-
Notifications
You must be signed in to change notification settings - Fork 0
[FEAT] 매칭 신청 API, 매칭 수락 API, 매칭 거절 API, 내가 보낸 매칭 요청 삭제 API 구현 #22
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
Conversation
📝 WalkthroughSummary by CodeRabbit릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings. Walkthrough매칭 요청·수락·거절·취소 API와 관련 서비스·리포지토리·알림/SSE 페이로드 및 이벤트 타입이 추가되었고, 인증에 타임존 전달이 도입되었으며 BaseEntity의 작성자/수정자 필드가 주석 처리되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Controller as MatchingController
participant Service as MatchingService
participant USP as UserSportProfileRepo
participant UR as UserRepo
participant MR as MatchingRepo
participant GR as GameReviewRepo
participant NTR as NotificationTemplateRepo
participant NS as NotificationService
participant OEP as OutboxPublisher
Client->>Controller: POST /api/v1/matchings/profiles/{receiverProfileId} (header: requesterUserId)
Controller->>Service: requestMatching(requesterUserId, receiverProfileId)
rect rgba(230,245,255,0.9)
Service->>USP: find receiverProfile (fetch)
Service->>UR: find receiver user
Service->>UR: find requester user
Service->>USP: find requesterProfile (userId,sportId fetch)
end
rect rgba(255,240,240,0.9)
Service->>MR: count today's matching between users
alt over limit
Service-->>Controller: throw MATCHING_DAILY_LIMIT_EXCEEDED
else
Service->>MR: save(Matching.createRequested(...))
end
end
rect rgba(235,255,235,0.9)
Service->>GR: count reviews for requester on sport
Service->>NTR: find template MATCHING_REQUESTED
Service->>NS: createMatchingRequested(receiver, requesterProfile)
NS-->>Service: saved notification
end
rect rgba(255,250,230,0.9)
Service->>OEP: publish MATCHING_RECEIVED payload (sse)
Service->>OEP: publish MATCHING_REQUEST_NOTIFICATION_CREATED payload (sse)
end
Service-->>Controller: 200 OK
Controller-->>Client: 응답
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
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.
Actionable comments posted: 3
🤖 Fix all issues with AI Agents
In
@src/main/kotlin/org/appjam/smashing/domain/matching/controller/MatchingController.kt:
- Around line 30-43: The endpoint is using an insecure @RequestHeader("userId");
replace it with server-validated authentication by re-enabling the commented
parameter: add the parameter @AuthenticationPrincipal principal:
CustomUserDetails to the requestMatching method signature, remove the
@RequestHeader("userId") requesterUserId parameter, and pass principal.username
as requesterUserId into matchingService.requestMatching (keep
receiverProfileId). Also remove the related TODO comments so the controller
relies solely on principal for identity.
In
@src/main/kotlin/org/appjam/smashing/domain/matching/service/MatchingService.kt:
- Around line 106-107: The hardcoded ZoneId("Asia/Seoul") in MatchingService
should be externalized; replace the inline val zoneId usage with a configurable
source (e.g., inject a ZoneId from application properties using @Value or a
configuration bean, read from an environment variable, or accept ZoneId as a
parameter in the matching method/constructor), provide a sensible fallback such
as ZoneId.systemDefault(), and keep a note to switch back to
SecurityUtils.currentZoneId() once auth is restored; update references to zoneId
in MatchingService accordingly.
In
@src/main/kotlin/org/appjam/smashing/domain/notification/entity/Notification.kt:
- Around line 61-72: The params JSON is being built via string interpolation in
createMatchingRequested which will break if requesterProfile.user.nickname or
requesterProfile.tier.name contains quotes, backslashes or special unicode; fix
by constructing a small data object/map (e.g., mapOf("requesterNickname" to
requesterProfile.user.nickname, "requesterTierName" to
requesterProfile.tier.name)) and serializing it with a JSON library
(ObjectMapper or kotlinx.serialization) and pass the resulting safe JSON string
into the Notification(params = ...). Ensure you update createMatchingRequested
to use the serializer and handle/propagate any checked serialization exceptions
as appropriate.
🧹 Nitpick comments (6)
src/main/kotlin/org/appjam/smashing/domain/common/entity/BaseEntity.kt (1)
26-43: 코드 주석 처리 대신 더 나은 접근 방식을 고려하세요.
createdBy와updatedBy필드를 주석 처리하는 것은 임시 방편으로는 괜찮지만, 장기적으로는 다음과 같은 대안을 고려해보세요:
- 감사 필드가 있는/없는 별도의 베이스 클래스 생성 (예:
BaseEntityWithAudit,BaseEntityWithoutAudit)@MappedSuperclass계층 구조를 활용하여 선택적 감사 지원- 설정 기반 접근 방식으로 런타임에 감사 활성화/비활성화
이렇게 하면 코드 주석을 제거하고 더 명확한 의도를 표현할 수 있으며, 나중에 인증/인가 구현 시 주석 해제를 잊어버리는 실수를 방지할 수 있습니다.
💡 별도 베이스 클래스 접근 방식 예시
감사 필드가 없는 기본 베이스 클래스:
@EntityListeners(AuditingEntityListener::class) @MappedSuperclass abstract class BaseTimeEntity { @CreatedDate @Column(name = "created_at", nullable = false, updatable = false, columnDefinition = "DATETIME") @ColumnDefault("CURRENT_TIMESTAMP") @Comment("등록일자") lateinit var createdAt: LocalDateTime protected set @LastModifiedDate @Column(name = "updated_at", nullable = false, columnDefinition = "DATETIME") @ColumnDefault("CURRENT_TIMESTAMP") @Comment("수정일자") lateinit var updatedAt: LocalDateTime protected set @Column(name = "deleted_at", insertable = false, updatable = false, columnDefinition = "DATETIME") @Comment("삭제일자") lateinit var deletedAt: LocalDateTime protected set }인증/인가 구현 후 사용할 전체 감사 베이스 클래스:
@EntityListeners(AuditingEntityListener::class) @MappedSuperclass abstract class BaseEntity : BaseTimeEntity() { @CreatedBy @Column(name = "created_by", nullable = false, updatable = false, length = 50) @Comment("등록자") lateinit var createdBy: String protected set @LastModifiedBy @Column(name = "updated_by", nullable = false, length = 50) @Comment("수정자") lateinit var updatedBy: String protected set }src/main/kotlin/org/appjam/smashing/domain/matching/service/MatchingService.kt (5)
42-42: Null 안전성 개선을 권장합니다.
receiverProfile.user.id!!처럼!!연산자를 사용하면 NPE 위험이 있습니다. 데이터베이스에서 조회한 엔티티의 ID는 일반적으로 null이 아니지만, 더 안전한 패턴을 사용하는 것이 좋습니다.🔎 안전한 null 처리 제안
- validateNotSelf(requesterUserId, receiverProfile.user.id!!) + val receiverUserId = requireNotNull(receiverProfile.user.id) { "Receiver user ID must not be null" } + validateNotSelf(requesterUserId, receiverUserId)
46-47: 더 구체적인 에러 코드 사용을 고려하세요.데이터베이스에서 로드된
sport.id가 null인 경우는 데이터 무결성 문제를 나타내므로, 일반적인BAD_REQUEST보다는 더 명확한 에러 코드(예:INTERNAL_SERVER_ERROR또는 전용 에러 코드)를 사용하는 것이 디버깅에 도움이 됩니다.
76-84: 일관된 null 처리 패턴을 적용하세요.라인 135에서는
requireNotNull을 사용하고 있지만, 여기서는 여전히!!연산자를 사용하고 있습니다. 코드베이스 전체에서 일관된 null 처리 패턴을 사용하는 것이 좋습니다.🔎 일관된 null 처리 제안
publishMatchingReceived( - receiverUserId = receiverProfile.user.id!!, + receiverUserId = requireNotNull(receiverProfile.user.id) { "Receiver user ID must not be null" }, matchingId = matchingId, requesterProfile = requesterProfile, reviewCount = reviewCount, ) publishNotificationCreated( - receiverUserId = receiverProfile.user.id!!, + receiverUserId = requireNotNull(receiverProfile.user.id) { "Receiver user ID must not be null" }, notificationId = notificationId, matchingId = matchingId, )
157-164: 또 다른!!연산자 사용이 발견되었습니다.라인 158의
requesterProfile.user.id!!도 더 안전한 null 처리로 개선할 수 있습니다.🔎 안전한 null 처리 제안
requester = MatchingReceivedPayload.MatchingRequesterSummary( - userId = requesterProfile.user.id!!, + userId = requireNotNull(requesterProfile.user.id) { "Requester user ID must not be null" }, nickname = requesterProfile.user.nickname, gender = requesterProfile.user.gender, tierName = requesterProfile.tier.name,
34-88: 전반적인 구조가 잘 설계되었습니다.매칭 요청 플로우가 논리적으로 잘 구성되어 있으며, 트랜잭션 처리, 유효성 검증, 알림 생성, 이벤트 발행이 적절하게 통합되어 있습니다. 다만 향후 매칭 수락/거절 등의 기능이 추가되면 서비스가 비대해질 수 있으니, 필요시 별도의 서비스나 파사드 패턴 적용을 고려하시기 바랍니다.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/main/kotlin/org/appjam/smashing/domain/common/entity/BaseEntity.ktsrc/main/kotlin/org/appjam/smashing/domain/matching/controller/MatchingController.ktsrc/main/kotlin/org/appjam/smashing/domain/matching/entity/Matching.ktsrc/main/kotlin/org/appjam/smashing/domain/matching/repository/MatchingRepository.ktsrc/main/kotlin/org/appjam/smashing/domain/matching/service/MatchingService.ktsrc/main/kotlin/org/appjam/smashing/domain/notification/entity/Notification.ktsrc/main/kotlin/org/appjam/smashing/domain/notification/repository/NotificationRepository.ktsrc/main/kotlin/org/appjam/smashing/domain/notification/repository/NotificationTemplateRepository.ktsrc/main/kotlin/org/appjam/smashing/domain/notification/service/NotificationService.ktsrc/main/kotlin/org/appjam/smashing/domain/outbox/dto/SsePayload.ktsrc/main/kotlin/org/appjam/smashing/domain/review/repository/GameReviewRepository.ktsrc/main/kotlin/org/appjam/smashing/domain/user/repository/UserRepository.ktsrc/main/kotlin/org/appjam/smashing/domain/user/repository/UserSportProfileRepository.ktsrc/main/kotlin/org/appjam/smashing/global/auth/jwt/components/JwtProvider.ktsrc/main/kotlin/org/appjam/smashing/global/auth/jwt/filter/JwtAuthenticationFilter.ktsrc/main/kotlin/org/appjam/smashing/global/auth/security/data/CustomUserDetails.ktsrc/main/kotlin/org/appjam/smashing/global/config/SecurityConfig.ktsrc/main/kotlin/org/appjam/smashing/global/config/TimeZoneProperties.ktsrc/main/kotlin/org/appjam/smashing/global/exception/ErrorCode.ktsrc/main/kotlin/org/appjam/smashing/global/util/SecurityUtils.kt
🧰 Additional context used
🧬 Code graph analysis (8)
src/main/kotlin/org/appjam/smashing/domain/notification/service/NotificationService.kt (2)
src/main/kotlin/org/appjam/smashing/domain/notification/entity/NotificationTemplate.kt (1)
strategy(7-27)src/main/kotlin/org/appjam/smashing/domain/notification/enums/NotificationType.kt (1)
MATCHING_REQUESTED(3-14)
src/main/kotlin/org/appjam/smashing/domain/matching/controller/MatchingController.kt (1)
src/main/kotlin/org/appjam/smashing/domain/matching/enums/MatchingStatus.kt (1)
REQUESTED(3-5)
src/main/kotlin/org/appjam/smashing/domain/matching/entity/Matching.kt (1)
src/main/kotlin/org/appjam/smashing/domain/matching/enums/MatchingStatus.kt (1)
REQUESTED(3-5)
src/main/kotlin/org/appjam/smashing/global/auth/security/data/CustomUserDetails.kt (1)
src/main/kotlin/org/appjam/smashing/global/auth/security/CustomUserDetails.kt (5)
userId(6-23)isAccountNonExpired(16-16)isAccountNonLocked(18-18)isCredentialsNonExpired(20-20)isEnabled(22-22)
src/main/kotlin/org/appjam/smashing/domain/matching/repository/MatchingRepository.kt (1)
src/main/kotlin/org/appjam/smashing/domain/matching/entity/Game.kt (1)
indexes(14-88)
src/main/kotlin/org/appjam/smashing/domain/matching/service/MatchingService.kt (6)
src/main/kotlin/org/appjam/smashing/domain/matching/enums/MatchingStatus.kt (1)
REQUESTED(3-5)src/main/kotlin/org/appjam/smashing/domain/matching/entity/GameResultSubmission.kt (1)
indexes(14-104)src/main/kotlin/org/appjam/smashing/domain/notification/enums/NotificationType.kt (1)
MATCHING_REQUESTED(3-14)src/main/kotlin/org/appjam/smashing/domain/matching/entity/Game.kt (1)
indexes(14-88)src/main/kotlin/org/appjam/smashing/domain/user/entity/UserSportProfile.kt (1)
name(12-69)src/main/kotlin/org/appjam/smashing/domain/review/entity/GameReview.kt (1)
indexes(13-76)
src/main/kotlin/org/appjam/smashing/global/config/TimeZoneProperties.kt (2)
src/main/kotlin/org/appjam/smashing/global/auth/jwt/config/JwtProperties.kt (1)
prefix(5-10)src/main/kotlin/org/appjam/smashing/SmashingApplication.kt (1)
@SpringBootApplication(7-9)
src/main/kotlin/org/appjam/smashing/domain/common/entity/BaseEntity.kt (2)
src/main/kotlin/org/appjam/smashing/global/config/JpaAuditingConfig.kt (2)
auditorAware(8-16)auditorAware(12-15)src/main/kotlin/org/appjam/smashing/domain/common/entity/BaseCreatedEntity.kt (1)
name(13-29)
🔇 Additional comments (22)
src/main/kotlin/org/appjam/smashing/domain/common/entity/BaseEntity.kt (1)
26-30: 이 리뷰 의견은 재고가 필요합니다.
BaseEntity와BaseCreatedEntity는 의도적으로 서로 다른 설계를 가지고 있으며, 이는 불일치가 아니라 명확한 아키텍처 의도입니다.
BaseEntity: 등록일자, 수정일자를 추적하는 일반적인 감사 기본 클래스 (createdBy/updatedBy는 인증 구현 대기 중)BaseCreatedEntity: 등록일자와 등록자만 추적하는 특화된 기본 클래스 (아웃박스 패턴용)현재
OutboxEvent만BaseCreatedEntity를 상속하며, 나머지 8개의 도메인 엔티티는BaseEntity를 상속합니다. 각 엔티티가 자신의 감사 요구사항에 맞는 기본 클래스를 사용하고 있으며,BaseEntity의 주석 처리된 createdBy/updatedBy는 명시적 TODO로 인증 구현 후 활성화될 예정입니다.이는 일관성 문제가 아니라 의도된 아키텍처 결정이므로 추가 조치가 필요하지 않습니다.
src/main/kotlin/org/appjam/smashing/global/exception/ErrorCode.kt (1)
42-51: LGTM!매칭 및 알림 도메인의 에러 코드가 기존 패턴을 잘 따르고 있습니다. HTTP 상태 코드도 적절하게 설정되어 있습니다.
src/main/kotlin/org/appjam/smashing/domain/user/repository/UserRepository.kt (1)
1-6: LGTM!표준 Spring Data JPA 리포지토리 패턴을 따르고 있습니다.
src/main/kotlin/org/appjam/smashing/global/config/TimeZoneProperties.kt (1)
5-8: LGTM!
JwtProperties와 동일한 패턴을 따르고 있습니다.application.yml에timezone.default-time-zone속성이 올바르게 설정되어 있는지 확인해 주세요.src/main/kotlin/org/appjam/smashing/domain/matching/entity/Matching.kt (1)
73-84: LGTM!팩토리 메서드가 깔끔하게 구현되었습니다.
MatchingStatus.REQUESTED상태로 초기화하고,@Tsid를 통해 ID가 자동 생성됩니다.src/main/kotlin/org/appjam/smashing/domain/notification/repository/NotificationRepository.kt (1)
1-6: LGTM!표준 Spring Data JPA 리포지토리 패턴을 따르고 있습니다.
src/main/kotlin/org/appjam/smashing/global/auth/security/data/CustomUserDetails.kt (1)
6-26: 중복 클래스 확인 불필요 - 승인합니다타임존 필드 추가가 적절히 구현되었습니다. 저장소에
CustomUserDetails클래스는 현재 경로(security/data/CustomUserDetails.kt)에 하나만 존재하므로 중복 클래스 우려는 없습니다.src/main/kotlin/org/appjam/smashing/global/auth/jwt/filter/JwtAuthenticationFilter.kt (2)
28-34: 타임존 기반 인증 처리가 올바르게 구현되었습니다.요청에서 타임존을 해석한 후 인증에 전달하는 흐름이 적절하며, 예외 발생 시 request attribute에 저장하여 진입점에서 처리할 수 있도록 한 구조가 좋습니다.
51-67: 타임존 해석 로직이 안전하게 구현되었습니다.헤더가 없거나 잘못된 경우 기본 타임존으로 폴백하는 처리가 적절합니다. TODO 주석에 언급된 대로 향후 DB UTC 전환 시 사용자별 타임존 처리가 필요할 수 있으니 유의하시기 바랍니다.
src/main/kotlin/org/appjam/smashing/domain/review/repository/GameReviewRepository.kt (1)
9-21: JPQL 쿼리가 정확하게 작성되었습니다.GameReview를 Game과 조인하여 특정 reviewee와 sport로 필터링하는 로직이 올바르며, 카운트 쿼리로서 효율적입니다.
src/main/kotlin/org/appjam/smashing/domain/notification/repository/NotificationTemplateRepository.kt (1)
7-9: 리포지토리 구현이 적절합니다.Spring Data JPA 네이밍 컨벤션을 따르고 있으며, nullable 반환 타입이 NotificationService에서 올바르게 처리되고 있습니다.
src/main/kotlin/org/appjam/smashing/domain/notification/service/NotificationService.kt (2)
19-32: 알림 생성 로직이 잘 구현되었습니다.템플릿 기반 알림 생성 패턴이 적절하며, 템플릿이 없을 경우 명확한 예외 처리가 되어 있습니다. 엔티티 팩토리 메서드를 활용한 생성 방식도 좋습니다.
34-34: @tsid 생성 전략이 적절히 구성되어 있어 현재 코드는 문제 없습니다.Notification 엔티티는
@Tsid어노테이션을 사용하여 ID를 자동으로 생성합니다. 이는 Hypersistence 라이브러리의 견고한 타임스탐프 기반 ID 생성 방식으로,save()이후 항상 id를 채웁니다. 따라서requireNotNull(notification.id)는 ID 생성 실패 같은 예외 상황을 적절히 처리합니다.src/main/kotlin/org/appjam/smashing/domain/matching/repository/MatchingRepository.kt (1)
10-26: 네이티브 쿼리로 soft delete를 우회하는 것이 의도적이며 올바릅니다.Matching 엔티티는 @SQLRestriction("deleted_at is null")과 @SQLDelete 애너테이션으로 soft delete가 설정되어 있으며, 이 메서드에서 nativeQuery = true를 사용하여 의도적으로 @SQLRestriction을 우회하고 있습니다. 메서드 이름 "IncludingDeleted"가 명확하게 이 동작을 문서화하고 있습니다.
이는 사용자가 매칭 요청을 삭제 후 즉시 재생성하여 일일 매칭 제한(같은 사용자 쌍당 최대 3회)을 우회하는 것을 방지하기 위한 올바른 구현입니다. 양방향 체크
(A->B or B->A)로직도 정확하게 구현되었습니다.src/main/kotlin/org/appjam/smashing/global/config/SecurityConfig.kt (1)
21-26: 타임존 지원 통합이 깔끔하게 구현되었습니다.
timeZoneProperties를 생성자에 추가하고JwtAuthenticationFilter에 전달하는 방식이 의존성 주입 패턴을 잘 따르고 있습니다.Also applies to: 52-52
src/main/kotlin/org/appjam/smashing/global/util/SecurityUtils.kt (2)
9-22: 현재 사용자 정보 조회 로직이 적절합니다.
SecurityContextHolder를 통한 안전한 캐스팅과 명시적인 인증 오류 처리가 잘 구현되어 있습니다.
24-28: 타임존 유효성 검증은 이미 구현되어 있습니다.JwtAuthenticationFilter.resolveTimeZone()메서드에서runCatching패턴을 사용하여ZoneId.of()의 유효성을 검증하고, 예외 발생 시 기본 타임존을 반환합니다. 따라서CustomUserDetails가 생성될 때 타임존 문자열은 항상 유효함이 보장되므로,SecurityUtils.currentZoneId()에서DateTimeException이 발생할 위험은 없습니다.src/main/kotlin/org/appjam/smashing/domain/user/repository/UserSportProfileRepository.kt (2)
9-23: JPQL 쿼리가 올바르게 작성되었습니다.
join fetch를 사용하여 N+1 문제를 방지하고, 필요한 연관 엔티티들을 효율적으로 로드합니다.
25-37: 일관된 페치 전략을 사용하고 있습니다.프로필 조회 시 필요한 모든 연관 엔티티를 함께 로드하여 매칭 서비스에서 효율적으로 사용할 수 있습니다.
src/main/kotlin/org/appjam/smashing/global/auth/jwt/components/JwtProvider.kt (1)
7-7: 타임존 파라미터 추가가 일관되게 적용되었습니다.
getAuthentication메서드 시그니처 변경과CustomUserDetails생성자 호출이 타임존 지원 기능과 일관성 있게 구현되었습니다. Import 경로 변경도 적절합니다.Also applies to: 35-38, 53-57
src/main/kotlin/org/appjam/smashing/domain/outbox/dto/SsePayload.kt (1)
16-31: 페이로드 구조가 잘 설계되었습니다.매칭 요청자 정보를 포함하는 중첩 데이터 클래스가 적절하게 추가되었으며, 모든 필드의 타입이 명확하고 null 안전성도 잘 유지되고 있습니다.
src/main/kotlin/org/appjam/smashing/domain/matching/service/MatchingService.kt (1)
112-116: 의도적인 설계이므로 추가 검증이 불필요합니다.
countTodayBetweenUsersIncludingDeleted메서드는 의도적으로 삭제된 매칭 요청도 포함합니다. 이는 메서드명의 "IncludingDeleted"와 네이티브 쿼리를 통한 SQLRestriction 우회로 확인됩니다. 이러한 설계는 일일 제한(3건)을 통해 매칭 요청 삭제 후 재요청을 방지하는 안티 어뷰스 메커니즘으로 보입니다. 또한 쿼리의 OR 조건으로 양방향(A→B와 B→A) 카운트가 모두 포함됩니다.
src/main/kotlin/org/appjam/smashing/domain/matching/service/MatchingService.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/org/appjam/smashing/domain/notification/entity/Notification.kt
Show resolved
Hide resolved
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/kotlin/org/appjam/smashing/domain/matching/service/MatchingService.kt (1)
43-43: 명시적 null 체크 사용을 권장합니다.여러 곳에서
!!연산자를 사용하고 있습니다. 데이터베이스에서 조회한 엔티티의 ID는 일반적으로 null이 아니지만,!!대신requireNotNull()을 사용하면 NPE 발생 시 더 명확한 오류 메시지를 제공할 수 있습니다.♻️ 개선 제안
예시:
-val sportId = receiverProfile.sport.id ?: throw CustomException(ErrorCode.BAD_REQUEST) +val sportId = requireNotNull(receiverProfile.sport.id) { "Sport ID must not be null" } -userId = receiverProfile.user.id!!, +userId = requireNotNull(receiverProfile.user.id) { "User ID must not be null" },Line 141에서는 이미
requireNotNull(matching.id)를 사용하고 있으니, 일관성을 위해 다른 곳도 동일한 패턴을 적용하는 것을 고려해보세요.Also applies to: 47-47, 53-53, 77-77, 87-87, 168-168, 171-171, 198-198, 200-200
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/kotlin/org/appjam/smashing/domain/matching/service/MatchingService.ktsrc/main/kotlin/org/appjam/smashing/domain/outbox/dto/SsePayload.ktsrc/main/kotlin/org/appjam/smashing/domain/outbox/enums/SseEventType.kt
🔇 Additional comments (5)
src/main/kotlin/org/appjam/smashing/domain/outbox/enums/SseEventType.kt (1)
14-16: LGTM!새로운 SSE 이벤트 타입이 기존 패턴과 일관되게 추가되었습니다.
src/main/kotlin/org/appjam/smashing/domain/outbox/dto/SsePayload.kt (2)
19-33: LGTM!
MatchingReceivedPayload에 신청자 상세 정보를 포함시켜 클라이언트가 추가 API 호출 없이 UI를 구성할 수 있도록 한 설계가 적절합니다.
61-76: LGTM!알림 생성 이벤트를 위한 새로운 페이로드가 적절하게 정의되었습니다.
RequesterSummary가MatchingRequesterSummary보다 간소화된 것은 알림 이벤트의 목적에 맞게 필요한 정보만 포함한 설계입니다.src/main/kotlin/org/appjam/smashing/domain/matching/service/MatchingService.kt (2)
36-94: LGTM!
requestMatching메서드의 흐름이 명확하고 논리적입니다. 검증 → 생성 → 알림 → 이벤트 발행 순서가 적절하며, 각 단계가 헬퍼 메서드로 잘 분리되어 있습니다.
96-109: LGTM!헬퍼 메서드들이 단일 책임 원칙을 잘 따르고 있으며, 명확한 에러 코드로 구분된 예외 처리가 적절합니다. 리뷰 카운트 조회 로직도 신청자가 받은 리뷰를 정확하게 조회하고 있습니다.
Also applies to: 129-150, 152-204
leeeyubin
left a comment
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.
제 눈ㄴ에는 너무나 완벽한 코드.....⭐️ 수고하셨어요!!
| // 자기 자신에게 매칭 요청 불가 | ||
| validateNotSelf(requesterUserId, receiverProfile.user.id!!) |
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.
여기서 !! 연산자를 사용해주신 이유가 있으신가용?
validateNotSelf 로직을 실행하기 전에 id가 null이면 500 오류가 나는 흐름일 것 같아서요..!
의도헤주신거라면 괜찮을 것 같습니다!
private fun User.requireId(): String =
this.id ?: throw CustomException(ErrorCode.BAD_REQUEST)아니면 반복되는 부분은 확장함수로 구현하는 방법도 있을 것 같아요:)
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.
현재 저희 서비스의 엔티티의 id는 거의 모두 nullable로 선언되어 있어, 서비스 레이어에서 사용하려면 !! 또는 별도 검증이 필요했습니다.
이 로직에서는 receiverProfile.user는 DB에서 이미 조회된 엔티티이고 해당 시점에 id가 없는 User가 존재할 수 없는 흐름이라고 판단하여 명시적으로 !!를 사용했습니다
| import org.springframework.data.jpa.repository.JpaRepository | ||
|
|
||
| interface UserRepository : JpaRepository<User, String> |
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.
옹 코드 보다가 깨달은 건데 Spring Data JPA는 @Repository 어노테이션을 달지 않아도 되는군요!
제 코드에서 수정해놓겠습니당
| @Query( | ||
| """ | ||
| select usp | ||
| from UserSportProfile usp | ||
| join fetch usp.user u | ||
| join fetch usp.tier t | ||
| join fetch usp.sport s | ||
| where u.id = :userId | ||
| and s.id = :sportId | ||
| """ |
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.
join fetch로 N+1 문제를 방지한다... 메모해두겠습니다..👍
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.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
@src/main/kotlin/org/appjam/smashing/domain/notification/entity/Notification.kt:
- Around line 60-84: The factory methods createMatchingRequested and
createMatchingRequestAccepted build JSON via string interpolation
(Notification.params), which breaks/unsafe for nicknames containing
quotes/backslashes; instead construct a small data holder (e.g., RequesterParams
/ AcceptorParams) or a Map with keys requesterNickname/requesterTierName and
acceptorNickname/acceptorTierId and serialize it with Jackson's ObjectMapper or
kotlinx.serialization (e.g., objectMapper.writeValueAsString(...)) when
assigning params in the Notification constructors; ensure you reuse/inject the
existing ObjectMapper instance rather than building JSON by hand.
In @src/main/kotlin/org/appjam/smashing/domain/outbox/dto/SsePayload.kt:
- Around line 79-82: KDoc for MatchingAcceptNotificationCreatedPayload is
incorrect—change the phrase "상대가 나에게 매칭을 신청한 순간" to "상대가 내 매칭 신청을 수락한 순간" in the
KDoc above the MatchingAcceptNotificationCreatedPayload declaration in
SsePayload.kt to accurately describe the payload; simply update the copied
comment to the correct description and keep the existing KDoc format.
In @src/main/kotlin/org/appjam/smashing/domain/outbox/entity/OutboxEvent.kt:
- Line 31: OutboxEvent.kt changed the eventType column length from 30 to 100 but
no DB migration was provided; add a migration SQL (Flyway V or Liquibase
changelog) that alters the outbox table to increase the eventType/varchar length
to 100 (and include a safe rollback if using Liquibase or a down script for
Flyway), ensure the migration is named/numbered consistently with existing
migrations and committed alongside the code so runtime schema matches the
@Column(nullable = false, length = 100) annotation.
🧹 Nitpick comments (5)
src/main/kotlin/org/appjam/smashing/domain/matching/controller/MatchingController.kt (1)
54-58: 매개변수 이름이 혼란을 야기할 수 있습니다.Line 54의 매개변수 이름이
requesterUserId로 되어 있지만, 실제로는 매칭을 수락하는 사용자(즉, 원래 요청의 수신자)를 나타냅니다. Line 58에서receiverUserId로 올바르게 전달되고 있으나, 매개변수 이름이 맥락과 맞지 않아 유지보수 시 혼란을 줄 수 있습니다.♻️ 명확성을 위한 리팩토링 제안
@PostMapping("/{matchingId}/accept") fun acceptMatching( - @RequestHeader("userId") requesterUserId: String, // TODO: 인증/인가 회복시 @AuthenticationPrincipal 으로 변경 + @RequestHeader("userId") receiverUserId: String, // TODO: 인증/인가 회복시 @AuthenticationPrincipal 으로 변경 @PathVariable matchingId: String, ): ResponseEntity<ApiResponse<Unit>> { matchingService.acceptMatching( - receiverUserId = requesterUserId, + receiverUserId = receiverUserId, matchingId = matchingId, )src/main/kotlin/org/appjam/smashing/domain/notification/service/NotificationService.kt (2)
19-33: 알림 생성 로직이 명확합니다.템플릿 조회 → 엔티티 생성 → 저장의 흐름이 명확하며, 템플릿이 없을 경우 적절한 예외 처리가 되어 있습니다.
선택사항: 중복 코드 제거 제안
두 메서드(
createMatchingRequested,createMatchingAccepted)가 유사한 구조를 가지고 있어, 향후 알림 타입이 더 추가될 경우 공통 패턴을 추출하는 것을 고려할 수 있습니다. 하지만 현재 2개 메서드만 있는 상황에서는 가독성을 위해 현재 구조를 유지하는 것도 합리적입니다.
35-49: 팩토리 메서드의 네이밍 패턴을 통일하세요.
Notification엔티티의 두 팩토리 메서드가 일관성 없는 네이밍을 사용합니다:
- Line 61:
createMatchingRequested- Line 73:
createMatchingRequestAccepted두 메서드 모두 "Request"를 명시적으로 포함하거나 모두 제거하여 패턴을 통일하는 것을 권장합니다. 예를 들어,
createMatchingAccepted로 정렬하면 서비스 메서드createMatchingAccepted와도 대응이 명확해집니다.src/main/kotlin/org/appjam/smashing/domain/outbox/dto/SsePayload.kt (1)
24-32: Summary 클래스 간 정보 수준 차이를 확인하세요.
MatchingRequesterSummary는 7개 필드(userId, nickname, gender, tierId, wins, losses, reviewCount)를 포함하는 반면,RequesterSummary와AcceptorSummary는 3개 필드(userId, nickname, tierId)만 포함합니다.이는 의도적인 설계일 수 있습니다(매칭 수신 이벤트는 상세 정보가 필요하고, 알림 이벤트는 최소 정보만 필요). 하지만 향후 유지보수 시 혼란을 방지하기 위해 이러한 차이가 의도적인지 확인하고, 필요하다면 문서화하는 것을 권장합니다.
Also applies to: 72-76, 94-98
src/main/kotlin/org/appjam/smashing/domain/matching/entity/Matching.kt (1)
85-91: 서비스 레이어에서 상태 검증이 이미 수행되고 있습니다.
MatchingService.acceptMatching()메서드는accept()를 호출하기 전에validateAcceptable()을 통해 상태가REQUESTED인지 확인합니다(lines 298-301). 따라서 현재 코드에서는 REJECTED나 COMPLETED 상태에서의 잘못된 전환이 방지됩니다.다만, 엔티티 메서드 자체에 상태 검증이 없으므로, 향후 다른 곳에서 직접 호출될 수 있는 경우를 대비하여 다음과 같이 방어적 검증을 추가하는 것을 권장합니다:
fun accept( now: LocalDateTime ) { + require(status == MatchingStatus.REQUESTED) { + "매칭은 REQUESTED 상태에서만 수락할 수 있습니다. 현재 상태: $status" + } status = MatchingStatus.ACCEPTED respondedAt = now confirmedAt = now }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/main/kotlin/org/appjam/smashing/domain/game/entity/Game.ktsrc/main/kotlin/org/appjam/smashing/domain/game/entity/GameResultSubmission.ktsrc/main/kotlin/org/appjam/smashing/domain/game/repository/GameRepository.ktsrc/main/kotlin/org/appjam/smashing/domain/matching/controller/MatchingController.ktsrc/main/kotlin/org/appjam/smashing/domain/matching/entity/LpHistory.ktsrc/main/kotlin/org/appjam/smashing/domain/matching/entity/Matching.ktsrc/main/kotlin/org/appjam/smashing/domain/matching/repository/MatchingRepository.ktsrc/main/kotlin/org/appjam/smashing/domain/matching/service/MatchingService.ktsrc/main/kotlin/org/appjam/smashing/domain/notification/entity/Notification.ktsrc/main/kotlin/org/appjam/smashing/domain/notification/service/NotificationService.ktsrc/main/kotlin/org/appjam/smashing/domain/outbox/dto/SsePayload.ktsrc/main/kotlin/org/appjam/smashing/domain/outbox/entity/OutboxEvent.ktsrc/main/kotlin/org/appjam/smashing/domain/outbox/enums/SseEventType.ktsrc/main/kotlin/org/appjam/smashing/domain/review/entity/GameReview.ktsrc/main/kotlin/org/appjam/smashing/global/exception/ErrorCode.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/kotlin/org/appjam/smashing/domain/matching/service/MatchingService.kt
🧰 Additional context used
🧬 Code graph analysis (11)
src/main/kotlin/org/appjam/smashing/domain/matching/controller/MatchingController.kt (1)
src/main/kotlin/org/appjam/smashing/domain/matching/enums/MatchingStatus.kt (1)
REQUESTED(3-5)
src/main/kotlin/org/appjam/smashing/domain/outbox/dto/SsePayload.kt (2)
src/main/kotlin/org/appjam/smashing/domain/notification/enums/NotificationType.kt (1)
MATCHING_REQUESTED(3-14)src/main/kotlin/org/appjam/smashing/domain/user/entity/UserSportProfile.kt (1)
name(12-69)
src/main/kotlin/org/appjam/smashing/global/exception/ErrorCode.kt (6)
src/main/kotlin/org/appjam/smashing/domain/notification/enums/NotificationType.kt (1)
MATCHING_REQUESTED(3-14)src/main/kotlin/org/appjam/smashing/global/exception/CustomException.kt (1)
errorCode(3-6)src/main/kotlin/org/appjam/smashing/domain/matching/enums/MatchingStatus.kt (1)
REQUESTED(3-5)src/main/kotlin/org/appjam/smashing/domain/matching/enums/SubmissionStatus.kt (1)
SUBMITTED(3-5)src/main/kotlin/org/appjam/smashing/global/common/enums/Status.kt (1)
SUCCESS(3-7)src/main/kotlin/org/appjam/smashing/domain/matching/enums/RejectReason.kt (1)
SCORE_MISMATCH(3-6)
src/main/kotlin/org/appjam/smashing/domain/game/entity/Game.kt (4)
src/main/kotlin/org/appjam/smashing/domain/matching/entity/Game.kt (1)
indexes(14-88)src/main/kotlin/org/appjam/smashing/domain/sport/entity/Sport.kt (1)
strategy(10-25)src/main/kotlin/org/appjam/smashing/domain/matching/enums/GameResultStatus.kt (1)
PENDING_RESULT(3-5)src/main/kotlin/org/appjam/smashing/domain/common/entity/BaseEntity.kt (1)
name(15-49)
src/main/kotlin/org/appjam/smashing/domain/notification/service/NotificationService.kt (2)
src/main/kotlin/org/appjam/smashing/domain/notification/entity/NotificationTemplate.kt (1)
strategy(7-27)src/main/kotlin/org/appjam/smashing/domain/notification/enums/NotificationType.kt (1)
MATCHING_REQUESTED(3-14)
src/main/kotlin/org/appjam/smashing/domain/matching/entity/Matching.kt (1)
src/main/kotlin/org/appjam/smashing/domain/matching/enums/MatchingStatus.kt (1)
REQUESTED(3-5)
src/main/kotlin/org/appjam/smashing/domain/outbox/enums/SseEventType.kt (2)
src/main/kotlin/org/appjam/smashing/domain/notification/enums/NotificationType.kt (1)
MATCHING_REQUESTED(3-14)src/main/kotlin/org/appjam/smashing/domain/notification/entity/NotificationTemplate.kt (1)
strategy(7-27)
src/main/kotlin/org/appjam/smashing/domain/notification/entity/Notification.kt (4)
src/main/kotlin/org/appjam/smashing/domain/notification/entity/NotificationTemplate.kt (1)
strategy(7-27)src/main/kotlin/org/appjam/smashing/domain/notification/enums/NotificationType.kt (1)
MATCHING_REQUESTED(3-14)src/main/kotlin/org/appjam/smashing/domain/user/entity/UserSportProfile.kt (1)
name(12-69)src/main/kotlin/org/appjam/smashing/domain/user/entity/User.kt (1)
sql(11-42)
src/main/kotlin/org/appjam/smashing/domain/review/entity/GameReview.kt (1)
src/main/kotlin/org/appjam/smashing/domain/matching/entity/Game.kt (1)
indexes(14-88)
src/main/kotlin/org/appjam/smashing/domain/matching/entity/LpHistory.kt (1)
src/main/kotlin/org/appjam/smashing/domain/matching/entity/Game.kt (1)
indexes(14-88)
src/main/kotlin/org/appjam/smashing/domain/game/entity/GameResultSubmission.kt (6)
src/main/kotlin/org/appjam/smashing/domain/matching/entity/Game.kt (1)
indexes(14-88)src/main/kotlin/org/appjam/smashing/domain/common/entity/BaseEntity.kt (1)
name(15-49)src/main/kotlin/org/appjam/smashing/domain/matching/enums/GameResultStatus.kt (1)
PENDING_RESULT(3-5)src/main/kotlin/org/appjam/smashing/domain/user/entity/User.kt (1)
sql(11-42)build.gradle.kts (1)
{(79-83)src/main/kotlin/org/appjam/smashing/domain/matching/enums/SubmissionStatus.kt (1)
SUBMITTED(3-5)
🔇 Additional comments (14)
src/main/kotlin/org/appjam/smashing/domain/game/entity/Game.kt (2)
1-16: LGTM! 패키지 재구성 및 명시적 임포트Game 엔티티를
matching.entity에서game.entity로 이동하고 wildcard import를 명시적 import로 변경한 것은 도메인 분리 및 가독성 측면에서 적절합니다.
103-111: 감사 필드 처리는 정상 - 이 항목은 우려할 필요 없음BaseEntity를 확인한 결과,
createdBy와updatedBy필드는 주석 처리되어 있으므로 엔티티 생성 시 초기화 오류가 발생하지 않습니다. 활성화된 감사 필드(createdAt,updatedAt)는@EntityListeners(AuditingEntityListener::class)데코레이터로 Spring Data JPA의AuditingEntityListener에 의해 자동으로 관리됩니다.따라서
createFromMatching팩토리 메서드는 문제없이 작동합니다. 모든 감사 필드는 엔티티 생성 후 자동 초기화됩니다.src/main/kotlin/org/appjam/smashing/domain/matching/entity/LpHistory.kt (1)
6-6: LGTM! Game 엔티티 패키지 이동 반영Game 엔티티의 패키지 변경에 따라 import 경로를 올바르게 업데이트했습니다.
src/main/kotlin/org/appjam/smashing/domain/review/entity/GameReview.kt (1)
6-6: LGTM! Game 엔티티 패키지 이동 반영Game 엔티티의 패키지 변경에 따라 import 경로를 올바르게 업데이트했습니다.
src/main/kotlin/org/appjam/smashing/domain/game/repository/GameRepository.kt (1)
1-14: LGTM! Spring Data JPA 리포지토리 구조 적절GameRepository 인터페이스가 올바르게 구현되었습니다:
JpaRepository<Game, String>확장existsByMatchingId: Matching에 연결된 Game 존재 여부 확인findByMatchingId: Matching ID로 Game 조회 (nullable 반환)Game과 Matching의
@OneToOne관계를 고려할 때, 쿼리 메서드 명명 규칙과 반환 타입이 적절합니다. Kotlin의 nullable 타입(Game?)을 사용한 것도 idiomatic합니다.src/main/kotlin/org/appjam/smashing/domain/game/entity/GameResultSubmission.kt (1)
1-1: 패키지 구조 개선이 잘 되었습니다.경기 결과 제출 엔티티를
matching패키지에서game패키지로 이동하고, 와일드카드 import를 명시적 import로 변경한 것은 도메인 구조를 명확히 하는 좋은 리팩토링입니다.Also applies to: 4-15
src/main/kotlin/org/appjam/smashing/global/exception/ErrorCode.kt (1)
42-53: 매칭 및 알림 도메인 에러 코드가 적절합니다.새로 추가된 에러 코드들이 명확한 네이밍과 적절한 HTTP 상태 코드를 사용하고 있으며, 기존 컨벤션을 잘 따르고 있습니다.
src/main/kotlin/org/appjam/smashing/domain/outbox/enums/SseEventType.kt (1)
15-19: SSE 이벤트 타입 추가가 적절합니다.매칭 요청 및 수락 시 알림 생성 이벤트를 명확히 구분하여 정의했으며, 기존 이벤트 네이밍 컨벤션과 일관성을 유지하고 있습니다.
src/main/kotlin/org/appjam/smashing/domain/matching/repository/MatchingRepository.kt (2)
10-26: 삭제된 매칭 포함 카운트 로직 검증 필요
countTodayBetweenUsersIncludingDeleted메서드가 native query를 사용하여deleted_at을 필터링하지 않고 있어, 소프트 삭제된 매칭도 일일 제한 횟수에 포함됩니다.메서드명이 "IncludingDeleted"로 명시되어 있어 의도적인 것으로 보이지만, 비즈니스 요구사항 관점에서 확인이 필요합니다:
- 사용자가 매칭 신청 후 삭제한 경우, 이를 일일 제한 횟수에 포함시키는 것이 올바른 정책인가요?
- 악의적 사용자가 반복적으로 신청→삭제를 할 수 없도록 하려는 의도라면 적절하지만, 일반 사용자 경험 측면에서는 삭제된 매칭은 카운트에서 제외하는 것이 더 합리적일 수 있습니다.
현재 비즈니스 정책을 확인하여 삭제된 매칭의 카운트 포함 여부를 검증해주세요.
28-40: 연관 엔티티 eager fetch가 잘 구현되었습니다.
findByIdFetchAll메서드가 JPQL의join fetch를 사용하여 필요한 연관 엔티티들을 한 번에 조회하고 있어 N+1 문제를 효과적으로 방지합니다.src/main/kotlin/org/appjam/smashing/domain/matching/entity/Matching.kt (2)
35-35: 상태 전환을 위한 mutable 필드 변경이 적절합니다.
status,respondedAt,confirmedAt필드를var로 변경한 것은 매칭 엔티티의 생명주기 관리에 적합합니다. 불변성을 유지하면서도 필요한 상태 전환을 허용하는 좋은 설계입니다.Also applies to: 39-39, 43-43
73-83: 팩토리 메서드 구현이 깔끔합니다.
createRequested팩토리 메서드는 필수 파라미터만 받아 REQUESTED 상태의 매칭을 생성하며, 기본값을 활용한 명확한 생성 패턴을 제공합니다.src/main/kotlin/org/appjam/smashing/domain/outbox/dto/SsePayload.kt (1)
16-33: 풍부한 페이로드 설계가 우수합니다.
MatchingReceivedPayload에 신청자의 요약 정보를 포함시켜 추가 API 호출 없이 UI 구성이 가능하도록 한 설계가 효율적입니다. 닉네임, 성별, 티어, 전적, 후기 개수 등 필요한 정보를 함께 제공하여 클라이언트 성능을 향상시킵니다.src/main/kotlin/org/appjam/smashing/domain/matching/controller/MatchingController.kt (1)
52-63: 동일한 인증 우회 취약점이 존재합니다.Line 54의
@RequestHeader("userId")패턴은 Line 30-43에서 이미 지적된 것과 동일한 보안 취약점입니다. 클라이언트가 임의의 userId를 전송하여 다른 사용자를 가장할 수 있습니다.인증/인가 복구 시
@AuthenticationPrincipal을 활성화하여 서버 측에서 검증된 인증 정보를 사용하세요.Likely an incorrect or invalid review comment.
| companion object { | ||
| fun createMatchingRequested( | ||
| receiver: User, | ||
| template: NotificationTemplate, | ||
| requesterProfile: UserSportProfile, | ||
| ) = Notification( | ||
| params = """{"requesterNickname":"${requesterProfile.user.nickname}","requesterTierName":"${requesterProfile.tier.name}"}""", | ||
| isRead = false, | ||
| linkUrl = "/api/v1/users/me/matchings/received", | ||
| user = receiver, | ||
| notificationTemplate = template, | ||
| ) | ||
|
|
||
| fun createMatchingRequestAccepted( | ||
| receiver: User, | ||
| template: NotificationTemplate, | ||
| acceptorProfile: UserSportProfile, | ||
| ) = Notification( | ||
| params = """{"acceptorNickname":"${acceptorProfile.user.nickname}","acceptorTierId":${acceptorProfile.tier.id!!}}""", | ||
| isRead = false, | ||
| linkUrl = "/api/v1/users/me/matchings/accepted/pending-result", | ||
| user = receiver, | ||
| notificationTemplate = template, | ||
| ) | ||
| } |
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.
JSON 문자열 직접 생성 시 특수문자 처리 취약점
createMatchingRequested (line 66)와 createMatchingRequestAccepted (line 78)에서 문자열 보간을 사용하여 JSON을 생성하고 있습니다.
사용자 닉네임에 ", \, 또는 특수 문자가 포함될 경우 JSON 파싱 오류가 발생하거나 보안 취약점이 될 수 있습니다.
Jackson의 ObjectMapper 또는 kotlinx.serialization을 사용하여 안전하게 직렬화하는 것을 권장합니다.
🔧 ObjectMapper를 사용한 수정 제안
+import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
+
) : BaseEntity() {
companion object {
+ private val objectMapper = jacksonObjectMapper()
+
fun createMatchingRequested(
receiver: User,
template: NotificationTemplate,
requesterProfile: UserSportProfile,
- ) = Notification(
- params = """{"requesterNickname":"${requesterProfile.user.nickname}","requesterTierName":"${requesterProfile.tier.name}"}""",
+ ): Notification {
+ val paramsMap = mapOf(
+ "requesterNickname" to requesterProfile.user.nickname,
+ "requesterTierName" to requesterProfile.tier.name
+ )
+ return Notification(
+ params = objectMapper.writeValueAsString(paramsMap),
isRead = false,
linkUrl = "/api/v1/users/me/matchings/received",
user = receiver,
notificationTemplate = template,
)
+ }
fun createMatchingRequestAccepted(
receiver: User,
template: NotificationTemplate,
acceptorProfile: UserSportProfile,
- ) = Notification(
- params = """{"acceptorNickname":"${acceptorProfile.user.nickname}","acceptorTierId":${acceptorProfile.tier.id!!}}""",
+ ): Notification {
+ val paramsMap = mapOf(
+ "acceptorNickname" to acceptorProfile.user.nickname,
+ "acceptorTierId" to acceptorProfile.tier.id!!
+ )
+ return Notification(
+ params = objectMapper.writeValueAsString(paramsMap),
isRead = false,
linkUrl = "/api/v1/users/me/matchings/accepted/pending-result",
user = receiver,
notificationTemplate = template,
)
+ }
}
}📝 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.
| companion object { | |
| fun createMatchingRequested( | |
| receiver: User, | |
| template: NotificationTemplate, | |
| requesterProfile: UserSportProfile, | |
| ) = Notification( | |
| params = """{"requesterNickname":"${requesterProfile.user.nickname}","requesterTierName":"${requesterProfile.tier.name}"}""", | |
| isRead = false, | |
| linkUrl = "/api/v1/users/me/matchings/received", | |
| user = receiver, | |
| notificationTemplate = template, | |
| ) | |
| fun createMatchingRequestAccepted( | |
| receiver: User, | |
| template: NotificationTemplate, | |
| acceptorProfile: UserSportProfile, | |
| ) = Notification( | |
| params = """{"acceptorNickname":"${acceptorProfile.user.nickname}","acceptorTierId":${acceptorProfile.tier.id!!}}""", | |
| isRead = false, | |
| linkUrl = "/api/v1/users/me/matchings/accepted/pending-result", | |
| user = receiver, | |
| notificationTemplate = template, | |
| ) | |
| } | |
| companion object { | |
| private val objectMapper = jacksonObjectMapper() | |
| fun createMatchingRequested( | |
| receiver: User, | |
| template: NotificationTemplate, | |
| requesterProfile: UserSportProfile, | |
| ): Notification { | |
| val paramsMap = mapOf( | |
| "requesterNickname" to requesterProfile.user.nickname, | |
| "requesterTierName" to requesterProfile.tier.name | |
| ) | |
| return Notification( | |
| params = objectMapper.writeValueAsString(paramsMap), | |
| isRead = false, | |
| linkUrl = "/api/v1/users/me/matchings/received", | |
| user = receiver, | |
| notificationTemplate = template, | |
| ) | |
| } | |
| fun createMatchingRequestAccepted( | |
| receiver: User, | |
| template: NotificationTemplate, | |
| acceptorProfile: UserSportProfile, | |
| ): Notification { | |
| val paramsMap = mapOf( | |
| "acceptorNickname" to acceptorProfile.user.nickname, | |
| "acceptorTierId" to acceptorProfile.tier.id!! | |
| ) | |
| return Notification( | |
| params = objectMapper.writeValueAsString(paramsMap), | |
| isRead = false, | |
| linkUrl = "/api/v1/users/me/matchings/accepted/pending-result", | |
| user = receiver, | |
| notificationTemplate = template, | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
In
@src/main/kotlin/org/appjam/smashing/domain/notification/entity/Notification.kt
around lines 60 - 84, The factory methods createMatchingRequested and
createMatchingRequestAccepted build JSON via string interpolation
(Notification.params), which breaks/unsafe for nicknames containing
quotes/backslashes; instead construct a small data holder (e.g., RequesterParams
/ AcceptorParams) or a Map with keys requesterNickname/requesterTierName and
acceptorNickname/acceptorTierId and serialize it with Jackson's ObjectMapper or
kotlinx.serialization (e.g., objectMapper.writeValueAsString(...)) when
assigning params in the Notification constructors; ensure you reuse/inject the
existing ObjectMapper instance rather than building JSON by hand.
|
|
||
| @Enumerated(EnumType.STRING) | ||
| @Column(nullable = false, length = 30) | ||
| @Column(nullable = false, length = 100) |
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.
🧩 Analysis chain
🏁 Script executed:
# Find the SseEventType enum
find . -type f -name "*.kt" | xargs grep -l "enum class SseEventType"Repository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 146
🏁 Script executed:
# Find the OutboxEvent.kt file
find . -type f -name "OutboxEvent.kt"Repository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 146
🏁 Script executed:
# Look for database migration files
find . -type f \( -name "*.sql" -o -name "*migration*" -o -name "*.py" -o -name "*.xml" \) | head -20Repository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 55
🏁 Script executed:
# Read SseEventType enum
cat ./src/main/kotlin/org/appjam/smashing/domain/outbox/enums/SseEventType.ktRepository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 586
🏁 Script executed:
# Read OutboxEvent.kt with context
cat -n ./src/main/kotlin/org/appjam/smashing/domain/outbox/entity/OutboxEvent.kt | head -50Repository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 1791
🏁 Script executed:
# Look for migration directories
find . -type d -name "*migration*" -o -type d -name "db" -o -type d -name "flyway" | head -10Repository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 55
🏁 Script executed:
# Check for migration files in resources directories
find . -type d -name resources -o -type d -name migrations | head -10Repository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 93
🏁 Script executed:
# Search for SQL files in src directory
find ./src -type f -name "*.sql" 2>/dev/nullRepository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 55
🏁 Script executed:
# Check for Liquibase or Flyway configuration
find . -type f \( -name "application*.yml" -o -name "application*.properties" \) | head -10Repository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 55
🏁 Script executed:
# Look for migration-related configuration in build files
find . -type f \( -name "build.gradle*" -o -name "pom.xml" \) | head -5Repository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 91
🏁 Script executed:
# Check build.gradle.kts for migration dependencies
cat ./build.gradle.kts | grep -i -A 5 -B 5 "migration\|liquibase\|flyway"Repository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 55
🏁 Script executed:
# List files in src/main/resources
find ./src/main/resources -type f | head -20Repository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 105
🏁 Script executed:
# Check if there are migration directories in src/main/resources
ls -la ./src/main/resources/ 2>/dev/null || echo "Directory not found"Repository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 241
🏁 Script executed:
# Search for any migration or schema related files anywhere
find . -type f \( -name "*.sql" -o -name "*schema*" -o -name "*migration*" \) 2>/dev/nullRepository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 55
🏁 Script executed:
# Double-check by measuring actual enum constant names
python3 << 'EOF'
event_types = [
"MATCHING_RECEIVED",
"MATCHING_UPDATED",
"NOTIFICATION_CREATED",
"MATCHING_REQUEST_NOTIFICATION_CREATED",
"MATCHING_ACCEPT_NOTIFICATION_CREATED"
]
print("Event Type Name Lengths:")
for et in event_types:
print(f" {et}: {len(et)} chars")
print(f"\nMaximum length: {max(len(et) for et in event_types)} chars")
print(f"Previous column length: 30 chars")
print(f"New column length: 100 chars")
print(f"Sufficient? {max(len(et) for et in event_types) <= 100}")
EOFRepository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 159
🏁 Script executed:
# Check git for recent migration-related changes
git log --oneline --all 2>/dev/null | grep -i "migration\|schema\|outbox" | head -10Repository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 333
🏁 Script executed:
# Look in subdirectories of src/main for any migration-related content
find ./src -type f -name "*.kt" | xargs grep -l "migration\|Migration\|Flyway\|Liquibase" 2>/dev/null | head -10Repository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 55
데이터베이스 마이그레이션 스크립트 필수 제출
eventType 컬럼 길이를 30에서 100으로 증가시킨 변경사항이 있으나, 대응하는 데이터베이스 마이그레이션 스크립트가 저장소에서 발견되지 않습니다. 다음을 확인해주세요:
-
마이그레이션 스크립트 제출: 이 스키마 변경을 위한 마이그레이션 스크립트(SQL)가 필요합니다. Flyway, Liquibase 또는 다른 마이그레이션 도구를 사용 중이라면 해당 스크립트를 준비하고 커밋해주세요.
-
이벤트 타입 길이 검증 ✓: 새로 추가된 이벤트 타입들은 모두 100자 이내입니다.
- 가장 긴 타입:
MATCHING_REQUEST_NOTIFICATION_CREATED(37자) - 참고: 이전 길이 30자는 최대 길이 37자를 초과하므로 이번 변경이 필수적이었습니다.
- 가장 긴 타입:
-
기존 데이터와의 호환성: 마이그레이션 스크립트가 확인되어야 검증 가능합니다.
🤖 Prompt for AI Agents
In @src/main/kotlin/org/appjam/smashing/domain/outbox/entity/OutboxEvent.kt at
line 31, OutboxEvent.kt changed the eventType column length from 30 to 100 but
no DB migration was provided; add a migration SQL (Flyway V or Liquibase
changelog) that alters the outbox table to increase the eventType/varchar length
to 100 (and include a safe rollback if using Liquibase or a down script for
Flyway), ensure the migration is named/numbered consistently with existing
migrations and committed alongside the code so runtime schema matches the
@Column(nullable = false, length = 100) annotation.
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.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
@src/main/kotlin/org/appjam/smashing/domain/matching/controller/MatchingController.kt:
- Around line 75-86: The parameter name in rejectMatching is inconsistent:
rename the method parameter requesterUserId to receiverUserId in the controller
function rejectMatching and update its usages so the call to
matchingService.rejectMatching uses receiverUserId = receiverUserId (matching
the acceptMatching naming convention); ensure any TODO/auth comment stays and
method signature, RequestHeader annotation, and the
matchingService.rejectMatching invocation all reflect the new parameter name.
- Around line 52-63: The parameter name in MatchingController.acceptMatching is
misleading: change the @RequestHeader parameter from requesterUserId to
receiverUserId (and update the TODO comment if needed) so it matches the
argument passed to matchingService.acceptMatching(receiverUserId = ...). Update
the function signature and all references inside acceptMatching to use
receiverUserId to keep intent consistent with the receiver semantics.
- Around line 30-43: The requestMatching handler still trusts a client-sent
userId header; remove the @RequestHeader("userId") requesterUserId parameter and
re-enable the server-side principal by uncommenting the @AuthenticationPrincipal
principal: CustomUserDetails parameter in MatchingController.requestMatching,
then pass requesterUserId = principal.username into
matchingService.requestMatching (and remove any TODO that suggests relying on
the header); this ensures the authenticated user identity is taken from the
server-side security principal rather than client input.
In @src/main/kotlin/org/appjam/smashing/domain/matching/entity/Matching.kt:
- Around line 85-91: The accept() method on Matching unconditionally sets status
to MatchingStatus.ACCEPTED and timestamps, risking invalid transitions; update
Matching.accept to validate current status (e.g., only allow transition from
MatchingStatus.PENDING), and if the current status is already ACCEPTED or is
REJECTED throw an appropriate exception (IllegalStateException or your
domain-specific exception) or return a failure result; ensure you only set
respondedAt and confirmedAt when the transition is valid and include references
to the status field and MatchingStatus enum in the check.
- Around line 93-98: The reject() method lacks the same state transition
validation as accept(), allowing duplicate rejections; update Matching.reject to
first verify the current status (e.g., ensure status == MatchingStatus.WAITING)
and throw an IllegalStateException (or the same domain exception used in
accept()) if the transition is invalid, then set status =
MatchingStatus.REJECTED and respondedAt = now; mirror the validation pattern
used in Matching.accept to keep behavior consistent.
🧹 Nitpick comments (2)
src/main/kotlin/org/appjam/smashing/domain/outbox/dto/SsePayload.kt (2)
61-77: 요약 DTO 간 중복이 있습니다.
RequesterSummary(3개 필드)와MatchingRequesterSummary(7개 필드)는 유사한 개념을 표현하지만 필드 구성이 다릅니다. 만약 두 페이로드가 서로 다른 컨텍스트에서 다른 수준의 정보를 제공해야 한다면 현재 설계가 적절하지만, 그렇지 않다면 통합을 고려할 수 있습니다.필드 차이가 의도적이라면 KDoc에 각 Summary 클래스의 사용 목적과 차이를 명시하는 것을 권장합니다.
72-76: 동일한 구조의 Summary 클래스들을 통합할 수 있습니다.
RequesterSummary와AcceptorSummary가 완전히 동일한 필드 구성(userId, nickname, tierId)을 가지고 있습니다. 공통UserSummary클래스를 만들어 재사용하면 코드 중복을 줄이고 유지보수성을 높일 수 있습니다.♻️ 통합 제안
파일 상단에 공통 Summary 클래스 추가:
data class UserSummary( val userId: String, val nickname: String, val tierId: Long, )그 후 각 페이로드에서 재사용:
data class MatchingRequestNotificationCreatedPayload( // ... - val requester: RequesterSummary, + val requester: UserSummary, ) : SsePayload { - - data class RequesterSummary( - val userId: String, - val nickname: String, - val tierId: Long, - ) } data class MatchingAcceptNotificationCreatedPayload( // ... - val acceptor: AcceptorSummary, + val acceptor: UserSummary, ) : SsePayload { - - data class AcceptorSummary( - val userId: String, - val nickname: String, - val tierId: Long, - ) }Also applies to: 94-98
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/kotlin/org/appjam/smashing/domain/matching/controller/MatchingController.ktsrc/main/kotlin/org/appjam/smashing/domain/matching/entity/Matching.ktsrc/main/kotlin/org/appjam/smashing/domain/matching/service/MatchingService.ktsrc/main/kotlin/org/appjam/smashing/domain/outbox/dto/SsePayload.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/kotlin/org/appjam/smashing/domain/matching/service/MatchingService.kt
🔇 Additional comments (5)
src/main/kotlin/org/appjam/smashing/domain/matching/entity/Matching.kt (3)
35-43: 필드 가변성 변경이 적절합니다.상태 전이 메서드(accept/reject)를 지원하기 위해
val에서var로 변경한 것은 합리적인 설계입니다. 기본값 설정도 적절합니다.
73-83: 팩토리 메서드 설계가 깔끔합니다.기본값을 활용한 팩토리 패턴이 잘 적용되었습니다. 도메인 규칙 검증은 서비스 레이어에서 처리하는 것으로 보입니다.
86-86: 타임존 처리는 현재 안전하게 구현되어 있습니다.Matching.kt의 accept()와 reject() 메서드(라인 86, 94)는 LocalDateTime을 파라미터로 받으며, 직접 LocalDateTime.now()를 호출하지 않습니다. MatchingService에서 호출할 때 이미
LocalDateTime.now(DEFAULT_ZONE_ID)를 통해 Asia/Seoul 타임존으로 생성된 시간을 주입하고 있어, 현재 KST 기반 운영 환경에서는 안전합니다. 또한 코드의 TODO 주석("인증 붙으면 receiver 타임존으로 교체", "DB UTC 변경시 추후 사용자 타임존 관련 처리 필요")에서 확인할 수 있듯이 향후 UTC 마이그레이션 계획이 명확하게 문서화되어 있습니다.src/main/kotlin/org/appjam/smashing/domain/outbox/dto/SsePayload.kt (2)
16-33: 페이로드 확장 설계가 우수합니다.
MatchingRequesterSummary에 신청자의 상세 정보(성별, 전적, 후기 개수 등)를 포함하여, 클라이언트가 추가 API 호출 없이 UI를 구성할 수 있도록 한 설계가 훌륭합니다. PR 목표와도 정확히 일치합니다.
79-99: KDoc 주석이 수정되었습니다.이전 리뷰에서 지적된 KDoc 오류가 올바르게 수정되었습니다. "상대가 나의 매칭을 수락한 순간"으로 정확히 표현되어 있습니다.
src/main/kotlin/org/appjam/smashing/domain/matching/controller/MatchingController.kt
Show resolved
Hide resolved
src/main/kotlin/org/appjam/smashing/domain/matching/controller/MatchingController.kt
Show resolved
Hide resolved
src/main/kotlin/org/appjam/smashing/domain/matching/controller/MatchingController.kt
Show resolved
Hide resolved
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/kotlin/org/appjam/smashing/domain/matching/entity/Matching.kt (1)
100-105: 새로 추가된cancel()메서드도 상태 검증이 누락되었습니다.
cancel()메서드가accept(),reject()와 동일한 패턴으로 구현되었으나, 현재 상태가REQUESTED인지 검증하지 않습니다.서비스 계층의
validateCancellableByRequester(Lines 258-271)에서 검증을 수행하고 있어 현재는 안전하지만, 엔티티의 일관성을 위해 향후 리팩토링 시 엔티티 메서드 내부에 상태 검증을 추가하는 것을 권장합니다.♻️ 향후 개선 시 참고할 방어적 검증 패턴
fun cancel( now: LocalDateTime ) { + require(status == MatchingStatus.REQUESTED) { + "매칭 취소는 REQUESTED 상태에서만 가능합니다. 현재 상태: $status" + } status = MatchingStatus.CANCELLED respondedAt = now }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/kotlin/org/appjam/smashing/domain/matching/controller/MatchingController.ktsrc/main/kotlin/org/appjam/smashing/domain/matching/entity/Matching.ktsrc/main/kotlin/org/appjam/smashing/domain/matching/enums/MatchingStatus.ktsrc/main/kotlin/org/appjam/smashing/domain/matching/service/MatchingService.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/kotlin/org/appjam/smashing/domain/matching/controller/MatchingController.kt
🔇 Additional comments (8)
src/main/kotlin/org/appjam/smashing/domain/matching/enums/MatchingStatus.kt (1)
4-8: LGTM! CANCELLED 상태 추가가 적절합니다.매칭 삭제 워크플로우를 지원하기 위한
CANCELLED상태 추가가 깔끔하게 구현되었습니다. 다른 상태들과 일관된 네이밍과 주석을 따르고 있습니다.src/main/kotlin/org/appjam/smashing/domain/matching/service/MatchingService.kt (6)
41-106: 매칭 요청 플로우가 체계적으로 구현되었습니다.전체 흐름이 논리적으로 잘 구성되어 있습니다:
- 수신자 프로필 조회 및 자기 자신 검증
- 요청자 정보 및 프로필 조회
- 일일 제한 검증
- 매칭 엔티티 생성
- 리뷰 개수 조회
- 알림 생성
- SSE 이벤트 발행 (매칭 수신, 알림 생성)
각 단계의 책임이 명확하고, 에러 처리도 적절히 되어 있습니다.
108-159: 매칭 수락 플로우가 올바르게 구현되었습니다.수락 플로우의 핵심 로직들이 잘 구현되어 있습니다:
- Lines 117, 394-407에서 수락 가능 여부 검증 (수신자 확인 + REQUESTED 상태 확인)
- Lines 124-126에서 게임 중복 생성 방지
- 알림 생성 및 SSE 이벤트 발행
Line 121의 타임존 TODO는 향후 인증 복구 시 개선 예정으로 확인됩니다.
161-183: 매칭 거절 플로우가 적절히 구현되었습니다.거절 로직이 간결하고 명확합니다:
- Line 169에서 거절 가능 여부 검증
- Line 172에서 상태 변경
- Lines 173-176에서 flush 후 soft delete (엔티티의
@SQLDelete어노테이션 활용)- SSE 이벤트 발행으로 실시간 알림
185-207: 매칭 요청 삭제 플로우가 올바르게 구현되었습니다.취소 로직이 거절과 유사한 패턴을 따르며 잘 구현되어 있습니다:
- Line 193에서 요청자 권한 검증
- 상태를 CANCELLED로 변경
- Soft delete 처리
- 수신자에게 취소 이벤트 발행
243-271: 검증 로직이 철저하게 구현되었습니다.
validateRejectable과validateCancellableByRequester모두 다음을 올바르게 검증합니다:
- 권한 확인: 거절은 수신자만, 취소는 요청자만 가능
- 상태 확인: REQUESTED 상태일 때만 처리 가능
MATCHING_ALREADY_RESPONDED에러로 중복 처리 방지이러한 서비스 계층 검증이 엔티티의 상태 전이를 안전하게 보호합니다.
296-442: SSE 이벤트 발행 로직이 체계적으로 구현되었습니다.각 이벤트 발행 메서드들이 일관된 패턴을 따르고 있으며:
- 적절한 수신자에게 이벤트 전송
- 풍부한 페이로드 구성 (matchingId, sportId, 사용자 요약 정보 등)
- 클라이언트가 추가 API 호출 없이 UI를 구성할 수 있도록 설계
Lines 312-319, 358-361, 435-439에서 신청자/수락자 요약 정보를 포함하는 것이 특히 좋은 설계입니다.
src/main/kotlin/org/appjam/smashing/domain/matching/entity/Matching.kt (1)
73-83: 팩토리 메서드가 깔끔하게 구현되었습니다.
createRequested팩토리 메서드가 매칭 생성의 명확한 진입점을 제공하며, 기본 상태를REQUESTED로 설정하는 것이 적절합니다.
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@src/main/kotlin/org/appjam/smashing/domain/game/repository/GameRepository.kt:
- Around line 18-36: The native query in countTodayConfirmedGamesBetweenUsers
uses an OR for bidirectional user checks which can prevent index use and slow
queries; refactor the query to use two UNION ALL branches (one for
requester_user_id = :userA & receiver_user_id = :userB and the other swapped) so
each branch can use indexes, and add or ensure a composite index on
game(result_status, confirmed_at) (and individual indexes on result_status and
confirmed_at if needed); also remove the hardcoded 'RESULT_CONFIRMED' string by
referencing the enum constant or a query parameter to avoid literal maintenance
issues.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/kotlin/org/appjam/smashing/domain/game/repository/GameRepository.ktsrc/main/kotlin/org/appjam/smashing/domain/matching/repository/MatchingRepository.ktsrc/main/kotlin/org/appjam/smashing/domain/matching/service/MatchingService.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/kotlin/org/appjam/smashing/domain/matching/service/MatchingService.kt
🔇 Additional comments (2)
src/main/kotlin/org/appjam/smashing/domain/matching/repository/MatchingRepository.kt (1)
10-22: LGTM! 효율적인 조회 쿼리 구현JOIN FETCH를 사용하여 연관된 requester, receiver, sport를 즉시 로딩하는 것은 N+1 쿼리 문제를 방지하는 좋은 방식입니다. 매칭 신청/수락/거절 API 흐름에서 필요한 모든 연관 데이터를 한 번의 쿼리로 가져올 수 있어 성능상 이점이 있습니다.
src/main/kotlin/org/appjam/smashing/domain/game/repository/GameRepository.kt (1)
10-16: 파생 쿼리 메서드가 올바르게 구현되었습니다.Spring Data JPA 네이밍 규칙을 준수하고 있으며, 반환 타입도 적절합니다.
src/main/kotlin/org/appjam/smashing/domain/game/repository/GameRepository.kt
Show resolved
Hide resolved
leeeyubin
left a comment
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.
확인했습니다!! 매칭 도메인 구현하시느라 너무 고생했어요🥹🥹🥹
📌 Related Issue
#️⃣ 요약 설명
매칭 신청 api를 작성합니다.
현재 저희 서비스 기능에 맞추어 해당 api에서 매칭 생성과 동시에 도메인 규칙 검증 → 알림 생성 → SSE 이벤트 전송까지 하나의 흐름으로 처리하도록 구성하고자 하였습니다.
매칭 신청 성공 시, 수신자에게 다음이 함께 발생합니다.
이때 matching.received 이벤트에는 단순 ID가 아닌, 신청자 요약 정보(닉네임, 성별, 티어, 전적, 종목 기준 후기 개수)를 함께 전달하여
추가 API 호출 없이 화면을 구성할 수 있도록 했습니다.
📝 작업 내용
👍 동작 확인
💬 리뷰 요구사항(선택)
현재 인증/인가 기능이 모두 완성되지 않았고, 원활한 테스트를 위해 관련 코드들을 주석 처리해놓은 상황입니다! 이후에 인증/인가 회복시 해당 부분의 주석 해제가 필요합니다.
현재는 한국만 고려하지만 (서비스 특성, 앱잼내 리소스 이슈) 이후에 DB와 서버의 시간 등을 UTC로 맞추고 알맞게 변환해 사용하는 로직 도입이 필요할 것같습니다. 또한 사용자 타임존을 도입해 (현재는 설정된 기본값으로 동작) 이후에 사용자의 타임존에 맞추어 서비스를 확장하기 용이할 수 있도록 하고자 하였습니다.
앱잼 기간동안 서버와 db의 타임존을 모두 한국으로 맞출 예정입니다. 관련해 .yml 파일의 수정이 있어 노션 한번 참고해주시면 좋을 것같습니다☺️