Skip to content

[FEAT] 모임 상세 조회와 커서 기반 검색 구현#68

Merged
LimdaeIl merged 3 commits intomainfrom
feat/group-cursor
Dec 10, 2025
Merged

[FEAT] 모임 상세 조회와 커서 기반 검색 구현#68
LimdaeIl merged 3 commits intomainfrom
feat/group-cursor

Conversation

@LimdaeIl
Copy link
Collaborator

@LimdaeIl LimdaeIl commented Dec 10, 2025

📝 Pull Request

📌 PR 종류

해당하는 항목에 체크해주세요.

  • 기능 추가 (Feature)
  • 버그 수정 (Fix)
  • 문서 수정 (Docs)
  • 코드 리팩터링 (Refactor)
  • 테스트 추가 (Test)
  • 기타 변경 (Chore)

✨ 변경 내용

  • 모임 상세 검색 API 구현
  • 모임 커서 기반 검색 API 구현

그룹 목록 엔드포인트에 커서 기반 페이지네이션 기능을 추가합니다. 이를 통해 특정 커서(그룹 ID) 이후의 그룹을 가져와 대규모 그룹 목록을 효율적으로 검색할 수 있습니다.페이지네이션된 데이터를 반환하는 GetGroupListResponse DTO를 도입하고, 목록 표시에 필요한 필드를 포함하도록 GroupListItemResponse를 업데이트합니다.

GroupImage 처리 방식을 변경하여 440x240 및 100x100 이미지 변형을 모두 저장하고 검색할 수 있도록 하며, 정렬 순서에 따라 이미지를 연결합니다. 또한 테스트를 위해 H2 데이터베이스 종속성을 포함합니다.

커서 기반 검색 API를 위한 더미 데이터 생성 GroupCursorDummyTest를 구현했습니다. 애플리케이션 실행 후, 실행하면 회원과 모임 데이터가 생성됩니다.

🔍 관련 이슈

🧪 테스트

변경된 기능에 대한 테스트 범위 또는 테스트 결과를 작성해주세요.

  • 유닛 테스트 추가 / 수정
  • 통합 테스트 검증
  • 수동 테스트 완료

🚨 확인해야 할 사항 (Checklist)

PR을 제출하기 전에 아래 항목들을 확인해주세요.

  • 코드 포매팅 완료
  • 불필요한 파일/코드 제거
  • 로직 검증 완료
  • 프로젝트 빌드 성공
  • 린트/정적 분석 통과 (해당 시)

🙋 기타 참고 사항

리뷰어가 참고하면 좋을 만한 추가 설명이 있다면 적어주세요.

Summary by CodeRabbit

릴리스 노트

  • 새로운 기능

    • 모임 목록 검색 및 페이지네이션 기능 추가 (키워드 검색, 커서 기반 페이징)
    • 모임 상세 정보 조회 시 사용자별 맞춤형 데이터 제공
    • 모임 이미지 처리 개선 (다양한 해상도 지원)
  • 개선 사항

    • 모임 상세 조회 엔드포인트 변경 (POST → GET)

✏️ Tip: You can customize this high-level summary in your review settings.

그룹 목록 엔드포인트에 커서 기반 페이지네이션 기능을 추가합니다. 이를 통해 특정 커서(그룹 ID) 이후의 그룹을 가져와 대규모 그룹 목록을 효율적으로 검색할 수 있습니다.
페이지네이션된 데이터를 반환하는 GetGroupListResponse DTO를 도입하고, 목록 표시에 필요한 필드를 포함하도록 GroupListItemResponse를 업데이트합니다.

GroupImage 처리 방식을 변경하여 440x240 및 100x100 이미지 변형을 모두 저장하고 검색할 수 있도록 하며, 정렬 순서에 따라 이미지를 연결합니다. 또한 테스트를 위해 H2 데이터베이스 종속성을 포함합니다.
@LimdaeIl LimdaeIl self-assigned this Dec 10, 2025
Copilot AI review requested due to automatic review settings December 10, 2025 05:06
@LimdaeIl LimdaeIl added the ✨enhancement New feature or request label Dec 10, 2025
@LimdaeIl LimdaeIl moved this from Backlog to In progress in WeGo-Together Backend Dec 10, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 10, 2025

Walkthrough

이 PR은 모임 조회 기능을 추가합니다. 커서 기반 페이지 나누기를 통한 모임 목록 조회, 모임 상세 정보 조회 엔드포인트 추가, 이미지 저장 방식 변경(주 이미지와 썸네일을 별도 행으로 저장), 관련 데이터 전송 객체 및 서비스 로직을 구현합니다.

Changes

Cohort / File(s) 요약
의존성 및 설정
build.gradle
H2 데이터베이스 테스트 의존성 추가
응답 DTO 신규 생성
src/main/java/team/wego/wegobackend/group/application/dto/response/GetGroupListResponse.java
src/main/java/team/wego/wegobackend/group/application/dto/response/GroupListItemResponse.java
모임 목록 응답 레코드와 개별 모임 목록 항목 레코드 신규 정의
응답 DTO 수정
src/main/java/team/wego/wegobackend/group/application/dto/response/GetGroupResponse.java
images 필드 타입을 List<String>에서 List<GroupImageItemResponse>로 변경
이미지 서비스 로직
src/main/java/team/wego/wegobackend/group/application/service/GroupImageService.java
입력 이미지당 두 개의 GroupImage 행(440x240, 100x100) 생성 및 배치 저장으로 변경
모임 서비스 로직
src/main/java/team/wego/wegobackend/group/application/service/GroupService.java
getGroupList(), getGroup() 메서드 추가; 커서 페이지 나누기 및 이미지 그룹화 로직 구현
저장소 쿼리
src/main/java/team/wego/wegobackend/group/domain/repository/GroupRepository.java
커서 기반 페이지 나누기를 지원하는 findGroupsWithKeywordAndCursor() 네이티브 쿼리 메서드 추가
컨트롤러 엔드포인트
src/main/java/team/wego/wegobackend/group/presentation/GroupController.java
GET /api/v1/groups 목록 조회 엔드포인트 추가; GET /{groupId} 상세 조회 엔드포인트 추가(userId 선택 매개변수)
HTTP 테스트 시나리오
src/test/http/group/get-cursor.http
커서 기반 페이지 나누기 검증을 위한 HTTP 테스트 스크립트
HTTP 테스트 업데이트
src/test/http/group/attend.http
src/test/http/group/cancel.http
인증 토큰 처리, 요청 헤더 및 응답 경로 업데이트
통합 테스트
src/test/java/team/wego/wegobackend/group/GroupCursorDummyTest.java
커서 페이지 나누기 테스트용 더미 데이터 생성 클래스

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant GroupController
    participant GroupService
    participant GroupRepository
    participant ImageService
    participant Database

    Client->>GroupController: GET /api/v1/groups?keyword=...&cursor=...&size=...
    GroupController->>GroupService: getGroupList(keyword, cursor, size)
    GroupService->>GroupRepository: findGroupsWithKeywordAndCursor(keyword, cursor, limit)
    GroupRepository->>Database: 쿼리 실행 (커서 기반 필터링)
    Database-->>GroupRepository: Group 목록 반환
    GroupRepository-->>GroupService: List<Group>
    
    loop 각 Group에 대해
        GroupService->>ImageService: Group의 이미지 조회 및 그룹화
        ImageService->>Database: GroupImage 조회
        Database-->>ImageService: 주 이미지(440x240) & 썸네일(100x100)
        ImageService-->>GroupService: List<GroupImageItemResponse>
        GroupService->>GroupService: toGroupListItemResponse() 변환
    end
    
    GroupService-->>GroupController: GetGroupListResponse(items, nextCursor)
    GroupController-->>Client: 200 OK + 응답 본문
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

  • 주의 필요 영역:
    • GroupService.getGroupList()toGroupListItemResponse(): 커서 기반 페이지 나누기 로직 및 이미지 그룹화 로직의 정확성 검증
    • GroupRepository.findGroupsWithKeywordAndCursor(): 네이티브 SQL 쿼리의 커서 경계 처리 및 키워드 필터링 정확성
    • GroupImageService: 주 이미지와 썸네일을 별도 행으로 저장하는 변경사항의 일관성 및 롤백 로직
    • GetGroupResponse 타입 변경: 기존 코드에서 List<String> 기대 위치들의 호환성 확인

