Feat: 동아리 소속 목록 조회 api, 동아리 목록 조회 api, 동아리 상세 정보 조회 api 구현#347
Feat: 동아리 소속 목록 조회 api, 동아리 목록 조회 api, 동아리 상세 정보 조회 api 구현#347
Conversation
Unit Test Results 82 files +1 82 suites +1 1m 32s ⏱️ -3s For more details on these failures, see this check. Results for commit ba06ea0. ± Comparison against base commit e5e2d49. ♻️ This comment has been updated with latest results. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Walkthrough동아리 조회 기능을 위한 새로운 REST API 엔드포인트(ClubQueryApiV2)를 추가합니다. 서비스 계층, 포트, 리포지토리, 데이터 전송 객체, 도메인 모델 및 DB 마이그레이션을 포함하여 동아리 목록, 상세정보, 지원 소속 조회를 지원합니다. Changes
Sequence DiagramsequenceDiagram
participant Client
participant ClubQueryApiV2
participant ClubQueryService
participant ClubQueryPort
participant ClubSubscriptionQueryPort
participant RootUserQueryPort
participant Database
Client->>ClubQueryApiV2: GET /api/v2/clubs/{id}<br/>(with Bearer Token)
activate ClubQueryApiV2
ClubQueryApiV2->>ClubQueryApiV2: extractToken & validate JWT
ClubQueryApiV2->>ClubQueryService: getClubDetail(ClubDetailCommand)
activate ClubQueryService
ClubQueryService->>RootUserQueryPort: findByEmail(email)
activate RootUserQueryPort
RootUserQueryPort->>Database: Query RootUser
Database-->>RootUserQueryPort: RootUser
RootUserQueryPort-->>ClubQueryService: RootUser (Optional)
deactivate RootUserQueryPort
ClubQueryService->>ClubQueryPort: findClubDetailById(clubId)
activate ClubQueryPort
ClubQueryPort->>Database: Query ClubDetail with Joins
Database-->>ClubQueryPort: ClubDetailDto
ClubQueryPort-->>ClubQueryService: ClubDetailDto (Optional)
deactivate ClubQueryPort
ClubQueryService->>ClubSubscriptionQueryPort: countSubscribers(clubId)
activate ClubSubscriptionQueryPort
ClubSubscriptionQueryPort->>Database: Count ClubSubscribe
Database-->>ClubSubscriptionQueryPort: Count
ClubSubscriptionQueryPort-->>ClubQueryService: Long
deactivate ClubSubscriptionQueryPort
alt User is Logged In
ClubQueryService->>ClubSubscriptionQueryPort: existsSubscription(rootUserId, clubId)
activate ClubSubscriptionQueryPort
ClubSubscriptionQueryPort->>Database: Check Subscription
Database-->>ClubSubscriptionQueryPort: boolean
ClubSubscriptionQueryPort-->>ClubQueryService: isSubscribed
deactivate ClubSubscriptionQueryPort
else User Not Logged In
ClubQueryService->>ClubQueryService: isSubscribed = false
end
ClubQueryService->>ClubQueryService: buildClubDetailResult<br/>(map DTOs & compute status)
ClubQueryService-->>ClubQueryApiV2: ClubDetailResult
deactivate ClubQueryService
ClubQueryApiV2->>ClubQueryApiV2: mapToResponse(ClubDetailResult)
ClubQueryApiV2-->>Client: BaseResponse<ClubDetailResponse>
deactivate ClubQueryApiV2
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 |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (6)
src/main/java/com/kustacks/kuring/club/application/port/in/dto/ClubItemResult.java (1)
10-11:ClubDetailResult와category/division타입 불일치
ClubDetailResult는 동일한 필드에ClubCategory,ClubDivision도메인 열거형을 사용하지만,ClubItemResult는String을 사용합니다. 두 DTO가 같은 개념의 데이터를 나타내면서 타입이 다르면, 매핑 레이어에서 변환 실수가 발생하거나 API 응답 일관성이 깨질 수 있습니다. 통일된 표현 방식을 사용하거나, 의도적인 차이라면 주석으로 명시하는 것을 권장합니다.🤖 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/in/dto/ClubItemResult.java` around lines 10 - 11, ClubItemResult uses String for the category and division fields while ClubDetailResult uses the domain enums ClubCategory and ClubDivision, causing inconsistency; change the types of the category and division fields in ClubItemResult to ClubCategory and ClubDivision respectively, then update any mappers (e.g., where ClubItemResult is populated) and unit tests to pass/format the enums consistently (or add clear javadoc if the String choice was intentional) so both DTOs represent the same domain types.src/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java (2)
43-75:mockReadModels필드에final선언 추가를 권장합니다.해당 필드는 초기화 후 변경되지 않으므로
final로 선언하는 것이 적절합니다.♻️ 제안된 변경
- private List<ClubReadModel> mockReadModels = + private final List<ClubReadModel> mockReadModels =🤖 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/ClubQueryServiceTest.java` around lines 43 - 75, The field mockReadModels in ClubQueryServiceTest is immutable after initialization but not declared final; change its declaration to add the final modifier (i.e., make the List<ClubReadModel> mockReadModels field final) so the intent is explicit and the compiler enforces immutability of the reference; locate the field by the symbol mockReadModels in the ClubQueryServiceTest class and update its declaration accordingly.
112-113:countClubs목(mock)이 반환하는 값(2)이searchClubs반환 목록 크기(3)와 불일치합니다.
totalCount가 2이지만 실제로 3개 항목이 반환되는 설정은 현실적이지 않아 테스트 독자에게 혼동을 줄 수 있습니다.totalCount는 반환 목록 크기 이상이어야 의미가 일관됩니다.♻️ 제안된 변경
when(clubQueryPort.countClubs(category, divisionList)) - .thenReturn(2); + .thenReturn(3);🤖 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/ClubQueryServiceTest.java` around lines 112 - 113, The test sets clubQueryPort.countClubs(...) to return 2 while searchClubs returns 3 items, which is inconsistent; update the mock return of countClubs in the test (the when(clubQueryPort.countClubs(category, divisionList)).thenReturn(...)) so that totalCount is at least the size of the list returned by ClubQueryService.searchClubs (e.g., change to 3) or alternatively reduce the mocked searchClubs result to match the count, ensuring totalCount >= returned list size.src/main/java/com/kustacks/kuring/club/application/port/out/dto/ClubDetailDto.java (1)
42-83:@QueryProjection이 실제로 사용되지 않습니다.
ClubQueryRepositoryImpl.findClubDetailById는QClubDetailDto를 이용한 QueryDSL 프로젝션 대신Tuple방식으로ClubDetailDto를 직접 생성합니다. 또한recruitmentStatus는 DB 컬럼이 아닌 계산된 값이므로 QueryDSL 프로젝션으로 처리하는 것 자체가 불가능합니다. 불필요한QClubDetailDto생성을 방지하려면@QueryProjection을 제거하는 것이 좋습니다.🤖 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/dto/ClubDetailDto.java` around lines 42 - 83, Remove the unused QueryDSL projection by deleting the `@QueryProjection` annotation from the ClubDetailDto constructor declaration in ClubDetailDto; ensure no other code relies on QClubDetailDto (the repository ClubQueryRepositoryImpl.findClubDetailById constructs ClubDetailDto from Tuple), and remove any unused import of com.querydsl.core.annotations.QueryProjection or unused generated QClubDetailDto artifacts if present.src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java (1)
200-220:start,end모두null이고isAlways = false일 때RECRUITING을 반환하는 것이 의도된 동작인지 확인하세요.모집 기간 정보가 전혀 없는 경우 기본값으로
RECRUITING을 반환하면 의도치 않게 모집 중으로 표시될 수 있습니다.CLOSED혹은 별도의UNKNOWN상태가 더 적합할 수 있습니다.🤖 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/ClubQueryRepositoryImpl.java` around lines 200 - 220, The current calculateRecruitmentStatus method returns RECRUITING when start and end are both null and isAlways is false; change this to return a safer default (e.g., introduce ClubRecruitmentStatus.UNKNOWN and return UNKNOWN) when start==null && end==null && Boolean.FALSE.equals(isAlways), update the ClubRecruitmentStatus enum to include UNKNOWN, adjust any callers/tests that expect RECRUITING for missing period data, and ensure calculateRecruitmentStatus still handles the existing branches (ALWAYS, BEFORE, CLOSED, RECRUITING) unchanged otherwise.src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java (1)
97-97:userToken파라미터가 사용되지 않습니다.
getClubDetail메서드에서userToken을 인자로 받지만 구현 내부에서 전혀 사용하지 않습니다. 향후 사용 예정이면// TODO주석을 추가하고, 그렇지 않다면 인터페이스와 함께 제거하는 것이 좋습니다.🤖 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/ClubQueryService.java` at line 97, getClubDetail 메서드의 userToken 파라미터가 사용되지 않으므로 사용자 토큰이 향후 필요하다면 ClubQueryService.getClubDetail(Long id, String userToken, Long loginUserId) 선언과 메서드 구현에 "// TODO: userToken will be used for X" 주석을 추가하고 호출부에 전달되는 값을 유지하되 사용 시점을 명시하세요; 필요 없다면 인터페이스와 구현에서 userToken 파라미터를 제거(메서드 시그니처 변경)하고, 관련 호출부들(및 테스트)을 찾아 모두 수정하여 컴파일 오류가 없도록 하세요.
🤖 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/adapter/in/web/ClubQueryApiV2.java`:
- Around line 78-114: The JWT handling between getClubs and getClubDetail is
inconsistent; update getClubDetail to match getClubs by removing the thrown
InvalidStateException and only setting loginUserId when
jwtTokenProvider.validateToken(jwt) returns true (i.e., if bearerToken != null
then extractAuthorizationValue(...), call validateToken(jwt) and if valid set
loginUserId = Long.parseLong(jwtTokenProvider.getPrincipal(jwt)), otherwise
leave loginUserId null), so getClubDetail uses the same silent fallback behavior
as getClubs (unless you intend to require authentication—if so, make getClubs
throw instead).
- Line 82: The code calls Long.parseLong(jwtTokenProvider.getPrincipal(jwt))
(used to set loginUserId at least in ClubQueryApiV2) which can throw
NumberFormatException if the JWT subject isn't a numeric string; add handling by
validating or parsing safely: either change JwtTokenProvider.getPrincipal() to
return a Long (or an Optional<Long>) if you can guarantee numeric subjects, or
wrap the parse in a try/catch that catches NumberFormatException and translates
it into a controlled response (e.g., throw a custom authentication/validation
exception or return a 401/400) so the service does not propagate 500; apply the
same fix for the other occurrence at the second parse around line 113.
In
`@src/main/java/com/kustacks/kuring/club/adapter/in/web/dto/ClubDetailResponse.java`:
- Around line 28-58: The null-check in ClubDetailResponse.from is ineffective
because ClubQueryService.getClubDetail always returns a
ClubDetailResult.Location instance even when all location fields are null; fix
this in ClubQueryService.getClubDetail by changing the logic that constructs
ClubDetailResult.Location so it returns null when all constituent fields
(building, room, lon, lat) are null/absent, i.e., only create new
ClubDetailResult.Location(...) when at least one of those fields is non-null;
keep ClubDetailResponse.from as-is so it will receive a true null when there is
no location.
In
`@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java`:
- Around line 31-58: The cursor check in searchClubs currently uses
idAfterCursor(cursor) which only applies club.id.gt(Long.parseLong(cursor)),
causing missing rows when sort order is not by id; update idAfterCursor (or
create a new method used by searchClubs) to decode a composite cursor that
includes the last sort key and lastId (e.g. "lastKey|lastId") and produce a
boolean expression matching keyset pagination: for sortBy "name" produce
(club.name.gt(lastName).or(club.name.eq(lastName).and(club.id.gt(lastId)))) and
for "recruitEndAt" produce
(club.recruitEndAt.gt(lastEndAt).or(club.recruitEndAt.eq(lastEndAt).and(club.id.gt(lastId)))),
falling back to club.id.gt(lastId) for id-only sort; ensure searchClubs uses
getOrderSpecifiers(sortBy) and the new composite cursor predicate so ordering
and cursor logic align.
- Around line 180-183: The idAfterCursor method currently calls
Long.parseLong(cursor) without handling NumberFormatException; update
idAfterCursor(String cursor) to validate or safely parse the cursor (e.g.,
try-catch NumberFormatException or use a numeric-checked parse) and if parsing
fails return null (or an appropriate empty predicate) instead of letting the
exception propagate; ensure you keep the existing behavior of returning null for
null/"0" and use the parsed long value with club.id.gt(...) when parsing
succeeds.
- Around line 39-58: The projection is mapping club.posterImagePath into the
ClubReadModel's 4th constructor parameter (iconImageUrl), which is semantically
incorrect; pick one resolution and implement it consistently: either add an
iconImagePath field to the Club entity and use club.iconImagePath in the
QClubReadModel projection inside ClubQueryRepositoryImpl, or rename/change
ClubReadModel's 4th parameter to posterImageUrl and update
QClubReadModel/constructor usages accordingly so the projected field names and
entity fields match (references: ClubQueryRepositoryImpl, QClubReadModel,
club.posterImagePath, ClubReadModel constructor).
In
`@src/main/java/com/kustacks/kuring/club/application/port/in/ClubQueryUseCase.java`:
- Line 15: getClubDetail 메서드의 파라미터 중 userToken과 loginUserId가 중복되어 책임 경계가 모호합니다:
ClubQueryUseCase 인터페이스의 getClubDetail(Long id, String userToken, Long
loginUserId)에서 userToken이 단순 식별용이라면 userToken을 제거하고 loginUserId만 사용하도록 시그니처를
변경하고, 컨트롤러/시큐리티 레이어에서 토큰을 파싱해 loginUserId를 전달하도록 수정하세요; 만약 userToken이 외부 API 호출
등 별도 목적이라면 getClubDetail 선언에 해당 의도를 주석으로 명확히 남기고 getClubs(Long id, Long
loginUserId)와의 일관성을 위해 다른 메서드들도 같은 패턴(userToken 포함)으로 맞추거나 인터페이스 설계 문서를 업데이트하세요.
In
`@src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java`:
- Around line 55-82: The current mapping in ClubQueryService iterates
cursorBasedList.getContents() and calls clubQueryPort.countSubscribers(...) and
clubQueryPort.existsSubscription(...) per club causing N+1 queries; instead add
port methods to fetch subscriber counts and subscription existence in bulk
(e.g., clubQueryPort.countSubscribersByClubIds(List<Long> clubIds) returning a
Map<Id,Integer> and clubQueryPort.findSubscribedClubIdsByUser(Long userId,
List<Long> clubIds) returning a Set<Id>), call those once before the stream,
then construct each ClubItemResult using the pre-fetched counts and subscription
set (replace per-item calls to countSubscribers and existsSubscription with
lookups into the returned collections).
- Around line 109-133: The code in ClubQueryService always constructs a new
ClubDetailResult.Location even when all location fields are null, causing
ClubDetailResponse.from()'s result.location() to be non-null with all-null
fields; change the return construction in ClubQueryService so you only create
and pass a new ClubDetailResult.Location when at least one of dto.getBuilding(),
dto.getRoom(), dto.getLon(), dto.getLat() is non-null (otherwise pass null),
ensuring Location is null for clubs without location data.
---
Nitpick comments:
In
`@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java`:
- Around line 200-220: The current calculateRecruitmentStatus method returns
RECRUITING when start and end are both null and isAlways is false; change this
to return a safer default (e.g., introduce ClubRecruitmentStatus.UNKNOWN and
return UNKNOWN) when start==null && end==null && Boolean.FALSE.equals(isAlways),
update the ClubRecruitmentStatus enum to include UNKNOWN, adjust any
callers/tests that expect RECRUITING for missing period data, and ensure
calculateRecruitmentStatus still handles the existing branches (ALWAYS, BEFORE,
CLOSED, RECRUITING) unchanged otherwise.
In
`@src/main/java/com/kustacks/kuring/club/application/port/in/dto/ClubItemResult.java`:
- Around line 10-11: ClubItemResult uses String for the category and division
fields while ClubDetailResult uses the domain enums ClubCategory and
ClubDivision, causing inconsistency; change the types of the category and
division fields in ClubItemResult to ClubCategory and ClubDivision respectively,
then update any mappers (e.g., where ClubItemResult is populated) and unit tests
to pass/format the enums consistently (or add clear javadoc if the String choice
was intentional) so both DTOs represent the same domain types.
In
`@src/main/java/com/kustacks/kuring/club/application/port/out/dto/ClubDetailDto.java`:
- Around line 42-83: Remove the unused QueryDSL projection by deleting the
`@QueryProjection` annotation from the ClubDetailDto constructor declaration in
ClubDetailDto; ensure no other code relies on QClubDetailDto (the repository
ClubQueryRepositoryImpl.findClubDetailById constructs ClubDetailDto from Tuple),
and remove any unused import of com.querydsl.core.annotations.QueryProjection or
unused generated QClubDetailDto artifacts if present.
In
`@src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java`:
- Line 97: getClubDetail 메서드의 userToken 파라미터가 사용되지 않으므로 사용자 토큰이 향후 필요하다면
ClubQueryService.getClubDetail(Long id, String userToken, Long loginUserId) 선언과
메서드 구현에 "// TODO: userToken will be used for X" 주석을 추가하고 호출부에 전달되는 값을 유지하되 사용
시점을 명시하세요; 필요 없다면 인터페이스와 구현에서 userToken 파라미터를 제거(메서드 시그니처 변경)하고, 관련 호출부들(및 테스트)을
찾아 모두 수정하여 컴파일 오류가 없도록 하세요.
In
`@src/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java`:
- Around line 43-75: The field mockReadModels in ClubQueryServiceTest is
immutable after initialization but not declared final; change its declaration to
add the final modifier (i.e., make the List<ClubReadModel> mockReadModels field
final) so the intent is explicit and the compiler enforces immutability of the
reference; locate the field by the symbol mockReadModels in the
ClubQueryServiceTest class and update its declaration accordingly.
- Around line 112-113: The test sets clubQueryPort.countClubs(...) to return 2
while searchClubs returns 3 items, which is inconsistent; update the mock return
of countClubs in the test (the when(clubQueryPort.countClubs(category,
divisionList)).thenReturn(...)) so that totalCount is at least the size of the
list returned by ClubQueryService.searchClubs (e.g., change to 3) or
alternatively reduce the mocked searchClubs result to match the count, ensuring
totalCount >= returned list size.
src/main/java/com/kustacks/kuring/club/adapter/in/web/ClubQueryApiV2.java
Outdated
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/adapter/in/web/ClubQueryApiV2.java
Outdated
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/adapter/in/web/dto/ClubDetailResponse.java
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/application/port/in/ClubQueryUseCase.java
Outdated
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java (1)
257-268:getOrderSpecifiers에서recruitmentGroup헬퍼 메서드를 재사용하지 않음Lines 259-262의 inline CASE 표현식이 lines 277-282의
recruitmentGroup(now)메서드와 완전히 동일합니다.cursorCondition은recruitmentGroup을 재사용하지만,getOrderSpecifiers는 복제합니다.♻️ 제안된 리팩터
case "recruitEndDate" -> { - - var statusOrder = new CaseBuilder() - .when(club.recruitEndAt.isNull()).then(2) - .when(club.recruitEndAt.lt(now)).then(1) - .otherwise(0); - + var statusOrder = recruitmentGroup(now); yield new OrderSpecifier[]{ statusOrder.asc(), club.recruitEndAt.asc().nullsLast(), club.id.asc() }; }🤖 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/ClubQueryRepositoryImpl.java` around lines 257 - 268, The order specifier for "recruitEndDate" duplicates the CASE logic already implemented in recruitmentGroup(now); modify getOrderSpecifiers to call recruitmentGroup(now) instead of inlining the CaseBuilder: obtain the CaseBuilder/Expression from recruitmentGroup(now) (as used by cursorCondition) and use it as statusOrder, then yield the same OrderSpecifier array (statusOrder.asc(), club.recruitEndAt.asc().nullsLast(), club.id.asc()); this removes duplication and ensures both cursorCondition and getOrderSpecifiers share the same recruitmentGroup logic.src/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java (1)
51-52:mockReadModels필드를final로 선언재할당되지 않는 테스트 픽스처 필드는
final로 선언하는 것이 관례입니다.♻️ 제안된 수정
- private List<ClubReadModel> mockReadModels = + private final List<ClubReadModel> mockReadModels =🤖 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/ClubQueryServiceTest.java` around lines 51 - 52, The field mockReadModels in ClubQueryServiceTest should be declared final since it’s a test fixture that is never reassigned; update the declaration of mockReadModels to be private final List<ClubReadModel> mockReadModels to reflect immutability and follow test convention, ensuring no other code tries to reassign it.src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java (1)
44-65:null일 때 불필요한rootUserQueryPort.findRootUserByEmail(null)DB 호출 발생
getClubs와getClubDetail모두email == null인 경우에도findRootUserByEmail(null)을 호출합니다. 비로그인 사용자의 요청마다 불필요한 DB 조회가 발생합니다.♻️ 제안된 수정 (getClubs 및 getClubDetail 모두 동일하게 적용)
- Optional<RootUser> optionalRootUser = rootUserQueryPort.findRootUserByEmail(email); - Long loginUserId = optionalRootUser.map(RootUser::getId).orElse(null); + Long loginUserId = email != null + ? rootUserQueryPort.findRootUserByEmail(email) + .map(RootUser::getId) + .orElse(null) + : null;Also applies to: 107-112
🤖 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/ClubQueryService.java` around lines 44 - 65, Avoid calling rootUserQueryPort.findRootUserByEmail when email is null: in getClubs (and mirror the same change in getClubDetail) guard the DB call with an email null check so you only call rootUserQueryPort.findRootUserByEmail(email) when email != null, otherwise keep loginUserId as null; locate the usage around Optional<RootUser> optionalRootUser = rootUserQueryPort.findRootUserByEmail(email) and the mapping to loginUserId (RootUser::getId) and modify control flow so the optional lookup is skipped for null email and the rest of the method uses the existing loginUserId variable unchanged.
🤖 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/adapter/out/persistence/ClubPersistenceAdapter.java`:
- Around line 64-99: countSubscribersByClubIds and findSubscribedClubIds
currently load full ClubSubscribe entities (risking lazy-load of club and OOM)
and aggregate in memory; change to repository-level aggregation/projection
queries that return clubId and count/exists directly. Add methods on
clubSubscribeRepository (e.g., findSubscriberCountsByClubIds(List<Long> clubIds)
with a `@Query` selecting s.club.id AS clubId, COUNT(s) AS cnt GROUP BY s.club.id
and findSubscribedClubIdsByUser(List<Long> clubIds, Long loginUserId) with a
`@Query` selecting s.club.id where s.user.loginUserId = :loginUserId) and adapt
countSubscribersByClubIds and findSubscribedClubIds to call those repo methods
and convert the projection results into Map<Long,Integer> / Map<Long,Boolean>
without loading full entities.
In
`@src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java`:
- Around line 161-181: The cursor parsing in cursorCondition breaks when club
names contain '|' because it uses split("\\|") and then
Long.parseLong(parts[1]); update the parsing logic (not generateCursor) to
locate the last '|' via lastIndexOf('|'), extract the id as the substring after
that last delimiter and parse it to Long, and treat the prefix (name) as
everything before that last delimiter so names containing '|' are preserved and
parsing never attempts to parse non-numeric parts; adjust any null/empty checks
and exception handling accordingly in the method that currently performs
split("\\|") (cursorCondition).
In
`@src/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java`:
- Around line 200-204: 테스트 getClubs_withoutLogin에서 잘못된 메서드를 검증하고 있으므로
verify(clubQueryPort, never()).existsSubscription(...) 대신 로그인 없을 때 구독 관련 조회가
호출되지 않음을 검증하도록 변경하세요; 구체적으로 ClubQueryServiceTest의 getClubs_withoutLogin에서
verify(clubQueryPort, never()).findSubscribedClubIds(anyLong()) 를 추가하거나
existsSubscription 검증을 삭제하고 findSubscribedClubIds 호출 부재를 검증하여 비로그인 경로에서
findSubscribedClubIds가 호출되지 않음을 확인하십시오.
---
Duplicate comments:
In
`@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java`:
- Line 47: The code maps Club.posterImagePath into the 4th parameter of
ClubReadModel (which is semantically iconImageUrl) causing a mismatch used later
by ClubQueryService (r.getIconImageUrl()); fix by making the mapping explicit
and correct: either pass club.getIconImageUrl() into the ClubReadModel
constructor (or add a getIconImageUrl() on Club that returns posterImagePath if
that is the intended source), or adjust the ClubReadModel constructor/parameter
order so the field names match; update ClubQueryRepositoryImpl to use the
correct symbol (iconImageUrl/getIconImageUrl) instead of posterImagePath so
ClubQueryService.r.getIconImageUrl() remains valid.
---
Nitpick comments:
In
`@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java`:
- Around line 257-268: The order specifier for "recruitEndDate" duplicates the
CASE logic already implemented in recruitmentGroup(now); modify
getOrderSpecifiers to call recruitmentGroup(now) instead of inlining the
CaseBuilder: obtain the CaseBuilder/Expression from recruitmentGroup(now) (as
used by cursorCondition) and use it as statusOrder, then yield the same
OrderSpecifier array (statusOrder.asc(), club.recruitEndAt.asc().nullsLast(),
club.id.asc()); this removes duplication and ensures both cursorCondition and
getOrderSpecifiers share the same recruitmentGroup logic.
In
`@src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java`:
- Around line 44-65: Avoid calling rootUserQueryPort.findRootUserByEmail when
email is null: in getClubs (and mirror the same change in getClubDetail) guard
the DB call with an email null check so you only call
rootUserQueryPort.findRootUserByEmail(email) when email != null, otherwise keep
loginUserId as null; locate the usage around Optional<RootUser> optionalRootUser
= rootUserQueryPort.findRootUserByEmail(email) and the mapping to loginUserId
(RootUser::getId) and modify control flow so the optional lookup is skipped for
null email and the rest of the method uses the existing loginUserId variable
unchanged.
In
`@src/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java`:
- Around line 51-52: The field mockReadModels in ClubQueryServiceTest should be
declared final since it’s a test fixture that is never reassigned; update the
declaration of mockReadModels to be private final List<ClubReadModel>
mockReadModels to reflect immutability and follow test convention, ensuring no
other code tries to reassign it.
src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapter.java
Outdated
Show resolved
Hide resolved
| private String generateCursor(ClubReadModel club, String sortBy, LocalDateTime now) { | ||
| return switch (sortBy) { | ||
| case "name" -> club.getName() + "|" + club.getId(); | ||
| case "recruitEndDate" -> { | ||
| int group; | ||
| if (club.getRecruitEndDate() == null) { | ||
| group = 2; | ||
| } else if (club.getRecruitEndDate().isBefore(now)) { | ||
| group = 1; | ||
| } else { | ||
| group = 0; | ||
| } | ||
|
|
||
| String datePart = club.getRecruitEndDate() == null | ||
| ? "null" | ||
| : club.getRecruitEndDate().toString(); | ||
|
|
||
| yield group + "|" + datePart + "|" + club.getId(); | ||
| } | ||
| default -> club.getId().toString(); | ||
| }; |
There was a problem hiding this comment.
name 정렬 커서에 | 구분자 사용 시 동아리명에 | 포함될 경우 페이지네이션 파괴
generateCursor에서 name 정렬 커서를 "동아리명|id" 형태로 생성하는데, 동아리명에 |가 포함될 경우 ("A|B 동아리" → 커서 "A|B 동아리|123"), cursorCondition에서 split("\\|")로 분리하면 parts[1]이 "B 동아리"가 되어 Long.parseLong("B 동아리")에서 NumberFormatException → try-catch에 잡혀 null 반환 → 커서 조건 없이 처음부터 재조회 → 중복 항목 노출 및 무한 루프성 페이지네이션 오류가 발생합니다.
ID는 항상 마지막 | 이후에 위치하므로, lastIndexOf('|')로 파싱하면 동아리명 내의 |에 안전합니다.
🐛 제안된 수정
- case "name" -> {
- if (parts.length < 2) yield null;
-
- String lastName = parts[0];
- Long lastId = Long.parseLong(parts[1]);
-
+ case "name" -> {
+ int lastPipe = cursor.lastIndexOf('|');
+ if (lastPipe < 0) yield null;
+
+ String lastName = cursor.substring(0, lastPipe);
+ Long lastId = Long.parseLong(cursor.substring(lastPipe + 1));
+
yield club.name.gt(lastName)
.or(
club.name.eq(lastName)
.and(club.id.gt(lastId))
);
}그리고 generateCursor의 name 케이스는 변경 불필요 ("동아리명|id" 형식 유지, 파싱 쪽에서 처리).
🤖 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/ClubQueryService.java`
around lines 161 - 181, The cursor parsing in cursorCondition breaks when club
names contain '|' because it uses split("\\|") and then
Long.parseLong(parts[1]); update the parsing logic (not generateCursor) to
locate the last '|' via lastIndexOf('|'), extract the id as the substring after
that last delimiter and parse it to Long, and treat the prefix (name) as
everything before that last delimiter so names containing '|' are preserved and
parsing never attempts to parse non-numeric parts; adjust any null/empty checks
and exception handling accordingly in the method that currently performs
split("\\|") (cursorCondition).
src/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.java
Outdated
Show resolved
Hide resolved
|
코드래빗 리뷰 확인해서 수정했습니다! 변경 사항
질문상세 조회 API 명세상 userToken을 함께 받도록 되어 있는데 현재 코드상에서는 사용되지 않고 있어서 이 파라미터를 제거해도 되는지 확인 부탁드립니다!! |
There was a problem hiding this comment.
고생하셨습니다 리뷰가 늦어져 죄송합니다! 코멘트 확인 후 답글 꼭 부탁드립니다~
전체적인 이야기를 해보자면 로직의 흐름은 잘 만들어진거 같으나, AI가 과도하게 영향을 끼쳐서 팀의 이전까지의 방향과 너무 다른 결로 작성하셨다는 생각이 드네요!!
-
팀 아키텍처에 맞지 않게 레이어, 객체 간 로직이 혼재되어 있다는 생각이 듭니다! 특히 비지니스 로직, 서비스의 중요 정책과 연관된 내용이 서비스 코드에 없다는 점이 가장 크게 느껴져요!
-
이전의 프로젝트에서 충분히 활용할 수 있는 점들도 활용이 안된 느낌입니다! 커서 관련 내용도 그러하고 전반적인 메서드 네이밍도 그렇구요! 이 또한 아마도 컨텍스트를 다 이해하지 못한 AI의 영향이지 않을까 싶어요..!! 활용하시는건 매우 좋지만 충분히 기능 기획과 팀과 동기화가 되길..!!
-
마지막으로 시간 걸리시더라도 테스트는 항상 꼭 작성해주세요! 만들어주신 서비스레이어 테스트도 좋지만 커버리지 만큼은 코어 흐름에 대한 테스트는 꼭..!!
+) AI를 사용하시는건 좋지만 단순 기능 구현도만 보지 말고 그 외적인 부분도 검토를 많이 해주세요!!
++) 전체 프로젝트 아키텍처와 이전까지의 객체지향 방향성을 조금 더 고민해주셨으면 합니다!
src/main/java/com/kustacks/kuring/common/dto/ResponseCodeAndMessages.java
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/application/port/out/ClubQueryPort.java
Outdated
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/club/domain/ClubRecruitmentStatus.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (9)
src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java (1)
28-37: 조회 메서드의readOnly트랜잭션 일관성을 맞춰주세요.Line 29의 조회 메서드에도
@Transactional(readOnly = true)를 붙이면 의도 표현과 일관성이 좋아집니다.수정 예시
`@Override` + `@Transactional`(readOnly = true) public List<Club> findClubsBetweenDates(LocalDateTime start, LocalDateTime end) { return queryFactory.selectFrom(club) .where(🤖 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/ClubQueryRepositoryImpl.java` around lines 28 - 37, Add the `@Transactional`(readOnly = true) annotation to the findClubsBetweenDates method in ClubQueryRepositoryImpl so its transactional read-only intent matches other query methods; locate the method named findClubsBetweenDates(LocalDateTime start, LocalDateTime end) and annotate it with `@Transactional`(readOnly = true) (ensure the import for org.springframework.transaction.annotation.Transactional is present).src/main/java/com/kustacks/kuring/club/application/port/in/dto/ClubDetailCommand.java (1)
3-7:clubId는 생성 시점에 null 방어를 두는 편이 안전합니다.입력 커맨드에서 조기 검증하면 하위 계층 예외 전파를 줄일 수 있습니다.
수정 예시
package com.kustacks.kuring.club.application.port.in.dto; +import java.util.Objects; + public record ClubDetailCommand( Long clubId, String email ) { + public ClubDetailCommand { + Objects.requireNonNull(clubId, "clubId 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/application/port/in/dto/ClubDetailCommand.java` around lines 3 - 7, ClubDetailCommand's clubId should be null-guarded at creation to fail fast; add a canonical constructor for the record ClubDetailCommand that checks clubId != null and throws an IllegalArgumentException (or a domain-specific validation exception) with a clear message if null, so callers get early validation instead of downstream errors.src/main/java/com/kustacks/kuring/club/application/port/out/dto/ClubDetailDto.java (1)
31-31:isAlways필드 접근제어자를private로 통일해주세요.Line 31만 package-private이라 DTO 캡슐화 일관성이 깨집니다.
수정 예시
- Boolean isAlways; + private Boolean isAlways;🤖 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/dto/ClubDetailDto.java` at line 31, The ClubDetailDto class has the Boolean field isAlways declared with package-private visibility; change its access modifier to private to match the other DTO fields' encapsulation and maintain consistency in ClubDetailDto. Locate the isAlways field in class ClubDetailDto and make it private, and ensure any existing getter/setter methods (e.g., getIsAlways/isAlways or setIsAlways) remain accessible or are added/updated accordingly so external code uses the public accessors rather than the field directly.src/main/java/com/kustacks/kuring/worker/notification/ClubNotificationScheduler.java (1)
19-30: 멀티 인스턴스 환경이라면 스케줄 중복 실행 방지를 고려해주세요.현재 구조는 인스턴스마다 동일 시각에 발송 로직이 실행될 수 있어 중복 알림이 날 수 있습니다. 운영환경이 다중 인스턴스라면 분산락/리더선출 같은 보호장치를 두는 편이 안전합니다.
🤖 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, The scheduled task in ClubNotificationScheduler.sendClubDeadlineNotifications can run concurrently on multiple instances causing duplicate notifications; wrap the execution of clubNotificationUseCase.sendDeadlineNotifications with a distributed lock or leader-election check (e.g., use ShedLock, Redis lock, Zookeeper, or your cluster's leader API) so only the instance holding the lock runs the send logic, ensure the lock is acquired before calling sendDeadlineNotifications and always released (or use a library-managed lock/annotation) and log/skipped execution when the lock is not obtained.src/test/java/com/kustacks/kuring/archunit/DependencyRuleTests.java (1)
183-185: Club 어댑터 검증에서in.web경계도 포함해 주세요.Line 183~185은
out.persistence만 선언되어 있어,club.adapter.in.web패키지의 계층 규칙 위반을 이 테스트가 잡아내지 못합니다.제안 수정안
.withAdaptersLayer("adapter") + .incoming("in.web") .outgoing("out.persistence") .and()🤖 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 183 - 185, The adapter layer rule built with withAdaptersLayer("adapter") currently only declares .outgoing("out.persistence"), so violations in the club.adapter.in.web boundary are not checked; update the rule that defines withAdaptersLayer("adapter") in DependencyRuleTests to also include the incoming boundary for the web adapter (e.g., add an incoming("in.web") or equivalent call before/outside the outgoing call) so that the club.adapter.in.web package is enforced alongside out.persistence.src/test/java/com/kustacks/kuring/acceptance/UserAcceptanceTest.java (1)
65-65: 고정 동아리 ID 의존은 테스트를 취약하게 만들 수 있습니다.Line 65의
TEST_CLUB_ID = 1L전제는 시드 데이터 변경 시 쉽게 깨집니다. 테스트 내부에서 동아리를 준비하거나, 안정적인 테스트 픽스처 ID를 명시적으로 참조하는 방식이 더 안전합니다.Also applies to: 537-579
🤖 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` at line 65, UserAcceptanceTest currently relies on a hardcoded TEST_CLUB_ID = 1L which breaks if seed data changes; replace this static constant by creating or fetching a test club during test setup and use its returned ID in tests (e.g., add a createTestClub() helper called from the `@BeforeAll/`@BeforeEach setup that creates a club via the same service/repository used in production and stores the resulting ID in a non-final field like testClubId), then update all usages of TEST_CLUB_ID (including the cases around the 537-579 area) to reference this dynamic testClubId so tests no longer depend on brittle seeded IDs.src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubSubscribeRepository.java (1)
21-27:List<Object[]>대신 타입 안전한 projection 사용 고려
countSubscribersByClubIds메서드가List<Object[]>를 반환하여 호출부에서 인덱스 기반 캐스팅이 필요합니다. 타입 안전성을 위해 projection 인터페이스나 record를 사용하는 것을 권장합니다.♻️ 제안: Projection 사용
// Projection 인터페이스 정의 public interface ClubSubscriberCount { Long getClubId(); Long getCount(); } // Repository 메서드 변경 `@Query`(""" SELECT cs.club.id as clubId, COUNT(cs) as count FROM ClubSubscribe cs WHERE cs.club.id IN :clubIds GROUP BY cs.club.id """) List<ClubSubscriberCount> countSubscribersByClubIds(List<Long> clubIds);🤖 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/ClubSubscribeRepository.java` around lines 21 - 27, The repository method countSubscribersByClubIds returns List<Object[]> which forces index-based casting in callers; replace this with a type-safe projection by defining a projection (interface or record) such as ClubSubscriberCount with methods getClubId() and getCount(), update the JPQL in countSubscribersByClubIds to select cs.club.id as clubId and COUNT(cs) as count, and change the return type to List<ClubSubscriberCount> so callers can use typed accessors instead of Object[] indexing.src/test/java/com/kustacks/kuring/club/application/service/ClubNotificationServiceTest.java (1)
72-80:@SuppressWarnings("unchecked")추가 권장Line 75에서
ReflectionTestUtils.getField의 반환값을Map<String, String>으로 캐스팅할 때 unchecked cast 경고가 발생합니다. 테스트 코드이므로 큰 문제는 아니지만, 명시적으로 경고를 억제하면 깔끔해집니다.♻️ 제안
+ `@SuppressWarnings`("unchecked") `@DisplayName`("내일 마감 대상 목록을 모두 발송한다") `@Test` void send_deadline_notifications_only_for_targets() throws Exception {🤖 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, Add a `@SuppressWarnings`("unchecked") to suppress the unchecked-cast warning when casting the ReflectionTestUtils.getField result to Map<String, String> in ClubNotificationServiceTest; specifically annotate the smallest possible scope (the local variable declaration or the test method containing the cast) so the cast of ReflectionTestUtils.getField(... ) to Map<String, String> for the variable data does not emit a compiler warning while keeping scope minimal.src/main/java/com/kustacks/kuring/club/application/service/ClubCommandService.java (1)
32-32: 변수명이 타입과 일관성이 없습니다.
countSubscriptionsQueryPort는ClubSubscriptionQueryPort타입이며, 이 포트는countSubscriptions외에도existsSubscription등 다른 쿼리도 제공합니다. 포트의 역할을 더 정확히 반영하는 이름으로 변경하는 것을 권장합니다.♻️ 제안된 수정
- private final ClubSubscriptionQueryPort countSubscriptionsQueryPort; + private final ClubSubscriptionQueryPort clubSubscriptionQueryPort;관련 사용처도 함께 수정 필요:
- Line 49, 63:
countSubscriptionsQueryPort.countSubscriptions→clubSubscriptionQueryPort.countSubscriptions- Line 67:
countSubscriptionsQueryPort.existsSubscription→clubSubscriptionQueryPort.existsSubscription🤖 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 32, Rename the field countSubscriptionsQueryPort in ClubCommandService to a name that reflects the port's broader role (e.g., clubSubscriptionQueryPort) and update all call sites to use the new name: replace usages of countSubscriptionsQueryPort.countSubscriptions(...) with clubSubscriptionQueryPort.countSubscriptions(...) and countSubscriptionsQueryPort.existsSubscription(...) with clubSubscriptionQueryPort.existsSubscription(...); ensure the constructor (or any injection point) and any imports/assignments that reference countSubscriptionsQueryPort are updated to the new identifier so the ClubSubscriptionQueryPort is consistently named across the class.
🤖 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/adapter/out/persistence/ClubPersistenceAdapter.java`:
- Around line 48-58: The findSubscribedClubIds method causes an N+1 due to lazy
Club loading when calling sub.getClub().getId(); update the repository method
findByClubIdInAndRootUserId (used by
ClubPersistenceAdapter.findSubscribedClubIds) to avoid lazy fetches by either:
1) adding an `@Query` that JOIN FETCHes cs.club so the Club entities are loaded in
one query, or 2) replacing the derived finder with a projection query that
directly selects the club id (e.g. SELECT cs.club.id FROM ClubSubscribe cs WHERE
cs.club.id IN :clubIds AND cs.rootUser.id = :rootUserId) and return List<Long>
to eliminate entity hydration.
In
`@src/main/java/com/kustacks/kuring/club/application/port/in/dto/ClubListCommand.java`:
- Around line 16-18: The division parsing currently maps and trims tokens but
does not remove empty entries, causing blank strings to appear for inputs like
", ,"; update the logic in ClubListCommand that builds the division list (the
stream starting from Arrays.stream(division.split(","))) to filter out empty or
blank tokens after trimming (e.g., call a filter that rejects strings where
isEmpty/isBlank returns true) before collecting toList so no empty tokens are
included.
In
`@src/main/java/com/kustacks/kuring/club/application/service/ClubNotificationService.java`:
- Around line 49-50: In findDeadlineClubs() in ClubNotificationService replace
the direct call to LocalDateTime.now() with a Clock-backed time: add a final
Clock field injected via the service constructor (or existing constructor), then
call LocalDateTime.now(clock) inside findDeadlineClubs() and pass that to
clubQueryPort.findNextDayRecruitEndClubs(...); ensure the Clock bean configured
in TimeConfig is used so behavior/tests become deterministic and consistent with
AlertService.
In `@src/main/java/com/kustacks/kuring/club/domain/ClubRecruitmentStatus.java`:
- Around line 21-36: The from method in ClubRecruitmentStatus
(ClubRecruitmentStatus.from(start,end,isAlways,now)) must guard against a null
now to avoid NPEs; add an explicit null check at the start of the method (e.g.,
if now == null) and throw a clear, consistent domain validation exception (for
example IllegalArgumentException or the project’s domain validation exception)
with a descriptive message so callers get a predictable error when now is
missing before proceeding with the existing isAlways/start/end logic.
In
`@src/main/java/com/kustacks/kuring/message/application/port/out/dto/NoticeMessageDto.java`:
- Line 53: The mapping uses String.valueOf(notice.getId()) which will produce
the literal "null" if notice.getId() is null; change the mapping to explicitly
handle a null id (e.g., set id to null or throw) instead of relying on
String.valueOf so the subsequent Assert.notNull(id, ...) at line 33 can work
correctly; locate the builder call setting .id(...) in NoticeMessageDto (the
.id(...) expression using notice.getId()) and replace it with an explicit
null-check (notice.getId() == null ? null : String.valueOf(notice.getId())) or
fail early with a clear exception.
In
`@src/main/java/com/kustacks/kuring/user/adapter/in/web/dto/UserClubSubscriptionRequest.java`:
- Line 6: The UserClubSubscriptionRequest DTO currently only uses `@NotNull` on
the id field allowing 0 or negative values; update the id validation in
UserClubSubscriptionRequest to enforce a positive lower bound (e.g., add
`@Positive` or `@Min`(1) to the Long id field) so only values > 0 are accepted, and
ensure any validation imports/annotations are added to the class.
In
`@src/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.java`:
- Around line 48-57: Remove the unused FCM token parameter from the controller
methods: delete the `@RequestHeader`(FCM_TOKEN_HEADER_KEY) String userToken
parameter from both addSubscription(...) and deleteSubscription(...) in
UserClubSubscriptionApiV2 (or alternatively implement its use if the FCM token
is required by business logic); update any related method signatures, callers,
and imports to match the new signatures so compilation and API consistency
remain intact.
In
`@src/main/resources/db/migration/V260217__Alter_club_subscribe_to_root_user.sql`:
- Around line 3-4: The migration currently issues an unconditional DELETE FROM
club_subscribe which risks production data loss; change the migration in
V260217__Alter_club_subscribe_to_root_user.sql to either (A) perform a safe
guard that checks for existing rows in club_subscribe (e.g., SELECT COUNT(*) and
RAISE EXCEPTION / abort the migration if count > 0) so the migration fails fast
when data exists, or (B) remove the destructive DELETE and instead implement a
separate backfill/cleanup migration that safely migrates or archives records
before deletion; locate the DELETE FROM club_subscribe statement and replace it
with the chosen guarded-check or split into a non-destructive backfill
migration.
---
Nitpick comments:
In
`@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java`:
- Around line 28-37: Add the `@Transactional`(readOnly = true) annotation to the
findClubsBetweenDates method in ClubQueryRepositoryImpl so its transactional
read-only intent matches other query methods; locate the method named
findClubsBetweenDates(LocalDateTime start, LocalDateTime end) and annotate it
with `@Transactional`(readOnly = true) (ensure the import for
org.springframework.transaction.annotation.Transactional is present).
In
`@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubSubscribeRepository.java`:
- Around line 21-27: The repository method countSubscribersByClubIds returns
List<Object[]> which forces index-based casting in callers; replace this with a
type-safe projection by defining a projection (interface or record) such as
ClubSubscriberCount with methods getClubId() and getCount(), update the JPQL in
countSubscribersByClubIds to select cs.club.id as clubId and COUNT(cs) as count,
and change the return type to List<ClubSubscriberCount> so callers can use typed
accessors instead of Object[] indexing.
In
`@src/main/java/com/kustacks/kuring/club/application/port/in/dto/ClubDetailCommand.java`:
- Around line 3-7: ClubDetailCommand's clubId should be null-guarded at creation
to fail fast; add a canonical constructor for the record ClubDetailCommand that
checks clubId != null and throws an IllegalArgumentException (or a
domain-specific validation exception) with a clear message if null, so callers
get early validation instead of downstream errors.
In
`@src/main/java/com/kustacks/kuring/club/application/port/out/dto/ClubDetailDto.java`:
- Line 31: The ClubDetailDto class has the Boolean field isAlways declared with
package-private visibility; change its access modifier to private to match the
other DTO fields' encapsulation and maintain consistency in ClubDetailDto.
Locate the isAlways field in class ClubDetailDto and make it private, and ensure
any existing getter/setter methods (e.g., getIsAlways/isAlways or setIsAlways)
remain accessible or are added/updated accordingly so external code uses the
public accessors rather than the field directly.
In
`@src/main/java/com/kustacks/kuring/club/application/service/ClubCommandService.java`:
- Line 32: Rename the field countSubscriptionsQueryPort in ClubCommandService to
a name that reflects the port's broader role (e.g., clubSubscriptionQueryPort)
and update all call sites to use the new name: replace usages of
countSubscriptionsQueryPort.countSubscriptions(...) with
clubSubscriptionQueryPort.countSubscriptions(...) and
countSubscriptionsQueryPort.existsSubscription(...) with
clubSubscriptionQueryPort.existsSubscription(...); ensure the constructor (or
any injection point) and any imports/assignments that reference
countSubscriptionsQueryPort are updated to the new identifier so the
ClubSubscriptionQueryPort is consistently named across the class.
In
`@src/main/java/com/kustacks/kuring/worker/notification/ClubNotificationScheduler.java`:
- Around line 19-30: The scheduled task in
ClubNotificationScheduler.sendClubDeadlineNotifications can run concurrently on
multiple instances causing duplicate notifications; wrap the execution of
clubNotificationUseCase.sendDeadlineNotifications with a distributed lock or
leader-election check (e.g., use ShedLock, Redis lock, Zookeeper, or your
cluster's leader API) so only the instance holding the lock runs the send logic,
ensure the lock is acquired before calling sendDeadlineNotifications and always
released (or use a library-managed lock/annotation) and log/skipped execution
when the lock is not obtained.
In `@src/test/java/com/kustacks/kuring/acceptance/UserAcceptanceTest.java`:
- Line 65: UserAcceptanceTest currently relies on a hardcoded TEST_CLUB_ID = 1L
which breaks if seed data changes; replace this static constant by creating or
fetching a test club during test setup and use its returned ID in tests (e.g.,
add a createTestClub() helper called from the `@BeforeAll/`@BeforeEach setup that
creates a club via the same service/repository used in production and stores the
resulting ID in a non-final field like testClubId), then update all usages of
TEST_CLUB_ID (including the cases around the 537-579 area) to reference this
dynamic testClubId so tests no longer depend on brittle seeded IDs.
In `@src/test/java/com/kustacks/kuring/archunit/DependencyRuleTests.java`:
- Around line 183-185: The adapter layer rule built with
withAdaptersLayer("adapter") currently only declares
.outgoing("out.persistence"), so violations in the club.adapter.in.web boundary
are not checked; update the rule that defines withAdaptersLayer("adapter") in
DependencyRuleTests to also include the incoming boundary for the web adapter
(e.g., add an incoming("in.web") or equivalent call before/outside the outgoing
call) so that the club.adapter.in.web package is enforced alongside
out.persistence.
In
`@src/test/java/com/kustacks/kuring/club/application/service/ClubNotificationServiceTest.java`:
- Around line 72-80: Add a `@SuppressWarnings`("unchecked") to suppress the
unchecked-cast warning when casting the ReflectionTestUtils.getField result to
Map<String, String> in ClubNotificationServiceTest; specifically annotate the
smallest possible scope (the local variable declaration or the test method
containing the cast) so the cast of ReflectionTestUtils.getField(... ) to
Map<String, String> for the variable data does not emit a compiler warning while
keeping scope minimal.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (47)
src/main/java/com/kustacks/kuring/club/adapter/in/web/ClubQueryApiV2.javasrc/main/java/com/kustacks/kuring/club/adapter/in/web/dto/ClubDetailResponse.javasrc/main/java/com/kustacks/kuring/club/adapter/in/web/dto/ClubDivisionListResponse.javasrc/main/java/com/kustacks/kuring/club/adapter/in/web/dto/ClubListResponse.javasrc/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapter.javasrc/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepository.javasrc/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.javasrc/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubSubscribeRepository.javasrc/main/java/com/kustacks/kuring/club/application/port/in/ClubNotificationUseCase.javasrc/main/java/com/kustacks/kuring/club/application/port/in/ClubQueryUseCase.javasrc/main/java/com/kustacks/kuring/club/application/port/in/ClubSubscriptionUseCase.javasrc/main/java/com/kustacks/kuring/club/application/port/in/dto/ClubDetailCommand.javasrc/main/java/com/kustacks/kuring/club/application/port/in/dto/ClubDetailResult.javasrc/main/java/com/kustacks/kuring/club/application/port/in/dto/ClubItemResult.javasrc/main/java/com/kustacks/kuring/club/application/port/in/dto/ClubListCommand.javasrc/main/java/com/kustacks/kuring/club/application/port/in/dto/ClubListResult.javasrc/main/java/com/kustacks/kuring/club/application/port/in/dto/ClubSubscriptionCommand.javasrc/main/java/com/kustacks/kuring/club/application/port/out/ClubQueryPort.javasrc/main/java/com/kustacks/kuring/club/application/port/out/ClubSubscriptionCommandPort.javasrc/main/java/com/kustacks/kuring/club/application/port/out/ClubSubscriptionQueryPort.javasrc/main/java/com/kustacks/kuring/club/application/port/out/dto/ClubDetailDto.javasrc/main/java/com/kustacks/kuring/club/application/service/ClubCommandService.javasrc/main/java/com/kustacks/kuring/club/application/service/ClubNotificationService.javasrc/main/java/com/kustacks/kuring/club/application/service/ClubQueryService.javasrc/main/java/com/kustacks/kuring/club/domain/Club.javasrc/main/java/com/kustacks/kuring/club/domain/ClubRecruitmentStatus.javasrc/main/java/com/kustacks/kuring/club/domain/ClubSubscribe.javasrc/main/java/com/kustacks/kuring/common/dto/ResponseCodeAndMessages.javasrc/main/java/com/kustacks/kuring/common/exception/code/ErrorCode.javasrc/main/java/com/kustacks/kuring/common/featureflag/KuringFeatures.javasrc/main/java/com/kustacks/kuring/message/application/port/out/dto/NoticeMessageDto.javasrc/main/java/com/kustacks/kuring/message/domain/MessageType.javasrc/main/java/com/kustacks/kuring/user/adapter/in/web/UserClubSubscriptionApiV2.javasrc/main/java/com/kustacks/kuring/user/adapter/in/web/dto/UserClubSubscriptionCountResponse.javasrc/main/java/com/kustacks/kuring/user/adapter/in/web/dto/UserClubSubscriptionRequest.javasrc/main/java/com/kustacks/kuring/worker/notification/ClubNotificationScheduler.javasrc/main/resources/ai/data/vectorstore.jsonsrc/main/resources/db/migration/V260217__Alter_club_subscribe_to_root_user.sqlsrc/test/java/com/kustacks/kuring/acceptance/UserAcceptanceTest.javasrc/test/java/com/kustacks/kuring/acceptance/UserStep.javasrc/test/java/com/kustacks/kuring/archunit/DependencyRuleTests.javasrc/test/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapterTest.javasrc/test/java/com/kustacks/kuring/club/application/service/ClubCommandServiceTest.javasrc/test/java/com/kustacks/kuring/club/application/service/ClubNotificationServiceTest.javasrc/test/java/com/kustacks/kuring/club/application/service/ClubQueryServiceTest.javasrc/test/java/com/kustacks/kuring/support/DatabaseConfigurator.javasrc/test/java/com/kustacks/kuring/worker/notification/ClubNotificationSchedulerTest.java
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/kustacks/kuring/club/domain/Club.java
🚧 Files skipped from review as they are similar to previous changes (5)
- src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepository.java
- src/main/java/com/kustacks/kuring/club/adapter/in/web/dto/ClubDivisionListResponse.java
- src/main/java/com/kustacks/kuring/club/adapter/in/web/ClubQueryApiV2.java
- src/main/java/com/kustacks/kuring/common/exception/code/ErrorCode.java
- src/main/java/com/kustacks/kuring/club/application/port/in/ClubQueryUseCase.java
| @Override | ||
| public List<Long> findSubscribedClubIds( | ||
| List<Long> clubIds, | ||
| Long rootUserId | ||
| ) { | ||
| return clubSubscribeRepository | ||
| .findByClubIdInAndRootUserId(clubIds, rootUserId) | ||
| .stream() | ||
| .map(sub -> sub.getClub().getId()) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "ClubSubscribeRepository*" -type fRepository: ku-ring/ku-ring-backend-web
Length of output: 164
🏁 Script executed:
find . -name "ClubSubscribe.java" -type fRepository: ku-ring/ku-ring-backend-web
Length of output: 137
🏁 Script executed:
rg -n "findByClubIdInAndRootUserId" --type java -A 15Repository: ku-ring/ku-ring-backend-web
Length of output: 3999
🏁 Script executed:
cat -n ./src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubSubscribeRepository.javaRepository: ku-ring/ku-ring-backend-web
Length of output: 1280
🏁 Script executed:
cat -n ./src/main/java/com/kustacks/kuring/club/domain/ClubSubscribe.javaRepository: ku-ring/ku-ring-backend-web
Length of output: 1609
findSubscribedClubIds에서 N+1 쿼리 문제 발생.
findByClubIdInAndRootUserId 메서드는 @Query 애너테이션 없이 Spring Data의 파생 쿼리로 정의되어 있고, ClubSubscribe.club은 FetchType.LAZY로 설정되어 있습니다. 결과적으로 sub.getClub().getId() 호출 시 각 구독마다 추가 쿼리가 발생합니다(1 + N 쿼리).
repository 메서드를 다음 중 하나로 개선하세요:
@Query에서JOIN FETCH를 사용하여 club을 즉시 로드- club ID만 직접 조회하는 프로젝션 쿼리 사용 (예:
SELECT cs.club.id FROM ClubSubscribe cs ...)
🤖 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 48 - 58, The findSubscribedClubIds method causes an N+1 due to lazy
Club loading when calling sub.getClub().getId(); update the repository method
findByClubIdInAndRootUserId (used by
ClubPersistenceAdapter.findSubscribedClubIds) to avoid lazy fetches by either:
1) adding an `@Query` that JOIN FETCHes cs.club so the Club entities are loaded in
one query, or 2) replacing the derived finder with a projection query that
directly selects the club id (e.g. SELECT cs.club.id FROM ClubSubscribe cs WHERE
cs.club.id IN :clubIds AND cs.rootUser.id = :rootUserId) and return List<Long>
to eliminate entity hydration.
| return Arrays.stream(division.split(",")) | ||
| .map(String::trim) | ||
| .toList(); |
There was a problem hiding this comment.
division 파싱 결과에서 빈 토큰을 제거해 주세요.
Line 16~18 로직은 ", ," 같은 입력에서 빈 문자열이 리스트에 포함됩니다. 필터에 빈 값이 전달되지 않도록 정제하는 게 안전합니다.
제안 수정안
- return Arrays.stream(division.split(","))
- .map(String::trim)
- .toList();
+ return Arrays.stream(division.split(","))
+ .map(String::trim)
+ .filter(token -> !token.isBlank())
+ .toList();📝 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.
| return Arrays.stream(division.split(",")) | |
| .map(String::trim) | |
| .toList(); | |
| return Arrays.stream(division.split(",")) | |
| .map(String::trim) | |
| .filter(token -> !token.isBlank()) | |
| .toList(); |
🤖 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/in/dto/ClubListCommand.java`
around lines 16 - 18, The division parsing currently maps and trims tokens but
does not remove empty entries, causing blank strings to appear for inputs like
", ,"; update the logic in ClubListCommand that builds the division list (the
stream starting from Arrays.stream(division.split(","))) to filter out empty or
blank tokens after trimming (e.g., call a filter that rejects strings where
isEmpty/isBlank returns true) before collecting toList so no empty tokens are
included.
| private List<Club> findDeadlineClubs() { | ||
| return clubQueryPort.findNextDayRecruitEndClubs(LocalDateTime.now()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 1) 스케줄러 설정과 LocalDateTime.now() 사용 위치 확인
rg -n --type java '@Scheduled|LocalDateTime\.now()' -A 2 -B 2Repository: ku-ring/ku-ring-backend-web
Length of output: 23948
🏁 Script executed:
# 2) 애플리케이션 설정 파일 확인
find . -type f \( -name "application.yml" -o -name "application.properties" -o -name "application-*.yml" -o -name "application-*.properties" \) | head -20Repository: ku-ring/ku-ring-backend-web
Length of output: 186
🏁 Script executed:
# 3) 스케줄러 관련 클래스 확인
fd -t f "ClubNotificationService" -x cat -n {}Repository: ku-ring/ku-ring-backend-web
Length of output: 7826
🏁 Script executed:
# 4) 타임존 관련 설정 확인
rg -n 'time-zone|timezone|Asia/Seoul|TimeZone|ZoneId' -t java -t yaml -t properties | head -30Repository: ku-ring/ku-ring-backend-web
Length of output: 105
🏁 Script executed:
cat src/main/resources/application.ymlRepository: ku-ring/ku-ring-backend-web
Length of output: 263
🏁 Script executed:
rg -n "TimeZone|Clock|clock" --type java src/main/java/com/kustacks/kuring/club/application/service/ -A 2 -B 2Repository: ku-ring/ku-ring-backend-web
Length of output: 53
🏁 Script executed:
# Search for Clock injection patterns in the codebase
rg -n "@Bean.*Clock|private.*Clock" --type javaRepository: ku-ring/ku-ring-backend-web
Length of output: 182
🏁 Script executed:
# Check for Clock bean configuration
rg -n "@Bean.*Clock|Clock\s*clock" --type java -B 3 -A 3Repository: ku-ring/ku-ring-backend-web
Length of output: 2838
🏁 Script executed:
# Check common.yml configuration
cat src/main/resources/config/environments/common.ymlRepository: ku-ring/ku-ring-backend-web
Length of output: 4795
🏁 Script executed:
# Check if AlertService uses Clock correctly
rg -n "Clock|clock" src/main/java/com/kustacks/kuring/alert/application/service/AlertService.java -B 2 -A 2Repository: ku-ring/ku-ring-backend-web
Length of output: 806
🏁 Script executed:
# Search for any JVM timezone system properties or configuration
rg -n "user\.timezone|TimeZone\.setDefault|setDefaultTimeZone" --type javaRepository: ku-ring/ku-ring-backend-web
Length of output: 189
LocalDateTime.now() 대신 의존성 주입된 Clock 사용을 권고합니다.
JVM 기본 타임존이 애플리케이션 시작 시 Asia/Seoul로 명시 설정되어 있으므로(KuringApplication.java 라인 22), 현재 코드는 정상 작동합니다. 다만 AlertService처럼 Clock 의존성을 주입받아 LocalDateTime.now(clock)을 사용하면 더 명확하고 테스트 가능한 구조가 됩니다.
TimeConfig에서 이미 @Bean public Clock clock()이 Asia/Seoul 타임존으로 설정되어 있으므로, 동일한 패턴을 적용하면 일관성이 개선됩니다.
🤖 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 - 50, In findDeadlineClubs() in ClubNotificationService replace
the direct call to LocalDateTime.now() with a Clock-backed time: add a final
Clock field injected via the service constructor (or existing constructor), then
call LocalDateTime.now(clock) inside findDeadlineClubs() and pass that to
clubQueryPort.findNextDayRecruitEndClubs(...); ensure the Clock bean configured
in TimeConfig is used so behavior/tests become deterministic and consistent with
AlertService.
| public static ClubRecruitmentStatus from( | ||
| LocalDateTime start, | ||
| LocalDateTime end, | ||
| Boolean isAlways, | ||
| LocalDateTime now | ||
| ) { | ||
| if (Boolean.TRUE.equals(isAlways)) { | ||
| return ALWAYS; | ||
| } | ||
|
|
||
| if (start != null && now.isBefore(start)) { | ||
| return BEFORE; | ||
| } | ||
|
|
||
| if (end != null && now.isAfter(end)) { | ||
| return CLOSED; |
There was a problem hiding this comment.
now null 방어 로직이 필요합니다.
now가 null이면 상태 계산 중 NPE가 발생합니다. 도메인 팩토리에서 명시적으로 검증해 예외를 일관되게 처리해 주세요.
🔧 제안 수정
public static ClubRecruitmentStatus from(
LocalDateTime start,
LocalDateTime end,
Boolean isAlways,
LocalDateTime now
) {
+ if (now == null) {
+ throw new IllegalArgumentException("now must not be null");
+ }
+
if (Boolean.TRUE.equals(isAlways)) {
return ALWAYS;
}🤖 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/ClubRecruitmentStatus.java`
around lines 21 - 36, The from method in ClubRecruitmentStatus
(ClubRecruitmentStatus.from(start,end,isAlways,now)) must guard against a null
now to avoid NPEs; add an explicit null check at the start of the method (e.g.,
if now == null) and throw a clear, consistent domain validation exception (for
example IllegalArgumentException or the project’s domain validation exception)
with a descriptive message so callers get a predictable error when now is
missing before proceeding with the existing isAlways/start/end logic.
src/main/java/com/kustacks/kuring/message/application/port/out/dto/NoticeMessageDto.java
Show resolved
Hide resolved
src/main/java/com/kustacks/kuring/user/adapter/in/web/dto/UserClubSubscriptionRequest.java
Show resolved
Hide resolved
| public ResponseEntity<BaseResponse<UserClubSubscriptionCountResponse>> addSubscription( | ||
| @RequestHeader(FCM_TOKEN_HEADER_KEY) String userToken, | ||
| @RequestHeader(AuthorizationExtractor.AUTHORIZATION) String bearerToken, | ||
| @Valid @RequestBody UserClubSubscriptionRequest request | ||
| ) { | ||
| String email = validateJwtAndGetEmail(extractAuthorizationValue(bearerToken, AuthorizationType.BEARER)); | ||
| long subscriptionCount = clubSubscriptionUseCase.addSubscription(new ClubSubscriptionCommand(email, request.id())); | ||
|
|
||
| return ResponseEntity.ok(new BaseResponse<>(CLUB_SUBSCRIPTION_ADD_SUCCESS, new UserClubSubscriptionCountResponse(subscriptionCount))); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. UserClubSubscriptionApiV2.java 파일 확인
find . -name "UserClubSubscriptionApiV2.java" -type f
# 2. 파일 내용 확인
git ls-files | grep -i "UserClubSubscriptionApiV2"Repository: ku-ring/ku-ring-backend-web
Length of output: 242
🏁 Script executed:
#!/bin/bash
# User-Token과 Authorization을 함께 사용하는 패턴 확인
rg -n "FCM_TOKEN_HEADER_KEY|User-Token" --type java -B 3 -A 3 | head -80Repository: ku-ring/ku-ring-backend-web
Length of output: 8090
🏁 Script executed:
#!/bin/bash
# 다른 컨트롤러에서 userToken 파라미터 사용 패턴 확인
rg -n "@RequestHeader.*FCM_TOKEN_HEADER_KEY|@RequestHeader.*User-Token" --type java -A 10 | head -120Repository: ku-ring/ku-ring-backend-web
Length of output: 15673
사용되지 않는 userToken 파라미터 제거 필요
addSubscription() 및 deleteSubscription() 메서드에서 userToken 파라미터가 선언되었으나 메서드 본문에 사용되지 않습니다. 메서드는 bearerToken만 사용하여 인증을 처리합니다.
다른 사용자 관련 API(UserCommandApiV2, UserQueryApiV2)는 FCM_TOKEN_HEADER_KEY 파라미터를 실제로 사용하므로, 이 API와의 일관성을 위해 다음 중 선택이 필요합니다:
- 파라미터 제거 (현재 사용 계획이 없다면 권장)
- API 스펙에 따라 실제로 사용하도록 구현 수정
🤖 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 48 - 57, Remove the unused FCM token parameter from the controller
methods: delete the `@RequestHeader`(FCM_TOKEN_HEADER_KEY) String userToken
parameter from both addSubscription(...) and deleteSubscription(...) in
UserClubSubscriptionApiV2 (or alternatively implement its use if the FCM token
is required by business logic); update any related method signatures, callers,
and imports to match the new signatures so compilation and API consistency
remain intact.
src/main/resources/db/migration/V260217__Alter_club_subscribe_to_root_user.sql
Show resolved
Hide resolved
|
리뷰 다 확인하고 수정했습니다! 변경 사항
기타
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/java/com/kustacks/kuring/archunit/DependencyRuleTests.java (1)
183-185:⚠️ Potential issue | 🟠 MajorClub 웹 어댑터(
in.web) 규칙 검증이 빠져 있습니다.이 PR 목적상 Club 조회 API가 추가되었는데, Club 아키텍처 규칙에서 incoming 어댑터 검증이 누락되어 경계 위반을 테스트가 놓칠 수 있습니다.
🔧 제안 수정
HexagonalArchitecture.boundedContext("com.kustacks.kuring.club") .withDomainLayer("domain") .withAdaptersLayer("adapter") + .incoming("in.web") .outgoing("out.persistence") .and()🤖 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 183 - 185, 테스트에서 Club 웹 어댑터(in.web)에 대한 incoming 검증이 빠져 있어 경계 위반을 놓칠 수 있으니 DependencyRuleTests의 해당 규칙 체인(현재 withAdaptersLayer("adapter").outgoing("out.persistence").and())에 Club의 incoming 어댑터 검증을 추가하여 .incoming("in.web") 또는 적절한 incoming 식별자(예: "in.web")를 포함하도록 수정하고, 필요하면 .and() 체인을 맞춰 Club 관련 규칙 블록에 포함시켜 테스트가 in.web으로부터의 접근을 올바르게 차단/허용하는지 검증하게 하세요.
🧹 Nitpick comments (3)
src/test/java/com/kustacks/kuring/archunit/DependencyRuleTests.java (1)
198-208: 테스트 이름/표시명과 실제 검증 대상을 맞춰 주세요.현재
"테스트 페키지 의존성 검증"이라는 이름과 달리, 실제 규칙은user.domain -> user.application의존성만 검사합니다. 오타(“페키지”)도 있어 의도가 모호합니다. 테스트 목적에 맞게 이름을 바꾸거나, 실제로 테스트 패키지 규칙을 검증하도록 정렬하는 편이 좋겠습니다.🤖 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 198 - 208, The test's DisplayName and method intent don't match the actual ArchUnit rule: method testPackageDependencies (and its DisplayName "테스트 페키지 의존성 검증") only asserts that classes in "com.kustacks.kuring.user.domain.." must not depend on "com.kustacks.kuring.user.application.."; rename the DisplayName (and optionally the test method) to something like "user.domain should not depend on user.application" or expand the rule to cover all package dependency checks you intended; also fix the typo "페키지" if keeping a Korean name so the label accurately reflects the assertion using the existing resideInAPackage("com.kustacks.kuring.user.domain..") and resideInAnyPackage("com.kustacks.kuring.user.application..") references.src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubSubscribeRepository.java (1)
21-27: 집계 결과를List<Object[]>로 반환하면 타입 안정성이 떨어집니다.Line 27의
List<Object[]>는 서비스/어댑터에서 인덱스 기반 캐스팅을 강제해서 깨지기 쉽습니다. projection(인터페이스/DTO)으로 바꾸는 쪽이 안전합니다. 또한 named parameter는@Param("clubIds")를 명시해 두는 편이 더 견고합니다.예시 수정안
+import org.springframework.data.repository.query.Param; + interface ClubSubscribeRepository extends JpaRepository<ClubSubscribe, Long> { + interface ClubSubscriberCountRow { + Long getClubId(); + Long getSubscriberCount(); + } + `@Query`(""" - SELECT cs.club.id, COUNT(cs) + SELECT cs.club.id AS clubId, COUNT(cs) AS subscriberCount FROM ClubSubscribe cs WHERE cs.club.id IN :clubIds GROUP BY cs.club.id """) - List<Object[]> countSubscribersByClubIds(List<Long> clubIds); + List<ClubSubscriberCountRow> countSubscribersByClubIds(`@Param`("clubIds") List<Long> clubIds); }🤖 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/ClubSubscribeRepository.java` around lines 21 - 27, Replace the raw List<Object[]> return in ClubSubscribeRepository.countSubscribersByClubIds with a type-safe projection (create an interface or DTO like ClubSubscriberCount with getClubId()/getCount()) and change the `@Query` to return List<ClubSubscriberCount>; also add a named parameter annotation `@Param`("clubIds") to the method signature so the parameter is explicitly bound. Locate the method countSubscribersByClubIds and update its return type and parameter annotation, and create the projection type used by the repository.src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java (1)
47-70: 결과 정렬 기준이 없어 비결정적 결과가 반환될 수 있습니다.
searchClubs메서드에ORDER BY절이 없어서 동일한 조건으로 호출해도 데이터베이스 쿼리 플랜에 따라 결과 순서가 달라질 수 있습니다. 클라이언트에서 일관된 순서를 기대하는 경우 문제가 될 수 있습니다.최소한 ID 기준 정렬을 추가하여 결정적 결과를 보장하는 것을 권장합니다.
♻️ 정렬 추가 제안
.from(club) .where( categoryEq(category), divisionIn(divisions) ) + .orderBy(club.id.asc()) .fetch();🤖 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/ClubQueryRepositoryImpl.java` around lines 47 - 70, searchClubs currently returns results without a deterministic order; update the Querydsl chain in searchClubs (the queryFactory.select(new QClubReadModel(...))...from(club).where(...).fetch()) to append an orderBy clause (e.g., orderBy(club.id.asc())) so results are consistently ordered by club.id (use club.id in the QClubReadModel selection to locate the field).
🤖 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/adapter/out/persistence/ClubQueryRepositoryImpl.java`:
- Around line 112-123: The loop in ClubQueryRepositoryImpl that iterates over
tuples and assigns String variables instagram, youtube, etc. overwrites previous
URLs for the same ClubSnsType, causing data loss; change the implementation to
collect all URLs per type (e.g., use a Map<ClubSnsType, List<String>> or three
List<String> fields) instead of single String fields, populate the lists inside
the loop when t.get(clubSns.type) and t.get(clubSns.url) are present, and then
adapt the method's return value or DTO construction to expose lists (or a
concatenated/structured representation) for instagram/youtube/etc so multiple
URLs are preserved rather than only the last one.
---
Outside diff comments:
In `@src/test/java/com/kustacks/kuring/archunit/DependencyRuleTests.java`:
- Around line 183-185: 테스트에서 Club 웹 어댑터(in.web)에 대한 incoming 검증이 빠져 있어 경계 위반을 놓칠
수 있으니 DependencyRuleTests의 해당 규칙 체인(현재
withAdaptersLayer("adapter").outgoing("out.persistence").and())에 Club의 incoming
어댑터 검증을 추가하여 .incoming("in.web") 또는 적절한 incoming 식별자(예: "in.web")를 포함하도록 수정하고,
필요하면 .and() 체인을 맞춰 Club 관련 규칙 블록에 포함시켜 테스트가 in.web으로부터의 접근을 올바르게 차단/허용하는지 검증하게
하세요.
---
Nitpick comments:
In
`@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.java`:
- Around line 47-70: searchClubs currently returns results without a
deterministic order; update the Querydsl chain in searchClubs (the
queryFactory.select(new QClubReadModel(...))...from(club).where(...).fetch()) to
append an orderBy clause (e.g., orderBy(club.id.asc())) so results are
consistently ordered by club.id (use club.id in the QClubReadModel selection to
locate the field).
In
`@src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubSubscribeRepository.java`:
- Around line 21-27: Replace the raw List<Object[]> return in
ClubSubscribeRepository.countSubscribersByClubIds with a type-safe projection
(create an interface or DTO like ClubSubscriberCount with
getClubId()/getCount()) and change the `@Query` to return
List<ClubSubscriberCount>; also add a named parameter annotation
`@Param`("clubIds") to the method signature so the parameter is explicitly bound.
Locate the method countSubscribersByClubIds and update its return type and
parameter annotation, and create the projection type used by the repository.
In `@src/test/java/com/kustacks/kuring/archunit/DependencyRuleTests.java`:
- Around line 198-208: The test's DisplayName and method intent don't match the
actual ArchUnit rule: method testPackageDependencies (and its DisplayName "테스트
페키지 의존성 검증") only asserts that classes in "com.kustacks.kuring.user.domain.."
must not depend on "com.kustacks.kuring.user.application.."; rename the
DisplayName (and optionally the test method) to something like "user.domain
should not depend on user.application" or expand the rule to cover all package
dependency checks you intended; also fix the typo "페키지" if keeping a Korean name
so the label accurately reflects the assertion using the existing
resideInAPackage("com.kustacks.kuring.user.domain..") and
resideInAnyPackage("com.kustacks.kuring.user.application..") references.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapter.javasrc/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubQueryRepositoryImpl.javasrc/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubSubscribeRepository.javasrc/test/java/com/kustacks/kuring/acceptance/UserAcceptanceTest.javasrc/test/java/com/kustacks/kuring/archunit/DependencyRuleTests.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/test/java/com/kustacks/kuring/acceptance/UserAcceptanceTest.java
- src/main/java/com/kustacks/kuring/club/adapter/out/persistence/ClubPersistenceAdapter.java
| for (Tuple t : tuples) { | ||
| ClubSnsType type = t.get(clubSns.type); | ||
| String url = t.get(clubSns.url); | ||
|
|
||
| if (type == null) continue; | ||
|
|
||
| switch (type) { | ||
| case INSTAGRAM -> instagram = url; | ||
| case YOUTUBE -> youtube = url; | ||
| case ETC -> etc = url; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and examine the ClubSns entity
echo "=== Finding ClubSns entity ==="
fd -e java | xargs grep -l "class ClubSns" | head -5
echo ""
echo "=== Examining ClubSns entity definition ==="
fd -e java | xargs grep -l "class ClubSns" | xargs catRepository: ku-ring/ku-ring-backend-web
Length of output: 1224
🏁 Script executed:
#!/bin/bash
# Search for uniqueness constraints and relationships
echo "=== Searching for unique constraints on ClubSns ==="
rg "@UniqueConstraint|@Column.*unique|uniqueConstraints" --type java -B 2 -A 2
echo ""
echo "=== Searching for Club entity SNS relationship ==="
rg "class Club" --type java -A 50 | head -80Repository: ku-ring/ku-ring-backend-web
Length of output: 14038
🏁 Script executed:
#!/bin/bash
# Check for database migration files that might show schema constraints
echo "=== Searching for ClubSns in migration files ==="
fd -e sql -o -e yml -o -e yaml | xargs rg -l "club_sns|ClubSns" 2>/dev/null | head -5
# If migration files found, examine them
fd -e sql | xargs grep -i "club_sns" -A 10 -B 2 2>/dev/null | head -100Repository: ku-ring/ku-ring-backend-web
Length of output: 2100
🏁 Script executed:
#!/bin/bash
# Search for ClubSns creation/validation logic
echo "=== Searching for ClubSns creation and validation ==="
rg "ClubSns" --type java -B 2 -A 5 | grep -E "(save|persist|validation|validator|@Valid|uniqueConstraint)" -B 3 -A 3 | head -50
echo ""
echo "=== Searching for tests with multiple ClubSns of same type ==="
fd -e java | xargs grep -l "ClubSns" | xargs grep -l "Test" | head -3
echo ""
echo "=== Examining ClubSns test files ==="
fd -e java | xargs grep -l "ClubSns.*Test\|Test.*ClubSns" | xargs cat 2>/dev/null | head -100Repository: ku-ring/ku-ring-backend-web
Length of output: 3437
🏁 Script executed:
#!/bin/bash
# Check the Club entity and its ClubSns relationship
echo "=== Checking Club entity definition ==="
fd -e java -path "*/club/domain/Club.java" | xargs cat | head -150Repository: ku-ring/ku-ring-backend-web
Length of output: 296
🏁 Script executed:
#!/bin/bash
# Look for comments or documentation about SNS type uniqueness
echo "=== Searching for SNS-related comments and documentation ==="
rg "SNS|sns" --type java -i | grep -i "unique\|duplicate\|multiple\|one per\|only" | head -20
echo ""
echo "=== Check ClubSnsType enum ==="
fd -e java -path "*/ClubSnsType.java" | xargs catRepository: ku-ring/ku-ring-backend-web
Length of output: 349
🏁 Script executed:
#!/bin/bash
# Check the Club entity definition
echo "=== Checking Club entity definition ==="
find src -name "Club.java" -path "*/club/domain/*" -exec cat {} \; | head -100
echo ""
echo "=== Checking ClubSnsType enum ==="
find src -name "ClubSnsType.java" -exec cat {} \;Repository: ku-ring/ku-ring-backend-web
Length of output: 2146
🏁 Script executed:
#!/bin/bash
# Search for how ClubSns is created/persisted
echo "=== Searching for ClubSns creation in service layer ==="
find src -name "*.java" -path "*/club/*" -exec grep -l "clubSns\|ClubSns" {} \; | xargs grep -l "save\|persist\|create" | head -5
echo ""
echo "=== Check if there are any validators or constraints ==="
rg "ClubSns" --type java -A 10 | grep -i "constraint\|valid\|unique" | head -20Repository: ku-ring/ku-ring-backend-web
Length of output: 391
🏁 Script executed:
#!/bin/bash
# Look for how the data is being created/inserted
echo "=== Searching for ClubSns in adapter/application layer ==="
find src -path "*/club/adapter/in/*" -name "*.java" -o -path "*/club/application/*" -name "*.java" | xargs grep -l "ClubSns" 2>/dev/null
echo ""
echo "=== Check the entire findClubDetailById method ==="
find src -name "ClubQueryRepositoryImpl.java" -exec cat {} \; | head -200Repository: ku-ring/ku-ring-backend-web
Length of output: 5630
동일 SNS 타입이 여러 개일 경우 마지막 URL만 유지되어 데이터 손실이 발생합니다.
루프에서 동일한 ClubSnsType을 만날 때마다 이전 값을 덮어쓰므로, 하나의 동아리에 같은 타입의 SNS URL이 여러 개 저장되어 있다면 마지막 URL만 반환됩니다. 데이터베이스 스키마에는 (club_id, type) 조합에 대한 유니크 제약이 없어 이러한 상황이 발생할 수 있습니다.
의도된 설계인 경우 명확한 문서화가 필요하며, 그렇지 않다면 유니크 제약 추가 또는 여러 URL을 모두 반환하는 방식 개선을 검토하세요.
🤖 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/ClubQueryRepositoryImpl.java`
around lines 112 - 123, The loop in ClubQueryRepositoryImpl that iterates over
tuples and assigns String variables instagram, youtube, etc. overwrites previous
URLs for the same ClubSnsType, causing data loss; change the implementation to
collect all URLs per type (e.g., use a Map<ClubSnsType, List<String>> or three
List<String> fields) instead of single String fields, populate the lists inside
the loop when t.get(clubSns.type) and t.get(clubSns.url) are present, and then
adapt the method's return value or DTO construction to expose lists (or a
concatenated/structured representation) for instagram/youtube/etc so multiple
URLs are preserved rather than only the last one.
#️⃣ 이슈
#336
📌 요약
동아리 소속 목록 조회 api, 동아리 목록 조회 api, 동아리 상세 정보 조회 api를 구현하고 관련 테스트도 추가하였습니다.
🛠️ 상세
GET /api/v2/clubs/divisions
GET /api/v2/clubs
GET /api/v2/clubs/{id}
💬 기타
Summary by CodeRabbit
Release Notes
New Features
Tests