Feat : 동아리 구독 추가/삭제, 구독자 알림 기능 추가#346
Merged
rlagkswn00 merged 34 commits intodevelopfrom Feb 25, 2026
Merged
Conversation
rlagkswn00
changed the title
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Walkthrough동아리 구독 CRUD와 구독 기반 디바이스 구독/구독해지, 내일 마감 대상 동아리 FCM 알림 전송, 관련 포트·퍼시스턴스·스케줄러·마이그레이션 및 테스트를 추가합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant API as UserClubSubscriptionApiV2
participant Service as ClubCommandService
participant QueryPort as ClubQueryPort
participant CommandPort as ClubSubscriptionCommandPort
participant UserEventPort as UserEventPort
participant Firebase as FirebaseMessaging
User->>API: POST /api/v2/users/subscriptions/clubs (Bearer, User-Token, clubId)
API->>Service: addSubscription(command)
Service->>QueryPort: findClubById(clubId)
QueryPort-->>Service: Club
Service->>CommandPort: saveSubscription(rootUser, club)
CommandPort-->>Service: 저장 완료
Service->>UserEventPort: subscribeDevices(topic)
UserEventPort->>Firebase: FCM 구독 요청
Firebase-->>UserEventPort: 응답
UserEventPort-->>Service: 완료
Service-->>API: subscriptionCount
API-->>User: 200 OK + 구독 개수
sequenceDiagram
participant Scheduler as ClubNotificationScheduler
participant FeatureFlag as FeatureFlags
participant Service as ClubNotificationService
participant QueryPort as ClubQueryPort
participant Firebase as FirebaseMessagingPort
Scheduler->>FeatureFlag: NOTIFY_CLUB_DEADLINE 활성 여부 확인
FeatureFlag-->>Scheduler: true/false
alt 활성화
Scheduler->>Service: sendDeadlineNotifications()
Service->>QueryPort: findNextDayRecruitEndClubs(now)
QueryPort-->>Service: List<Club>
loop 각 Club
Service->>Firebase: send(Message(topic, notification, data))
Firebase-->>Service: 전송결과
end
Service-->>Scheduler: 완료
else 비활성화
Scheduler-->>Scheduler: 작업 스킵
end
Possibly related PRs
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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (14)
src/main/java/com/kustacks/kuring/user/adapter/in/web/dto/UserClubSubscriptionCountResponse.java (1)
3-5:Long→long(원시 타입) 사용 권장구독 수는 의미상
null이 될 수 없습니다. 박싱 타입Long을 그대로 사용하면 직렬화 시 의도치 않게null이 응답에 포함될 수 있습니다.♻️ 제안된 수정
public record UserClubSubscriptionCountResponse( - Long subscriptionCount + long subscriptionCount ) { }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/user/adapter/in/web/dto/UserClubSubscriptionCountResponse.java` around lines 3 - 5, UserClubSubscriptionCountResponse currently uses the boxed type Long for subscriptionCount which allows nulls; change the record component type to the primitive long in the UserClubSubscriptionCountResponse declaration (and update any callers/constructors or tests to supply a primitive long or handle unboxing) so subscriptionCount cannot be null and serialization will always return a numeric value.src/test/java/com/kustacks/kuring/archunit/DependencyRuleTests.java (1)
176-177:@DisplayName이름 패턴 불일치기존 테스트는 모두
"XXX 아키텍처 검증"패턴을 따르고 있으나, 신규 테스트는"Club Domain 의존성 검증"을 사용하고 있습니다. 일관성을 위해"Club 아키텍처 검증"으로 통일하는 것을 권장합니다.♻️ 제안된 수정
- `@DisplayName`("Club Domain 의존성 검증") + `@DisplayName`("Club 아키텍처 검증")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/kustacks/kuring/archunit/DependencyRuleTests.java` around lines 176 - 177, Update the `@DisplayName` annotation on the new test from "Club Domain 의존성 검증" to match the project's naming pattern "Club 아키텍처 검증"; locate the test annotated with `@DisplayName` and `@Test` in DependencyRuleTests (the new Club test) and change the display string so it follows the "XXX 아키텍처 검증" convention.src/test/java/com/kustacks/kuring/acceptance/UserAcceptanceTest.java (1)
495-545: JWT 미제공/유효하지 않은 JWT 실패 시나리오 테스트 누락
UserStep에동아리_구독_추가_요청_유저토큰없음이 정의되어 있음에도 이를 활용하는 인증 실패 케이스 테스트가 없습니다. 다른 엔드포인트(예:withdraw_user_with_invalid_token)와의 일관성을 위해 인증 없이 구독 시도 시 실패하는 테스트를 추가하는 것을 권장합니다.✅ 추가 테스트 예시
`@DisplayName`("[v2] 인증되지 않은 사용자는 동아리를 구독할 수 없다") `@Test` void add_club_subscription_fail_without_jwt() { var response = 동아리_구독_추가_요청_유저토큰없음(USER_FCM_TOKEN, TEST_CLUB_ID); 실패_응답_확인(response, HttpStatus.UNAUTHORIZED); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/kustacks/kuring/acceptance/UserAcceptanceTest.java` around lines 495 - 545, Add an authentication-failure test to UserAcceptanceTest that uses the existing helper 동아리_구독_추가_요청_유저토큰없음 to attempt subscribing without a JWT and assert an unauthorized response with 실패_응답_확인(response, HttpStatus.UNAUTHORIZED); implement it as a new `@Test` (e.g., add_club_subscription_fail_without_jwt) with a `@DisplayName` matching other tests so it follows existing patterns and uses USER_FCM_TOKEN and TEST_CLUB_ID.src/main/java/com/kustacks/kuring/club/application/port/out/ClubQueryPort.java (1)
13-15:findNextDayRecruitEndClubs를default메서드로 리팩터링 고려
findNextDayRecruitEndClubs는findClubsBetweenDates의 래퍼로, 어댑터 구현이 내일 날짜 범위를 계산한 후findClubsBetweenDates를 호출합니다(라인 39). 이 로직은 포트 레이어의default메서드로 캡슐화하여 DRY 원칙을 따르는 것을 고려하세요.♻️ default 메서드로 리팩터링하는 방안
public interface ClubQueryPort { Optional<Club> findClubById(Long id); List<Club> findClubsBetweenDates(LocalDateTime start, LocalDateTime end); - List<Club> findNextDayRecruitEndClubs(LocalDateTime now); + default List<Club> findNextDayRecruitEndClubs(LocalDateTime now) { + LocalDate tomorrow = now.toLocalDate().plusDays(1); + LocalDateTime startInclusive = tomorrow.atStartOfDay(); + LocalDateTime endExclusive = tomorrow.plusDays(1).atStartOfDay(); + return findClubsBetweenDates(startInclusive, endExclusive); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/club/application/port/out/ClubQueryPort.java` around lines 13 - 15, Introduce a default implementation of findNextDayRecruitEndClubs in the ClubQueryPort interface that computes the start and end LocalDateTime for "tomorrow" and delegates to the existing findClubsBetweenDates(start, end); specifically, add a default method named findNextDayRecruitEndClubs() in the ClubQueryPort interface which calculates tomorrow's start (e.g., tomorrow at 00:00) and end (e.g., tomorrow at 23:59:59.999 or plusDays(1).minusNanos(1)) using LocalDateTime/LocalDate and then returns findClubsBetweenDates(start, end), removing the need for adapter classes to implement this wrapper logic.src/main/java/com/kustacks/kuring/worker/notification/ClubNotificationScheduler.java (1)
19-30: Feature flag 비활성 시 로그가 남지 않는 점 고려Feature flag가 OFF일 때 아무 로그도 남지 않아, 스케줄러가 정상적으로 실행은 됐지만 flag에 의해 건너뛰었는지 확인하기 어렵습니다. 운영 환경에서 디버깅 시 유용할 수 있으므로 debug 레벨 로그 추가를 권장합니다.
📋 Feature flag OFF 로그 추가 제안
`@Scheduled`(cron = "0 0 18 * * *", zone = "Asia/Seoul") public void sendClubDeadlineNotifications() { if (featureFlags.isEnabled(KuringFeatures.NOTIFY_CLUB_DEADLINE.getFeature())) { log.info("******** 동아리 마감 임박 알림 발송 시작 ********"); try { clubNotificationUseCase.sendDeadlineNotifications(); log.info("******** 동아리 마감 임박 알림 발송 완료 ********"); } catch (Exception e) { log.error("동아리 마감 임박 알림 발송 중 오류가 발생했습니다.", e); } + } else { + log.debug("NOTIFY_CLUB_DEADLINE feature flag가 비활성 상태이므로 알림 발송을 건너뜁니다."); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/worker/notification/ClubNotificationScheduler.java` around lines 19 - 30, Add a debug-level log when the feature flag check in sendClubDeadlineNotifications skips execution so operators can tell if the scheduler ran but was disabled by the flag; specifically, inside sendClubDeadlineNotifications where it calls featureFlags.isEnabled(KuringFeatures.NOTIFY_CLUB_DEADLINE.getFeature()), add a log.debug statement (using the existing log) in the else/disabled branch indicating the notification job was skipped due to the feature flag being off, keeping existing info/error logs unchanged.src/main/resources/db/migration/V260217__Alter_club_subscribe_to_root_user.sql (1)
24-35:root_user_id단일 컬럼 인덱스가 중복될 수 있음25~28번 줄에서 FK 추가 시 MySQL(InnoDB)이
root_user_id에 자동으로 인덱스를 생성합니다. 이후 35번 줄에서 동일 컬럼에 명시적 인덱스를 다시 생성하면 중복 인덱스가 될 수 있습니다. 마이그레이션 적용 후SHOW INDEX FROM club_subscribe로 중복 여부를 확인해 보세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/V260217__Alter_club_subscribe_to_root_user.sql` around lines 24 - 35, The migration creates a foreign key fk_club_subscribe_root_user on club_subscribe(root_user_id), which InnoDB will auto-index, and then later creates an explicit index idx_club_subscribe_root_user on the same root_user_id causing a duplicate index; fix by removing or guarding the explicit CREATE INDEX for idx_club_subscribe_root_user (or use a conditional/create-if-not-exists pattern) so only one index exists for root_user_id after adding the FK, ensuring you still satisfy any needed lookup patterns without creating a redundant index.src/main/java/com/kustacks/kuring/club/domain/ClubSubscribe.java (1)
40-43: 생성자에서 null 검증 누락
rootUser또는club이 null로 전달되면 DB 저장 시점까지 오류가 지연됩니다. 도메인 계층에서 빠르게 실패(fail-fast)하도록 null 검증을 추가하는 것을 권장합니다.🛡️ null 검증 추가 제안
+import java.util.Objects; + public ClubSubscribe(RootUser rootUser, Club club) { - this.rootUser = rootUser; - this.club = club; + this.rootUser = Objects.requireNonNull(rootUser, "rootUser must not be null"); + this.club = Objects.requireNonNull(club, "club must not be null"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/club/domain/ClubSubscribe.java` around lines 40 - 43, Constructor ClubSubscribe(RootUser rootUser, Club club) lacks null checks causing delayed failures; update the ClubSubscribe constructor to validate inputs (e.g., use Objects.requireNonNull or explicit checks) for both rootUser and club and throw a clear exception (NullPointerException/IllegalArgumentException) on null so the domain fails fast when creating a subscription.src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepository.java (1)
8-11: 메서드명이 조회 대상 날짜 필드를 명시하지 않음
findClubsBetweenDates만으로는 어떤 날짜 기준(모집 마감일, 생성일 등)인지 알기 어렵습니다.findClubsByRecruitEndDateBetween처럼 구체적인 이름을 사용하면 의도가 더 명확해집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepository.java` around lines 8 - 11, The method name findClubsBetweenDates on the ClubQueryRepository interface is ambiguous about which date field is used; rename it to a clear, intent-revealing name such as findClubsByRecruitEndDateBetween (or another specific field name like findClubsByCreatedAtBetween if different) and update all implementations and callers to use the new method name (look for usages of ClubQueryRepository.findClubsBetweenDates and implementations in classes that implement ClubQueryRepository).src/main/java/com/kustacks/kuring/club/application/service/ClubCommandService.java (1)
30-30: 필드명countSubscriptionsQueryPort가 역할을 정확히 반영하지 않음이 포트는
existsSubscription()과countSubscriptions()모두를 제공하지만, 필드명이 count만을 암시합니다.clubSubscriptionQueryPort가 타입명과 일관되고 역할을 더 잘 표현합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/club/application/service/ClubCommandService.java` at line 30, Rename the field countSubscriptionsQueryPort to clubSubscriptionQueryPort to reflect that the port provides both existsSubscription() and countSubscriptions(); update the field declaration (private final ... countSubscriptionsQueryPort), constructor parameters, assignments, and all usages in ClubCommandService (and any referenced classes) to use clubSubscriptionQueryPort so the name matches the type and expressed responsibilities.src/main/java/com/kustacks/kuring/club/application/service/ClubNotificationService.java (2)
49-51:LocalDateTime.now()직접 호출 — 테스트 용이성 저하
findDeadlineClubs()에서LocalDateTime.now()를 직접 호출하면 시간 기반 테스트가 어려워집니다.java.time.Clock을 주입받거나,now를 매개변수로 전달받는 방식을 고려해 볼 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/club/application/service/ClubNotificationService.java` around lines 49 - 51, The method findDeadlineClubs currently calls LocalDateTime.now() directly which hinders time-based testing; refactor it to accept the current time via injection or parameter: either inject a java.time.Clock into ClubNotificationService and replace LocalDateTime.now() with LocalDateTime.now(clock), or change findDeadlineClubs() to accept a LocalDateTime (e.g., now) and pass that to clubQueryPort.findNextDayRecruitEndClubs; update callers and unit tests to provide a fixed clock or fixed LocalDateTime so tests become deterministic and reference findDeadlineClubs, clubQueryPort.findNextDayRecruitEndClubs, and LocalDateTime.now() when making the changes.
25-27:CLUB_TOPIC_PREFIX상수 중복
CLUB_TOPIC_PREFIX = "club."이ClubCommandService에도 동일하게 정의되어 있습니다. 토픽 생성 로직을 공통 도메인 상수 또는 유틸리티로 추출하면 불일치 위험을 줄일 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/club/application/service/ClubNotificationService.java` around lines 25 - 27, CLUB_TOPIC_PREFIX is duplicated in ClubNotificationService and ClubCommandService; extract this literal into a shared constant (e.g., a new ClubTopicConstants or TopicConstants class with CLUB_TOPIC_PREFIX = "club.") and replace the local CLUB_TOPIC_PREFIX declarations in both ClubNotificationService and ClubCommandService to reference that shared constant, removing the duplicate fields so both services use the single source of truth for topic generation.src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapter.java (1)
17-19: 어댑터가 3개의 포트를 구현하고 있습니다 — 현재는 적절합니다.현재 메서드 수가 많지 않아 하나의 어댑터로 충분하지만, 향후 포트별 메서드가 늘어나면
ClubQueryAdapter와ClubSubscriptionAdapter로 분리하는 것을 고려해 보세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapter.java` around lines 17 - 19, ClubPersistenceAdapter currently implements three ports (ClubQueryPort, ClubSubscriptionCommandPort, ClubSubscriptionQueryPort); keep as-is now but prepare to split when port method counts grow by creating two adapters (e.g., ClubQueryAdapter to implement ClubQueryPort and ClubSubscriptionAdapter to implement ClubSubscriptionCommandPort and ClubSubscriptionQueryPort), move the relevant methods from ClubPersistenceAdapter into those new classes, wire them with the same constructor-injected dependencies (`@RequiredArgsConstructor`) and the `@PersistenceAdapter` stereotype, and update any DI bindings/usages to reference the new adapter classes instead of ClubPersistenceAdapter.src/test/java/com/kustacks/kuring/club/application/service/ClubCommandServiceTest.java (1)
117-138:countSubscriptions스텁이 명시적으로 설정되지 않아 테스트 의도가 불분명합니다
remove_subscription_success테스트에서countSubscriptions(1L)의 반환값을 명시적으로 스텁하지 않고, Mockito의 기본 반환값0L에 의존하여assertThat(count).isEqualTo(0L)검증을 통과합니다.테스트 의도를 명확히 하기 위해
when(clubSubscriptionQueryPort.countSubscriptions(1L)).thenReturn(0L);을 명시적으로 추가하는 것이 좋겠습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/kustacks/kuring/club/application/service/ClubCommandServiceTest.java` around lines 117 - 138, The test remove_subscription_success in ClubCommandServiceTest relies on Mockito's default for countSubscriptions(1L), so explicitly stub clubSubscriptionQueryPort.countSubscriptions(1L) to return 0L to make the test intent clear: add when(clubSubscriptionQueryPort.countSubscriptions(1L)).thenReturn(0L) in the given/setup section before calling service.removeSubscription(new ClubSubscriptionCommand(...)); keep existing stubs for rootUserQueryPort.findRootUserByEmail, clubQueryPort.findClubById, clubSubscriptionQueryPort.existsSubscription, and userQueryPort.findByLoggedInUserId so only countSubscriptions is now explicitly defined.src/test/java/com/kustacks/kuring/club/application/service/ClubNotificationServiceTest.java (1)
72-80:ReflectionTestUtils를 통한 FirebaseMessage내부 필드 접근은 취약합니다Firebase SDK 버전 업그레이드 시
Message클래스의 내부 필드명(topic,data)이 변경되면 테스트가 깨질 수 있습니다. 현재로서는 대안이 제한적이지만, SDK 업그레이드 시 이 테스트의 유지보수 비용을 인지하고 계시면 좋겠습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/kustacks/kuring/club/application/service/ClubNotificationServiceTest.java` around lines 72 - 80, 현재 테스트 ClubNotificationServiceTest가 ReflectionTestUtils로 Firebase Message의 내부 필드("topic", "data")에 접근해 SDK 변경에 취약하므로, ReflectionTestUtils 사용을 제거하고 대신 Mockito의 ArgumentCaptor<Message>로 전송된 Message 객체를 캡처한 뒤 Message의 공개 API(예: getTopic(), getData() 또는 getDataMap() 등 현재 SDK에 제공되는 접근자)를 사용해 검증하도록 변경하세요; 만약 현재 SDK에 직접 접근자가 없으면 Message를 빌더로 재구성하거나 FirebaseMessaging 호출(예: send/sendAsync)을 검증하는 행위 기반 어서션으로 대체해 내부 필드에 의존하지 않도록 수정하세요.
🤖 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/java/com/kustacks/kuring/club/application/service/ClubCommandService.java`:
- Around line 36-48: Wrap the call to
clubSubscriptionCommandPort.saveSubscription(...) inside
addSubscription(ClubSubscriptionCommand) with a try/catch that catches the
DataIntegrityViolationException (or the specific persistence exception your
stack throws) and rethrows an InvalidStateException with
ErrorCode.CLUB_ALREADY_SUBSCRIBED; keep the existing flow
(subscribeAllLoggedInDevices and countSubscriptionsQueryPort.countSubscriptions)
for the success path and only translate the DB unique-constraint exception to
the CLUB_ALREADY_SUBSCRIBED error to handle TOCTOU race conditions.
- Around line 92-94: The makeTopic(Club club) method returns CLUB_TOPIC_PREFIX +
club.getId() but omits applying serverProperties.ifDevThenAddSuffix(), causing a
mismatch with ClubNotificationService.buildTopic(); update makeTopic to call
serverProperties.ifDevThenAddSuffix(CLUB_TOPIC_PREFIX + club.getId()) (or
equivalent) so the same dev suffix logic is applied, ensuring topic names match
across ClubCommandService and ClubNotificationService.
- Around line 78-90: The current subscribeAllLoggedInDevices and
unsubscribeAllLoggedInDevices use
userQueryPort.findByLoggedInUserId(rootUserId).forEach(...) so a thrown
exception from userEventPort.subscribeEvent or unsubscribeEvent aborts
processing remaining devices; change the loop to handle each device individually
with a try/catch around the call to
userEventPort.subscribeEvent(user.getFcmToken(), topic) and
userEventPort.unsubscribeEvent(user.getFcmToken(), topic) respectively, catching
Exception, logging the failure including rootUserId, the failing
user.getFcmToken() and the exception, and then continuing to the next device so
one failure does not stop the rest.
In
`@src/main/java/com/kustacks/kuring/club/application/service/ClubNotificationService.java`:
- Around line 72-74: The dev vs prod topic suffix is inconsistent:
ClubNotificationService.buildTopic(Club) applies
serverProperties.ifDevThenAddSuffix(...) but ClubCommandService.makeTopic(...)
does not, causing dev users to subscribe to "club.{id}" while notifications are
sent to "club.{id}.dev". Fix by making topic construction consistent—either call
serverProperties.ifDevThenAddSuffix(...) inside
ClubCommandService.makeTopic(...) as well or extract a shared helper (e.g.,
ClubTopicBuilder.buildTopic(Club) or buildTopicForId(long id)) and use it from
both ClubNotificationService.buildTopic and ClubCommandService.makeTopic so both
publish and subscribe use the exact same string.
In `@src/main/java/com/kustacks/kuring/club/domain/Club.java`:
- Around line 79-89: Update Club.equals and Club.hashCode to be proxy-safe and
avoid treating two transient entities with null ids as equal: in equals, first
check reference equality (this == o), then use instanceof Club (not getClass())
to allow Hibernate proxies, cast to Club, and return false if either id is null;
only compare id.equals(other.id) when both ids are non-null. In hashCode, return
a fixed value derived from the class (e.g., getClass().hashCode()) instead of
Objects.hashCode(id) so transient vs persistent state doesn't change the hash;
modify the equals and hashCode methods in the Club class accordingly.
In `@src/main/java/com/kustacks/kuring/common/dto/ResponseCodeAndMessages.java`:
- Line 66: ResponseCodeAndMessages에 정의된 CLUB_SUBSCRIPTION_LOOKUP_SUCCESS 상수가 현재
참조되지 않으니 처리하세요: UserClubSubscriptionApiV2의 엔드포인트들(addSubscription,
deleteSubscription)을 확인하고 향후 구독 목록 조회를 구현할 계획이면
CLUB_SUBSCRIPTION_LOOKUP_SUCCESS에 대한 주석을 추가해 의도를 명시하거나, 해당 기능을 바로 구현할 예정이라면
UserClubSubscriptionApiV2에 조회 메서드(예: listSubscriptions)와 매핑을 추가하고 메시지를 사용하도록
연결하세요; 계획이 없다면 Constants 클래스에서 CLUB_SUBSCRIPTION_LOOKUP_SUCCESS 항목을 제거해 미사용 코드를
정리하세요.
In `@src/main/java/com/kustacks/kuring/common/exception/code/ErrorCode.java`:
- Line 62: Update the ErrorCode enum so CLUB_NOT_FOUND uses HttpStatus.NOT_FOUND
instead of HttpStatus.BAD_REQUEST to match the semantics and the other
*_NOT_FOUND constants (e.g., NOTICE_NOT_FOUND, USER_NOT_FOUND,
ROOT_USER_NOT_FOUND); locate the enum constant CLUB_NOT_FOUND in the ErrorCode
class and change its status value to HttpStatus.NOT_FOUND.
In
`@src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java`:
- Around line 45-54: The addSubscription and deleteSubscription handlers declare
the userToken parameter (RequestHeader "User-Token") but never use it; either
pass it through to the service if it's needed or stop requiring it: if FCM token
is required for subscription/notification, add it to the ClubSubscriptionCommand
and forward it via clubSubscriptionUseCase.addSubscription / deleteSubscription
so the service receives the token; otherwise remove the userToken parameter from
the method signatures (or mark `@RequestHeader`(required = false)) and update any
API docs/tests to avoid forcing the header. Ensure changes reference the
addSubscription/deleteSubscription methods, the userToken parameter, and
ClubSubscriptionCommand/clubSubscriptionUseCase.
- Around line 59-68: The DELETE handler deleteSubscription currently reads
clubId from a `@RequestBody` (UserClubSubscriptionRequest), which can be dropped
by proxies; change it to accept the clubId as a path variable (e.g. update
`@DeleteMapping` to `@DeleteMapping`("/{clubId}") and add a `@PathVariable` Long
clubId parameter), remove the `@RequestBody` UserClubSubscriptionRequest
parameter, and construct ClubSubscriptionCommand(email, clubId) when calling
clubSubscriptionUseCase.removeSubscription; also update any API
signatures/tests/docs that reference UserClubSubscriptionRequest for this
endpoint.
In
`@src/main/resources/db/migration/V260217__Alter_club_subscribe_to_root_user.sql`:
- Around line 6-8: The UPDATE using "JOIN user u" can leave cs.root_user_id NULL
for orphaned club_subscribe rows or when u.login_user_id is NULL; change the
UPDATE in V260217__Alter_club_subscribe_to_root_user.sql to use a LEFT JOIN and
assign cs.root_user_id = COALESCE(u.login_user_id, cs.root_user_id) (so you
don't overwrite with NULL), and add a pre-migration check SELECT that finds rows
where club_subscribe.user_id has no matching user or user.login_user_id IS NULL
(e.g. WHERE NOT EXISTS (SELECT 1 FROM user u WHERE u.id = cs.user_id) OR
u.login_user_id IS NULL) so you can remediate them before applying the NOT NULL
constraint on root_user_id.
In `@src/test/java/com/kustacks/kuring/acceptance/UserStep.java`:
- Around line 110-118: The helper method 동아리_구독_추가_요청_유저토큰없음 in UserStep is
defined but never used; either remove it or add a test that asserts
authentication failure when User-Token is missing. To fix, either delete
동아리_구독_추가_요청_유저토큰없음 from UserStep, or add a new test in UserAcceptanceTest that
calls UserStep.동아리_구독_추가_요청_유저토큰없음(...) and asserts the expected
unauthorized/error response (mirroring the existing 동아리_구독_추가_요청-based tests for
success/failure scenarios). Ensure the new test covers the missing User-Token
scenario and validates the same response structure used elsewhere.
In `@src/test/java/com/kustacks/kuring/archunit/DependencyRuleTests.java`:
- Around line 183-185: The Club hexagonal architecture test is missing the
incoming port assertion; update the fluent rule chain that configures the Club
module (the call sequence using
withAdaptersLayer("adapter").outgoing("out.persistence").and()) to include
.incoming("in.web") so the rule also validates incoming adapters for web; locate
the Club test's rule definition in DependencyRuleTests and insert
.incoming("in.web") into the adapter layer clause to match the other modules'
patterns.
---
Nitpick comments:
In
`@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapter.java`:
- Around line 17-19: ClubPersistenceAdapter currently implements three ports
(ClubQueryPort, ClubSubscriptionCommandPort, ClubSubscriptionQueryPort); keep
as-is now but prepare to split when port method counts grow by creating two
adapters (e.g., ClubQueryAdapter to implement ClubQueryPort and
ClubSubscriptionAdapter to implement ClubSubscriptionCommandPort and
ClubSubscriptionQueryPort), move the relevant methods from
ClubPersistenceAdapter into those new classes, wire them with the same
constructor-injected dependencies (`@RequiredArgsConstructor`) and the
`@PersistenceAdapter` stereotype, and update any DI bindings/usages to reference
the new adapter classes instead of ClubPersistenceAdapter.
In
`@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepository.java`:
- Around line 8-11: The method name findClubsBetweenDates on the
ClubQueryRepository interface is ambiguous about which date field is used;
rename it to a clear, intent-revealing name such as
findClubsByRecruitEndDateBetween (or another specific field name like
findClubsByCreatedAtBetween if different) and update all implementations and
callers to use the new method name (look for usages of
ClubQueryRepository.findClubsBetweenDates and implementations in classes that
implement ClubQueryRepository).
In
`@src/main/java/com/kustacks/kuring/club/application/port/out/ClubQueryPort.java`:
- Around line 13-15: Introduce a default implementation of
findNextDayRecruitEndClubs in the ClubQueryPort interface that computes the
start and end LocalDateTime for "tomorrow" and delegates to the existing
findClubsBetweenDates(start, end); specifically, add a default method named
findNextDayRecruitEndClubs() in the ClubQueryPort interface which calculates
tomorrow's start (e.g., tomorrow at 00:00) and end (e.g., tomorrow at
23:59:59.999 or plusDays(1).minusNanos(1)) using LocalDateTime/LocalDate and
then returns findClubsBetweenDates(start, end), removing the need for adapter
classes to implement this wrapper logic.
In
`@src/main/java/com/kustacks/kuring/club/application/service/ClubCommandService.java`:
- Line 30: Rename the field countSubscriptionsQueryPort to
clubSubscriptionQueryPort to reflect that the port provides both
existsSubscription() and countSubscriptions(); update the field declaration
(private final ... countSubscriptionsQueryPort), constructor parameters,
assignments, and all usages in ClubCommandService (and any referenced classes)
to use clubSubscriptionQueryPort so the name matches the type and expressed
responsibilities.
In
`@src/main/java/com/kustacks/kuring/club/application/service/ClubNotificationService.java`:
- Around line 49-51: The method findDeadlineClubs currently calls
LocalDateTime.now() directly which hinders time-based testing; refactor it to
accept the current time via injection or parameter: either inject a
java.time.Clock into ClubNotificationService and replace LocalDateTime.now()
with LocalDateTime.now(clock), or change findDeadlineClubs() to accept a
LocalDateTime (e.g., now) and pass that to
clubQueryPort.findNextDayRecruitEndClubs; update callers and unit tests to
provide a fixed clock or fixed LocalDateTime so tests become deterministic and
reference findDeadlineClubs, clubQueryPort.findNextDayRecruitEndClubs, and
LocalDateTime.now() when making the changes.
- Around line 25-27: CLUB_TOPIC_PREFIX is duplicated in ClubNotificationService
and ClubCommandService; extract this literal into a shared constant (e.g., a new
ClubTopicConstants or TopicConstants class with CLUB_TOPIC_PREFIX = "club.") and
replace the local CLUB_TOPIC_PREFIX declarations in both ClubNotificationService
and ClubCommandService to reference that shared constant, removing the duplicate
fields so both services use the single source of truth for topic generation.
In `@src/main/java/com/kustacks/kuring/club/domain/ClubSubscribe.java`:
- Around line 40-43: Constructor ClubSubscribe(RootUser rootUser, Club club)
lacks null checks causing delayed failures; update the ClubSubscribe constructor
to validate inputs (e.g., use Objects.requireNonNull or explicit checks) for
both rootUser and club and throw a clear exception
(NullPointerException/IllegalArgumentException) on null so the domain fails fast
when creating a subscription.
In
`@src/main/java/com/kustacks/kuring/user/adapter/in/web/dto/UserClubSubscriptionCountResponse.java`:
- Around line 3-5: UserClubSubscriptionCountResponse currently uses the boxed
type Long for subscriptionCount which allows nulls; change the record component
type to the primitive long in the UserClubSubscriptionCountResponse declaration
(and update any callers/constructors or tests to supply a primitive long or
handle unboxing) so subscriptionCount cannot be null and serialization will
always return a numeric value.
In
`@src/main/java/com/kustacks/kuring/worker/notification/ClubNotificationScheduler.java`:
- Around line 19-30: Add a debug-level log when the feature flag check in
sendClubDeadlineNotifications skips execution so operators can tell if the
scheduler ran but was disabled by the flag; specifically, inside
sendClubDeadlineNotifications where it calls
featureFlags.isEnabled(KuringFeatures.NOTIFY_CLUB_DEADLINE.getFeature()), add a
log.debug statement (using the existing log) in the else/disabled branch
indicating the notification job was skipped due to the feature flag being off,
keeping existing info/error logs unchanged.
In
`@src/main/resources/db/migration/V260217__Alter_club_subscribe_to_root_user.sql`:
- Around line 24-35: The migration creates a foreign key
fk_club_subscribe_root_user on club_subscribe(root_user_id), which InnoDB will
auto-index, and then later creates an explicit index
idx_club_subscribe_root_user on the same root_user_id causing a duplicate index;
fix by removing or guarding the explicit CREATE INDEX for
idx_club_subscribe_root_user (or use a conditional/create-if-not-exists pattern)
so only one index exists for root_user_id after adding the FK, ensuring you
still satisfy any needed lookup patterns without creating a redundant index.
In `@src/test/java/com/kustacks/kuring/acceptance/UserAcceptanceTest.java`:
- Around line 495-545: Add an authentication-failure test to UserAcceptanceTest
that uses the existing helper 동아리_구독_추가_요청_유저토큰없음 to attempt subscribing without
a JWT and assert an unauthorized response with 실패_응답_확인(response,
HttpStatus.UNAUTHORIZED); implement it as a new `@Test` (e.g.,
add_club_subscription_fail_without_jwt) with a `@DisplayName` matching other tests
so it follows existing patterns and uses USER_FCM_TOKEN and TEST_CLUB_ID.
In `@src/test/java/com/kustacks/kuring/archunit/DependencyRuleTests.java`:
- Around line 176-177: Update the `@DisplayName` annotation on the new test from
"Club Domain 의존성 검증" to match the project's naming pattern "Club 아키텍처 검증";
locate the test annotated with `@DisplayName` and `@Test` in DependencyRuleTests
(the new Club test) and change the display string so it follows the "XXX 아키텍처
검증" convention.
In
`@src/test/java/com/kustacks/kuring/club/application/service/ClubCommandServiceTest.java`:
- Around line 117-138: The test remove_subscription_success in
ClubCommandServiceTest relies on Mockito's default for countSubscriptions(1L),
so explicitly stub clubSubscriptionQueryPort.countSubscriptions(1L) to return 0L
to make the test intent clear: add
when(clubSubscriptionQueryPort.countSubscriptions(1L)).thenReturn(0L) in the
given/setup section before calling service.removeSubscription(new
ClubSubscriptionCommand(...)); keep existing stubs for
rootUserQueryPort.findRootUserByEmail, clubQueryPort.findClubById,
clubSubscriptionQueryPort.existsSubscription, and
userQueryPort.findByLoggedInUserId so only countSubscriptions is now explicitly
defined.
In
`@src/test/java/com/kustacks/kuring/club/application/service/ClubNotificationServiceTest.java`:
- Around line 72-80: 현재 테스트 ClubNotificationServiceTest가 ReflectionTestUtils로
Firebase Message의 내부 필드("topic", "data")에 접근해 SDK 변경에 취약하므로, ReflectionTestUtils
사용을 제거하고 대신 Mockito의 ArgumentCaptor<Message>로 전송된 Message 객체를 캡처한 뒤 Message의 공개
API(예: getTopic(), getData() 또는 getDataMap() 등 현재 SDK에 제공되는 접근자)를 사용해 검증하도록
변경하세요; 만약 현재 SDK에 직접 접근자가 없으면 Message를 빌더로 재구성하거나 FirebaseMessaging 호출(예:
send/sendAsync)을 검증하는 행위 기반 어서션으로 대체해 내부 필드에 의존하지 않도록 수정하세요.
src/main/java/com/kustacks/kuring/club/application/service/ClubCommandService.java
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/application/service/ClubCommandService.java
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/application/service/ClubCommandService.java
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/application/service/ClubNotificationService.java
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java
Outdated
Show resolved
Hide resolved
src/main/resources/db/migration/V260217__Alter_club_subscribe_to_root_user.sql
Outdated
Show resolved
Hide resolved
jiyun921
reviewed
Feb 22, 2026
Comment on lines
22
to
45
| @Transactional | ||
| @RequiredArgsConstructor | ||
| public class ClubCommandService implements ClubSubscriptionUseCase { | ||
|
|
||
| private static final String CLUB_TOPIC_PREFIX = "club."; | ||
|
|
||
| private final ClubQueryPort clubQueryPort; | ||
| private final ClubSubscriptionCommandPort clubSubscriptionCommandPort; | ||
| private final ClubSubscriptionQueryPort countSubscriptionsQueryPort; | ||
| private final RootUserQueryPort rootUserQueryPort; | ||
| private final UserQueryPort userQueryPort; | ||
| private final UserEventPort userEventPort; | ||
|
|
||
| @Override | ||
| public long addSubscription(ClubSubscriptionCommand command) { | ||
| RootUser rootUser = findRootUserByEmail(command.email()); | ||
| Club club = findClubById(command.clubId()); | ||
|
|
||
| if (isAlreadySubscription(rootUser, club)) { | ||
| throw new InvalidStateException(ErrorCode.CLUB_ALREADY_SUBSCRIBED); | ||
| } | ||
|
|
||
| clubSubscriptionCommandPort.saveSubscription(rootUser, club); | ||
| subscribeAllLoggedInDevices(rootUser.getId(), makeTopic(club)); |
Collaborator
There was a problem hiding this comment.
fcm 구독 로직이 트랜잭션 내부에 있어서 subscribeAllLoggedInDevices 중간에 예외 발생하면 db만 롤백되고 이미 수행된 일부 fcm 구독은 롤백 안되는 문제가 생길수도 있을 것 같아요!
Member
Author
There was a problem hiding this comment.
구독 Event가 TransactionalEventListner로 동작하고 있어서 트랜잭션 커밋 후에 이벤트가 진행하도록 되어있는걸로 확인했습니다! 그래서 DB가 롤백될 상황이라면 FCM구독도 이루어지지 않을거 같습니다!
src/main/java/com/kustacks/kuring/club/application/service/ClubCommandService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java (1)
47-47:⚠️ Potential issue | 🟡 Minor
userToken파라미터가 여전히 사용되지 않음
addSubscription과deleteSubscription모두에서userToken이 선언되었지만 메서드 본문에서 전혀 참조되지 않습니다. FCM 토큰이 실제로 필요하지 않다면 파라미터를 제거하거나, 필요하다면ClubSubscriptionCommand에 포함하여 서비스 레이어로 전달하세요.Also applies to: 62-62
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java` at line 47, The userToken RequestHeader parameter is declared but unused in UserClubSubscriptionApiV2; either remove it from the addSubscription and deleteSubscription method signatures or propagate it into the command object and service call: update addSubscription(...) and deleteSubscription(...) to either drop the userToken parameter (and remove FCM_TOKEN_HEADER_KEY usage) or set the token into ClubSubscriptionCommand (or a new field on that command) and pass that command to the subscriptionService methods so the service layer receives the FCM token.
🧹 Nitpick comments (1)
src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java (1)
72-77:validateJwtAndGetEmail헬퍼 메서드 중복 정리 권장해당 JWT 검증 + 이메일 추출 로직이 최소 7개의 웹 어댑터 클래스(UserClubSubscriptionApiV2, UserQueryApiV2, EmailCommandApiV2, UserCommandApiV2, NoticeQueryApiV2, NoticeCommandApiV2, RAGQueryApiV2)에서 동일하게 구현되어 있습니다. 공통 베이스 클래스나 유틸리티 클래스로 추출하면 유지보수성을 높일 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java` around lines 72 - 77, Extract the duplicated JWT validation+email extraction into a single shared helper (either a common abstract web adapter base class or a JwtUtil/AuthenticationService) and replace the validateJwtAndGetEmail implementations across the web adapters (e.g., UserClubSubscriptionApiV2.validateJwtAndGetEmail, UserQueryApiV2, EmailCommandApiV2, UserCommandApiV2, NoticeQueryApiV2, NoticeCommandApiV2, RAGQueryApiV2) with calls to that shared method; the shared method should call jwtTokenProvider.validateToken(jwt) and jwtTokenProvider.getPrincipal(jwt) and throw InvalidStateException(ErrorCode.JWT_INVALID_TOKEN) when validation fails so behavior remains identical.
🤖 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/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java`:
- Line 47: The userToken RequestHeader parameter is declared but unused in
UserClubSubscriptionApiV2; either remove it from the addSubscription and
deleteSubscription method signatures or propagate it into the command object and
service call: update addSubscription(...) and deleteSubscription(...) to either
drop the userToken parameter (and remove FCM_TOKEN_HEADER_KEY usage) or set the
token into ClubSubscriptionCommand (or a new field on that command) and pass
that command to the subscriptionService methods so the service layer receives
the FCM token.
---
Nitpick comments:
In
`@src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java`:
- Around line 72-77: Extract the duplicated JWT validation+email extraction into
a single shared helper (either a common abstract web adapter base class or a
JwtUtil/AuthenticationService) and replace the validateJwtAndGetEmail
implementations across the web adapters (e.g.,
UserClubSubscriptionApiV2.validateJwtAndGetEmail, UserQueryApiV2,
EmailCommandApiV2, UserCommandApiV2, NoticeQueryApiV2, NoticeCommandApiV2,
RAGQueryApiV2) with calls to that shared method; the shared method should call
jwtTokenProvider.validateToken(jwt) and jwtTokenProvider.getPrincipal(jwt) and
throw InvalidStateException(ErrorCode.JWT_INVALID_TOKEN) when validation fails
so behavior remains identical.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/kustacks/kuring/common/dto/ResponseCodeAndMessages.javasrc/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.javasrc/test/java/com/kustacks/kuring/acceptance/UserStep.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/kustacks/kuring/common/dto/ResponseCodeAndMessages.java
- src/test/java/com/kustacks/kuring/acceptance/UserStep.java
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java (1)
49-49:⚠️ Potential issue | 🟡 Minor
userToken파라미터가 두 핸들러 모두에서 여전히 사용되지 않음
addSubscription과deleteSubscription모두@RequestHeader(FCM_TOKEN_HEADER_KEY) String userToken을 필수로 선언하지만, 메서드 본문에서 전혀 사용하지 않습니다. 클라이언트에게 불필요한 헤더 전송을 강요하는 문제가 남아 있습니다.FCM 토큰이 실제로 필요 없다면 파라미터를 제거하거나
required = false로 설정하고, 필요하다면ClubSubscriptionCommand를 통해 서비스 레이어로 전달하세요.Also applies to: 64-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java` at line 49, Both addSubscription and deleteSubscription currently declare `@RequestHeader`(FCM_TOKEN_HEADER_KEY) String userToken but never use it; either remove the header parameter from the method signatures or mark it optional (required = false) and forward it into the service via the existing ClubSubscriptionCommand if the token is needed; update the signatures of addSubscription and deleteSubscription and adjust construction of ClubSubscriptionCommand (or the service call) to include userToken when present, referencing addSubscription, deleteSubscription, FCM_TOKEN_HEADER_KEY, and ClubSubscriptionCommand to locate changes.
🧹 Nitpick comments (4)
src/test/java/com/kustacks/kuring/club/application/service/ClubCommandServiceTest.java (2)
121-143: 구독 삭제 포트 호출도 함께 검증하면 더 견고합니다.
삭제 누락 회귀를 조기에 잡을 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/kustacks/kuring/club/application/service/ClubCommandServiceTest.java` around lines 121 - 143, The test currently verifies unsubscribeEvent but doesn't assert that the subscription deletion port is invoked; update remove_subscription_success to also verify the deletion call (e.g., verify(clubSubscriptionPort).deleteSubscription(1L, club.getId()) or the actual delete method used by ClubCommandService) after calling service.removeSubscription(new ClubSubscriptionCommand("[email protected]", 1L)); ensure you reference the same identifiers used in the test (service.removeSubscription, ClubSubscriptionCommand, clubSubscriptionQueryPort.existsSubscription, userEventPort.unsubscribeEvent) and assert the delete invocation with the correct arguments.
75-99: 구독 저장 포트 호출도 검증해 주세요.
이벤트/카운트만 검증하면 실제 저장 누락을 놓칠 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/kustacks/kuring/club/application/service/ClubCommandServiceTest.java` around lines 75 - 99, The test add_subscription_success currently only asserts events and count but doesn't verify the subscription persistence; update the test (ClubCommandServiceTest.add_subscription_success) to also verify that the command/save port is called when service.addSubscription(...) is executed — e.g., add a verify(...) for the subscription saving port (such as clubSubscriptionCommandPort.save(...) or clubSubscriptionStorePort.saveSubscription(...)) with the expected ClubSubscription or parameters to ensure the subscription is actually persisted.src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java (1)
38-38: SecurityScheme 정의는 올바르게 구성되었으나 일관성 패턴을 정비해야 함
@SecurityRequirement(name = "JWT")에 대응하는 Swagger SecurityScheme은 SwaggerConfiguration.java에서.addSecuritySchemes("JWT", JwtAuth())로 정상 정의되어 있습니다.다만 코드베이스에서 보안 스킴 참조 방식이 일관되지 않습니다: UserClubSubscriptionApiV2와 같이
JWT_TOKEN_HEADER_KEY상수를 사용하는 어댑터가 있는 반면, AdminQueryApiV2/AdminCommandApiV2 등은@SecurityRequirement(name = "JWT")로 직접 문자열을 사용하고 있습니다. 코드 유지보수성을 위해 전체 어댑터에서 상수 기반 접근법으로 통일하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java` at line 38, Multiple adapters reference the "JWT" security scheme inconsistently; unify them to use the constant. Replace literal "JWT" usages in SecurityRequirement annotations and any other places (e.g., AdminQueryApiV2, AdminCommandApiV2) with the shared constant JWT_TOKEN_HEADER_KEY defined in UserClubSubscriptionApiV2 (or move that constant to a common place and use it everywhere) so that `@SecurityRequirement`(name = JWT_TOKEN_HEADER_KEY) and SwaggerConfiguration.addSecuritySchemes(JWT_TOKEN_HEADER_KEY, JwtAuth()) use the same symbol; ensure the constant is publicly accessible to other classes if you relocate it.src/main/resources/db/migration/V260217__Alter_club_subscribe_to_root_user.sql (1)
23-34: Line 23-34: FK 추가 후 별도 인덱스가 중복될 가능성을 확인해 주세요.InnoDB는 FK 추가 시 참조 컬럼 인덱스가 없으면 자동 생성합니다. 현재는 FK 추가 후
idx_club_subscribe_root_user를 만들고 있어 중복 인덱스가 생길 수 있습니다. 인덱스를 FK 전에 생성하거나 자동 생성 인덱스를 재사용하는 방향을 검토해 주세요.예:
SHOW INDEX FROM club_subscribe WHERE Column_name = 'root_user_id';♻️ 인덱스 선생성 예시
-ALTER TABLE club_subscribe - ADD CONSTRAINT fk_club_subscribe_root_user - FOREIGN KEY (root_user_id) REFERENCES root_user (id) - ON DELETE CASCADE; - --- 7) root_user 기준 조회 성능을 위한 인덱스 추가 -CREATE INDEX idx_club_subscribe_root_user ON club_subscribe (root_user_id); +-- 7) root_user 기준 조회 성능을 위한 인덱스 추가 (FK 전에 생성) +CREATE INDEX idx_club_subscribe_root_user ON club_subscribe (root_user_id); + +ALTER TABLE club_subscribe + ADD CONSTRAINT fk_club_subscribe_root_user + FOREIGN KEY (root_user_id) REFERENCES root_user (id) + ON DELETE CASCADE;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/V260217__Alter_club_subscribe_to_root_user.sql` around lines 23 - 34, The FK creation for fk_club_subscribe_root_user may cause a duplicate index for root_user_id because InnoDB auto-creates an index when adding a FK; before creating CREATE INDEX idx_club_subscribe_root_user you should check for an existing index on club_subscribe.root_user_id (e.g. SHOW INDEX FROM club_subscribe WHERE Column_name='root_user_id') and either: 1) create the explicit index before adding the FOREIGN KEY so the FK reuses it, or 2) skip CREATE INDEX if the auto-created index already exists, or 3) drop the manually created idx_club_subscribe_root_user and rely on the FK index; update the migration to perform that check/ordering and ensure fk_club_subscribe_root_user and uk_club_root_user are applied without producing duplicate indexes.
🤖 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/resources/db/migration/V260217__Alter_club_subscribe_to_root_user.sql`:
- Around line 1-3: The migration currently contains an unconditional "DELETE
FROM club_subscribe" which will wipe production subscriptions; change the
migration in V260217__Alter_club_subscribe_to_root_user.sql to be safe by either
(A) adding a pre-check that aborts the migration if "SELECT COUNT(*) FROM
club_subscribe" > 0 (fail-fast/gate the deploy), or (B) implement an explicit
data-migration path that preserves and transforms existing rows into the new
schema before deleting; update the migration to perform the check and abort with
a clear error message or include the data-migration steps instead of the raw
DELETE statement.
---
Duplicate comments:
In
`@src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java`:
- Line 49: Both addSubscription and deleteSubscription currently declare
`@RequestHeader`(FCM_TOKEN_HEADER_KEY) String userToken but never use it; either
remove the header parameter from the method signatures or mark it optional
(required = false) and forward it into the service via the existing
ClubSubscriptionCommand if the token is needed; update the signatures of
addSubscription and deleteSubscription and adjust construction of
ClubSubscriptionCommand (or the service call) to include userToken when present,
referencing addSubscription, deleteSubscription, FCM_TOKEN_HEADER_KEY, and
ClubSubscriptionCommand to locate changes.
---
Nitpick comments:
In
`@src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java`:
- Line 38: Multiple adapters reference the "JWT" security scheme inconsistently;
unify them to use the constant. Replace literal "JWT" usages in
SecurityRequirement annotations and any other places (e.g., AdminQueryApiV2,
AdminCommandApiV2) with the shared constant JWT_TOKEN_HEADER_KEY defined in
UserClubSubscriptionApiV2 (or move that constant to a common place and use it
everywhere) so that `@SecurityRequirement`(name = JWT_TOKEN_HEADER_KEY) and
SwaggerConfiguration.addSecuritySchemes(JWT_TOKEN_HEADER_KEY, JwtAuth()) use the
same symbol; ensure the constant is publicly accessible to other classes if you
relocate it.
In
`@src/main/resources/db/migration/V260217__Alter_club_subscribe_to_root_user.sql`:
- Around line 23-34: The FK creation for fk_club_subscribe_root_user may cause a
duplicate index for root_user_id because InnoDB auto-creates an index when
adding a FK; before creating CREATE INDEX idx_club_subscribe_root_user you
should check for an existing index on club_subscribe.root_user_id (e.g. SHOW
INDEX FROM club_subscribe WHERE Column_name='root_user_id') and either: 1)
create the explicit index before adding the FOREIGN KEY so the FK reuses it, or
2) skip CREATE INDEX if the auto-created index already exists, or 3) drop the
manually created idx_club_subscribe_root_user and rely on the FK index; update
the migration to perform that check/ordering and ensure
fk_club_subscribe_root_user and uk_club_root_user are applied without producing
duplicate indexes.
In
`@src/test/java/com/kustacks/kuring/club/application/service/ClubCommandServiceTest.java`:
- Around line 121-143: The test currently verifies unsubscribeEvent but doesn't
assert that the subscription deletion port is invoked; update
remove_subscription_success to also verify the deletion call (e.g.,
verify(clubSubscriptionPort).deleteSubscription(1L, club.getId()) or the actual
delete method used by ClubCommandService) after calling
service.removeSubscription(new ClubSubscriptionCommand("[email protected]",
1L)); ensure you reference the same identifiers used in the test
(service.removeSubscription, ClubSubscriptionCommand,
clubSubscriptionQueryPort.existsSubscription, userEventPort.unsubscribeEvent)
and assert the delete invocation with the correct arguments.
- Around line 75-99: The test add_subscription_success currently only asserts
events and count but doesn't verify the subscription persistence; update the
test (ClubCommandServiceTest.add_subscription_success) to also verify that the
command/save port is called when service.addSubscription(...) is executed —
e.g., add a verify(...) for the subscription saving port (such as
clubSubscriptionCommandPort.save(...) or
clubSubscriptionStorePort.saveSubscription(...)) with the expected
ClubSubscription or parameters to ensure the subscription is actually persisted.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/kustacks/kuring/club/domain/Club.javasrc/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.javasrc/main/resources/db/migration/V260217__Alter_club_subscribe_to_root_user.sqlsrc/test/java/com/kustacks/kuring/club/application/service/ClubCommandServiceTest.java
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/kustacks/kuring/club/domain/Club.java
Comment on lines
+1
to
+3
| -- root_user로 바꾸면 데이터 이관을 해야하나, 아직 데이터가 존재하지 않는 개발 단계이므로 데이터 삭제하고 진행. | ||
| -- 1) 기존 device(user) 기준 구독 데이터는 유지하지 않는다. | ||
| DELETE FROM club_subscribe; |
There was a problem hiding this comment.
Line 1-3: 구독 데이터 전량 삭제는 운영 데이터 손실 위험이 큽니다.
테이블에 데이터가 존재하는 환경에서 이 마이그레이션을 실행하면 모든 구독 정보가 영구 삭제됩니다. 배포 전에 club_subscribe가 비어있음을 자동 검증하거나(배포 체크리스트/게이트), 데이터 이관 경로를 마련해 두는 것이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/resources/db/migration/V260217__Alter_club_subscribe_to_root_user.sql`
around lines 1 - 3, The migration currently contains an unconditional "DELETE
FROM club_subscribe" which will wipe production subscriptions; change the
migration in V260217__Alter_club_subscribe_to_root_user.sql to be safe by either
(A) adding a pre-check that aborts the migration if "SELECT COUNT(*) FROM
club_subscribe" > 0 (fail-fast/gate the deploy), or (B) implement an explicit
data-migration path that preserves and transforms existing rows into the new
schema before deleting; update the migration to perform the check and abort with
a clear error message or include the data-migration steps instead of the raw
DELETE statement.
061f054 to
23496af
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#️⃣ 이슈
📌 요약
root_user기준으로 구현했습니다.🛠️ 상세
API
POST /api/v2/users/subscriptions/clubsDELETE /api/v2/users/subscriptions/clubsBearer JWT + User-Token필수DB/Flyway
V260217__Alter_club_subscribe_to_root_user.sqlclub_subscribe.user_id->root_user_idFK 전환(club_id, root_user_id)unique 및 cascade 정책 유지Repository & PersistenceAdapter 구조
ClubQueryRepositoryImpl추가findNextDayRecruitEndClubs(now)에서[내일 00:00, 내일모레 00:00)계산 후 조회club_subscribe직접 조회 방식으로 구성알림/스케줄러
ClubNotificationService,ClubNotificationScheduler추가Asia/Seoul)KuringFeatures.NOTIFY_CLUB_DEADLINEclub.{clubId}orclub.{clubId}.dev테스트
UserAcceptanceTest,UserStep에 동아리 구독 시나리오 통합ClubCommandServiceTest,ClubNotificationServiceTestClubPersistenceAdapterTestClubNotificationSchedulerTestclub.domain의존성 규칙 검증 추가DatabaseConfigurator에 club 시드 1건 추가💬 기타
Summary by CodeRabbit
새 기능
테스트
기타