Possibly related PRs

Poem

🐰 커서 넘나들며 페이지 나누고,
썸네일과 주사진 이제 따로 담고,
검색 조건 담아 목록을 그려내니,
모임들이 한 줄로 쭉 펼쳐지는군요!
상세조회도 추가되어 더욱 풍요로워졌어요! 🎉

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 23.81% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed 제목 '[FEAT] 모임 상세 조회와 커서 기반 검색 구현'은 변경 사항의 주요 목표(상세 조회 및 커서 기반 검색)를 명확하게 요약하고 있습니다.
Linked Issues check ✅ Passed PR의 모든 코드 변경사항은 #67의 요구사항(모임 상세 조회 및 커서 기반 검색 구현)을 충족합니다.
Out of Scope Changes check ✅ Passed 모든 변경사항이 모임 상세 조회와 커서 기반 검색 구현이라는 범위 내에 있으며, 추가적인 범위 이탈 변경은 없습니다.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/group-cursor

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/test/http/group/attend.http (2)

95-110: MEMBER 1 회원가입 응답 파싱이 잘못됨 – 반드시 response.body.data.userId 사용

회원가입 API는 { "status": 201, "success": true, "data": { "userId": ... } } 형태로 응답합니다. HOST 회원가입(라인 15)은 올바르게 response.body.data.userId를 사용하지만, MEMBER 1 회원가입(라인 109)은 response.body.userId를 사용하고 있어 memberId1undefined가 되고, 이후 참석 요청(라인 113)이 실패합니다.

> {%
-  // 응답 구조에 따라 수정 필요
-  // 예: { "userId": 1, ... } 라면 아래처럼 유지
-  client.global.set("memberId1", response.body.userId);
+  client.global.set("memberId1", response.body.data.userId);
%}

