629-devicetoken을 별도 테이블로 분리#647
Hidden character warning
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdmin과 User의 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant LoginSvc as UserLoginService
participant SaveTokenPort as SaveUserDeviceTokenPort
participant TokenAdapter as UserDeviceTokenPersistenceAdapter
participant TokenRepo as UserDeviceTokenRepository
participant UserRepo as UserRepository
participant JWT as JWTGenerator
Client->>LoginSvc: login(accountId, password, deviceToken?, os?)
LoginSvc->>UserRepo: findByAccountId(accountId)
UserRepo-->>LoginSvc: user
LoginSvc->>LoginSvc: validatePassword()
LoginSvc->>JWT: generateToken(user)
JWT-->>LoginSvc: jwt
alt deviceToken and os present
LoginSvc->>SaveTokenPort: save(UserDeviceToken(userId, deviceToken, os))
SaveTokenPort->>TokenAdapter: save(domain)
TokenAdapter->>TokenRepo: findByDeviceToken(deviceToken)
TokenRepo-->>TokenAdapter: existingEntity?
alt exists
TokenAdapter->>TokenAdapter: updateEntity(existing, domain)
else
TokenAdapter->>TokenAdapter: toEntity(domain)
end
TokenAdapter->>TokenRepo: save(entity)
TokenRepo-->>TokenAdapter: savedEntity
TokenAdapter-->>SaveTokenPort: domain
SaveTokenPort-->>LoginSvc: domain
end
LoginSvc-->>Client: jwt
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Fix all issues with AI agents
In
`@src/main/kotlin/dsm/pick2024/domain/admin/presentation/dto/request/AdminLoginRequest.kt`:
- Around line 8-9: AdminLoginRequest currently has deviceToken: String? but os:
OSType (non-nullable), which forces clients to send os even when deviceToken is
absent; change os to nullable (OSType?) in the AdminLoginRequest DTO (or
alternatively wrap deviceToken and os into a single nullable sub-DTO) and update
any validation/consumers that assume non-null os (e.g., login handling code that
reads AdminLoginRequest.os) to handle the null case accordingly so behavior
remains correct when deviceToken is omitted.
In
`@src/main/kotlin/dsm/pick2024/domain/admin/presentation/dto/request/AdminSignUpRequest.kt`:
- Line 22: AdminSignUpRequest의 deviceToken 필드에 유효성 어노테이션이 빠져 있습니다; 이 필드가 필수면
AdminSignUpRequest 클래스의 deviceToken에 `@field`:NotBlank을 추가하고, 선택적이면 타입을 String?으로
변경하여 nullable로 선언하도록 수정하세요(필드 이름: deviceToken, 클래스: AdminSignUpRequest).
- Around line 26-27: The annotation on the AdminSignUpRequest.kt `os: OSType`
field is incorrect: replace `@field:NotBlank` with `@field:NotNull` on the `os`
property (type OSType) so the enum is validated for non-null; also add or update
the import to use `javax.validation.constraints.NotNull` if not already present.
Ensure the change targets the `os` property inside the AdminSignUpRequest class.
In `@src/main/kotlin/dsm/pick2024/domain/admin/service/AdminSignUpService.kt`:
- Around line 44-52: In AdminSignUpService, the AdminDeviceToken is constructed
without an explicit id which results in the default UUID(0,0) being used; update
the construction inside the request.deviceToken?.let block to supply a generated
id (e.g., id = UUID.randomUUID()) when calling adminDeviceTokenPort.save with
AdminDeviceToken so it matches the approach used by
UserLoginService/UserDeviceToken, or alternatively adjust the AdminDeviceToken
domain class to generate a random UUID by default; ensure the change references
AdminSignUpService, adminDeviceTokenPort.save, and AdminDeviceToken (or the
domain class) so IDs are unique and consistent with UserLoginService.
In
`@src/main/kotlin/dsm/pick2024/domain/application/service/ChangeApplicationStatusService.kt`:
- Around line 33-36: The deviceTokens collection in
ChangeApplicationStatusService may contain duplicates because
applications.flatMap { ...
queryUserDeviceTokenPort.findAllByUserId(application.userId).map {
it.deviceToken } } will add the same user's tokens multiple times when
request.idList contains multiple applications for the same user; update the
logic that builds deviceTokens (the variable named deviceTokens in
ChangeApplicationStatusService) to deduplicate entries (e.g., apply a
distinct-like operation after collecting tokens) so each deviceToken from
queryUserDeviceTokenPort.findAllByUserId is sent only once.
In `@src/main/kotlin/dsm/pick2024/domain/devicetoken/domain/AdminDeviceToken.kt`:
- Around line 7-13: Two fixes: give each JPA entity a unique table name to avoid
collision and add a DB-level unique constraint for (adminId, deviceToken), and
change repository lookup to include adminId to avoid race conditions. Update
AdminDeviceTokenJpaEntity and UserDeviceTokenJpaEntity to use distinct
`@Entity`(name=...) / `@Table`(name=..., uniqueConstraints=...) values and add a
UniqueConstraint for ("admin_id","device_token"); add or change repository
method findByAdminIdAndDeviceToken(...) (or update usages of findByDeviceToken
to pass adminId) and handle DataIntegrityViolationException on insert to map DB
uniqueness violations. Also add a migration SQL that explicitly creates the
device token table with the same unique index/constraint so ddl-auto:none
deployments are safe.
In
`@src/main/kotlin/dsm/pick2024/domain/devicetoken/entity/AdminDeviceTokenJpaEntity.kt`:
- Around line 12-24: Add a unique constraint on the AdminDeviceTokenJpaEntity
for the (adminId, deviceToken) pair (e.g., via `@Table`(uniqueConstraints = ...))
and/or an index to enforce DB-level uniqueness; add a repository method
findByAdminIdAndDeviceToken(adminId: UUID, deviceToken: String) to replace the
existing findByDeviceToken usage; update resolveEntity() to call
findByAdminIdAndDeviceToken(...) so lookups consider adminId; and fix
AdminDeviceTokenMapper.updateEntity(entity, domain) to actually copy fields from
the incoming domain (deviceToken, os, etc.) into the entity before returning so
updates are applied.
In
`@src/main/kotlin/dsm/pick2024/domain/devicetoken/entity/UserDeviceTokenJpaEntity.kt`:
- Around line 11-12: The `@Entity` annotation on UserDeviceTokenJpaEntity is
incorrectly set to "tbl_admin_device_token" (causing collision with
AdminDeviceTokenJpaEntity); update the `@Entity`(name = ...) value on the
UserDeviceTokenJpaEntity class to the correct user device token table name
(e.g., "tbl_user_device_token") so it maps to a distinct table and verify there
are no other annotations or constants referencing the admin table name in
UserDeviceTokenJpaEntity.
In
`@src/main/kotlin/dsm/pick2024/domain/devicetoken/mapper/AdminDeviceTokenMapper.kt`:
- Around line 30-40: The updateEntity function in AdminDeviceTokenMapper
currently ignores the incoming domain object and recreates the JPA entity using
only existingEntity fields; change AdminDeviceTokenMapper.updateEntity to
preserve the existingEntity.id (throwing EntityStateException if null) but copy
updated values from the domain parameter into the new AdminDeviceTokenJpaEntity
(set adminId = domain.adminId, deviceToken = domain.deviceToken, os = domain.os)
so the JPA entity reflects the intended updates from AdminDeviceToken.
In
`@src/main/kotlin/dsm/pick2024/domain/devicetoken/mapper/UserDeviceTokenMapper.kt`:
- Around line 30-40: The updateEntity function in UserDeviceTokenMapper
currently ignores the domain parameter and returns a copy of existingEntity;
change updateEntity to preserve the existingEntity id (throwing
EntityStateException if null) but populate the other fields from the domain
object so updates are applied — specifically update UserDeviceTokenJpaEntity
constructed in updateEntity to use id = existingEntity.id ?: throw
EntityStateException and userId = domain.userId, deviceToken =
domain.deviceToken, os = domain.os.
In
`@src/main/kotlin/dsm/pick2024/domain/devicetoken/persistence/repository/UserDeviceTokenRepository.kt`:
- Around line 7-10: The repository method findByDeviceToken in
UserDeviceTokenRepository assumes device tokens are unique but the
UserDeviceTokenJpaEntity's deviceToken column lacks a uniqueness constraint;
update the entity (UserDeviceTokenJpaEntity) to declare the deviceToken column
as unique (e.g., set unique = true on the `@Column` for deviceToken or add a
unique constraint on the table) so the DB enforces uniqueness and prevents
IncorrectResultSizeDataAccessException when using findByDeviceToken.
In
`@src/main/kotlin/dsm/pick2024/domain/selfstudy/service/SendNotificationSelfStudyTeacher.kt`:
- Line 22: The field selfStudy.teacherName is misnamed (it holds an admin ID)
and is passed to adminFinderUseCase.findByAdminIdOrThrow; rename the field to a
clearer identifier like teacherAdminId across the SelfStudy data class and all
usages (including SendNotificationSelfStudyTeacher where admin is resolved) to
reflect its actual purpose, update constructors/property accessors, and run the
build to catch and fix remaining references.
In
`@src/main/kotlin/dsm/pick2024/domain/user/presentation/dto/request/UserLoginRequest.kt`:
- Around line 5-10: UserLoginRequest currently declares deviceToken as String?
but os as non-null OSType, causing a mismatch when deviceToken is null; update
the DTO by either making os nullable (change os: OSType to os: OSType?) so both
fields can be absent, or make deviceToken non-null (change deviceToken: String?
to deviceToken: String) so both are required—modify the data class
UserLoginRequest accordingly and ensure any callers/consumers of
UserLoginRequest and validations are updated to match the chosen contract.
In
`@src/main/kotlin/dsm/pick2024/domain/weekendmeal/service/NotificationWeekendMealService.kt`:
- Around line 27-32: The code calls LocalDate.now() twice causing potential
date-flip inconsistencies; capture LocalDate.now() once at the start of the
method (e.g., val today = LocalDate.now()) and use that single today variable
for both the call to weekendMealPeriodPort.queryWeekendPeriodByDate(...) and the
subsequent when (...) branch so weekendMealPeriod and the when condition are
computed from the same instant.
- Around line 46-47: The notification message uses
weekendMealPeriod.month.ordinal which yields a 0-based month; update the string
construction in NotificationWeekendMealService to use
weekendMealPeriod.month.value so the message shows 1-12 month numbers (e.g.,
replace occurrences of month.ordinal with month.value in the message that
includes "${user.name}님은 ${weekendMealPeriod.month.ordinal}월 주말급식을 ..."). Ensure
only the month accessor is changed and the rest of the formatting and status
handling (Status.OK check) remains intact.
In `@src/main/kotlin/dsm/pick2024/global/error/exception/ErrorCode.kt`:
- Around line 56-57: ErrorCode enum entry ENTITY_STATE_EXCEPTION has a typo in
its message string ("존재히지" → "존재하지"); update the message for
ENTITY_STATE_EXCEPTION in ErrorCode.kt to the corrected Korean text "조회된 엔티티에
ID가 존재하지 않습니다" so the enum constant's second parameter reflects the fixed
message.
🧹 Nitpick comments (12)
src/main/kotlin/dsm/pick2024/domain/devicetoken/enum/OSType.kt (1)
3-7:ADMIN은 OS 타입이 아닙니다.
OSType열거형에AOS,IOS와 함께ADMIN이 포함되어 있습니다.ADMIN은 운영체제가 아니라 사용자 역할 또는 플랫폼 유형에 해당합니다. 이미AdminDeviceToken과UserDeviceToken으로 엔티티가 분리되어 있으므로,ADMIN을 OS 타입으로 사용하는 것은 의미적으로 부적절합니다.관리자가 웹 브라우저를 통해 접속하는 경우
WEB으로 명명하거나, 별도의 열거형으로 분리하는 것을 권장합니다.src/main/kotlin/dsm/pick2024/domain/devicetoken/exception/EntityStateException.kt (1)
1-8: 범용적인 예외가devicetoken패키지에 위치합니다.
EntityStateException은 "조회된 엔티티에 ID가 존재하지 않습니다"라는 범용적인 메시지를 가지고 있어, device token 외 다른 엔티티에서도 발생할 수 있는 예외입니다.global.error.exception패키지로 이동하는 것이 적절합니다. 또는 device token 전용이라면 이름을DeviceTokenEntityStateException처럼 구체적으로 변경하는 것을 권장합니다.src/main/kotlin/dsm/pick2024/domain/admin/service/AdminSignUpService.kt (1)
30-31:AdminDeviceTokenPort(composite) 대신SaveAdminDeviceTokenPort만 주입해도 충분합니다.이 서비스에서는
save만 호출하므로, 인터페이스 분리 원칙(ISP)에 따라SaveAdminDeviceTokenPort만 주입하면 의존 범위를 최소화할 수 있습니다.src/main/kotlin/dsm/pick2024/domain/devicetoken/domain/UserDeviceToken.kt (1)
7-13: 도메인 모델 구조는 깔끔합니다.
@Aggregate어노테이션과 필드 구성이 적절합니다. 다만,id의 기본값UUID(0, 0)이 "미저장" 상태를 나타내는 sentinel 값으로 사용되고 있는데, 이 패턴이AdminDeviceToken에서도 동일하게 적용되는지 확인이 필요합니다. 또한, 동일deviceToken문자열이 여러 사용자에게 중복 등록되지 않도록 영속성 계층에서 unique constraint를 적용하는 것을 권장합니다.src/main/kotlin/dsm/pick2024/domain/devicetoken/persistence/repository/AdminDeviceTokenRepository.kt (1)
7-10: 토큰 정리(cleanup) 및 중복 방지를 위한 메서드 추가를 고려해 주세요.
findByDeviceToken이 선언되어 있지만, 서비스 계층에서 저장 전 중복 확인에 활용되고 있지 않습니다. 또한, 관리자 디바이스 변경 시 기존 토큰을 정리할 수 있도록deleteByAdminId또는deleteByDeviceToken같은 메서드도 필요할 수 있습니다.src/main/kotlin/dsm/pick2024/domain/user/service/UserLoginService.kt (1)
22-23:userSavePort제거를 권장합니다.클래스 내 어떤 메서드에서도
userSavePort가 사용되지 않습니다. deviceToken 저장 로직이saveUserDeviceTokenPort로 완전히 이전된 후, 미사용 의존성으로 남겨져 있습니다. 정리를 위해 생성자 매개변수에서 제거하시기 바랍니다.src/main/kotlin/dsm/pick2024/domain/earlyreturn/service/ChangeEarlyReturnStatusService.kt (1)
33-37: 다른 서비스와의 빈 토큰 필터링 일관성 부재이 서비스에서는
.filter { it.isNotBlank() }로 빈 토큰을 필터링하고 있지만,CreateEarlyReturnService,ApplicationService,SendNotificationSelfStudyTeacher등 다른 서비스에서는 동일한 필터링이 없습니다. 빈 토큰이 알림 전송 시 오류를 발생시킬 수 있다면, 모든 서비스에 일관되게 적용하거나, 포트/어댑터 레벨에서 필터링하는 것이 바람직합니다.src/main/kotlin/dsm/pick2024/domain/user/service/UserSignUpService.kt (1)
24-24: 와일드카드 임포트 사용 지양
java.util.*와일드카드 임포트는 네임스페이스 충돌 가능성을 높이고 가독성을 낮춥니다. 실제 사용하는java.util.UUID만 명시적으로 임포트하는 것이 좋습니다.제안
-import java.util.* +import java.util.UUIDsrc/main/kotlin/dsm/pick2024/domain/selfstudy/service/SendNotificationSelfStudyTeacher.kt (1)
19-19:.map { it }는 불필요한 identity 매핑입니다
.map { it }은 리스트를 그대로 복사할 뿐 의미 있는 변환을 수행하지 않습니다. 제거하는 것이 좋습니다.제안
- val selfStudies = selfStudyFinderUseCase.findByDaySelfStudyOrThrow(LocalDate.now()).map { it } + val selfStudies = selfStudyFinderUseCase.findByDaySelfStudyOrThrow(LocalDate.now())src/main/kotlin/dsm/pick2024/domain/earlyreturn/service/CreateEarlyReturnService.kt (1)
63-69: 알림 전송 방식이ApplicationService와 일관되지 않습니다이 서비스는 토큰별로
outboxFacadeUseCase.sendNotification()을 개별 호출하지만,ApplicationService는 토큰 목록을FcmRequest에 담아saveOutboxPort.saveOutbox()로 한 번에 저장합니다. 동일한 알림 전송 목적인데 두 가지 다른 패턴이 사용되고 있어, 다음과 같은 차이가 발생할 수 있습니다:
- 트랜잭션 내에서의 동작 차이
- 실패 시 부분 전송 여부
- 성능 (N회 호출 vs 1회 호출)
하나의 패턴으로 통일하는 것을 권장합니다.
src/main/kotlin/dsm/pick2024/domain/weekendmeal/service/NotificationWeekendMealService.kt (1)
29-49: 전체 사용자 조회 후 개별 쿼리 반복 — N+1 성능 문제
queryUserPort.findAll()로 모든 사용자를 가져온 후, 각 사용자에 대해findByUserIdOrThrow와findAllByUserId를 개별 호출하므로, 사용자 수 N에 대해 최소 2N+1개의 쿼리가 발생합니다. 사용자 수가 많아지면 심각한 성능 저하로 이어질 수 있습니다. 배치 조회를 도입하거나 별도 이슈로 추적하는 것을 권장합니다.src/main/kotlin/dsm/pick2024/domain/devicetoken/entity/AdminDeviceTokenJpaEntity.kt (1)
11-11:@Entity(name = ...)대신@Table(name = ...)을 사용하세요
@Entity의name속성은 JPQL에서 사용되는 엔티티 이름을 지정하며, 데이터베이스 테이블 이름을 지정하는 것은@Table(name = ...)입니다. 현재 코드는 Hibernate가 엔티티 이름을 테이블 이름으로 매핑하기 때문에 동작하지만, JPQL에서FROM tbl_admin_device_token처럼 테이블 이름을 엔티티 이름으로 사용해야 하는 부자연스러운 상황이 발생합니다. 프로젝트의 다른 엔티티(OutboxJpaEntity)에서도@Table(name = "...")을 사용하는 패턴을 따릅니다.제안
import javax.persistence.Column import javax.persistence.Entity +import javax.persistence.Table import javax.persistence.EnumType import javax.persistence.Enumerated -@Entity(name = "tbl_admin_device_token") +@Entity +@Table(name = "tbl_admin_device_token") class AdminDeviceTokenJpaEntity(
src/main/kotlin/dsm/pick2024/domain/admin/presentation/dto/request/AdminLoginRequest.kt
Show resolved
Hide resolved
src/main/kotlin/dsm/pick2024/domain/admin/presentation/dto/request/AdminSignUpRequest.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/dsm/pick2024/domain/admin/presentation/dto/request/AdminSignUpRequest.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/dsm/pick2024/domain/application/service/ChangeApplicationStatusService.kt
Outdated
Show resolved
Hide resolved
| val admin = adminFinderUseCase.findByAdminNameOrThrow(it.teacherName) | ||
| admin.deviceToken?.let { token -> | ||
| selfStudies.forEach { selfStudy -> | ||
| val admin = adminFinderUseCase.findByAdminIdOrThrow(selfStudy.teacherName) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find SelfStudy.kt file to check teacherName field type
fd "SelfStudy.kt" --type fRepository: DSM-PICK/PiCK_CORE_SERVER
Length of output: 134
🏁 Script executed:
# Find AdminFinderUseCase to check findByAdminIdOrThrow signature
fd "AdminFinderUseCase.kt" --type fRepository: DSM-PICK/PiCK_CORE_SERVER
Length of output: 140
🏁 Script executed:
cat -n src/main/kotlin/dsm/pick2024/domain/selfstudy/domain/SelfStudy.ktRepository: DSM-PICK/PiCK_CORE_SERVER
Length of output: 442
🏁 Script executed:
cat -n src/main/kotlin/dsm/pick2024/domain/admin/port/in/AdminFinderUseCase.ktRepository: DSM-PICK/PiCK_CORE_SERVER
Length of output: 449
🏁 Script executed:
cat -n src/main/kotlin/dsm/pick2024/domain/selfstudy/service/SendNotificationSelfStudyTeacher.ktRepository: DSM-PICK/PiCK_CORE_SERVER
Length of output: 1828
필드 이름 리팩터링 고려
selfStudy.teacherName은 findByAdminIdOrThrow에 전달되지만, 필드명이 실제 용도와 일치하지 않습니다. 필드명 teacherName은 교사의 이름을 암시하지만, 실제로는 관리자 ID를 저장하고 있습니다. 코드의 의도를 명확히 하기 위해 필드명을 teacherAdminId 또는 유사한 이름으로 리팩터링하는 것을 권장합니다.
🤖 Prompt for AI Agents
In
`@src/main/kotlin/dsm/pick2024/domain/selfstudy/service/SendNotificationSelfStudyTeacher.kt`
at line 22, The field selfStudy.teacherName is misnamed (it holds an admin ID)
and is passed to adminFinderUseCase.findByAdminIdOrThrow; rename the field to a
clearer identifier like teacherAdminId across the SelfStudy data class and all
usages (including SendNotificationSelfStudyTeacher where admin is resolved) to
reflect its actual purpose, update constructors/property accessors, and run the
build to catch and fix remaining references.
src/main/kotlin/dsm/pick2024/domain/user/presentation/dto/request/UserLoginRequest.kt
Show resolved
Hide resolved
src/main/kotlin/dsm/pick2024/domain/weekendmeal/service/NotificationWeekendMealService.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/dsm/pick2024/domain/weekendmeal/service/NotificationWeekendMealService.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/dsm/pick2024/global/error/exception/ErrorCode.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@src/main/kotlin/dsm/pick2024/domain/devicetoken/entity/UserDeviceTokenJpaEntity.kt`:
- Around line 18-19: 엔티티 UserDeviceTokenJpaEntity의 deviceToken 속성에 현재
`@Column`(name = "device_token")만 설정되어 있어 NULL 허용으로 남아 있으니, 데이터 정합성을 위해
deviceToken 컬럼에 NOT NULL 제약을 추가하도록 `@Column`(name = "device_token", nullable =
false)로 변경하고 관련 생성자/매핑(필요 시)도 nullable=false 제약을 가정한 상태로 유지되도록 수정하세요.
- Around line 21-23: Commit message and code disagree about making os nullable;
inspect intent and either make os nullable across JPA and domain or revert the
commit. If making nullable, change the JPA entities UserDeviceTokenJpaEntity and
AdminDeviceTokenJpaEntity by setting `@Column`(name = "os", nullable = true) and
update the Kotlin property type from OSType to OSType? (nullable) and adjust
constructors/mappers that convert between the JPA entities and domain classes;
also update the domain classes UserDeviceToken and AdminDeviceToken to use
OSType? and handle possible null values in any code that reads os. If the commit
was wrong, instead revert the commit so os remains non-nullable everywhere
(leave `@Column`(nullable = false) and domain types as OSType).
src/main/kotlin/dsm/pick2024/domain/devicetoken/entity/UserDeviceTokenJpaEntity.kt
Outdated
Show resolved
Hide resolved
| @Enumerated(EnumType.STRING) | ||
| @Column(name = "os", nullable = false) | ||
| val os: OSType |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd src/main/kotlin/dsm/pick2024/domain/devicetoken && find . -name "*.kt" -type fRepository: DSM-PICK/PiCK_CORE_SERVER
Length of output: 759
🏁 Script executed:
rg -n "nullable\s*=\s*(true|false)" --type=kotlin -B5 -A1 src/main/kotlin/dsm/pick2024/domain/devicetoken/Repository: DSM-PICK/PiCK_CORE_SERVER
Length of output: 3153
🏁 Script executed:
rg -n "class.*DeviceToken\|os\s*:\s*OSType\|osType" --type=kotlin src/main/kotlin/dsm/pick2024/domain/devicetoken/Repository: DSM-PICK/PiCK_CORE_SERVER
Length of output: 51
🏁 Script executed:
cat -n src/main/kotlin/dsm/pick2024/domain/devicetoken/domain/UserDeviceToken.ktRepository: DSM-PICK/PiCK_CORE_SERVER
Length of output: 471
🏁 Script executed:
cat -n src/main/kotlin/dsm/pick2024/domain/devicetoken/domain/AdminDeviceToken.ktRepository: DSM-PICK/PiCK_CORE_SERVER
Length of output: 473
커밋 메시지와 실제 코드 구현이 불일치합니다.
커밋 메시지에서 "osType을 nullable하게 변경"이라고 명시했으나, UserDeviceTokenJpaEntity와 AdminDeviceTokenJpaEntity 모두에서 os 필드는 nullable = false로 설정되어 있습니다. 도메인 클래스(UserDeviceToken, AdminDeviceToken)에서도 os: OSType은 non-nullable입니다. 의도된 변경 사항인지, 또는 커밋 메시지가 잘못되었는지 확인이 필요합니다.
🤖 Prompt for AI Agents
In
`@src/main/kotlin/dsm/pick2024/domain/devicetoken/entity/UserDeviceTokenJpaEntity.kt`
around lines 21 - 23, Commit message and code disagree about making os nullable;
inspect intent and either make os nullable across JPA and domain or revert the
commit. If making nullable, change the JPA entities UserDeviceTokenJpaEntity and
AdminDeviceTokenJpaEntity by setting `@Column`(name = "os", nullable = true) and
update the Kotlin property type from OSType to OSType? (nullable) and adjust
constructors/mappers that convert between the JPA entities and domain classes;
also update the domain classes UserDeviceToken and AdminDeviceToken to use
OSType? and handle possible null values in any code that reads os. If the commit
was wrong, instead revert the commit so os remains non-nullable everywhere
(leave `@Column`(nullable = false) and domain types as OSType).
There was a problem hiding this comment.
entity 저장단계에서는 null인 애들은 저장이 되지 않고, 실제 값이 있는 애들만 저장이 되는 로직이라 이렇게 작성됨
There was a problem hiding this comment.
@1210thatman 설명 감사합니다!
저장 로직에서 null 값을 필터링하여 실제 값이 있는 경우만 저장하는 구조이므로, 엔티티 레벨에서는 nullable = false로 설정하는 것이 맞네요. API 요청 단계에서는 nullable로 받지만, 실제 DB에 저장되는 시점에는 항상 값이 존재하도록 보장하는 설계 방식으로 이해했습니다.
커밋 메시지는 아마도 요청 DTO 레벨에서의 nullable 변경을 의미하는 것으로 보이며, 엔티티 제약 조건은 의도대로 non-nullable을 유지하고 있습니다. 👍
📝 Learnings were identified and not saved (knowledge base disabled). Enable
…PICK/PiCK_CORE_SERVER into 629-devicetoken을-별도-테이블로-분리
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/kotlin/dsm/pick2024/domain/application/service/ChangeApplicationStatusService.kt (1)
44-48:⚠️ Potential issue | 🟡 Minor빈
deviceTokens리스트 처리 최적화 권장
deviceTokens가 비어있을 경우 불필요한 Outbox 레코드가 생성됩니다. 프로세서들이 빈 토큰 리스트를 처리할 때 OutboxFacade에서 검증 없이 Outbox 레코드를 생성하므로, 실제 발송할 토큰이 없어도 레코드가 저장됩니다. 호출 전에deviceTokens.isEmpty()체크를 추가하여 불필요한 Outbox 레코드 생성을 방지하는 것을 권장합니다.src/main/kotlin/dsm/pick2024/domain/earlyreturn/service/CreateEarlyReturnService.kt (2)
63-72:⚠️ Potential issue | 🟡 Minor
tokens가 빈 리스트일 경우 알림이 무시됩니다.
findAllByAdminId가 빈 리스트를 반환하면forEach블록이 실행되지 않아 알림이 조용히 누락됩니다. 관리자에게 등록된 디바이스 토큰이 없는 경우, 의도된 동작인지 확인이 필요합니다. 최소한 로그를 남기거나, 비즈니스 요건에 따라 예외 처리를 고려해 주세요.💡 빈 토큰 목록에 대한 로깅 추가 제안
val tokens = queryAdminDeviceTokenPort.findAllByAdminId(admin.id) .map { it.deviceToken } + if (tokens.isEmpty()) { + log.warn { "No device tokens found for admin ${admin.id}, skipping notification" } + } + tokens.forEach { token ->
log인스턴스가 없다면 companion object에 로거를 추가해야 합니다.
58-72:⚠️ Potential issue | 🟠 Major@transactional 내에서 관리자 조회 실패 시 조기귀가 신청이 함께 롤백됩니다.
Line 43-57에서 Application을 저장한 후 Line 58에서
findByGradeAndClassNumOrThrow가 예외를 던지면 전체 트랜잭션이 롤백되어 학생의 신청 데이터가 저장되지 않습니다. 이 패턴은 git 히스토리상 오래전부터 존재해왔으나, 알림 전송 실패(관리자 미존재)가 신청 저장 자체를 무효화하는 것이 의도된 설계인지 재검토가 필요합니다. 신청 저장과 알림 전송을 분리하거나(예: 별도 트랜잭션), 또는 알림 조회 실패를 안전하게 처리할지 결정하시기 바랍니다.
🤖 Fix all issues with AI agents
In
`@src/main/kotlin/dsm/pick2024/domain/user/presentation/dto/request/UserSignUpRequest.kt`:
- Around line 30-31: The UserSignUpRequest currently allows deviceToken and os
to be set independently which causes the UserSignUpService's nested ?.let chain
to silently drop the token when one is null; update validation so these fields
are consistent: in the DTO (UserSignUpRequest) add a custom validator (or use a
class-level constraint) that enforces both deviceToken and os must be either
both non-null or both null, or alternatively update UserSignUpService to
explicitly check the pair (deviceToken and os/OSType) and throw/return a
validation error when only one is present; reference the fields deviceToken and
os and the UserSignUpService entry point when implementing the check.
In `@src/main/kotlin/dsm/pick2024/domain/user/service/UserLoginService.kt`:
- Around line 34-46: 중첩된 let 블록을 제거하고 request.deviceToken과 request.os가 모두
non-null일 때만 UserDeviceToken을 생성해 saveUserDeviceTokenPort.save()를 호출하도록 간소화하세요:
UserLoginService의 해당 블록에서 nested let 대신 if (request.deviceToken != null &&
request.os != null) { val userDeviceToken = UserDeviceToken(...);
saveUserDeviceTokenPort.save(userDeviceToken) } 같은 단일 조건문으로 바꾸고, 중복 토큰 제거는 이미
UserDeviceTokenPersistenceAdapter.save()/findByDeviceToken()에서 처리된다는 점을 유지하세요.
In
`@src/main/kotlin/dsm/pick2024/domain/weekendmeal/service/NotificationWeekendMealService.kt`:
- Around line 34-50: The loop currently calls
weekendMealFinderUseCase.findByUserIdOrThrow inside users.forEach which lets a
WeekendMealNotFoundException abort notifications for all users; fix by isolating
failures per user — either wrap the per-user logic (the call to
weekendMealFinderUseCase.findByUserIdOrThrow and subsequent token/send steps) in
a try-catch that logs and continues, or replace the throwy call with a
non-throwing lookup (use queryWeekendMealPort.findByUserId) and skip users with
no weekend meal record; ensure tokens are still fetched via
queryUserDeviceTokenPort.findAllByUserId and notifications sent via
outboxFacadeUseCase.sendNotification only for users with valid weekend meal
data.
🧹 Nitpick comments (6)
src/main/kotlin/dsm/pick2024/domain/devicetoken/entity/UserDeviceTokenJpaEntity.kt (1)
11-11:@Entity(name=...)대신@Table(name=...)을 사용하는 것이 권장됩니다.현재
@Entity(name = "tbl_user_device_token")은 JPA 엔티티 이름을 테이블 이름으로 설정하고 있습니다. 이 방식은 동작하지만, JPQL 쿼리에서 엔티티를 참조할 때tbl_user_device_token이라는 이름을 사용해야 합니다. 표준 관행은@Table로 테이블 이름을 지정하고,@Entity는 클래스 이름을 그대로 사용하는 것입니다.♻️ 수정 제안
+import javax.persistence.Table + -@Entity(name = "tbl_user_device_token") +@Entity +@Table(name = "tbl_user_device_token") class UserDeviceTokenJpaEntity(src/main/kotlin/dsm/pick2024/domain/user/service/UserSignUpService.kt (1)
53-66: 중첩된?.let체인의 가독성과 안전성 개선이 필요합니다.현재
os가 null이면UserDeviceToken생성이 건너뛰어지면서deviceToken이 조용히 무시됩니다. 의도적인 동작인지 확인이 필요하며, 가독성 측면에서도 중첩let보다 명시적 조건문이 더 명확합니다.♻️ 가독성 개선을 위한 리팩터링 제안
- request.deviceToken?.let { token -> - request.os?.let { - UserDeviceToken( - id = UUID.randomUUID(), - userId = savedUser.id, - deviceToken = token, - os = it - ) - }?.let { - saveUserDeviceTokenPort.save( - it - ) - } - } + val token = request.deviceToken + val os = request.os + if (token != null && os != null) { + saveUserDeviceTokenPort.save( + UserDeviceToken( + id = UUID.randomUUID(), + userId = savedUser.id, + deviceToken = token, + os = os + ) + ) + }src/main/kotlin/dsm/pick2024/domain/application/service/ChangeApplicationStatusService.kt (1)
40-43: 중복 제거가 잘 반영되었습니다. 다만, 동일 userId에 대한 반복 쿼리 최적화를 고려해 주세요.
.distinct()로 중복 디바이스 토큰 문제가 해결되었습니다. 그러나request.idList에 동일 사용자의 신청이 여러 건 포함될 경우, 같은userId로findAllByUserId가 중복 호출됩니다. userId를 먼저 deduplicate하면 불필요한 DB 호출을 줄일 수 있습니다.♻️ 선택적 최적화 제안
- val deviceTokens = applications.flatMap { application -> - queryUserDeviceTokenPort.findAllByUserId(application.userId) - .map { it.deviceToken } - }.distinct() + val deviceTokens = applications.map { it.userId }.distinct() + .flatMap { userId -> + queryUserDeviceTokenPort.findAllByUserId(userId) + .map { it.deviceToken } + }src/main/kotlin/dsm/pick2024/domain/weekendmeal/service/NotificationWeekendMealService.kt (1)
38-49: 사용자별 디바이스 토큰 조회가 N+1 쿼리 패턴입니다.
findAll()로 전체 사용자를 가져온 뒤, 각 사용자마다findAllByUserId를 개별 호출하고 있어 사용자 수(N)만큼 추가 쿼리가 발생합니다. 사용자 수가 많아지면 성능에 영향을 줄 수 있습니다.모든 디바이스 토큰을 한 번에 조회(
findAll또는findAllByUserIds)한 뒤, userId 기준으로 그룹핑하는 방식을 고려해 보세요.Also applies to: 55-64
src/main/kotlin/dsm/pick2024/domain/user/service/UserLoginService.kt (2)
15-15: 와일드카드 import 대신 명시적 import를 권장합니다.
java.util.*대신java.util.UUID만 import하면 의도가 더 명확해집니다.♻️ 제안
-import java.util.* +import java.util.UUID
22-23: 불필요한userSavePort의존성 제거
userSavePort가 이 서비스에서 사용되지 않습니다. 생성자에 주입되지만execute()메서드 내에서 참조되는 곳이 없으므로 제거해 주세요.
| val deviceToken: String?, | ||
| val os: OSType? |
There was a problem hiding this comment.
deviceToken과 os의 일관성 검증이 없습니다.
deviceToken만 제공되고 os가 null인 경우(또는 반대), UserSignUpService에서 중첩된 ?.let 체인으로 인해 디바이스 토큰이 조용히 무시됩니다. 두 필드가 항상 함께 존재하거나 함께 null이어야 한다면, 커스텀 유효성 검증을 추가하거나 서비스 레이어에서 명시적으로 처리하는 것이 좋습니다.
🤖 Prompt for AI Agents
In
`@src/main/kotlin/dsm/pick2024/domain/user/presentation/dto/request/UserSignUpRequest.kt`
around lines 30 - 31, The UserSignUpRequest currently allows deviceToken and os
to be set independently which causes the UserSignUpService's nested ?.let chain
to silently drop the token when one is null; update validation so these fields
are consistent: in the DTO (UserSignUpRequest) add a custom validator (or use a
class-level constraint) that enforces both deviceToken and os must be either
both non-null or both null, or alternatively update UserSignUpService to
explicitly check the pair (deviceToken and os/OSType) and throw/return a
validation error when only one is present; reference the fields deviceToken and
os and the UserSignUpService entry point when implementing the check.
There was a problem hiding this comment.
deviceToken을 넘기지 않더라도 os는 넘어와
src/main/kotlin/dsm/pick2024/domain/weekendmeal/service/NotificationWeekendMealService.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/kotlin/dsm/pick2024/domain/weekendmeal/service/NotificationWeekendMealService.kt (1)
35-55: QueryUserDeviceTokenPort와 WeekendMealFinderUseCase에 일괄 조회 메서드 추가를 고려하세요.현재 코드에서 사용자 수(N)만큼 N+1 쿼리 문제가 발생합니다:
- 라인 37:
weekendMealFinderUseCase.findByUserIdOrThrow(user.id)— 각 사용자마다 호출- 라인 43, 60:
queryUserDeviceTokenPort.findAllByUserId(user.id)— 각 사용자마다 호출QueryUserDeviceTokenPort에
findAllByUserIds(userIds: List<UUID>)메서드를, WeekendMealFinderUseCase에findByUserIds(userIds: List<UUID>)메서드를 추가한 후, 루프 진입 전에 모든 데이터를 일괄 조회하는 방식으로 리팩터링하면 N+1 쿼리 문제를 해결할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/dsm/pick2024/domain/weekendmeal/service/NotificationWeekendMealService.kt` around lines 35 - 55, This loop causes N+1 queries; add batch methods QueryUserDeviceTokenPort.findAllByUserIds(userIds: List<UUID>) and WeekendMealFinderUseCase.findByUserIds(userIds: List<UUID>), then before the users.forEach collect userIds and call those two methods to fetch all weekend meal statuses and device tokens in one go, convert their results into lookup maps (e.g., userId -> Status and userId -> List<deviceToken>), and update NotificationWeekendMealService.send logic to use the maps (skip users missing a weekend meal entry as the current catch does) so you no longer call findByUserIdOrThrow or findAllByUserId inside the per-user loop.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@src/main/kotlin/dsm/pick2024/domain/weekendmeal/service/NotificationWeekendMealService.kt`:
- Around line 28-29: You created a local val today = LocalDate.now() but still
call weekendMealPeriodPort.queryWeekendPeriodByDate(LocalDate.now()); change
that call to use the today variable instead (i.e.,
weekendMealPeriodPort.queryWeekendPeriodByDate(today)) so the same date instance
is used for period lookup and subsequent when/branch logic in
NotificationWeekendMealService (ensure any other LocalDate.now() duplicates in
that method are likewise replaced with today).
---
Nitpick comments:
In
`@src/main/kotlin/dsm/pick2024/domain/weekendmeal/service/NotificationWeekendMealService.kt`:
- Around line 35-55: This loop causes N+1 queries; add batch methods
QueryUserDeviceTokenPort.findAllByUserIds(userIds: List<UUID>) and
WeekendMealFinderUseCase.findByUserIds(userIds: List<UUID>), then before the
users.forEach collect userIds and call those two methods to fetch all weekend
meal statuses and device tokens in one go, convert their results into lookup
maps (e.g., userId -> Status and userId -> List<deviceToken>), and update
NotificationWeekendMealService.send logic to use the maps (skip users missing a
weekend meal entry as the current catch does) so you no longer call
findByUserIdOrThrow or findAllByUserId inside the per-user loop.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/kotlin/dsm/pick2024/domain/user/service/UserLoginService.kt (1)
24-46:⚠️ Potential issue | 🟠 Major디바이스 토큰 저장 실패 시 로그인 전체가 롤백됩니다.
@Transactional이 선언된execute메서드 내부에서saveUserDeviceTokenPort.save()를 호출하고 있습니다. 데이터베이스 제약 조건 위반, 연결 오류 등 어떤 예외라도 발생하면 트랜잭션 전체가 롤백되어jwtTokenProvider.generateToken()이 실행되지 않고 사용자가 로그인에 실패합니다. 디바이스 토큰 저장은 로그인의 핵심 경로가 아니므로, 저장 실패가 로그인을 막아서는 안 됩니다.권장 접근 방법은 두 가지입니다:
방법 1 – 예외를 로컬에서 흡수 (가장 간단)
🛡️ 제안 수정안 (try-catch)
val token = request.deviceToken val os = request.os if (token != null && os != null) { + try { saveUserDeviceTokenPort.save( UserDeviceToken( id = UUID.randomUUID(), userId = user.id, deviceToken = token, os = os ) ) + } catch (e: Exception) { + log.warn("디바이스 토큰 저장 실패: userId={}, error={}", user.id, e.message) + } }방법 2 – 어댑터
save()에REQUIRES_NEW전파 설정 (부모 트랜잭션과 분리)
UserDeviceTokenPersistenceAdapter의save()메서드에@Transactional(propagation = Propagation.REQUIRES_NEW)를 적용하면, 디바이스 토큰 저장이 독립 트랜잭션으로 처리되어 실패해도 로그인 트랜잭션이 보호됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/dsm/pick2024/domain/user/service/UserLoginService.kt` around lines 24 - 46, The execute method in UserLoginService currently calls saveUserDeviceTokenPort.save(...) inside the `@Transactional` boundary so any exception during device token persistence rolls back the whole login; fix by making device-token persistence non-fatal: either wrap the saveUserDeviceTokenPort.save(...) call in a local try-catch that logs errors and swallows exceptions so jwtTokenProvider.generateToken(request.accountId, user.role.name) still runs, or change the adapter implementation (e.g., UserDeviceTokenPersistenceAdapter.save) to use a separate transaction with `@Transactional`(propagation = Propagation.REQUIRES_NEW) so failures there do not affect the parent execute transaction; update references to UserDeviceToken and saveUserDeviceTokenPort accordingly and ensure errors are logged for diagnostics.
🧹 Nitpick comments (3)
src/main/kotlin/dsm/pick2024/domain/user/service/UserLoginService.kt (1)
14-14: 와일드카드 임포트 대신 명시적 임포트를 사용하세요.
import java.util.*대신 실제로 사용되는UUID만 명시적으로 임포트하는 것이 Kotlin 관례에 부합합니다.♻️ 제안 수정안
-import java.util.* +import java.util.UUID🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/dsm/pick2024/domain/user/service/UserLoginService.kt` at line 14, The file UserLoginService.kt currently uses a wildcard import `import java.util.*`; replace it with the explicit import `import java.util.UUID` (or the specific types actually used) so only UUID is imported, and then run an import/optimize step to remove any unused imports; target the import statement at the top of UserLoginService.kt and ensure any references to UUID remain unchanged.src/main/kotlin/dsm/pick2024/domain/weekendmeal/finder/WeekendMealFinder.kt (1)
20-22: 표현식 바디(expression body)로 간소화 가능기존
findByUserIdOrThrow와 동일한 스타일로 표현식 바디를 사용하면 더 간결합니다.♻️ 제안 리팩터링
- override fun findByUserIds(userIds: List<UUID>): List<WeekendMeal> { - return queryWeekendMealPort.findAllByUserIds(userIds) - } + override fun findByUserIds(userIds: List<UUID>): List<WeekendMeal> = + queryWeekendMealPort.findAllByUserIds(userIds)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/dsm/pick2024/domain/weekendmeal/finder/WeekendMealFinder.kt` around lines 20 - 22, 함수 본문을 블록 바디에서 표현식 바디로 간결하게 바꿔주세요: 현재의 `findByUserIds(userIds: List<UUID>): List<WeekendMeal>` 구현을 중괄호와 return문 대신 표현식 바디로 축약하고 `queryWeekendMealPort.findAllByUserIds(userIds)`를 바로 반환하도록 수정하세요 (참조 심볼: findByUserIds, queryWeekendMealPort.findAllByUserIds).src/main/kotlin/dsm/pick2024/domain/weekendmeal/service/NotificationWeekendMealService.kt (1)
27-34: 기간 내 start/end가 아닌 날에도 불필요한 DB 쿼리 실행
queryWeekendPeriodByDate(today)는 오늘이 신청 기간 안에 속하면 period를 반환합니다. 즉, 기간 중간의 날(start도 end도 아닌 날)에도queryUserPort.findAll()과queryUserDeviceTokenPort.findByUserIds(userIds)가 실행됩니다. 스케줄러가 매일 실행된다면 기간 내 대부분의 날 동안 전체 사용자·토큰 조회가 낭비됩니다.
when분기 진입 조건에 해당하지 않을 때는 일찍 반환하도록 개선하세요.♻️ 제안 리팩터링
override fun execute() { val today = LocalDate.now() val weekendMealPeriod = weekendMealPeriodPort.queryWeekendPeriodByDate(today) ?: return + if (today != weekendMealPeriod.start && today != weekendMealPeriod.end) return + val users = queryUserPort.findAll() val userIds = users.map { it.id } val deviceTokensByUserId = queryUserDeviceTokenPort.findByUserIds(userIds) .groupBy({ it.userId }, { it.deviceToken })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/dsm/pick2024/domain/weekendmeal/service/NotificationWeekendMealService.kt` around lines 27 - 34, The code currently calls queryUserPort.findAll() and queryUserDeviceTokenPort.findByUserIds(userIds) immediately after queryWeekendPeriodByDate(today), which causes full user/token queries on every day inside the period; change the flow in NotificationWeekendMealService so you first call queryWeekendPeriodByDate(today) to get weekendMealPeriod, then check whether today is one of the trigger days (e.g., weekendMealPeriod.start or weekendMealPeriod.end) and return early if not; only when the condition matches, invoke queryUserPort.findAll() and queryUserDeviceTokenPort.findByUserIds(userIds) and proceed with grouping device tokens—move those calls below the when/conditional that handles start/end to avoid unnecessary DB queries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/kotlin/dsm/pick2024/domain/devicetoken/persistence/UserDeviceTokenPersistenceAdapter.kt`:
- Around line 20-23: The method findByUserIds currently passes userIds directly
to userDeviceTokenRepository.findAllByUserIdIn which will generate an invalid
SQL when given an empty list; add an early-return guard in
UserDeviceTokenPersistenceAdapter.findByUserIds that checks if userIds.isEmpty()
and returns emptyList() before calling
userDeviceTokenRepository.findAllByUserIdIn, then map results with
userDeviceTokenMapper.toDomain as before; ensure callers such as
NotificationWeekendMealService (which uses queryUserPort.findAll()) still work
when an empty list is returned.
- Around line 25-43: The save/resolveEntity flow can produce TOCTOU duplicates
because findByDeviceToken and repository.save are separate operations; add a
database-level uniqueness constraint by marking the deviceToken column as unique
in UserDeviceTokenJpaEntity (e.g., set unique = true on the `@Column` for
deviceToken) to prevent duplicate rows, and apply the same change to
AdminDeviceTokenJpaEntity; no behavioral change needed in save or resolveEntity
— rely on the DB constraint to enforce uniqueness and handle any resulting
DataIntegrityViolationException at the repository/service layer if necessary.
---
Outside diff comments:
In `@src/main/kotlin/dsm/pick2024/domain/user/service/UserLoginService.kt`:
- Around line 24-46: The execute method in UserLoginService currently calls
saveUserDeviceTokenPort.save(...) inside the `@Transactional` boundary so any
exception during device token persistence rolls back the whole login; fix by
making device-token persistence non-fatal: either wrap the
saveUserDeviceTokenPort.save(...) call in a local try-catch that logs errors and
swallows exceptions so jwtTokenProvider.generateToken(request.accountId,
user.role.name) still runs, or change the adapter implementation (e.g.,
UserDeviceTokenPersistenceAdapter.save) to use a separate transaction with
`@Transactional`(propagation = Propagation.REQUIRES_NEW) so failures there do not
affect the parent execute transaction; update references to UserDeviceToken and
saveUserDeviceTokenPort accordingly and ensure errors are logged for
diagnostics.
---
Nitpick comments:
In `@src/main/kotlin/dsm/pick2024/domain/user/service/UserLoginService.kt`:
- Line 14: The file UserLoginService.kt currently uses a wildcard import `import
java.util.*`; replace it with the explicit import `import java.util.UUID` (or
the specific types actually used) so only UUID is imported, and then run an
import/optimize step to remove any unused imports; target the import statement
at the top of UserLoginService.kt and ensure any references to UUID remain
unchanged.
In `@src/main/kotlin/dsm/pick2024/domain/weekendmeal/finder/WeekendMealFinder.kt`:
- Around line 20-22: 함수 본문을 블록 바디에서 표현식 바디로 간결하게 바꿔주세요: 현재의
`findByUserIds(userIds: List<UUID>): List<WeekendMeal>` 구현을 중괄호와 return문 대신 표현식
바디로 축약하고 `queryWeekendMealPort.findAllByUserIds(userIds)`를 바로 반환하도록 수정하세요 (참조
심볼: findByUserIds, queryWeekendMealPort.findAllByUserIds).
In
`@src/main/kotlin/dsm/pick2024/domain/weekendmeal/service/NotificationWeekendMealService.kt`:
- Around line 27-34: The code currently calls queryUserPort.findAll() and
queryUserDeviceTokenPort.findByUserIds(userIds) immediately after
queryWeekendPeriodByDate(today), which causes full user/token queries on every
day inside the period; change the flow in NotificationWeekendMealService so you
first call queryWeekendPeriodByDate(today) to get weekendMealPeriod, then check
whether today is one of the trigger days (e.g., weekendMealPeriod.start or
weekendMealPeriod.end) and return early if not; only when the condition matches,
invoke queryUserPort.findAll() and
queryUserDeviceTokenPort.findByUserIds(userIds) and proceed with grouping device
tokens—move those calls below the when/conditional that handles start/end to
avoid unnecessary DB queries.
...main/kotlin/dsm/pick2024/domain/devicetoken/persistence/UserDeviceTokenPersistenceAdapter.kt
Show resolved
Hide resolved
...main/kotlin/dsm/pick2024/domain/devicetoken/persistence/UserDeviceTokenPersistenceAdapter.kt
Outdated
Show resolved
Hide resolved
| val password: String, | ||
| val deviceToken: String? | ||
| val deviceToken: String?, | ||
| val os: OSType? |
There was a problem hiding this comment.
os를 nullable type으로 설정하신 이유가 있을까요? 저는 nullable이 아니어도 괜찮을 것 같습니다.
There was a problem hiding this comment.
os의 경우 시뮬레이터에서 값을 넘겨줄 수 없다고 생각해서 저렇게 했던 것 같습니다.
수정하겠습니다
| request.deviceToken?.let { token -> | ||
| request.os?.let { | ||
| AdminDeviceToken( | ||
| adminId = admin.id, | ||
| deviceToken = token, | ||
| os = it | ||
| ) | ||
| }?.let { | ||
| adminDeviceTokenPort.save( | ||
| it | ||
| ) |
There was a problem hiding this comment.
os 타입에서 nullable을 제거하면 let문도 한 겹 제거할 수 있을 것 같습니다.
There was a problem hiding this comment.
확인했습니다! 반영하겠습니다
| fun updateEntity( | ||
| existingEntity: AdminDeviceTokenJpaEntity, | ||
| domain: AdminDeviceToken | ||
| ): AdminDeviceTokenJpaEntity { | ||
| return AdminDeviceTokenJpaEntity( | ||
| id = existingEntity.id ?: throw EntityStateException, | ||
| adminId = existingEntity.adminId, | ||
| deviceToken = existingEntity.deviceToken, | ||
| os = existingEntity.os | ||
| ) | ||
| } |
There was a problem hiding this comment.
update에서 같은 id의 새 엔티티 객체를 생성하는 로직을 Mapper에 두신 이유가 있을까요? 기존 픽 코드에서는 해당 도메인에 update 메서드를 두고 기존 객체에서 바꿀 컬럼을 인자로 받아 copy()하는 방식을 사용하고 있습니다.
| domain: AdminDeviceToken | ||
| ): AdminDeviceTokenJpaEntity { | ||
| return AdminDeviceTokenJpaEntity( | ||
| id = existingEntity.id ?: throw EntityStateException, |
There was a problem hiding this comment.
조회했던 Entity의 id가 없는 상황은 고려하지 않아도 될 것 같습니다.. 애초에 DB에서 PK가 존재할 수 없는 경우는 없고 생기더라도 서버에서 값을 검증하는게 아닌 DB가 픽 서버 외엔 변경되지 않도록 만드는게 더 나을듯 합니다.
| override fun save(adminDeviceToken: AdminDeviceToken): AdminDeviceToken { | ||
| val token = adminDeviceToken.deviceToken | ||
|
|
||
| val entityToSave = resolveEntity(token, adminDeviceToken) | ||
|
|
||
| return adminDeviceTokenMapper.toDomain( | ||
| adminDeviceTokenRepository.save(entityToSave) | ||
| ) | ||
| } | ||
|
|
||
| private fun resolveEntity(token: String, domain: AdminDeviceToken): AdminDeviceTokenJpaEntity { | ||
| val existingEntity = adminDeviceTokenRepository.findByDeviceToken(token) | ||
|
|
||
| return if (existingEntity != null) { | ||
| val updatedDomain = adminDeviceTokenMapper.toDomain(existingEntity).update(domain.adminId) | ||
| adminDeviceTokenMapper.toEntity(updatedDomain) | ||
| } else { | ||
| adminDeviceTokenMapper.toEntity(domain) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
resolveEntity()가 단순 persistence 보조가 아니라, 토큰이 이미 존재할 때 소유자를 변경할지 여부를 결정하는 비즈니스 규칙까지 포함하고 있는 것 같습니다. 이런 정책은 adapter 내부보다는 application/domain 레이어에서 드러나게 두는 편이 역할 분리가 더 명확해 보입니다.
There was a problem hiding this comment.
확실히 domain 레이어에 있는 것이 더 적절하다고 생각되네요. 수정하겠습니다
| fun update(adminId: UUID): AdminDeviceToken { | ||
| return this.copy(adminId = adminId) | ||
| } |
There was a problem hiding this comment.
어떤 컬럼이 update되는지 메서드명에 드러나는게 좋아보입니다. udpate메서드를 사용하면 추후 다른 컬럼을 update하게 될 때 메서드간의 혼란이 올 수 있습니다.
2 participants
deviceToken을 별도 테이블로 분리하였습니다.
주요 작업 사항은 이슈번호 629번을 참고해주세요
폰으로 올린 pr이라 링크 첨부가 어려운 점 양해 부탁드립니다.
Summary by CodeRabbit
릴리스 노트
신규 기능
변경사항
문서/오류