-
Notifications
You must be signed in to change notification settings - Fork 0
[#16] 상품 아이템 정보 조회 기능 구현 #18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
8d45aa0
0d4d886
56ced04
9a40999
1330c15
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
.env | ||
data.sql | ||
application.properties | ||
HELP.md | ||
.gradle | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,7 @@ dependencies { | |
runtimeOnly("com.h2database:h2") | ||
runtimeOnly("com.mysql:mysql-connector-j") | ||
annotationProcessor("org.springframework.boot:spring-boot-configuration-processor") | ||
developmentOnly("org.springframework.boot:spring-boot-docker-compose") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 구두로도 질문드렸지만 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. developmentOnly, testAndDevelopmentOnly는 스프링 플러그인이 제공하는 의존성 스코프입니다. 스프링 플러그인에서 제공하는 bootRun, bootJar는 runtimeClassPath를 각각 다르게 구성합니다. 기본적으로 gradle 기본 스코프인 implementation, runtimeOnly와 스프링 플러그인으로 추가된 developmentOnly, testAndDevelopmentOnly 총 4개 스코프가 runtimeClassPath 하위에 포함됩니다. bootJar는 구성시 developmentOnly, testAndDevelopmentOnly를 runtimeClassPath에서 제외합니다. bootRun은 소스 코드 컴파일본과 4개 스코프 의존성 라이브러리 모두 포함하여 jar로 묶지 않고 바로 실행합니다. 이렇게 bootJar로 만든 jar를 java -jar 로 실행하는 환경을 개발 환경이 아니라고 보고, |
||
testImplementation("org.springframework.boot:spring-boot-starter-test") | ||
testImplementation("io.kotest:kotest-runner-junit5:5.8.0") | ||
testImplementation("com.appmattus.fixture:fixture:1.2.0") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
services: | ||
mysql: | ||
image: 'mysql:latest' | ||
environment: | ||
- MYSQL_DATABASE=${MYSQL_DATABASE} | ||
- MYSQL_ROOT_PASSWORD=${MYSQL_ROOT_PASSWORD} | ||
- MYSQL_USER=${MYSQL_USER} | ||
- MYSQL_PASSWORD=${MYSQL_PASSWORD} | ||
ports: | ||
- '3306:3306' |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package com.nilgil.commerce.common.error | ||
|
||
import org.springframework.http.HttpStatus | ||
|
||
enum class CoreError( | ||
override val status: HttpStatus, | ||
override val code: String, | ||
override val message: String, | ||
) : ErrorType { | ||
UNKNOWN_ERROR(HttpStatus.INTERNAL_SERVER_ERROR, "C001", "알 수 없는 오류가 발생했습니다."), | ||
VALIDATION_ERROR(HttpStatus.BAD_REQUEST, "C002", "요청 값이 올바르지 않습니다."), | ||
AUTHENTICATION_ERROR(HttpStatus.UNAUTHORIZED, "C003", "인증되지 않은 사용자입니다."), | ||
AUTHORIZATION_ERROR(HttpStatus.FORBIDDEN, "C004", "접근 권한이 없습니다."), | ||
NOT_FOUND_ERROR(HttpStatus.NOT_FOUND, "C005", "리소스를 찾을 수 없습니다."), | ||
CONFLICT_ERROR(HttpStatus.CONFLICT, "C006", "충돌되는 리소스가 존재합니다."), | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
package com.nilgil.commerce.common.error | ||
|
||
import org.springframework.boot.logging.LogLevel | ||
|
||
data class CoreException( | ||
val type: ErrorType = CoreError.UNKNOWN_ERROR, | ||
override val cause: Throwable? = null, | ||
val logLevel: LogLevel = LogLevel.ERROR, | ||
val detail: Any? = null, | ||
) : RuntimeException(type.message, cause) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
package com.nilgil.commerce.common.error | ||
|
||
data class ErrorResponse( | ||
val code: String, | ||
val message: String, | ||
val detail: Any? = null, | ||
) | ||
|
||
fun CoreException.toResponse(): ErrorResponse = | ||
ErrorResponse( | ||
code = this.type.code, | ||
message = this.type.message, | ||
detail = this.detail, | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package com.nilgil.commerce.common.error | ||
|
||
import org.springframework.http.HttpStatus | ||
|
||
interface ErrorType { | ||
val status: HttpStatus | ||
val code: String | ||
val message: String | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
package com.nilgil.commerce.common.error | ||
|
||
import org.slf4j.Logger | ||
import org.slf4j.LoggerFactory | ||
import org.springframework.boot.logging.LogLevel | ||
import org.springframework.http.ResponseEntity | ||
import org.springframework.web.bind.annotation.ExceptionHandler | ||
import org.springframework.web.bind.annotation.RestControllerAdvice | ||
|
||
@RestControllerAdvice | ||
class GlobalExceptionHandler { | ||
private val log: Logger = LoggerFactory.getLogger(javaClass) | ||
|
||
@ExceptionHandler(CoreException::class) | ||
fun handleCoreException(e: CoreException): ResponseEntity<ErrorResponse> { | ||
log.logOnLevel( | ||
level = e.logLevel, | ||
message = "[ERR-${e.type.code}] ${e.message}", | ||
throwable = e, | ||
) | ||
return ResponseEntity(e.toResponse(), e.type.status) | ||
} | ||
|
||
@ExceptionHandler(Exception::class) | ||
fun handleException(e: Exception): ResponseEntity<ErrorResponse> = | ||
handleCoreException(CoreException(type = CoreError.UNKNOWN_ERROR, cause = e)) | ||
|
||
@ExceptionHandler(IllegalArgumentException::class) | ||
fun handleIllegalArgumentException(e: Exception): ResponseEntity<ErrorResponse> = | ||
handleCoreException(CoreException(type = CoreError.VALIDATION_ERROR, cause = e)) | ||
|
||
@ExceptionHandler(IllegalStateException::class) | ||
fun handleIllegalStateException(e: Exception): ResponseEntity<ErrorResponse> = | ||
handleCoreException(CoreException(type = CoreError.CONFLICT_ERROR, cause = e)) | ||
} | ||
|
||
private fun Logger.logOnLevel( | ||
level: LogLevel, | ||
message: String, | ||
throwable: Throwable? = null, | ||
) { | ||
when (level) { | ||
LogLevel.ERROR -> error(message, throwable) | ||
LogLevel.WARN -> warn(message, throwable) | ||
LogLevel.INFO -> info(message, throwable) | ||
LogLevel.DEBUG -> debug(message, throwable) | ||
LogLevel.TRACE -> trace(message, throwable) | ||
else -> error(message, throwable) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package com.nilgil.commerce.order | ||
|
||
import com.nilgil.commerce.common.error.ErrorType | ||
import org.springframework.http.HttpStatus | ||
|
||
enum class OrderError( | ||
override val status: HttpStatus, | ||
override val code: String, | ||
override val message: String, | ||
) : ErrorType { | ||
INVALID_STATUS_TRANSITION(HttpStatus.CONFLICT, "O001", "주문 상태를 변경할 수 없습니다."), | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
package com.nilgil.commerce.product | ||
|
||
import org.springframework.data.jpa.repository.JpaRepository | ||
|
||
interface ProductImageRepository : JpaRepository<ProductImage, Long> { | ||
fun findByProductAndType( | ||
product: Product, | ||
type: ProductImageType, | ||
): ProductImage? | ||
|
||
fun findAllByProductInAndType( | ||
products: List<Product>, | ||
type: ProductImageType, | ||
): List<ProductImage> | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
package com.nilgil.commerce.product | ||
|
||
import org.springframework.web.bind.annotation.GetMapping | ||
import org.springframework.web.bind.annotation.PathVariable | ||
import org.springframework.web.bind.annotation.RequestParam | ||
import org.springframework.web.bind.annotation.RestController | ||
|
||
@RestController | ||
class ProductItemController( | ||
private val service: ProductItemService, | ||
) { | ||
@GetMapping("/product-items/{id}") | ||
fun getProductItem( | ||
@PathVariable id: Long, | ||
): ProductItemResponse = service.getProductItem(id) | ||
|
||
@GetMapping("/product-items") | ||
fun getProductItems( | ||
@RequestParam ids: List<Long>, | ||
): List<ProductItemResponse> = service.getProductItems(ids) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package com.nilgil.commerce.product | ||
|
||
import org.springframework.data.jpa.repository.EntityGraph | ||
import org.springframework.data.jpa.repository.JpaRepository | ||
|
||
interface ProductItemOptionElementRepository : JpaRepository<ProductItemOptionElement, Long> { | ||
@EntityGraph(attributePaths = ["optionElement.option"]) | ||
fun findAllWithOptionDetailsByItem(item: ProductItem): List<ProductItemOptionElement> | ||
|
||
@EntityGraph(attributePaths = ["optionElement.option"]) | ||
fun findAllWithOptionDetailsByItemIn(items: List<ProductItem>): List<ProductItemOptionElement> | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package com.nilgil.commerce.product | ||
|
||
import org.springframework.data.jpa.repository.EntityGraph | ||
import org.springframework.data.jpa.repository.JpaRepository | ||
|
||
interface ProductItemRepository : JpaRepository<ProductItem, Long> { | ||
@EntityGraph(attributePaths = ["product"]) | ||
fun findWithProductById(productItemId: Long): ProductItem? | ||
|
||
@EntityGraph(attributePaths = ["product"]) | ||
fun findWithProductByIdIn(productItemIds: List<Long>): List<ProductItem> | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
package com.nilgil.commerce.product | ||
|
||
data class ProductItemResponse( | ||
val id: Long, | ||
val productName: String, | ||
val options: Map<String, String>, | ||
val thumbnailImageUrl: String?, | ||
val price: Int, | ||
val stock: Int, | ||
) { | ||
companion object { | ||
fun from( | ||
productItem: ProductItem, | ||
options: Map<String, String>, | ||
thumbnailUrl: String?, | ||
): ProductItemResponse = | ||
ProductItemResponse( | ||
id = productItem.id, | ||
productName = productItem.product.name, | ||
options = options, | ||
thumbnailImageUrl = thumbnailUrl, | ||
price = productItem.getAdjustedPrice(), | ||
stock = productItem.stock, | ||
) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
package com.nilgil.commerce.product | ||
|
||
import com.nilgil.commerce.common.error.CoreError | ||
import com.nilgil.commerce.common.error.CoreException | ||
import org.springframework.stereotype.Service | ||
|
||
@Service | ||
class ProductItemService( | ||
private val productItemRepository: ProductItemRepository, | ||
private val productImageRepository: ProductImageRepository, | ||
private val productItemOptionElementRepository: ProductItemOptionElementRepository, | ||
) { | ||
fun getProductItem(id: Long): ProductItemResponse { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 요런 메소드들에 @transactional(readOnly = true) 를 추가하는 것에 대해 어떻게 생각하시는지 궁금합니다~ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @transactional이 불필요하다 생각했고, 커넥션도 길게 잡을 것이기에 빼는게 좋다고 생각했었는데 @transactional 사용에 대하여오류 발생 가능성프로젝트 전체적으로 연관관계를 lazy fetch로 설정해 놨습니다. 그러나 필요한 연관관계들을 미리 로딩해 왔기에 lazy loading이 사용되지 않아 당장 문제는 없습니다. 그런데 추후 미리 로딩되지 않은 연관관계 필드를 사용하려 하면 트랜잭션 범위에 있지 않아 영속성 컨텍스트를 사용하지 못해 오류가 발생할 것 같습니다. 눈에 잘 보이지 않는 오류 발생 가능 포인트를 만든 것 같습니다. 조회 데이터의 일관성조회만 하는 경우에 트랜잭션이 필요하다는 생각을 못 했습니다. 현재 상황같이 ProductItem이라는 일관성이 보장되어야 하는 단위를 조회할 때 한 번의 쿼리로 가져오지 않고 여러 번의 쿼리로 각 테이블을 조회하여 조합하는 상황에서 일관성이 깨질 수 있다는 생각을 못 했습니다. REPEATABLE READ 격리 수준으로 모두 트랜잭션 시작 시점의 데이터를 가져와 조합하여야 할 것 같습니다. 그러기 위해서는 @transactional로 묶을 필요가 있습니다. 커넥션 대여 시간과 대여&반납 오버헤드 관점위와 같이 일관성이 보장되어야 하는 경우가 아닌 별도의 데이터들을 각 쿼리로 조회하는 경우에는 트랜잭션이 필수는 아닙니다. 현재는 각 쿼리마다 매번 커넥션을 풀에서 가져와 사용하고 바로 반납합니다. 그러나 트랜잭션으로 묶었다면 한 번 가져온 커넥션을 계속 사용했을 것입니다. 전자의 경우 커넥션을 오래 잡지 않고 빠르게 반납하는 것은 맞지만 만약 지금과 같이 커넥션 대여가 길어지지 않을 경우라면 오히려 커넥션 대여&반납 오버헤드가 더 클 것 같다는 생각이 듭니다. 결론기본적으로 @transactional을 걸고 트랜잭션이 길어질 수 있는 경우 최적화를 시도하는 것이 좋을 것 같습니다. @transactional readOnly 옵션조회만 하는 경우에 사용하면 내부적으로 최적화해 준다고만 알고 있었습니다. 자세히 알아보니 다음과 같은 이점이 있는 것을 알게 되었습니다.
|
||
val productItem = findProductItemOrThrow(id) | ||
val options = findOptionMap(productItem) | ||
val thumbnailUrl = findThumbnailUrlOrNull(productItem.product) | ||
|
||
return ProductItemResponse.from( | ||
productItem = productItem, | ||
options = options, | ||
thumbnailUrl = thumbnailUrl, | ||
) | ||
} | ||
|
||
fun getProductItems(ids: List<Long>): List<ProductItemResponse> { | ||
val productItems = productItemRepository.findWithProductByIdIn(ids) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ids 의 크기를 제한하거나 chunking 할 필요는 없을까요?_? |
||
if (productItems.isEmpty()) { | ||
return emptyList() | ||
} | ||
|
||
val thumbnailUrlMap = getProductIdThumbnailUrlMap(productItems) | ||
val optionElementsMap = getProductItemIdOptionElementsMap(productItems) | ||
|
||
val responseMap = | ||
productItems.associateBy( | ||
keySelector = { it.id }, | ||
valueTransform = { item -> | ||
ProductItemResponse.from( | ||
productItem = item, | ||
options = optionElementsMap[item.id] ?: emptyMap(), | ||
thumbnailUrl = thumbnailUrlMap[item.product.id], | ||
) | ||
}, | ||
) | ||
|
||
return ids.mapNotNull { responseMap[it] } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 요건 request 한 id 의 순서보장을 위해 이렇게 한건가용? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 네 맞습니다. 서버에서 임의로 정렬을 수행하지 않고 요청한 순서에 맞게 응답해 줘야겠다고 생각했습니다. 그런데 없는 데이터를 요청했을 때 오류 없이 빼고 주도록 했는데, 그냥 서버에서 순서 보장 없고, 중복 제거한 채로 응답하고, 요청한 쪽에서 알아서 잘 사용하도록 하는 게 더 나아보이기도 합니다. |
||
} | ||
|
||
private fun findProductItemOrThrow(id: Long): ProductItem = | ||
productItemRepository.findWithProductById(id) | ||
?: throw CoreException( | ||
type = CoreError.NOT_FOUND_ERROR, | ||
detail = "상품 아이템을 찾을 수 없습니다. id: $id", | ||
) | ||
|
||
private fun findOptionMap(item: ProductItem): Map<String, String> = | ||
productItemOptionElementRepository | ||
.findAllWithOptionDetailsByItem(item) | ||
.associate { it.optionElement.option.name to it.optionElement.name } | ||
|
||
private fun findThumbnailUrlOrNull(product: Product): String? = | ||
productImageRepository.findByProductAndType(product, ProductImageType.THUMBNAIL)?.url | ||
|
||
private fun getProductIdThumbnailUrlMap(productItems: List<ProductItem>): Map<Long, String> { | ||
val products = productItems.map { productItem -> productItem.product } | ||
return productImageRepository | ||
.findAllByProductInAndType(products, ProductImageType.THUMBNAIL) | ||
.associate { it.product.id to it.url } | ||
} | ||
|
||
private fun getProductItemIdOptionElementsMap(productItems: List<ProductItem>): Map<Long, Map<String, String>> = | ||
productItemOptionElementRepository | ||
.findAllWithOptionDetailsByItemIn(productItems) | ||
.groupBy { it.item.id } | ||
.mapValues { it.value.associate { it -> it.optionElement.option.name to it.optionElement.name } } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 요고 컬렉션 처리 자체는 효율적인 것으로 보이는데, 중첩 |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,14 @@ | ||
spring: | ||
application.name: generic-commerce | ||
jpa: | ||
open-in-view: false | ||
hibernate: | ||
ddl-auto: create | ||
show-sql: true | ||
defer-datasource-initialization: true | ||
properties: | ||
hibernate: | ||
format_sql: true | ||
default_batch_fetch_size: 100 | ||
sql.init.mode: always | ||
docker.compose.lifecycle-management: start_only |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
꼼꼼하네욥 👍