112-116: URL 형식 통일 및 Authorization 헤더 추가 필요

  1. URL 줄바꿈 형식 통일
    다른 요청들(POST http://.../images/upload?userId={{userId}}, POST http://.../create?userId={{userId}})과 달리 여기만 URL과 쿼리 파라미터를 분리했습니다. 한 줄로 정리하세요.

  2. Authorization 헤더 필수
    이 엔드포인트는 JwtAuthenticationFilter에 의해 보호되며, Authorization 헤더가 없으면 인증 실패(401)가 발생합니다. 이미지 업로드/모임 생성과 동일하게 Authorization: Bearer {{accessToken}}를 추가하세요.

-### 모임 참여
-POST http://localhost:8080/api/v1/groups/{{groupId_png_jpg}}/attend
-    ?userId={{memberId1}}
-Content-Type: application/json
+### 모임 참여
+POST http://localhost:8080/api/v1/groups/{{groupId_png_jpg}}/attend?userId={{memberId1}}
+Content-Type: application/json
+Authorization: Bearer {{accessToken}}
🧹 Nitpick comments (11)
src/test/http/group/cancel.http (1)

122-128: 참여 요청에 userId 쿼리 파라미터와 빈 JSON 바디를 명시한 점

/attend?userId={{memberId1}} 형태로 쿼리 파라미터를 넘기고, {}를 바디로 보내도록 한 변경이 서버 측 계약(쿼리 기반 userId + POST 바디)을 잘 반영하고 있습니다. 이 스크립트만 봤을 때는 MEMBER 1 플로우가 명확하게 분리되어 있어 디버깅에도 도움이 될 것 같습니다.

src/test/http/group/attend.http (3)

12-16: 회원가입(HOST) 응답 파싱 경로와 주석 일관성 정리 제안

response.body.data.userId 사용은 공통 응답 래퍼(data) 구조를 따른 것으로 보이지만, 위 주석 예시는 { "userId": 1 } 구조라 혼동을 줄 수 있습니다. 실제 응답이 {"data": {"userId": ...}}라면 주석 예시도 함께 맞춰 주세요.
또한 실제 응답 스키마가 바뀌면 이 스크립트가 바로 깨지니, 최신 스펙과 일치하는지 한 번 더 확인해 두는 것이 좋겠습니다.


18-29: 로그인 후 accessToken 전역 저장 흐름은 적절해 보입니다

로그인 응답에서 response.body.data.accessToken을 전역에 저장해 이후 Authorization 헤더에서 재사용하는 구조는 명확하고 재사용성도 좋습니다.
요청 실패 시(예: 4xx) 이 값이 undefined로 설정될 수 있으니, 필요하다면 최소한의 응답 코드/필드 체크를 추가하는 것도 고려해 볼 수 있습니다.


31-56: 이미지 업로드 인증 처리 및 다중 이미지 핸들링 방향은 적절하나, 방어 코드 고려 여지 있음

  • Authorization: Bearer {{accessToken}} 추가로 다른 그룹 관련 API들과 인증 방식이 일관되어 좋습니다.
  • 두 장의 이미지를 업로드하고 response.body.data.images[0..1]에서 각각 440x240/100x100 URL을 꺼내 전역 변수로 저장하는 흐름도 로직상 문제 없어 보입니다.

다만 테스트 서버가 예상과 달리 한 장만 반환하거나 배열이 비어 있을 경우 스크립트 실행 시 오류가 날 수 있으니, 필요하다면 길이 체크 정도를 추가해 방어적으로 작성하는 것도 가능합니다.

src/test/http/group/get-cursor.http (1)

30-132: 커서 흐름 디버깅용 스크립트 구성이 잘 되어 있습니다.

각 단계에서 nextCursor를 저장·로그 출력하는 구조라 키셋 페이지네이션 동작을 수동으로 검증하기 좋아 보입니다.
선택 사항이지만, 1-1 / 2-1 첫 페이지에서도 data.items를 로그로 찍어 두면 데이터 분포를 한 번에 비교하기 더 편할 것 같습니다.

src/main/java/team/wego/wegobackend/group/domain/repository/GroupRepository.java (1)

3-35: 키셋 페이지네이션 쿼리는 타당하지만, 태그 조인 + DISTINCT 에서 성능 이슈 여지가 있습니다.

  • g.group_id < :cursor + ORDER BY g.group_id DESC LIMIT :limit 로 구현한 키셋 페이지네이션은 방향성과 조건이 잘 맞습니다.
  • 다만 LEFT JOIN v1_group_tags/v1_tags + LIKE '%:keyword%' + DISTINCT g.* 조합이라, 태그가 많은 그룹이 많을 경우 정렬/중복제거 비용이 커질 수 있습니다.

장기적으로 대용량 트래픽을 고려하신다면,

  • title/location/tag 각각에 적절한 인덱스(전방 일치로 바꾸는 등)나,
  • 태그 검색을 EXISTS (SELECT 1 FROM v1_group_tags ... ) 패턴으로 바꾸거나,
  • 별도 검색 인덱스(예: 검색 전용 컬럼/테이블)를 두는 것도 한번 검토해 보시면 좋겠습니다.
src/main/java/team/wego/wegobackend/group/application/service/GroupImageService.java (1)

23-70: 이미지 2배수 저장/롤백 구조는 좋지만, 검증 재사용과 sortOrder 제약 확인이 필요해 보입니다.

  • 한 장당 main(440x240) + thumb(100x100) 두 엔티티를 만들고, 응답은 한 세트로 묶는 구조는 의도가 명확하고 서비스/DTO 쪽과도 잘 맞습니다.
  • 다만 몇 가지는 한 번 점검해두시는 게 좋아 보입니다.
    1. saveGroupImages(...)에서는 validateCreateGroupImageRequest를 호출하지 않아서, 이 메서드를 직접 사용하는 경로가 있으면 “3장 제한” 제약이 깨질 수 있습니다. 여기서도 같은 검증을 재사용하거나, service 레이어에서 saveGroupImages 진입 전에 항상 검증이 거치도록 보장해 두는 편이 안전합니다.
    2. GroupImage.create(group, url, i)를 main/thumbnail 모두에 대해 호출하면서 동일한 sortOrder = i를 사용하고 있는데, DB 스키마에 (group_id, sort_order) unique 제약이 있다면 지금 방식으로는 제약을 위반하게 됩니다. 스키마에 그런 제약이 없는지, 또는 variant 구분 컬럼을 추가할지 한 번 확인해 보시는 게 좋겠습니다.

그 외에는 uploadedKeys 기반 S3 롤백 처리와 saveAll 배치 저장 방식 모두 자연스럽습니다.

Also applies to: 80-125

src/test/java/team/wego/wegobackend/group/GroupCursorDummyTest.java (1)

35-118: 로컬 전용 더미 데이터 생성 테스트라는 점을 코드 상에서 더 명확히 해 두는 편이 좋겠습니다.

  • @Rollback(false) + 실제 DB에 누적되는 특성상, 일반 테스트 스위트에 항상 포함되기보다는 로컬 전용으로 관리되는 게 안전합니다.
    • 예: @Disabled("로컬 더미 데이터용") 또는 특정 프로파일/태그에만 포함되도록 조정.
  • cursor-host@example.com 호스트 유저를 생성(또는 재사용)하고 있지만, 아래에서는 실제로 member를 호스트로 넘기고 있어서 host 변수는 사용되지 않습니다. 혼동을 줄이기 위해 사용하지 않는 호스트 생성은 제거하거나, 실제로 일부 그룹은 host로 생성하도록 쓰임새를 맞추는 편이 깔끔합니다.
src/main/java/team/wego/wegobackend/group/application/service/GroupService.java (3)

441-451: resolveThumbnailUrl 는 현재 사용되지 않는 dead code 로 보입니다

  • 과거 “440x240 URL 에서 100x100 URL 을 유추” 하던 시절의 유틸로 보이는데, 현재는 440/100 각각 별도 행을 저장하는 방식으로 바뀌어 사용처가 없습니다.
  • private 메서드이고 호출도 없으므로, 혼란을 줄이려면 제거하거나, 나중에 다시 사용할 계획이 있다면 Javadoc/TODO 로 의도를 남겨 두는 편이 좋겠습니다.

162-223: Fix inconsistent list access patterns and import style for ArrayList

The project targets Java 21 (per build.gradle), so List#getFirst() is fully compatible and the JDK version concern does not apply. However, the code has two separate issues worth addressing:

  1. Inconsistent list access patterns: The same class uses both list.getFirst() (lines 209, 381) and list.get(0) (line 341). For consistency, unify to one approach throughout the class.

  2. Unnecessary fully qualified ArrayList: Line 173 uses new java.util.ArrayList<>() instead of importing ArrayList at the top of the file. This reduces readability; add import java.util.ArrayList; and use the unqualified form.


367-438: 상세 조회용 buildGetGroupResponse 구성은 요구사항을 잘 캡슐화하고 있습니다 (이미지/JOIN 상태 포함)

좋은 점:

  • 이미지
    • sortOrder 로 그룹핑 후, 각 묶음에서 440x240/100x100 를 찾아 GroupImageItemResponse 로 재조합하는 구조가 목록/상세 모두에 대해 일관적입니다.
    • 440x240 이 없는 경우 list.getFirst() fallback 으로 대표 이미지를 사용하는 것은 합리적이며, 프로젝트가 Java 21을 타겟하므로 문제없습니다.
  • 태그/참여자 수
    • 태그 이름 목록, status == ATTEND 인 인원만 카운트/노출하는 정책이 서비스 로직과 잘 맞습니다.
  • 로그인 유저 상태
    • currentUserId == null 인 경우 notJoined, 그렇지 않으면 ATTEND 인 row 존재 여부로 참여 상태를 계산하는 로직이 명확합니다.
  • 참여 멤버 리스트
    • ATTEND 상태만 필터링하고 joinedAt 기준 정렬 후 DTO 변환하는 구조가 직관적입니다.

개선 제안(선택 사항):

  • 이미지/태그/참여자 수 계산 로직이 toGroupListItemResponse 와 일부 중복됩니다. 나중에 스펙이 바뀔 때 두 곳을 동시에 수정해야 하므로, 내부 private 헬퍼(예: extractTagNames, groupImagesBySortOrder) 로 공통화하면 유지보수성이 조금 더 좋아질 수 있습니다.
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a993e29 and 38c2e39.

📒 Files selected for processing (12)
  • build.gradle (1 hunks)
  • src/main/java/team/wego/wegobackend/group/application/dto/response/GetGroupListResponse.java (1 hunks)
  • src/main/java/team/wego/wegobackend/group/application/dto/response/GetGroupResponse.java (3 hunks)
  • src/main/java/team/wego/wegobackend/group/application/dto/response/GroupListItemResponse.java (1 hunks)
  • src/main/java/team/wego/wegobackend/group/application/service/GroupImageService.java (4 hunks)
  • src/main/java/team/wego/wegobackend/group/application/service/GroupService.java (5 hunks)
  • src/main/java/team/wego/wegobackend/group/domain/repository/GroupRepository.java (1 hunks)
  • src/main/java/team/wego/wegobackend/group/presentation/GroupController.java (3 hunks)
  • src/test/http/group/attend.http (2 hunks)
  • src/test/http/group/cancel.http (6 hunks)
  • src/test/http/group/get-cursor.http (1 hunks)
  • src/test/java/team/wego/wegobackend/group/GroupCursorDummyTest.java (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Agent
  • GitHub Check: CodeQL analysis (java)
🔇 Additional comments (14)
build.gradle (2)

46-46: H2 테스트 종속성 추가는 적절합니다.

커서 기반 페이지네이션과 모임 상세 조회 기능 테스트를 위해 H2 데이터베이스를 테스트 스코프로 추가한 것은 올바른 구성입니다. 기존 developmentOnly 설정(45줄)과 새로운 testImplementation 설정(46줄)의 이중 구성은 개발 환경과 테스트 환경을 명확히 분리하는 Gradle 모범 사례를 따릅니다.


1-49: 전체 PR 변경사항에 대한 제한된 가시성 주의

현재 검토 대상 파일이 build.gradle만 제공되어 있으나, PR 목표는 다음을 포함합니다:

  • 모임 상세 검색 API 구현
  • 커서 기반 검색 API 및 페이지네이션 구현
  • GetGroupListResponse DTO 및 GroupListItemResponse 업데이트
  • GroupImage 처리 방식 변경 (다중 해상도 이미지 저장)

완전한 코드 검토를 위해 다음 파일들의 검토가 필요합니다:

  • Controller 클래스 (API 엔드포인트)
  • Service 클래스 (비즈니스 로직)
  • Repository 인터페이스 (커서 기반 쿼리)
  • DTO 클래스 (GetGroupListResponse, GroupListItemResponse)
  • Entity 클래스 (GroupImage 변경)
  • 관련 테스트 클래스

현재 제공된 build.gradle 변경사항만으로는 전체 PR의 기능 정합성을 확인할 수 없습니다.

src/test/http/group/cancel.http (4)

12-28: 응답 래핑 구조에 맞춘 전역 변수 설정이 잘 반영되었습니다

response.body.data.userId / response.body.data.accessToken으로 전역 변수를 설정하도록 바뀐 부분이 현재 응답 스펙({ success, data: {...} })과 잘 맞습니다. HOST용 토큰과 userId를 분리해 두어서 이후 시나리오 구성에도 사용성이 좋아 보입니다.


62-92: 모임 생성 페이로드와 groupId 추출 방식이 새 스펙과 일관적입니다

endTime 필드를 startTime과 같은 포맷으로 추가한 점, 그리고 생성 응답에서 response.body.data.id를 읽어 groupId_png_jpg로 저장하는 부분이 자연스럽고 이후 플로우(참여/취소 요청)와도 잘 이어집니다. 테스트용 제목/설명도 어떤 시나리오인지 바로 이해하기 좋습니다.


129-163: MEMBER 2 플로우 추가가 직관적이고 MEMBER 1과 대칭적으로 잘 구성되었습니다

memberId2, accessTokenByMember2를 별도로 저장해 MEMBER 1과 완전히 독립된 가입/로그인/참여 시나리오를 구성한 점이 좋습니다. 이메일과 닉네임도 구분되어 있어 실제로 두 명의 서로 다른 사용자가 같은 모임에 참여하는 케이스를 쉽게 재현할 수 있겠습니다.


164-169: 참여 취소(cancel) 엔드포인트 호출이 앞선 패턴과 잘 맞습니다

/cancel?userId={{memberId1}}로 MEMBER 1의 취소 요청을 보내고, 동일하게 빈 JSON 바디를 사용한 부분이 앞선 attend 요청들과 일관적입니다. HOST/참여자 토큰과 userId 구분도 유지되어 있어, 권한 관련 시나리오를 테스트할 때도 유용해 보입니다.

src/test/http/group/attend.http (1)

58-93: 모임 생성 시 Authorization 및 groupId 추출 로직은 방향성 OK

  • 모임 생성 요청에 Authorization: Bearer {{accessToken}}를 추가해 이미지 업로드와 인증 방식이 통일된 점은 좋습니다.
  • response.body.data.idgroupId_png_jpg로 전역 저장해서 이후 참석 요청에 재사용하는 패턴도 자연스럽습니다.

단, 응답 DTO가 변경되면 이 스크립트가 바로 깨지므로, 실제 모임 생성 API의 응답 스펙(data.id)과 현재 코드가 정확히 일치하는지 한 번 더 확인해 두시면 좋겠습니다.

src/main/java/team/wego/wegobackend/group/application/dto/response/GetGroupResponse.java (1)

17-55: 단건 조회에서 이미지 DTO 타입 변경 설계 괜찮습니다.

상세 조회에서 List<GroupImageItemResponse>를 직접 노출하는 방향이 이미지 메타데이터(정렬, 썸네일 등)를 확장하기에 좋아 보이고, of(...) 팩토리와 필드 타입도 일관되게 맞춰져 있어서 추가 수정 없이 잘 동작할 것 같습니다.

src/main/java/team/wego/wegobackend/group/application/dto/response/GetGroupListResponse.java (1)

1-13: 커서 기반 목록 응답 DTO 구조가 직관적입니다.

items + nextCursor 패턴이 HTTP 테스트 스크립트와도 잘 맞고, nextCursorLong nullable 로 둔 것도 “마지막 페이지 여부” 표현에 무난해 보입니다.

src/main/java/team/wego/wegobackend/group/application/dto/response/GroupListItemResponse.java (1)

1-50: 목록 전용 GroupListItemResponse 설계가 상세 DTO와 잘 분리되어 있습니다.

  • 리스트에서는 List<String> images(URL만)만 노출하고, 단건 상세에서는 List<GroupImageItemResponse>로 더 풍부한 정보를 주는 역할 분리가 명확합니다.
  • createdByGetGroupResponse.CreatedByResponse를 재사용한 것도 DTO 중복을 줄이면서 표현력을 유지하는 방향으로 괜찮아 보입니다.
src/main/java/team/wego/wegobackend/group/presentation/GroupController.java (1)

71-96: The size parameter already has range validation in the service layer; no controller-level changes needed.

The concern about unbounded size parameter is valid in principle, but the actual implementation already addresses it. GroupService.getGroupList() (line 303) applies the exact clamping logic you suggested: int pageSize = Math.max(1, Math.min(size, 50));. This constrains the value to 1–50, preventing excessive DB/network load.

The controller parameter remains undecorated with @Min/@Max annotations and has no defaultValue set, but the service layer's runtime clamping effectively neutralizes the risk. The current implementation is functionally safe.

Likely an incorrect or invalid review comment.

src/main/java/team/wego/wegobackend/group/application/service/GroupService.java (3)

18-25: 새 DTO import 추가는 목적에 부합합니다

GetGroupListResponse, GroupListItemResponse 추가 import 는 서비스에서 사용하는 공개 API 와 일치하고, 불필요한 의존성도 없어 보입니다.


270-281: 예외 메시지/코드 처리는 기존 attend 로직과 일관적입니다

GROUP_NOT_FOUND_BY_ID, NOT_ATTEND_GROUP 예외 처리 포맷만 정리된 것으로 보이고, 동작 상 문제는 없어 보입니다.


453-471: getGroup 오버로드 설계는 잘 되어 있으나, null 처리와 예외 정책이 명확해 좋습니다

  • findByIdAndDeletedAtIsNull 재사용, 동일한 GROUP_NOT_FOUND_BY_ID 예외 코드 사용 등 예외 정책이 일관적입니다.
  • 로그인 여부에 따라 (groupId, currentUserId) 버전과 (groupId) 버전으로 나눈 뒤, 내부적으로 모두 buildGetGroupResponse 로 위임하는 구조가 재사용성을 잘 살리고 있습니다.
  • currentUserId = null 인 경우 userStatus.notJoined() 이 되도록 빌더에서 일관되게 처리하는 점도 좋습니다.

Comment on lines +299 to +326
@Transactional(readOnly = true)
public GetGroupListResponse getGroupList(String keyword, Long cursor, int size) {

private GetGroupResponse buildGetGroupResponse(Group group, Long currentUserId) {
// 이미지 URL (sortOrder 기준 정렬)
var imageUrls = group.getImages().stream()
.sorted(Comparator.comparing(GroupImage::getSortOrder))
.map(GroupImage::getImageUrl)
// 1. size 기본 방어 로직 (1 ~ 50 사이로 제한)
int pageSize = Math.max(1, Math.min(size, 50));

// 2. keyword가 비었으면 null로 통일해서 쿼리 단에서 처리
String normalizedKeyword = (keyword == null || keyword.isBlank()) ? null : keyword;

// 3. limit = pageSize + 1: 다음 페이지 여부 판단용
List<Group> groups = groupRepository.findGroupsWithKeywordAndCursor(
normalizedKeyword,
cursor,
pageSize + 1
);

Long nextCursor = null;
if (groups.size() > pageSize) {
Group lastExtra = groups.remove(pageSize); // size+1 중 마지막 하나 제거
nextCursor = lastExtra.getId(); // 그 id를 다음 커서로 사용
}

List<GroupListItemResponse> items = groups.stream()
.map(this::toGroupListItemResponse)
.toList();

return GetGroupListResponse.of(items, nextCursor);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Find the GroupRepository or related repository interfaces/implementations
find . -type f -name "*Repository*.java" | grep -i group | head -20

Repository: WeGo-Together/WeGo_BackEnd

Length of output: 413


🏁 Script executed:

# Search for findGroupsWithKeywordAndCursor definition
rg -n "findGroupsWithKeywordAndCursor" . --type java

Repository: WeGo-Together/WeGo_BackEnd

Length of output: 366


🏁 Script executed:

# Look for cursor-related tests
rg -n "cursor|Cursor" src/test --type java -l | head -20

Repository: WeGo-Together/WeGo_BackEnd

Length of output: 137


🏁 Script executed:

# Read the GroupRepository interface and implementation
cat -n src/main/java/team/wego/wegobackend/group/domain/repository/GroupRepository.java

Repository: WeGo-Together/WeGo_BackEnd

Length of output: 1664


🏁 Script executed:

# Read the GroupCursorDummyTest to see what behavior is being tested
cat -n src/test/java/team/wego/wegobackend/group/GroupCursorDummyTest.java

Repository: WeGo-Together/WeGo_BackEnd

Length of output: 5374


🏁 Script executed:

# Search for actual cursor pagination test assertions (not just dummy data creation)
rg -n "getGroupList|GetGroupListResponse" src/test --type java -A 5 | head -100

Repository: WeGo-Together/WeGo_BackEnd

Length of output: 52


🏁 Script executed:

# Also check if there are integration tests for the GroupService
rg -n "class.*Test.*Group" src/test --type java -l

Repository: WeGo-Together/WeGo_BackEnd

Length of output: 52


🏁 Script executed:

# Let's check the query one more time to be absolutely sure about the operator
cat -n src/main/java/team/wego/wegobackend/group/domain/repository/GroupRepository.java | sed -n '14,30p'

Repository: WeGo-Together/WeGo_BackEnd

Length of output: 884


🏁 Script executed:

# Check if there are any other cursor-based queries or pagination patterns in the codebase for reference
rg -n "cursor|Cursor" src/main/java --type java -B 2 -A 2 | grep -E "(Query|@|cursor|Cursor)" | head -30

Repository: WeGo-Together/WeGo_BackEnd

Length of output: 2844


Repository query uses id < cursor (descending order), so current nextCursor logic is correct—but the concern about operator dependency is valid

The query at line 20 of GroupRepository uses g.group_id < :cursor with ORDER BY g.group_id DESC, which means the current implementation (setting nextCursor to the removed extra element's ID) is correct and will not cause duplicates or gaps.

However, your concern about tight coupling is valid: if the query operator were changed to > or >= (as in ascending-order pagination), the logic would break. Additionally, there are no cursor boundary tests to verify correctness or prevent regressions if the query is modified.

Recommendation:

  • Add explicit cursor boundary tests (e.g., verifying no gaps/duplicates across multiple pages) rather than just dummy data creation
  • Add a code comment in GroupService clarifying that nextCursor must be the removed extra element's ID specifically because the query uses < with DESC ordering
  • Consider documenting or enforcing the query pattern to prevent accidental changes that would break pagination
🤖 Prompt for AI Agents
In
src/main/java/team/wego/wegobackend/group/application/service/GroupService.java
around lines 299 to 326, the nextCursor computation relies on the repository
query using "id < :cursor" with ORDER BY id DESC so it must set nextCursor to
the removed extra element's ID; add a concise code comment above the nextCursor
logic stating this dependency (that we remove the pageSize+1'th item and use its
id because the repo queries descending with '<'), add explicit cursor-boundary
pagination tests that page across multiple pages asserting no gaps or duplicates
and that nextCursor equals the removed extra element's id for each page, and
consider adding a repository-level comment or test that enforces/validates the
query uses descending '<' semantics to prevent accidental refactors that would
break this pagination logic.

Comment on lines +328 to +364
private GroupListItemResponse toGroupListItemResponse(Group group) {
// sortOrder 기준으로 묶어서 대표(440x240)만 선택
Map<Integer, List<GroupImage>> byOrder = group.getImages().stream()
.collect(Collectors.groupingBy(GroupImage::getSortOrder));

List<String> imageUrls = byOrder.entrySet().stream()
.sorted(Map.Entry.comparingByKey())
.limit(3) // 논리적인 3장까지만
.map(entry -> {
List<GroupImage> list = entry.getValue();
return list.stream()
.filter(img -> img.getImageUrl().contains("440x240"))
.findFirst()
.orElse(list.get(0))
.getImageUrl();
})
.toList();

// 태그 이름
var tagNames = group.getGroupTags().stream()
List<String> tagNames = group.getGroupTags().stream()
.map(GroupTag::getTag)
.map(Tag::getName)
.toList();

// 현재 참여 인원 수 (status == ATTEND 인 인원만)
int participantCount = (int) group.getUsers().stream()
.filter(gu -> gu.getStatus() == ATTEND)
.count();

// 만든 사람 정보
CreatedByResponse createdBy = CreatedByResponse.from(group.getHost());

// 현재 요청 유저의 참여 상태
UserStatusResponse userStatus = group.getUsers().stream()
.filter(gu -> gu.getUser().getId().equals(currentUserId))
return GroupListItemResponse.of(
group,
imageUrls,
tagNames,
participantCount,
createdBy
);
}
Copy link

@coderabbitai coderabbitai bot Dec 10, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's find the GroupRepository and related query methods

echo "=== Finding GroupRepository interface ==="
find . -name "GroupRepository*" -type f

echo -e "\n=== Searching for findGroupsWithKeywordAndCursor method ==="
rg -n "findGroupsWithKeywordAndCursor" --type java

echo -e "\n=== Finding GroupService.getGroupList method ==="
rg -n "getGroupList|public.*Group.*List" src/main/java/team/wego/wegobackend/group/application/service/ --type java

echo -e "\n=== Searching for Group entity class ==="
find . -name "Group.java" -path "*/domain/entity/*" -type f

Repository: WeGo-Together/WeGo_BackEnd

Length of output: 1054


🏁 Script executed:

#!/bin/bash
# Search for Group entity to check relationship configurations

echo "=== Group entity relationship mappings ==="
rg -n "class Group|@OneToMany|@ManyToOne|@FetchType|private.*List.*Image|private.*List.*User|private.*List.*Tag" \
  src/main/java/team/wego/wegobackend/group/domain/entity/Group.java 2>/dev/null || \
  echo "File not found, searching broadly..."

find . -name "Group.java" -path "*/domain/*" | head -5

Repository: WeGo-Together/WeGo_BackEnd

Length of output: 710


🏁 Script executed:

#!/bin/bash
# Broader search for fetch configurations and query definitions

echo "=== Searching for @Query annotations with fetch join ==="
rg -n "@Query.*fetch|FetchType|LEFT JOIN FETCH" --type java src/main/java/team/wego/wegobackend/ 2>/dev/null | head -20

echo -e "\n=== Searching for batch size configurations ==="
rg -n "batch-size|batchSize" --type java src/main/java/team/wego/wegobackend/ 2>/dev/null

Repository: WeGo-Together/WeGo_BackEnd

Length of output: 1288


🏁 Script executed:

#!/bin/bash
# Read the GroupRepository to see the full implementation of findGroupsWithKeywordAndCursor

echo "=== Full GroupRepository content ==="
cat -n src/main/java/team/wego/wegobackend/group/domain/repository/GroupRepository.java

Repository: WeGo-Together/WeGo_BackEnd

Length of output: 1701


🏁 Script executed:

#!/bin/bash
# Also check the actual getGroupList method in GroupService to confirm the flow

echo "=== GroupService.getGroupList implementation ==="
sed -n '300,320p' src/main/java/team/wego/wegobackend/group/application/service/GroupService.java

Repository: WeGo-Together/WeGo_BackEnd

Length of output: 908


🏁 Script executed:

#!/bin/bash
# Search for GroupImage entity to understand the URL pattern and structure

echo "=== GroupImage entity ==="
cat -n src/main/java/team/wego/wegobackend/group/domain/entity/GroupImage.java

echo -e "\n=== Searching for any image URL resolution logic or configuration ==="
rg -n "440x240|imageUrl|image.*url" --type java src/main/java/team/wego/wegobackend/group/domain/entity/

Repository: WeGo-Together/WeGo_BackEnd

Length of output: 2363


🏁 Script executed:

#!/bin/bash
# Verify if there's any batch size configuration in application properties

echo "=== Checking application configuration files ==="
find . -name "application*.yml" -o -name "application*.properties" | xargs cat 2>/dev/null | grep -i "batch-size\|spring.jpa" || echo "No batch size config found"

echo -e "\n=== Double-check: search for any QueryDSL or custom implementation ==="
rg -n "QueryRepository|CustomRepository|@Modifying" src/main/java/team/wego/wegobackend/group/domain/repository/ --type java

Repository: WeGo-Together/WeGo_BackEnd

Length of output: 217


🏁 Script executed:

#!/bin/bash
# Check if there are any test files that demonstrate the N+1 issue or query performance

echo "=== Search for test files related to GroupService or GroupRepository ==="
find . -path "*test*" -name "*Group*Test*.java" | head -5

echo -e "\n=== Check for any documentation about the current limitations ==="
rg -n "TODO|FIXME|XXX|BUG|N\+1" src/main/java/team/wego/wegobackend/group/ --type java | head -10

Repository: WeGo-Together/WeGo_BackEnd

Length of output: 694


N+1 쿼리 문제가 실제로 발생합니다. findGroupsWithKeywordAndCursor 메서드를 개선해야 합니다.

현재 구조에서 심각한 성능 문제가 있습니다:

  • findGroupsWithKeywordAndCursornativeQuery를 사용하고 있어 JPA의 fetch join을 활용할 수 없습니다
  • Group의 images, groupTags, users 컬렉션이 모두 LAZY 로딩으로 설정되어 있고, 배치 사이즈 설정이 없습니다
  • toGroupListItemResponse에서 세 컬렉션을 모두 접근하므로, 페이지당 그룹 N개당 최소 3N개의 추가 쿼리가 발생합니다

예를 들어 한 페이지에 20개 그룹이 있으면: 1개(목록) + 60개(3개 컬렉션 × 20) = 61개의 쿼리가 실행됩니다.

해결 방안:

  • JPQL 기반 쿼리로 변경하고 LEFT JOIN FETCH g.images, g.groupTags, g.users 적용
  • 또는 QueryDSL을 사용하여 fetch join 구성
  • 또는 별도의 배치 쿼리 추가 + spring.jpa.properties.hibernate.default_batch_fetch_size 설정

이미지 URL의 "440x240" 패턴 의존성도 장기적으로는 GroupImage에 타입/사이즈 필드를 추가하는 것이 좋습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

해당 로직은 리팩토링으로 나중에 QueryDSL 로 수정할 예정

Copy link

Choose a reason for hiding this comment

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

@LimdaeIl 알겠습니다! QueryDSL로 리팩토링하실 계획이시군요. QueryDSL을 사용하면 타입 안전성을 유지하면서도 fetch join을 통해 N+1 문제를 효과적으로 해결하실 수 있을 것입니다.

리팩토링 시 참고하시면 좋을 포인트:

  • leftJoin().fetch() 또는 fetchJoin()을 사용하여 연관 엔티티를 한 번에 조회
  • 여러 컬렉션을 동시에 fetch join하는 경우 MultipleBagFetchException 발생 가능성 주의 (Set 사용 또는 두 쿼리로 분리)
  • 커서 기반 페이지네이션과 fetch join 조합 시 성능 최적화 고려

이 작업을 추적하기 위한 이슈를 생성해드릴까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

API 요청과 응답 생성 우선 구현부터 수행

Copy link

Choose a reason for hiding this comment

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

@LimdaeIl 네, 이해했습니다! API 요청/응답 구현을 우선하시고, 성능 최적화는 QueryDSL 리팩토링 단계에서 진행하시겠다는 접근 방식이 합리적입니다.

현재 PR에서는 기능 구현에 집중하시고, 나중에 QueryDSL을 도입하여 fetch join을 통해 N+1 문제를 해결하실 계획이시군요. 이 리팩토링 작업을 추적하기 위한 이슈를 생성해드릴까요?

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements group detail retrieval and cursor-based pagination for group listing, enabling efficient browsing of large group datasets. The implementation introduces a new pagination mechanism using group IDs as cursors and updates the image handling system to store both 440x240 and 100x100 image variants separately in the database.

Key Changes:

  • Added cursor-based pagination API endpoint (GET /api/v1/groups) with keyword search support across title, location, and tags
  • Implemented group detail retrieval endpoint (GET /api/v1/groups/{groupId}) with optional user context
  • Refactored image storage to save both 440x240 and 100x100 variants as separate database rows with matching sortOrder

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
GroupController.java Added GET endpoints for group list (with pagination) and group detail retrieval
GroupRepository.java Implemented native SQL query for cursor-based pagination with keyword search
GroupService.java Added business logic for group listing, detail retrieval, and updated image grouping logic
GroupImageService.java Updated to save both image variants (440x240 and 100x100) as separate database entities
GetGroupListResponse.java New DTO for paginated group list responses with nextCursor
GroupListItemResponse.java New DTO for individual group items in list view
GetGroupResponse.java Updated to return structured image objects instead of plain URLs
build.gradle Added H2 database dependency for test environment
GroupCursorDummyTest.java Test utility for generating dummy data (not a proper unit test)
*.http files Manual test scripts for API endpoint verification

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

.filter(img -> img.getImageUrl().contains("440x240"))
.findFirst()
.orElse(null);
.orElse(list.getFirst()); // 혹시 패턴이 안 맞으면 첫 번째를 대표로
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

Similar to line 341, this has potential for IndexOutOfBoundsException with list.getFirst() if the list is somehow empty. While unlikely given the logic flow (list comes from grouping existing entities), defensive coding would be safer. Consider checking if list is empty before calling getFirst(), or use .findFirst().orElse(null) pattern consistently.

Copilot uses AI. Check for mistakes.
Comment on lines +381 to +393
.orElse(list.getFirst());

GroupImage thumb = list.stream()
.filter(img -> img.getImageUrl().contains("100x100"))
.findFirst()
.orElse(null);

return GroupImageItemResponse.from(
main,
main.getImageUrl(),
(thumb != null) ? thumb.getImageUrl() : null
);
})
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

Same issue as lines 209 and 341: potential IndexOutOfBoundsException with list.getFirst() if the list is empty. Consider using consistent error handling across all three occurrences of this pattern.

Suggested change
.orElse(list.getFirst());
GroupImage thumb = list.stream()
.filter(img -> img.getImageUrl().contains("100x100"))
.findFirst()
.orElse(null);
return GroupImageItemResponse.from(
main,
main.getImageUrl(),
(thumb != null) ? thumb.getImageUrl() : null
);
})
.orElse(null);
GroupImage thumb = list.stream()
.filter(img -> img.getImageUrl().contains("100x100"))
.findFirst()
.orElse(null);
if (main == null) {
// Option 1: skip this entry by returning null (will be filtered out later)
return null;
}
return GroupImageItemResponse.from(
main,
main.getImageUrl(),
(thumb != null) ? thumb.getImageUrl() : null
);
})
.filter(Objects::nonNull)

