Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package com.loopers.application.user

import com.loopers.domain.user.Gender
import com.loopers.domain.user.UserService
import com.loopers.support.error.CoreException
import com.loopers.support.error.ErrorType
import org.springframework.stereotype.Component

@Component
class UserFacade(
private val service: UserService,
) {
fun registerUser(userId: String, email: String, birth: String, gender: Gender): UserInfo {
return service.registerUser(userId, email, birth, gender)
.let { UserInfo.from(it) }
}

fun getUser(userId: String): UserInfo {
return service.getUserByUserId(userId)
?.let { UserInfo.from(it) }
?: throw CoreException(ErrorType.NOT_FOUND, "User not found")
}

fun getPoint(userId: String): Int {
return service.getPointByUserId(userId)
?: throw CoreException(ErrorType.NOT_FOUND, "User not found")
}

fun chargePoint(userId: String, amount: Int): Int {
return service.chargePointByUserId(userId, amount)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package com.loopers.application.user

import com.loopers.domain.user.Gender
import com.loopers.domain.user.User

data class UserInfo(
val userId: String,
val email: String,
val birth: String,
val gender: Gender,
) {
companion object {
fun from(user: User): UserInfo {
return UserInfo(
userId = user.userId,
email = user.email,
birth = user.birth,
gender = user.gender,
)
}
}
}
72 changes: 72 additions & 0 deletions apps/commerce-api/src/main/kotlin/com/loopers/domain/user/User.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package com.loopers.domain.user

import com.loopers.domain.BaseEntity
import com.loopers.support.error.CoreException
import com.loopers.support.error.ErrorType
import jakarta.persistence.Entity
import jakarta.persistence.Table

@Entity
@Table(name = "users")
class User(
userId: String,
email: String,
birth: String,
gender: Gender,
) : BaseEntity() {
var userId: String = userId
protected set

var email: String = email
protected set

var birth: String = birth
protected set

var gender: Gender = gender
protected set

var point: Int = 0

init {
if (!validateUserId(userId)) throw CoreException(ErrorType.BAD_REQUEST, "invalid user id")

if (!validateEmail(email)) throw CoreException(ErrorType.BAD_REQUEST, "invalid email")

if (!validateBirth(birth)) throw CoreException(ErrorType.BAD_REQUEST, "invalid birth")
}

fun chargePoint(point: Int): Int {
if (point <= 0) throw CoreException(ErrorType.BAD_REQUEST, "invalid point")

this.point += point
return this.point
}
Comment on lines +39 to +44
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

동시성 제어 필요 (Critical) 및 재저장 질문에 대한 답변

1. 동시성 문제 (Race Condition):
현재 구현은 동시에 여러 포인트 충전 요청이 들어올 때 업데이트 손실이 발생할 수 있습니다.

예시:

  • 스레드 A: point=100 읽음 → +50 → 150으로 저장
  • 스레드 B: point=100 읽음 → +30 → 130으로 저장
  • 결과: 130 (스레드 A의 +50이 손실됨)

2. 재저장 관련 질문 답변:
JPA의 dirty checking 메커니즘이 @Transactional 내에서 엔티티 변경을 자동으로 감지하여 저장하므로, 명시적인 save() 호출은 불필요합니다. 다만, 반드시 서비스 레이어에서 @Transactional 안에서 호출되어야 합니다.

해결 방법:

낙관적 잠금을 위한 @Version 필드 추가를 권장합니다:

 @Entity
 @Table(name = "users")
 class User(
     userId: String,
     email: String,
     birth: String,
     gender: Gender,
 ) : BaseEntity() {
+    @Version
+    var version: Long = 0
+        protected set
+
     var userId: String = userId
         protected set

이렇게 하면 동시 수정 시 OptimisticLockException이 발생하여 데이터 무결성을 보호할 수 있습니다.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In apps/commerce-api/src/main/kotlin/com/loopers/domain/user/User.kt around
lines 39 to 44, the chargePoint method is vulnerable to race conditions when
concurrent requests update the same entity; fix by adding an optimistic lock
version field (annotated @Version) to the User entity class, ensure all
modifications occur inside a service-layer method annotated with @Transactional
(so JPA dirty-checking will flush changes), and handle OptimisticLockException
by retrying the charge operation a bounded number of times or propagating a
clear error; do not add explicit save() calls—let the transaction commit persist
changes.


companion object {
private fun validateUserId(userId: String): Boolean {
val userIdValidator = Regex("^[A-Za-z0-9]{1,10}$")

return userIdValidator.matches(userId)
}

private fun validateEmail(email: String): Boolean {
val emailValidator = Regex("^[0-9a-zA-Z]([-_.]?[0-9a-zA-Z])*@[0-9a-zA-Z]([-_.]?[0-9a-zA-Z])*\\.[a-zA-Z]{2,3}$")

return emailValidator.matches(email)
}

private fun validateBirth(birth: String): Boolean {
val birthValidator = Regex("^\\d{4}-\\d{2}-\\d{2}$")

return birthValidator.matches(birth)
}
}
Comment on lines +46 to +64
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

데이터베이스 제약조건 및 인덱스 추가 권장

검증 로직은 잘 구현되어 있지만, 데이터베이스 레벨의 보장이 필요합니다.

User 엔티티에 다음 어노테이션을 추가하세요:

 @Entity
-@Table(name = "users")
+@Table(
+    name = "users",
+    indexes = [Index(name = "idx_user_id", columnList = "userId")],
+    uniqueConstraints = [UniqueConstraint(name = "uk_user_id", columnNames = ["userId"])]
+)
 class User(

그리고 userId 필드에:

+    @Column(unique = true, nullable = false, length = 10)
     var userId: String = userId
         protected set

이유:

  • uniqueConstraints: 애플리케이션 레벨 검증만으로는 동시성 상황에서 중복 방지 불가
  • Index: findByUserId 쿼리 성능 최적화

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In apps/commerce-api/src/main/kotlin/com/loopers/domain/user/User.kt around
lines 46–64, add database-level uniqueness and an index: annotate the User
entity class with a @Table that declares a UniqueConstraint on the userId column
and an @Index for userId; also annotate the userId field to be non-nullable and
unique at the column level (e.g., @Column(nullable = false, unique = true) or
equivalent) so the DB enforces uniqueness and the findByUserId query is indexed
for performance.

}

enum class Gender {
NONE,
MALE,
FEMALE,
OTHER,
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.loopers.domain.user

interface UserRepository {
fun save(user: User): User
fun existsByUserId(userId: String): Boolean
fun findByUserId(userId: String): User?
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package com.loopers.domain.user

import com.loopers.support.error.CoreException
import com.loopers.support.error.ErrorType
import org.springframework.stereotype.Component
import org.springframework.transaction.annotation.Transactional

@Component
class UserService(
private val userRepository: UserRepository,
) {
@Transactional
fun registerUser(
userId: String,
email: String,
birth: String,
gender: Gender,
): User {
if (userRepository.existsByUserId(userId)) throw CoreException(ErrorType.CONFLICT, "User already exists")

return userRepository.save(User(userId, email, birth, gender))
}

fun getUserByUserId(userId: String): User? {
return userRepository.findByUserId(userId)
}

fun getPointByUserId(userId: String): Int? {
return userRepository.findByUserId(userId)?.point
}

fun chargePointByUserId(userId: String, point: Int): Int {
val user = userRepository.findByUserId(userId)
?: throw CoreException(ErrorType.NOT_FOUND, "user not found")

user.chargePoint(point)
return user.point
}
Comment on lines +32 to +38
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

트랜잭션 경계가 누락되어 포인트 변경사항이 저장되지 않습니다.

chargePoint() 호출로 엔티티가 변경되지만, @Transactional 어노테이션이 없고 명시적인 save() 호출도 없어서 변경사항이 데이터베이스에 반영되지 않습니다.

작성자 질문에 대한 답변:

  1. User 엔티티에 포인트를 저장하는 것은 현재 요구사항에서는 적절합니다. 다만 향후 포인트 이력 추적, 트랜잭션 관리 등이 필요해지면 별도의 PointTransaction 엔티티를 고려해야 합니다.
  2. 포인트 충전 후 명시적인 저장이 필요한지에 대해: JPA의 더티 체킹(dirty checking)을 활용하려면 @Transactional을 추가해야 하며, 그렇지 않으면 userRepository.save(user)를 명시적으로 호출해야 합니다.

해결 방법 1 (권장): @Transactional 추가

+    @Transactional
     fun chargePointByUserId(userId: String, point: Int): Int {
         val user = userRepository.findByUserId(userId)
             ?: throw CoreException(ErrorType.NOT_FOUND, "user not found")
 
         user.chargePoint(point)
         return user.point
     }

해결 방법 2: 명시적 저장

     fun chargePointByUserId(userId: String, point: Int): Int {
         val user = userRepository.findByUserId(userId)
             ?: throw CoreException(ErrorType.NOT_FOUND, "user not found")
 
         user.chargePoint(point)
+        userRepository.save(user)
         return user.point
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fun chargePointByUserId(userId: String, point: Int): Int {
val user = userRepository.findByUserId(userId)
?: throw CoreException(ErrorType.NOT_FOUND, "user not found")
user.chargePoint(point)
return user.point
}
@Transactional
fun chargePointByUserId(userId: String, point: Int): Int {
val user = userRepository.findByUserId(userId)
?: throw CoreException(ErrorType.NOT_FOUND, "user not found")
user.chargePoint(point)
return user.point
}
Suggested change
fun chargePointByUserId(userId: String, point: Int): Int {
val user = userRepository.findByUserId(userId)
?: throw CoreException(ErrorType.NOT_FOUND, "user not found")
user.chargePoint(point)
return user.point
}
fun chargePointByUserId(userId: String, point: Int): Int {
val user = userRepository.findByUserId(userId)
?: throw CoreException(ErrorType.NOT_FOUND, "user not found")
user.chargePoint(point)
userRepository.save(user)
return user.point
}
🤖 Prompt for AI Agents
In apps/commerce-api/src/main/kotlin/com/loopers/domain/user/UserService.kt
around lines 32-38, the method mutates the User entity via
user.chargePoint(point) but lacks a transactional boundary or an explicit save,
so the change isn't persisted; fix by annotating the method (or the service
class) with @Transactional and import
org.springframework.transaction.annotation.Transactional so JPA dirty-checking
will flush the updated point, or alternatively call userRepository.save(user)
after chargePoint(point) if you prefer explicit saves; ensure the chosen
approach is consistently applied and the method still returns user.point.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.loopers.infrastructure.user

import com.loopers.domain.user.User
import org.springframework.data.jpa.repository.JpaRepository

interface UserJpaRepository : JpaRepository<User, Long> {
fun existsByUserId(userId: String): Boolean
fun findByUserId(userId: String): User?
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package com.loopers.infrastructure.user

import com.loopers.domain.user.User
import com.loopers.domain.user.UserRepository
import org.springframework.stereotype.Component

@Component
class UserRepositoryImpl(
private val userJpaRepository: UserJpaRepository,
) : UserRepository {
override fun save(user: User): User {
return userJpaRepository.save(user)
}

override fun existsByUserId(userId: String): Boolean {
return userJpaRepository.existsByUserId(userId)
}

override fun findByUserId(userId: String): User? {
return userJpaRepository.findByUserId(userId)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package com.loopers.interfaces.api.user

import com.loopers.interfaces.api.ApiResponse
import io.swagger.v3.oas.annotations.Operation
import io.swagger.v3.oas.annotations.media.Schema
import io.swagger.v3.oas.annotations.tags.Tag

@Tag(name = "User V1 API", description = "회원 API")
interface UserV1ApiSpec {
@Operation(
summary = "회원 가입",
description = "ID, email, birth, gender 정보로 회원 가입 시도",
)
fun registerUser(req: UserV1Dto.RegisterUserRequest): ApiResponse<UserV1Dto.UserResponse>

@Operation(
summary = "회원 조회",
description = "ID로 회원을 조회합니다.",
)
fun getUser(
@Schema(name = "회원 ID", description = "조회할 회원의 ID")
userId: String,
): ApiResponse<UserV1Dto.UserResponse>

@Operation(
summary = "회원 포인트 조회",
description = "ID로 회원 포인트를 조회합니다.",
)
fun getPoint(
@Schema(name = "회원 ID", description = "조회할 회원의 ID")
userId: String?,
): ApiResponse<Int>
Comment on lines +25 to +32
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

파라미터 nullable 불일치 확인 필요

getPointuserId 파라미터가 nullable(String?)로 선언되어 있는데, 이는 getUser의 non-nullable userId와 일관성이 없습니다.

만약 인증 컨텍스트에서 userId를 가져오는 등의 이유로 의도적으로 optional이라면, 이를 주석이나 문서에 명시하는 것이 좋습니다. 그렇지 않다면 non-nullable로 변경해야 합니다.

일관성을 위해 다음과 같이 수정하는 것을 권장합니다:

 fun getPoint(
     @Schema(name = "회원 ID", description = "조회할 회원의 ID")
-    userId: String?,
+    userId: String,
 ): ApiResponse<Int>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Operation(
summary = "회원 포인트 조회",
description = "ID로 회원 포인트를 조회합니다.",
)
fun getPoint(
@Schema(name = "회원 ID", description = "조회할 회원의 ID")
userId: String?,
): ApiResponse<Int>
@Operation(
summary = "회원 포인트 조회",
description = "ID로 회원 포인트를 조회합니다.",
)
fun getPoint(
@Schema(name = "회원 ID", description = "조회할 회원의 ID")
userId: String,
): ApiResponse<Int>
🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/kotlin/com/loopers/interfaces/api/user/UserV1ApiSpec.kt
around lines 25–32, the getPoint parameter userId is declared nullable (String?)
but getUser uses a non-nullable userId; make this consistent by changing userId
to non-nullable String if it must always be provided, and update the @Schema
annotation to reflect required=true; if nullability was intentional, add a KDoc
comment and update the OpenAPI/@Schema to document that the parameter is
optional and how null is handled.


@Operation(
summary = "회원 포인트 충전",
description = "ID와 포인트로 회원 포인트를 충전합니다.",
)
fun chargePoint(req: UserV1Dto.ChargePointRequest): ApiResponse<Int>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package com.loopers.interfaces.api.user

import com.loopers.application.user.UserFacade
import com.loopers.domain.user.Gender
import com.loopers.interfaces.api.ApiResponse
import com.loopers.support.error.CoreException
import com.loopers.support.error.ErrorType
import org.springframework.web.bind.annotation.GetMapping
import org.springframework.web.bind.annotation.PathVariable
import org.springframework.web.bind.annotation.PostMapping
import org.springframework.web.bind.annotation.RequestBody
import org.springframework.web.bind.annotation.RequestHeader
import org.springframework.web.bind.annotation.RequestMapping
import org.springframework.web.bind.annotation.RestController

@RestController
@RequestMapping("/api/v1/users")
class UserV1Controller(
private val userFacade: UserFacade,
) : UserV1ApiSpec {
@PostMapping("/register")
override fun registerUser(@RequestBody req: UserV1Dto.RegisterUserRequest): ApiResponse<UserV1Dto.UserResponse> {
if (req.gender == Gender.NONE) throw CoreException(ErrorType.BAD_REQUEST)

return userFacade.registerUser(req.userId, req.email, req.birth, req.gender)
.let { UserV1Dto.UserResponse.from(it) }
.let { ApiResponse.success(it) }
}

override fun getUser(
@PathVariable userId: String,
): ApiResponse<UserV1Dto.UserResponse> {
return userFacade.getUser(userId)
.let { UserV1Dto.UserResponse.from(it) }
.let { ApiResponse.success(it) }
}

@GetMapping("/point")
override fun getPoint(
@RequestHeader("X-USER-ID", required = false) userId: String?,
): ApiResponse<Int> {
if (userId.isNullOrBlank()) throw CoreException(ErrorType.BAD_REQUEST)

return userFacade.getPoint(userId)
.let { ApiResponse.success(it) }
}

@PostMapping("/chargePoint")
override fun chargePoint(@RequestBody req: UserV1Dto.ChargePointRequest): ApiResponse<Int> {
return userFacade.chargePoint(req.userId, req.amount)
.let { ApiResponse.success(it) }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package com.loopers.interfaces.api.user

import com.loopers.application.user.UserInfo
import com.loopers.domain.user.Gender

class UserV1Dto {
data class RegisterUserRequest(val userId: String, val email: String, val birth: String, val gender: Gender)
data class UserResponse(val userId: String, val email: String, val birth: String, val gender: Gender) {
companion object {
fun from(user: UserInfo): UserResponse {
return UserResponse(
userId = user.userId,
email = user.email,
birth = user.birth,
gender = user.gender,
)
}
}
}
data class ChargePointRequest(val userId: String, val amount: Int)
}
Loading