Conversation
한 개 이미지 저장 요청에 대해 여러 크기의 webp 이미지를 생성해야 하는 경우가 있습니다. 구현 편의성을 위해 가로, 세로를 받아서 유연한 처리하도록 수정 했습니다.
요청 검증, 태그 관리, 호스트 할당을 포함하여 그룹 생성을 위한 핵심 로직을 구현합니다. 이미지 관리를 포함하여 요청 및 응답에 대한 DTO를 추가합니다. 그룹 데이터를 데이터베이스에 유지합니다.
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough모임 생성 및 이미지 업로드 기능을 추가했습니다. 요청 DTO/응답, 서비스(모임·이미지), 컨트롤러, 도메인 엔티티/열거형/예외, 저장소, 이미지 업로드 API 시그니처 변경 및 HTTP 테스트 스크립트가 포함됩니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GroupController
participant GroupService
participant UserRepo as UserRepository
participant TagRepo as TagRepository
participant GroupRepo as GroupRepository
participant GroupTagRepo as GroupTagRepository
participant GroupUserRepo as GroupUserRepository
participant DB as Database
Client->>GroupController: POST /api/v1/groups/create (userId, CreateGroupRequest)
GroupController->>GroupService: create(userId, request)
GroupService->>GroupService: validateCreateGroupRequest(request)
GroupService->>UserRepo: findById(userId)
UserRepo->>DB: SELECT user
DB-->>UserRepo: User
GroupService->>GroupRepo: save(group)
GroupRepo->>DB: INSERT group
DB-->>GroupRepo: persisted Group
GroupService->>TagRepo: findByNameIn(tagNames)
TagRepo->>DB: SELECT tags
DB-->>TagRepo: existing Tags
GroupService->>TagRepo: saveAll(newTags)
TagRepo->>DB: INSERT tags
GroupService->>GroupTagRepo: saveAll(groupTags)
GroupTagRepo->>DB: INSERT group_tag
GroupService->>GroupUserRepo: save(host as HOST)
GroupUserRepo->>DB: INSERT group_user
GroupService-->>GroupController: CreateGroupResponse
GroupController-->>Client: 201 Created (ApiResponse)
Note over Client,DB: 이미지 업로드 흐름 (별도 요청)
Client->>GroupImageController: POST /api/v1/groups/images/{groupId}/upload (userId, images)
GroupImageController->>GroupImageService: createGroupImage(userId, groupId, images)
GroupImageService->>GroupImageService: validateCreateGroupImageRequest(images)
GroupImageService->>ImageUploadService: uploadAsWebpWithSizes(...) per image
ImageUploadService-->>GroupImageService: List<ImageFile> (440x240,100x100)
GroupImageService->>GroupRepo: findById(groupId) (verify host)
GroupImageService->>GroupRepo: save(GroupImage)
GroupImageService-->>GroupImageController: CreateGroupImageResponse
GroupImageController-->>Client: 201 Created (ApiResponse)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (13)
src/main/java/team/wego/wegobackend/tag/domain/repository/TagRepository.java (1)
3-8: 태그 이름 목록 조회 메서드 설계 괜찮습니다Spring Data JPA 관례에 맞는 시그니처이고, 배치 조회용으로 잘 맞습니다.
추가로,tagNames가 빈 리스트일 때 JPA 구현체마다IN ()처리 방식이 다를 수 있으니, 상위 계층에서 빈 리스트를 넘기지 않는다는 전제를 유지하면 더 안전할 것 같습니다.src/main/java/team/wego/wegobackend/image/application/service/ImageUploadService.java (1)
96-129: width/height 쌍 검증을 서비스로 내린 점은 좋지만, 예외 처리 방식만 한 번 정리 필요
widths.size() != heights.size()를ImageUploadService에서 검증하도록 옮긴 건 책임 분리 측면에서 괜찮아 보입니다.- 다만 여기서
IllegalArgumentException을 던지면, 기존ImageException기반의 에러 응답(예: 공통 에러코드/응답 포맷)과 다르게 처리될 수 있으니,
전역 예외 핸들러에서IllegalArgumentException을 어떤 HTTP 코드/메시지로 매핑하는지 한 번만 확인해 두면 좋겠습니다.
필요하다면ImageExceptionCode에 별도 코드를 추가해서 도메인 예외로 통일하는 것도 옵션입니다.현재 로직 자체(루프 인덱스 기반으로
ImageSize생성, WebP 변환 및 S3 업로드)는 요구사항(여러 사이즈 변형 생성)에 잘 맞습니다.src/main/java/team/wego/wegobackend/group/application/dto/response/GroupImageItemResponse.java (1)
1-24: 두 사이즈 URL을 묶은 응답 DTO 설계가 요구사항과 잘 일치합니다
id,sortOrder,imageUrl440x240,imageUrl100x100를 하나의 레코드로 묶어 내려주는 구조가 “이미지별로 두 사이즈를 묶어서 제공”이라는 요구사항을 그대로 반영하고 있습니다.- 정렬 순서를 클라이언트에 그대로 노출하므로, 대표 이미지를
sortOrder가 가장 작은 항목으로 선택하는 프론트 처리에도 잘 맞습니다.추가로, 440x240 URL이 항상
GroupImage엔티티에 저장된 값과 동일하다면,from메서드에서 엔티티에서 바로 읽고 100x100만 인자로 받는 형태로 단순화하는 것도 고려해 볼 수 있습니다만, 현재 형태도 명확해서 유지해도 무방해 보입니다.src/main/java/team/wego/wegobackend/group/domain/entity/Group.java (1)
94-104: 양방향 연관관계 편의 메서드의 일관성을 개선하세요.현재
addImage,addUser,addTag메서드는 컬렉션에만 엔티티를 추가하고 역방향 참조를 설정하지 않습니다.GroupImage.create()처럼 정적 팩토리 메서드를 통해서만 생성되는 경우에는 문제가 없지만, 직접 이 메서드들을 호출할 경우 양방향 관계가 불완전해질 수 있습니다.다음과 같이 양방향 관계를 모두 설정하도록 개선하는 것을 권장합니다:
public void addImage(GroupImage image) { this.images.add(image); + if (image.getGroup() != this) { + image.setGroup(this); + } } public void addUser(GroupUser groupUser) { this.users.add(groupUser); + if (groupUser.getGroup() != this) { + groupUser.setGroup(this); + } } public void addTag(GroupTag groupTag) { this.groupTags.add(groupTag); + if (groupTag.getGroup() != this) { + groupTag.setGroup(this); + } }참고: 이를 위해서는 각 엔티티에 패키지 레벨
setGroup메서드가 필요합니다.src/main/java/team/wego/wegobackend/group/presentation/GroupController.java (3)
26-26: RESTful 엔드포인트 경로를 개선하세요.POST 메서드는 이미 생성을 의미하므로
/create경로는 중복입니다. RESTful 규약에 따라POST /api/v1/groups로 간소화하는 것을 권장합니다.-@PostMapping("/create") +@PostMapping public ResponseEntity<ApiResponse<CreateGroupResponse>> createGroupResponse(
27-27: 메서드 이름을 개선하세요.메서드 이름
createGroupResponse는 응답 객체를 생성하는 것처럼 보입니다.createGroup이 더 명확합니다.-public ResponseEntity<ApiResponse<CreateGroupResponse>> createGroupResponse( +public ResponseEntity<ApiResponse<CreateGroupResponse>> createGroup(
16-16: 사용되지 않는 import를 제거하세요.
GetGroupResponse가 이 파일에서 사용되지 않습니다.import team.wego.wegobackend.group.application.dto.response.CreateGroupResponse; -import team.wego.wegobackend.group.application.dto.response.GetGroupResponse; import team.wego.wegobackend.group.application.service.GroupService;src/main/java/team/wego/wegobackend/group/domain/entity/GroupUser.java (1)
49-60:status컬럼의 null 허용 여부를 명확히 하는 것이 좋습니다
- 새로 생성되는
GroupUser는GroupUserStatus.ATTEND로 항상 세팅되어 도메인 상으로는 null이 없어야 할 것 같습니다.- 현재
@Column(name = "group_user_status")에nullable = false가 없어 DB 스키마 상 null이 허용될 여지가 있습니다. 마이그레이션에서 기본값을 채우고 NOT NULL 제약을 추가하는 쪽이 도메인 인variant를 더 잘 지켜줄 것 같습니다.src/main/java/team/wego/wegobackend/group/application/service/GroupImageService.java (2)
113-116:IMAGE_UPLOAD_FAILED에 예외 객체를 포맷 인자로 넘기는 부분 확인이 필요합니다
new GroupException(GroupErrorCode.IMAGE_UPLOAD_FAILED, e)에서, 에러 코드 메시지 템플릿에는%s플레이스홀더가 없습니다.AppException에서String.format(messageTemplate, args)방식으로 구현되어 있다면, 인자 개수와 플레이스홀더 개수가 맞지 않아MissingFormatArgumentException이 발생할 수 있습니다.- 원인이 되는 예외를 남기고 싶다면:
- (1) 메시지 템플릿에
%s를 추가해e.getMessage()정도만 포맷으로 넣거나,- (2)
AppException쪽에 cause를 받는 생성자를 추가해new GroupException(GroupErrorCode.IMAGE_UPLOAD_FAILED, e)형태로 cause를 전달하도록 분리하는 것을 고려해 주세요.
119-148: host 검증 및 응답 DTO 구성 흐름이 명확합니다
userId로 호스트 조회 →groupId로 모임 조회 →group.getHost().getId()와 비교하는 순서가 직관적입니다.- 유효성 검사 후
saveGroupImages호출, 마지막에CreateGroupImageResponse로 모임 ID와 각 이미지 정보(두 사이즈 URL)를 묶어 반환하는 구조가 프론트 요구사항과도 맞습니다.- 다만, 호스트가 아닌 사용자가 호출한 경우에도
HOST_USER_NOT_FOUND를 재사용하고 있어, 나중에 권한 오류를 구분하고 싶다면 별도의 에러 코드(예: USER_NOT_HOST)를 추가하는 것도 고려해볼 수 있습니다.src/main/java/team/wego/wegobackend/group/application/dto/response/CreateGroupResponse.java (1)
51-66:joinedCount하드코딩(1) 부분의 의미를 도메인 기준으로 다시 정리하는 게 좋겠습니다
- 현재
joinedCount(1) // TODO: 다시 한 번 체크로 고정값을 내려보내고 있어, 필드 의미(누적 참여자 수, 오늘 새로 참여한 인원 등)에 따라 잘못된 정보가 내려갈 수 있습니다.- 의도에 따라:
- 현재 참여 인원과 동일하다면
attendUserCount를 재사용,- 누적 참여자 수라면 별도 도메인 값에서 계산
- 아직 확정되지 않았다면 일단 제거하고, 요구사항이 명확해질 때 다시 추가
등을 고려해 보시면 좋겠습니다.src/main/java/team/wego/wegobackend/group/application/service/GroupService.java (1)
42-50: 서비스 레벨 유효성 검증에서 null 가드 추가를 고려해 볼 수 있습니다
- 현재 컨트롤러에서
@Valid CreateGroupRequest로 들어와 Bean Validation이 선행되므로 실제 요청 흐름에서는startTime,endTime,maxParticipants가 null이 아닐 것입니다.- 다만
GroupService.create가 다른 컨텍스트에서 직접 호출될 가능성을 열어둔다면,request.startTime()/request.endTime()접근 전에 null 가드를 두는 것이 방어적으로 더 안전합니다. (예:Objects.requireNonNull또는 명시적인 예외)필수는 아니지만, 재사용을 고려하면 한 번쯤 점검해볼 만한 부분입니다.
src/main/java/team/wego/wegobackend/group/domain/exception/GroupErrorCode.java (1)
8-31: 그룹 도메인 에러 코드 정의가 명확하고 일관적입니다
- 각 케이스별로
HttpStatus와 한글 메시지가 잘 붙어 있어, API 응답을 해석하기 쉬울 것 같습니다.HOST_USER_NOT_FOUND,GROUP_NOT_FOUND_BY_ID,IMAGE_UPLOAD_EXCEED처럼%s플레이스홀더를 가진 코드와, 단순 메시지 코드가 구분되어 있는 점도 좋습니다. (다만IMAGE_UPLOAD_FAILED는 현재 인자를 넘기지 않는 방향으로 사용하는 것이 더 자연스러워 보입니다.)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
src/main/java/team/wego/wegobackend/group/application/dto/request/CreateGroupRequest.java(1 hunks)src/main/java/team/wego/wegobackend/group/application/dto/response/CreateGroupImageResponse.java(1 hunks)src/main/java/team/wego/wegobackend/group/application/dto/response/CreateGroupResponse.java(1 hunks)src/main/java/team/wego/wegobackend/group/application/dto/response/GroupImageItemResponse.java(1 hunks)src/main/java/team/wego/wegobackend/group/application/service/GroupImageService.java(1 hunks)src/main/java/team/wego/wegobackend/group/application/service/GroupService.java(1 hunks)src/main/java/team/wego/wegobackend/group/domain/GroupRole.java(0 hunks)src/main/java/team/wego/wegobackend/group/domain/entity/Group.java(1 hunks)src/main/java/team/wego/wegobackend/group/domain/entity/GroupImage.java(1 hunks)src/main/java/team/wego/wegobackend/group/domain/entity/GroupRole.java(1 hunks)src/main/java/team/wego/wegobackend/group/domain/entity/GroupTag.java(1 hunks)src/main/java/team/wego/wegobackend/group/domain/entity/GroupUser.java(2 hunks)src/main/java/team/wego/wegobackend/group/domain/entity/GroupUserStatus.java(1 hunks)src/main/java/team/wego/wegobackend/group/domain/exception/GroupErrorCode.java(1 hunks)src/main/java/team/wego/wegobackend/group/domain/exception/GroupException.java(1 hunks)src/main/java/team/wego/wegobackend/group/domain/repository/GroupImageRepository.java(1 hunks)src/main/java/team/wego/wegobackend/group/domain/repository/GroupRepository.java(1 hunks)src/main/java/team/wego/wegobackend/group/domain/repository/GroupTagRepository.java(1 hunks)src/main/java/team/wego/wegobackend/group/domain/repository/GroupUserRepository.java(1 hunks)src/main/java/team/wego/wegobackend/group/presentation/GroupController.java(1 hunks)src/main/java/team/wego/wegobackend/group/presentation/GroupImageController.java(1 hunks)src/main/java/team/wego/wegobackend/image/application/service/ImageUploadService.java(2 hunks)src/main/java/team/wego/wegobackend/image/presentation/ImageController.java(1 hunks)src/main/java/team/wego/wegobackend/tag/domain/repository/TagRepository.java(1 hunks)src/test/http/group/create.http(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/team/wego/wegobackend/group/domain/GroupRole.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/team/wego/wegobackend/group/application/service/GroupService.java (1)
src/main/java/team/wego/wegobackend/group/domain/exception/GroupException.java (1)
GroupException(6-15)
src/main/java/team/wego/wegobackend/group/presentation/GroupImageController.java (2)
src/main/java/team/wego/wegobackend/group/presentation/GroupController.java (1)
RequiredArgsConstructor(19-37)src/main/java/team/wego/wegobackend/image/presentation/ImageController.java (1)
RestController(22-119)
🔇 Additional comments (16)
src/main/java/team/wego/wegobackend/group/domain/repository/GroupTagRepository.java (1)
1-8: GroupTag 전용 리포지토리 추가 방향 좋습니다도메인 엔티티 패키지(
group.domain.entity.GroupTag)와 리포지토리 패키지 분리가 잘 되어 있고, 기본 CRUD 용도로 충분해 보입니다.src/main/java/team/wego/wegobackend/group/domain/entity/GroupTag.java (1)
1-44: 엔티티 패키지 재정렬 일관성 좋아 보입니다
Group,GroupImage등과 같이group.domain.entity로 위치를 통일한 점이 도메인 구조 파악에 도움 됩니다.
팩토리 메서드에서group.addTag(groupTag)로 양방향 연관관계도 즉시 정리하고 있어 유지해도 문제 없어 보입니다.src/main/java/team/wego/wegobackend/image/presentation/ImageController.java (1)
89-99: 서비스 시그니처 변경 반영 및 책임 위임이 자연스럽습니다
uploadAsWebpWithSizes(dir, file, 0, widths, heights)로 변경해 컨트롤러에서 사이즈 리스트 조립 로직을 제거한 점이 깔끔합니다.
width/height 길이 불일치는 이제 서비스에서 검증하므로, 컨트롤러 쪽에서는 현재 정도의 단순 위임으로 충분해 보입니다.src/main/java/team/wego/wegobackend/group/domain/entity/GroupUserStatus.java (1)
1-7: 참여/이탈 상태를 enum 으로 분리한 점 좋습니다
ATTEND,LEFT두 상태만으로 현재 요구사항(모임 생성 및 기본 참여 관리)에는 충분해 보입니다.
향후 대기/차단 등 상태가 늘어나더라도 enum 확장으로 대응 가능해 보여 도메인 모델링 방향이 괜찮습니다.src/main/java/team/wego/wegobackend/group/domain/entity/GroupRole.java (1)
1-7: HOST/MEMBER 역할 분리가 모임 기능 요구사항과 잘 맞습니다모임 생성 시 HOST 설정과 일반 참여자 구분을 Enum으로 명확히 표현한 점이 도메인 가독성에 도움이 됩니다.
추후 운영자/관리자 등의 추가 역할이 필요하면 enum 확장으로 충분히 대응 가능해 보입니다.src/main/java/team/wego/wegobackend/group/domain/repository/GroupRepository.java (1)
6-8: LGTM!기본적인 CRUD 연산을 제공하는 표준 JPA Repository 인터페이스로 적절합니다.
src/main/java/team/wego/wegobackend/group/domain/repository/GroupUserRepository.java (1)
6-8: LGTM!GroupUser 엔티티에 대한 표준 Repository 인터페이스로 적절합니다.
src/main/java/team/wego/wegobackend/group/domain/entity/GroupImage.java (1)
37-44: LGTM!정적 팩토리 메서드가 양방향 연관관계를 올바르게 설정하고 있습니다.
imageUrl필드는 PR 목표에 따라 대표 이미지 URL(440x240)만 저장하도록 설계되었습니다.src/main/java/team/wego/wegobackend/group/domain/exception/GroupException.java (1)
6-15: LGTM!표준 예외 패턴을 따르는 깔끔한 구현입니다.
src/main/java/team/wego/wegobackend/group/domain/repository/GroupImageRepository.java (1)
7-9: LGTM!기본 CRUD 연산을 제공하는 표준 Repository입니다. PR 목표에서 언급된 대표 이미지 조회(sortOrder 기반)가 필요한 경우 향후 커스텀 쿼리 메서드를 추가할 수 있습니다.
src/main/java/team/wego/wegobackend/group/application/dto/response/CreateGroupImageResponse.java (1)
6-10: LGTM!불변 DTO를 위한 깔끔한 record 구현입니다.
src/main/java/team/wego/wegobackend/group/presentation/GroupImageController.java (1)
27-38: 그룹 이미지 업로드 엔드포인트 설계 깔끔합니다
userId파라미터,groupIdpath,images를@RequestPart(required = false)로 받는 구조가 요구사항과 잘 맞습니다.- 서비스로 바로 委任하고
201 Created + ApiResponse.success로 감싸는 패턴이 기존GroupController/ImageController와도 일관적입니다.src/main/java/team/wego/wegobackend/group/application/dto/request/CreateGroupRequest.java (1)
12-38: 요청 DTO 필드 및 Bean Validation 제약이 요구사항을 잘 반영합니다
- 제목/위치/설명 필수, 시작·종료 시간의 미래 제약, 최대 인원 2~12명 제약이 명확하게 선언되어 있습니다.
- 태그/상세 위치는 선택 항목으로 열어두어 실제 도메인 요구에 맞습니다.
- 시간 순서(endTime > startTime)는 서비스 레벨에서 추가 검증하고 있어 계층 분리가 잘 되어 있습니다.
src/main/java/team/wego/wegobackend/group/application/service/GroupImageService.java (1)
49-112: 이미지 업로드 + 롤백 처리 흐름이 요구사항에 잘 맞습니다
- 한 장당 440x240 / 100x100 두 가지 WebP 변형을 올리고, DB에는 대표(440x240) URL만 저장하는 구조가 PR 설명과 일치합니다.
- 업로드 중 예외 발생 시, 이미 올라간 키들을
uploadedKeys로 모아deleteAll로 정리하는 보상 트랜잭션 패턴도 좋습니다.- null/빈 이미지 리스트, 개별 파일 null/empty를 건너뛰는 방어 코드도 잘 되어 있습니다.
src/main/java/team/wego/wegobackend/group/application/service/GroupService.java (2)
52-96: 태그 정규화 및 생성/매핑 로직이 잘 구성되어 있습니다
- null/공백 제거 → trim → 중복 제거 → 기존 태그 조회 → 없는 것만 신규 생성 →
GroupTag를 normalized 순서대로 저장하는 플로우가 깔끔합니다.- 나중에 태그 개수가 많아질 경우를 대비해
findByNameIn에 인덱스만 잘 잡혀 있으면 성능도 크게 문제 없을 것 같습니다.
103-133: 모임 생성 전체 트랜잭션 플로우가 요구사항을 잘 충족합니다
- 호스트 사용자 조회 실패 시
HOST_USER_NOT_FOUND로 예외 처리.- 비즈니스 유효성 검증(시간/최대 인원) → Group 생성/저장 → 태그 저장 → 호스트를
GroupUser(HOST)로 등록하는 순서가 자연스럽습니다.- 마지막에
CreateGroupResponse.from(group)으로 도메인 → 응답 DTO 매핑을 위임하는 구조도 좋습니다.
src/main/java/team/wego/wegobackend/group/application/dto/response/CreateGroupResponse.java
Show resolved
Hide resolved
src/main/java/team/wego/wegobackend/group/application/service/GroupImageService.java
Show resolved
Hide resolved
|
|
||
| @PostMapping("/create") | ||
| public ResponseEntity<ApiResponse<CreateGroupResponse>> createGroupResponse( | ||
| @RequestParam Long userId, |
There was a problem hiding this comment.
보안 문제: userId를 요청 파라미터로 받지 마세요.
@RequestParam Long userId를 사용하면 클라이언트가 임의의 userId를 전달하여 다른 사용자를 사칭할 수 있습니다. 인증된 사용자 정보는 보안 컨텍스트에서 추출해야 합니다.
다음과 같이 수정하세요:
+import org.springframework.security.core.annotation.AuthenticationPrincipal;
+// 또는 커스텀 인증 principal 사용
+
@PostMapping("/create")
public ResponseEntity<ApiResponse<CreateGroupResponse>> createGroupResponse(
- @RequestParam Long userId,
+ @AuthenticationPrincipal UserDetails userDetails,
@RequestBody @Valid CreateGroupRequest request
) {
+ Long userId = extractUserIdFrom(userDetails);
CreateGroupResponse response = groupService.create(userId, request);또는 프로젝트의 인증 방식에 맞는 방법으로 현재 인증된 사용자의 ID를 추출하세요.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/team/wego/wegobackend/group/presentation/GroupController.java
around line 28, the controller currently accepts @RequestParam Long userId which
allows clients to impersonate other users; remove the @RequestParam userId
parameter and instead obtain the authenticated user's ID from the security
context (e.g., add an Authentication or @AuthenticationPrincipal
UserDetails/CustomPrincipal parameter or call
SecurityContextHolder.getContext().getAuthentication()), then pass that
extracted ID to the service; update the method signature and any callers/tests
accordingly to use the authenticated user ID rather than a request parameter.
| POST http://localhost:8080/api/v1/groups/images/{{groupId}}/upload | ||
| ?userId=1 | ||
| Content-Type: multipart/form-data; boundary=boundary | ||
|
|
||
| --boundary | ||
| Content-Disposition: form-data; name="images"; filename="test-webp1.webp" | ||
| Content-Type: image/webp | ||
|
|
||
| < ../image/resources/test-webp1.webp | ||
| --boundary | ||
| Content-Disposition: form-data; name="images"; filename="test-webp1.webp" | ||
| Content-Type: image/webp | ||
|
|
||
| < ../image/resources/test-webp2.webp | ||
| --boundary-- | ||
|
|
||
| --boundary | ||
| Content-Disposition: form-data; name="images"; filename="img1.png" | ||
| Content-Type: image/png | ||
|
|
||
| < ../image/resources/img1.png | ||
| --boundary-- | ||
|
|
||
| --boundary | ||
| Content-Disposition: form-data; name="images"; filename="img2.jpg" | ||
| Content-Type: image/jpeg | ||
|
|
||
| < ../image/resources/img2.jpg | ||
| --boundary-- |
There was a problem hiding this comment.
4장 업로드 케이스의 multipart boundary 순서가 잘못되어 실제로는 4장이 전송되지 않습니다
- Line 153에서 이미
--boundary--로 폼이 종료된 뒤, 155/162에서 다시--boundary를 사용하고 있어 이후 파일들은 유효한 파트로 인식되지 않습니다. - 의도는 4개의 파트를 모두 추가한 뒤 마지막 한 번만
--boundary--로 닫는 것이어야 합니다.
예시 수정안(개념적으로):
...
...
-
...
-
...또한 149/152에서 파일명(test-webp1.webp)과 실제 전송 파일(test-webp2.webp)이 달라 혼동을 줄 수 있으니 함께 정리하는 것이 좋겠습니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/test/http/group/create.http around lines 139 to 167 the multipart body
prematurely closes the multipart at line 153 with "--boundary--", so the
subsequent parts (lines 155–162 and 164–167) are not treated as valid form
parts; remove the premature "--boundary--" markers so each part is separated by
a single "--boundary" and leave a single final "--boundary--" at the end, and
also fix the mismatched filenames (test-webp1.webp vs test-webp2.webp) so each
Content-Disposition filename matches the actual file being sent.
There was a problem hiding this comment.
Pull request overview
This PR implements a group creation feature that supports creating groups with metadata and uploading associated images. The implementation separates group creation from image uploads into two distinct endpoints, stores only the main 440x240 image URL in the database, and generates a 100x100 thumbnail variant that can be derived from S3 naming conventions.
Key Changes:
- Group creation endpoint with validation for title, location, time range, and participant limits
- Image upload endpoint supporting up to 3 images with automatic webp conversion to two sizes (440x240, 100x100)
- Tag management with automatic creation of new tags and association with groups
- Package restructuring moving domain entities from
group.domaintogroup.domain.entity
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
GroupController.java |
Adds POST endpoint for group creation accepting JSON request body |
GroupImageController.java |
Adds POST endpoint for uploading group images as multipart/form-data |
GroupService.java |
Implements group creation logic with tag handling and host assignment |
GroupImageService.java |
Handles image upload, S3 storage, and database persistence with compensation transaction |
ImageUploadService.java |
Refactors method signature to accept separate width/height lists instead of ImageSize objects |
CreateGroupRequest.java |
Defines validation rules for group creation including time and participant constraints |
CreateGroupResponse.java |
Returns group details including tags, participant count, and creator information |
GroupImageItemResponse.java |
Returns individual image data with both size URLs bundled together |
Group.java, GroupImage.java, GroupTag.java, GroupUser.java |
Moved from group.domain to group.domain.entity package |
GroupRole.java |
Relocated to group.domain.entity package |
GroupUserStatus.java |
New enum defining ATTEND and LEFT statuses for group members |
GroupException.java, GroupErrorCode.java |
New exception handling for group-related errors |
TagRepository.java |
Adds findByNameIn query method for bulk tag lookup |
GroupRepository.java, GroupImageRepository.java, GroupTagRepository.java, GroupUserRepository.java |
New JPA repositories for group-related entities |
create.http |
HTTP test scenarios for group creation and image upload with various test cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @RequestParam Long userId, | ||
| @PathVariable Long groupId, | ||
| @RequestPart(value = "images", required = false) List<MultipartFile> images | ||
| ) { |
There was a problem hiding this comment.
The images parameter is marked as required = false but the validation logic at line 41 assumes it's not null (calling images.size()). If images is null, this will throw a NullPointerException. Either remove required = false to make it required, or add a null check in the validation method before accessing images.size().
| ) { | |
| ) { | |
| if (images == null) { | |
| images = List.of(); | |
| } |
|
|
||
| < ../image/resources/test-webp1.webp | ||
| --boundary | ||
| Content-Disposition: form-data; name="images"; filename="test-webp1.webp" |
There was a problem hiding this comment.
There's a duplicate filename "test-webp1.webp" at both lines 144 and 149, but line 152 loads "test-webp2.webp". The filename at line 149 should be "test-webp2.webp" to match the actual file being loaded.
| Content-Disposition: form-data; name="images"; filename="test-webp1.webp" | |
| Content-Disposition: form-data; name="images"; filename="test-webp2.webp" |
| .participantCount(attendUserCount) | ||
| .maxParticipants(group.getMaxParticipants()) | ||
| .createdBy(createdByHost) | ||
| .createdAt(group.getCreatedAt()) | ||
| .updatedAt(group.getUpdatedAt()) | ||
| .joinedCount(1) // TODO: 다시 한 번 체크 |
There was a problem hiding this comment.
The response field participantCount (line 60) and joinedCount (line 65) appear to represent similar concepts but have different values. participantCount uses the actual count of ATTEND status users, while joinedCount is hardcoded to 1. This inconsistency is confusing. Consider either:
- Using the same value for both if they represent the same thing
- Renaming them to clarify their distinct purposes (e.g.,
currentParticipantCountvsinitialHostCount) - Removing one if redundant
|
|
||
| @Transactional | ||
| public CreateGroupResponse create(Long userId, CreateGroupRequest request) { | ||
| // 1. 회원 조회(HOST) TODO: 회원 정보 파싱 구현되면 연결 |
There was a problem hiding this comment.
The TODO comment "회원 정보 파싱 구현되면 연결" (connect when user info parsing is implemented) is unclear. Since the code already fetches the user from the repository, it's not clear what "parsing" refers to or what needs to be "connected". Consider either:
- Removing the TODO if the current implementation is sufficient
- Clarifying what specific parsing/connection is needed (e.g., "TODO: Replace userId query param with JWT token parsing when authentication is implemented")
| // 1. 회원 조회(HOST) TODO: 회원 정보 파싱 구현되면 연결 | |
| // 1. 회원 조회(HOST) TODO: 인증 구현 시 userId 파라미터를 JWT 토큰 파싱으로 대체 |
| INVALID_TIME_RANGE(HttpStatus.BAD_REQUEST, "모임: 종료 시간은 시작 시간보다 뒤여야 합니다."), | ||
| INVALID_MAX_PARTICIPANTS(HttpStatus.BAD_REQUEST, "모임: 최대 인원은 최소 2명 이상이어야 합니다."), | ||
| TAG_NOT_FOUND(HttpStatus.NOT_FOUND, "모임: 요청한 태그를 찾을 수 없습니다."), | ||
| IMAGE_UPLOAD_FAILED(HttpStatus.BAD_REQUEST, "모임: 이미지 업로드 요청에 실패했습니다."), |
There was a problem hiding this comment.
The error message template uses a placeholder %s but it's only appropriate when the exception is constructed with the images count argument. However, the error message "모임: 이미지 업로드 요청에 실패했습니다." at line 16 doesn't have a placeholder, yet it's used with an exception argument e at line 115. Consider using a more descriptive error message like "모임: 이미지 업로드 요청에 실패했습니다: %s" to include the underlying error details.
| IMAGE_UPLOAD_FAILED(HttpStatus.BAD_REQUEST, "모임: 이미지 업로드 요청에 실패했습니다."), | |
| IMAGE_UPLOAD_FAILED(HttpStatus.BAD_REQUEST, "모임: 이미지 업로드 요청에 실패했습니다: %s"), |
src/test/http/group/create.http
Outdated
| Content-Type: image/jpeg | ||
|
|
||
| < ../image/resources/img2.jpg | ||
| --boundary--} |
There was a problem hiding this comment.
There are extra closing braces } at line 56 that shouldn't be part of the multipart boundary. The line should just be --boundary-- without the trailing }.
| --boundary--} | |
| --boundary-- |
| import team.wego.wegobackend.common.response.ApiResponse; | ||
| import team.wego.wegobackend.group.application.dto.request.CreateGroupRequest; | ||
| import team.wego.wegobackend.group.application.dto.response.CreateGroupResponse; | ||
| import team.wego.wegobackend.group.application.dto.response.GetGroupResponse; |
There was a problem hiding this comment.
The import GetGroupResponse is unused in this file. Consider removing it to keep the imports clean.
| import team.wego.wegobackend.group.application.dto.response.GetGroupResponse; |
| @PostMapping("/create") | ||
| public ResponseEntity<ApiResponse<CreateGroupResponse>> createGroupResponse( | ||
| @RequestParam Long userId, | ||
| @RequestBody @Valid CreateGroupRequest request | ||
| ) { | ||
| CreateGroupResponse response = groupService.create(userId, request); | ||
|
|
||
| return ResponseEntity | ||
| .status(HttpStatus.CREATED) | ||
| .body(ApiResponse.success(response)); | ||
| } | ||
| } |
There was a problem hiding this comment.
[nitpick] The API separates group creation and image upload into two endpoints, which could lead to inconsistent states. If a user creates a group but fails to upload images (or vice versa), the group exists without images. Consider either:
- Accepting images as part of the group creation request (combined multipart request with both JSON and files)
- Adding validation/business logic to handle groups without images explicitly
- Implementing a transactional pattern that ensures both operations succeed or fail together
This is especially important since the PR description emphasizes handling both "모임 이미지 저장" and "모임 생성 내용 저장" together.
| private void validateCreateGroupImageRequest(List<MultipartFile> images) { | ||
| // // TODO: 이미지가 필수는 아니라고 가정한다면? (필수면 여기서 예외 던지면 됨) | ||
| // if (images == null || images.isEmpty()) { | ||
| // return; | ||
| // } | ||
|
|
||
| if (images.size() > 3) { | ||
| throw new GroupException(GroupErrorCode.IMAGE_UPLOAD_EXCEED, images.size()); | ||
| } |
There was a problem hiding this comment.
The validation method validateCreateGroupImageRequest doesn't handle the null case properly. If images is null (which is allowed by the controller's required = false), calling images.size() at line 41 will throw a NullPointerException. Add a null check before the size validation:
if (images == null || images.isEmpty()) {
return;
}
if (images.size() > 3) {
throw new GroupException(GroupErrorCode.IMAGE_UPLOAD_EXCEED, images.size());
}| .createdBy(createdByHost) | ||
| .createdAt(group.getCreatedAt()) | ||
| .updatedAt(group.getUpdatedAt()) | ||
| .joinedCount(1) // TODO: 다시 한 번 체크 |
There was a problem hiding this comment.
The TODO comment references "다시 한 번 체크" (check again) but doesn't explain what needs to be checked. The joinedCount is hardcoded to 1, which might be intentional since the host joins automatically. Either remove the TODO if this is the correct behavior, or clarify what specific aspect needs to be verified (e.g., "TODO: Verify joinedCount matches the actual ATTEND status users count").
| .joinedCount(1) // TODO: 다시 한 번 체크 | |
| .joinedCount((int) attendUserCount) // joinedCount reflects the number of users with ATTEND status |
GroupUser.status가 null일 가능성이 생기면 .equals(GroupUserStatus.ATTEND) 호출 시 NPE가 발생합니다. 방어적으로 상수를 기준으로 비교하는 패턴을 쓰는 것이 안전합니다.
컨트롤러에서 @RequestPart(value = "images", required = false)로 받고 있어, 이미지 없이 호출되면 images가 null일 수 있습니다. 현재 validateCreateGroupImageRequest는 바로 images.size()를 호출하므로 null일 때 NPE가 발생합니다. 아래처럼 null/empty를 먼저 허용 처리한 후 개수 제한을 검사하는 편이 안전합니다.
바운더리를 올바르게 표기해서 예외 상황이 생기지 않도록 재수정했습니다.
github-project-automation
bot
moved this from In progress
to Done
in WeGo-Together Backend
1 participant
📝 Pull Request
📌 PR 종류
해당하는 항목에 체크해주세요.
✨ 변경 내용
모임 이미지 저장( multipart/form-data)과 모임 생성 내용(application/json) 저장을 수행합니다.
DB에만 440x240(대표)만 넣고, 100x100 같은 나머지 사이즈는 안 넣었습니다.
DB는 “도메인 정보” 중심, S3는 “파일 상세 정보” 중심으로 GroupImage 엔티티에서 진짜로 비즈니스적으로 필요한 정보는 뭐냐 하면,
정도로 나타낼 수 있습니다.
반대로, 440x240, 100x100 같은 구체적인 렌더링 사이즈는 “도메인 개념”이라기보다, 프론트/뷰 레이어를 위해 준비한 표현용 데이터에 가깝습니다. 그래서,
즉, DB는 비즈니스의 진짜 상태, S3/파일 시스템은 표현을 위한 파생 상태를 담당하게 됩니다.
URL 여러 개를 DB에 넣으면 생기는 단점들
만약 이미지 한 장당 440x240 URL, 100x100 URL 두 개를 다 DB에 넣는다고 치면, 갑자기 할 일이 늘어나게 됩니다.
예를 들어 이런 선택지들이 생기게 됩니다.
아니면 GroupImageVariant 같은 서브 엔티티를 만들어서
을 따로 관리해야 합니다.둘 다 결국 도메인 모델 복잡도와 쿼리 복잡도가 증가합니다. 특히 나중에 “사이즈 하나 더 추가하자 (예: 1024x576)” 이러면? 곤란해집니다.
컬럼 하나 더 추가하거나, enum 하나 더 추가하고 코드, 쿼리, DTO 전부 수정해야 합니다. 즉, 뷰 레이어 요구사항 때문에 도메인 모델이 오염되는 느낌이 됩니다.. 지금처럼 S3 키에 _440x240, _100x100를 붙이는 방식이면, 사이즈를 추가해도 DB 스키마는 안 건드려도 됩니다.
“대표 URL 하나만 저장”의 장점
실제로 서비스에서 가장 많이 조회되는 건 하나의 대표 이미지입니다.
정도로만 쓰일 가능성이 크다고 판단됩니다... 그래서 DB에서는,
아예 프론트에서 440x240 → 100x100 규칙을 써서 조합해 쓸 수 있게 설계하는 게 꽤 효율적이라고 판단했습니다..
만약 대표 이미지를 다른 파일로 바꾸고 싶을 때를 생각했습니다.
S3에 새 440x240, 100x100 두 개를 올리고, DB에 저장된 URL들을 전부 교체해야 한다면? 컬럼이 많을수록, 엔티티가 많을수록 변경 포인트도 늘어나게 됩니다. 반대로 지금처럼, DB에는 대표(주요 용도) 1개만 저장하고 나머지 사이즈는 파일명 규칙으로 파생이면 바꿔야 하는 건 딱 하나의 URL이라서 관리가 훨씬 단순해집니다.
DB에는 비즈니스적으로 꼭 필요한 대표 이미지(440x240)만 저장하고, 나머지 사이즈는 S3 파일명 규칙 + 응답 DTO로 다루면 도메인 모델은 단순해지고, 스키마 변경 없이 사이즈 확장/변경이 쉬워집니다.
🔍 관련 이슈
🧪 테스트
변경된 기능에 대한 테스트 범위 또는 테스트 결과를 작성해주세요.
🚨 확인해야 할 사항 (Checklist)
PR을 제출하기 전에 아래 항목들을 확인해주세요.
🙋 기타 참고 사항
리뷰어가 참고하면 좋을 만한 추가 설명이 있다면 적어주세요.
Summary by CodeRabbit
New Features
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.