Copilot uses AI. Check for mistakes.
Comment on lines +35 to +38
@Test
@Transactional
@Rollback(false) // 👉 테스트 끝나도 롤백하지 말고 실제 DB에 남겨두기
void createDummyUsersAndGroupsForCursorTest() {
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

[nitpick] The test method createDummyUsersAndGroupsForCursorTest has @Rollback(false) which means it will persist data to the actual database. This is a data setup utility rather than a proper test. Consider:

  1. Renaming to indicate it's a setup method (e.g., setupDummyDataForManualTesting)
  2. Moving it to a separate utility class or manual test suite
  3. Or adding proper assertions if this is meant to be a real test
Suggested change
@Test
@Transactional
@Rollback(false) // 👉 테스트 끝나도 롤백하지 말고 실제 DB에 남겨두기
void createDummyUsersAndGroupsForCursorTest() {
/**
* [Manual] 더미 데이터 생성용 메서드입니다. 필요시 직접 호출하세요.
*/
@Transactional
@Rollback(false) // 👉 테스트 끝나도 롤백하지 말고 실제 DB에 남겨두기
void setupDummyDataForManualTesting() {

Copilot uses AI. Check for mistakes.
Comment on lines +14 to +29
@Query(value = """
SELECT DISTINCT g.*
FROM v1_groups g
LEFT JOIN v1_group_tags gt ON gt.group_id = g.group_id
LEFT JOIN v1_tags t ON t.tag_id = gt.tag_id
WHERE g.deleted_at IS NULL
AND (:cursor IS NULL OR g.group_id < :cursor)
AND (
:keyword IS NULL
OR :keyword = ''
OR g.title LIKE CONCAT('%', :keyword, '%')
OR g.location LIKE CONCAT('%', :keyword, '%')
OR t.name LIKE CONCAT('%', :keyword, '%')
)
ORDER BY g.group_id DESC
LIMIT :limit
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The DISTINCT keyword in the query may cause performance issues with large datasets. When a group has multiple tags, DISTINCT needs to deduplicate at the database level which can be expensive. Consider:

  1. Using GROUP BY g.group_id instead of DISTINCT g.* for better performance
  2. Or fetch groups and tags separately if performance becomes an issue

Additionally, the query may return incorrect results with pagination when using LIMIT with DISTINCT - groups with more matching tags might be prioritized over those with fewer, affecting the consistency of cursor-based pagination.

Copilot uses AI. Check for mistakes.
.map(entry -> {
List<GroupImage> list = entry.getValue();
return list.stream()
.filter(img -> img.getImageUrl().contains("440x240"))
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The image filtering logic relies on string matching with .contains("440x240") and .contains("100x100"). This is fragile and could break if:

  1. Image URLs don't follow the expected naming convention
  2. URLs contain these strings in unexpected places
  3. Future changes to URL structure

Consider:

  1. Adding a type or variant field to GroupImage entity to explicitly track image types
  2. Or validate URL patterns more strictly during image creation
  3. Add error handling for when neither pattern matches

This would make the code more maintainable and less prone to bugs from URL changes.

Suggested change
.filter(img -> img.getImageUrl().contains("440x240"))
.filter(img -> img.getImageUrl().matches(".*_440x240\\.(jpg|jpeg|png)$"))

Copilot uses AI. Check for mistakes.
Comment on lines +299 to +326
@Transactional(readOnly = true)
public GetGroupListResponse getGroupList(String keyword, Long cursor, int size) {

private GetGroupResponse buildGetGroupResponse(Group group, Long currentUserId) {
// 이미지 URL (sortOrder 기준 정렬)
var imageUrls = group.getImages().stream()
.sorted(Comparator.comparing(GroupImage::getSortOrder))
.map(GroupImage::getImageUrl)
// 1. size 기본 방어 로직 (1 ~ 50 사이로 제한)
int pageSize = Math.max(1, Math.min(size, 50));

// 2. keyword가 비었으면 null로 통일해서 쿼리 단에서 처리
String normalizedKeyword = (keyword == null || keyword.isBlank()) ? null : keyword;

// 3. limit = pageSize + 1: 다음 페이지 여부 판단용
List<Group> groups = groupRepository.findGroupsWithKeywordAndCursor(
normalizedKeyword,
cursor,
pageSize + 1
);

Long nextCursor = null;
if (groups.size() > pageSize) {
Group lastExtra = groups.remove(pageSize); // size+1 중 마지막 하나 제거
nextCursor = lastExtra.getId(); // 그 id를 다음 커서로 사용
}

List<GroupListItemResponse> items = groups.stream()
.map(this::toGroupListItemResponse)
.toList();

return GetGroupListResponse.of(items, nextCursor);
}
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The new cursor-based pagination feature (getGroupList) and group detail retrieval (getGroup) methods lack test coverage. While there's a dummy data generator test, there are no unit or integration tests that verify:

  1. Correct pagination behavior (nextCursor generation, size limiting)
  2. Keyword search functionality across title, location, and tags
  3. Edge cases (empty results, last page, invalid cursor)
  4. Group detail retrieval with and without userId

Consider adding proper test coverage for these critical features.

Copilot uses AI. Check for mistakes.
public ResponseEntity<ApiResponse<GetGroupListResponse>> getGroupList(
@RequestParam(required = false) String keyword,
@RequestParam(required = false) Long cursor,
@RequestParam int size
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The size parameter is required but lacks validation at the API level. If a client sends a negative number or zero, the service will clamp it to 1, but very large numbers (e.g., Integer.MAX_VALUE) will be clamped to 50. Consider adding explicit validation:

  1. Add @Min(1) and @Max(50) annotations to the parameter
  2. Return a clear error message when invalid size is provided
    This provides better API feedback to clients rather than silently clamping values.

Copilot uses AI. Check for mistakes.
Comment on lines +85 to +96
@GetMapping
public ResponseEntity<ApiResponse<GetGroupListResponse>> getGroupList(
@RequestParam(required = false) String keyword,
@RequestParam(required = false) Long cursor,
@RequestParam int size
) {
GetGroupListResponse response = groupService.getGroupList(keyword, cursor, size);

return ResponseEntity
.status(HttpStatus.OK)
.body(ApiResponse.success(HttpStatus.OK.value(), response));
}
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

[nitpick] The public API methods getGroupList and getGroup lack documentation. Consider adding JavaDoc comments that describe:

  • Purpose and behavior of the methods
  • Parameter descriptions (especially the cursor-based pagination mechanism)
  • Return value structure
  • Example usage or important notes about pagination

This is especially important for getGroupList as cursor-based pagination can be non-intuitive for API consumers.

Copilot uses AI. Check for mistakes.
Comment on lines +441 to +452
private String resolveThumbnailUrl(String mainUrl) {
if (mainUrl == null || mainUrl.isBlank()) {
return null;
}

if (mainUrl.contains("440x240")) {
return mainUrl.replace("440x240", "100x100");
}

return mainUrl;
}

Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The resolveThumbnailUrl method is defined but never used in the codebase. This method appears to be leftover from refactoring since the image handling now stores both 440x240 and 100x100 URLs separately in the database. Consider removing this unused method to improve maintainability.

Suggested change
private String resolveThumbnailUrl(String mainUrl) {
if (mainUrl == null || mainUrl.isBlank()) {
return null;
}
if (mainUrl.contains("440x240")) {
return mainUrl.replace("440x240", "100x100");
}
return mainUrl;
}

Copilot uses AI. Check for mistakes.
Comment on lines +338 to +342
return list.stream()
.filter(img -> img.getImageUrl().contains("440x240"))
.findFirst()
.orElse(list.get(0))
.getImageUrl();
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

Potential IndexOutOfBoundsException if the list is empty. While this shouldn't happen in normal flow (since images are grouped by sortOrder from existing GroupImage entities), defensive coding would be better. Consider using .findFirst().orElse(null) and handling the null case, or adding a check for empty lists.

Suggested change
return list.stream()
.filter(img -> img.getImageUrl().contains("440x240"))
.findFirst()
.orElse(list.get(0))
.getImageUrl();
GroupImage image = list.stream()
.filter(img -> img.getImageUrl().contains("440x240"))
.findFirst()
.orElse(list.isEmpty() ? null : list.get(0));
return image != null ? image.getImageUrl() : "";

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✨enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[FEAT] 모임 상세 조회와 커서 기반 검색 구현

1 participant