[fix] 하나의 웨이블존에 여러개의 저장한 리스트를 추가할 수 있도록 로직 변경 #173
Conversation
Walkthrough컨트롤러의 삭제 경로를 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as 클라이언트
participant Ctrl as UserPlaceController
participant Svc as UserPlaceService
participant UR as UserRepository
participant PR as UserPlaceRepository
participant ZR as WaybleZoneRepository
participant MR as UserPlaceWaybleZoneMappingRepository
C->>Ctrl: POST /api/v1/users/{id}/places/zones { placeIds[], waybleZoneId }
Ctrl->>Svc: addZoneToPlaces(userId, placeIds, waybleZoneId)
Svc->>UR: findById(userId)
UR-->>Svc: User
Svc->>ZR: findById(waybleZoneId)
ZR-->>Svc: WaybleZone
Svc->>Svc: dedupe(placeIds)
loop 각 placeId 처리
Svc->>PR: findByIdAndUserId(placeId, userId)
PR-->>Svc: UserPlace
Svc->>MR: existsByUserPlace_IdAndWaybleZone_Id(placeId, waybleZoneId)?
MR-->>Svc: true/false
alt 미존재
Svc->>MR: save(mapping)
Svc->>PR: increment place count & save(place)
Svc->>Svc: added++
end
end
alt added > 0
Svc->>ZR: increment likes & save(zone)
end
Svc-->>Ctrl: int added
Ctrl-->>C: 200 OK (응답 메시지에 추가된 개수 포함)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/main/java/com/wayble/server/user/dto/UserPlaceAddZonesRequestDto.java (1)
8-9: ID 유효성 검증 보강 제안: @positive 추가비즈니스 상 ID는 양수여야 하므로 양수 제약을 추가하면 조기 검증과 오류 메시지 일관성에 도움이 됩니다.
아래와 같이 변경을 제안합니다.
@@ -import jakarta.validation.constraints.NotEmpty; +import jakarta.validation.constraints.NotEmpty; import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Positive; @@ public record UserPlaceAddZonesRequestDto( - @NotEmpty List<@NotNull Long> placeIds, - @NotNull Long waybleZoneId + @NotEmpty List<@NotNull @Positive Long> placeIds, + @NotNull @Positive Long waybleZoneId ) {}src/main/java/com/wayble/server/user/service/UserPlaceService.java (4)
83-85: JPA 더티 체킹 활용: 루프 내 save(place) 제거 제안@transactional 범위 내에서 영속 엔티티의 필드 변경은 커밋 시 플러시되므로, 매 반복마다 save 호출은 불필요한 UPDATE를 유발할 수 있습니다.
place.increaseCount(); - userPlaceRepository.save(place);
86-94: likes 누적 갱신을 루프 밖에서 한 번만 수행반복마다 zone.addLikes(1) 호출 대신, 최종 added 누계를 한 번에 더하면 가독성과 의도가 명확해집니다. DB save도 한 번이면 충분합니다.
- zone.addLikes(1); // 리스트 하나에 추가될 때마다 +1 added++; } - if (added > 0) { - waybleZoneRepository.save(zone); - } + if (added > 0) { + zone.addLikes(added); // 리스트에 추가된 개수만큼 한 번에 증가 + waybleZoneRepository.save(zone); + } return added;
75-83: 동시성에 의한 중복 insert 가능성: 유니크 제약 및 예외 흡수로 방지 권장exists 체크와 save 사이 레이스가 발생하면 중복 매핑 시도가 생길 수 있습니다. DB 유니크 인덱스로 근본 차단하고, DataIntegrityViolationException을 잡아 멱등하게 처리하는 방식을 권장합니다.
- DB/엔티티에 유니크 제약 추가 권장
예: user_place_id, wayble_zone_id 복합 유니크예시(엔티티에 반영):
// src/main/java/com/wayble/server/user/entity/UserPlaceWaybleZoneMapping.java // @Table 어노테이션에 uniqueConstraints 추가 @Table( name = "user_place_wayble_zone_mapping", uniqueConstraints = { @UniqueConstraint( name = "uq_user_place_wayble_zone", columnNames = {"user_place_id", "wayble_zone_id"} ) } )
- 서비스 단에서 동시성에 의한 제약 위반을 흡수(선택 사항)
- mappingRepository.save(UserPlaceWaybleZoneMapping.builder() - .userPlace(place) - .waybleZone(zone) - .build()); - - place.increaseCount(); - userPlaceRepository.save(place); + try { + mappingRepository.save(UserPlaceWaybleZoneMapping.builder() + .userPlace(place) + .waybleZone(zone) + .build()); + place.increaseCount(); + // save(place)는 Dirty Checking에 맡기는 것을 권장 + } catch (org.springframework.dao.DataIntegrityViolationException ignore) { + // 동시성 중복 삽입 시 멱등하게 무시 + continue; + }확인 요청:
- 현재 DB 스키마(DDL)에 복합 유니크 인덱스가 있는지 확인 부탁드립니다. 없다면 위 제약 추가를 권장합니다.
71-85: 소규모 최적화(선택): place 로딩을 배치 조회로 전환placeIds가 많을 경우 반복적 단건 조회(findByIdAndUser_Id)가 누적됩니다. Repository에 findAllByIdInAndUser_Id(...)를 추가해 한번에 로딩 후 Map으로 조회하면 쿼리 수를 줄일 수 있습니다. 규모가 작으면 현 구현도 충분합니다.
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (2)
35-37: Swagger 문서의 중복 제목 응답코드 400 → 409로 정합성 맞추기UserErrorCase.PLACE_TITLE_DUPLICATED가 409로 변경되었으므로 API 문서도 409로 업데이트하는 것이 좋습니다.
@ApiResponses({ @ApiResponse(responseCode = "200", description = "리스트 생성 성공"), - @ApiResponse(responseCode = "400", description = "동일한 리스트명이 이미 존재") + @ApiResponse(responseCode = "409", description = "동일한 리스트명이 이미 존재") })
90-103: 핸들러 메서드명 의미 구체화 제안: addZonesToPlace → addZoneToPlaces현재는 “하나의 웨이블존을 여러 리스트에 추가”가 핵심이므로 서비스 명세와 동일하게 메서드명을 단수 Zone, 복수 Places로 맞추면 가독성이 좋아집니다. 라우팅은 동일합니다.
- public CommonResponse<String> addZonesToPlace( + public CommonResponse<String> addZoneToPlaces( @Parameter(hidden = true) @CurrentUser Long userId, @RequestBody @Valid UserPlaceAddZonesRequestDto request ) { int added = userPlaceService.addZoneToPlaces(userId, request.placeIds(), request.waybleZoneId()); return CommonResponse.success(String.format("%d개 리스트에 추가되었습니다.", added)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java(3 hunks)src/main/java/com/wayble/server/user/dto/UserPlaceAddZonesRequestDto.java(1 hunks)src/main/java/com/wayble/server/user/dto/UserPlaceRequestDto.java(0 hunks)src/main/java/com/wayble/server/user/dto/UserResponseDto.java(0 hunks)src/main/java/com/wayble/server/user/exception/UserErrorCase.java(1 hunks)src/main/java/com/wayble/server/user/service/UserPlaceService.java(2 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/com/wayble/server/user/dto/UserResponseDto.java
- src/main/java/com/wayble/server/user/dto/UserPlaceRequestDto.java
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/main/java/com/wayble/server/user/dto/UserPlaceAddZonesRequestDto.java (4)
src/main/java/com/wayble/server/user/dto/UserPlaceRemoveRequestDto.java (1)
UserPlaceRemoveRequestDto(5-8)src/main/java/com/wayble/server/user/dto/UserPlaceRequestDto.java (1)
UserPlaceRequestDto(5-9)src/main/java/com/wayble/server/user/dto/UserPlaceZonesResponseDto.java (1)
Builder(8-15)src/main/java/com/wayble/server/user/entity/UserPlaceWaybleZoneMapping.java (1)
Entity(10-29)
src/main/java/com/wayble/server/user/exception/UserErrorCase.java (5)
src/main/java/com/wayble/server/wayblezone/exception/WaybleZoneErrorCase.java (1)
Getter(7-18)src/main/java/com/wayble/server/direction/exception/WalkingErrorCase.java (1)
Getter(7-21)src/main/java/com/wayble/server/direction/exception/DirectionErrorCase.java (1)
Getter(7-19)src/main/java/com/wayble/server/admin/exception/AdminErrorCase.java (1)
Getter(7-17)src/main/java/com/wayble/server/user/entity/UserPlace.java (1)
Entity(13-47)
src/main/java/com/wayble/server/user/service/UserPlaceService.java (2)
src/main/java/com/wayble/server/user/entity/UserPlaceWaybleZoneMapping.java (1)
Entity(10-29)src/main/java/com/wayble/server/user/repository/UserPlaceWaybleZoneMappingRepository.java (2)
deleteByUserPlace_IdAndWaybleZone_Id(20-20)UserPlaceWaybleZoneMappingRepository(14-43)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (1)
src/main/java/com/wayble/server/user/dto/UserPlaceRemoveRequestDto.java (1)
UserPlaceRemoveRequestDto(5-8)
🔇 Additional comments (5)
src/main/java/com/wayble/server/user/exception/UserErrorCase.java (1)
26-26: 409 Conflict으로 상향한 것 적절합니다리소스 중복(리스트명 중복)에 409를 매핑한 선택이 REST 관례에 부합합니다.
src/main/java/com/wayble/server/user/service/UserPlaceService.java (1)
68-74: 입력 placeIds 중복 제거 로직 적절합니다LinkedHashSet으로 중복 제거 및 입력 순서 유지하는 접근이 깔끔합니다.
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (3)
58-59: 요약 API 설명 보완 LGTM설명과 반환 모델이 일치합니다. 변경 의도에 잘 맞습니다.
72-76: DELETE 경로를 /zones로 변경: 클라이언트 영향도 점검 권장기존 엔드포인트를 사용하던 클라이언트가 있다면 브레이킹 체인지입니다. 공개 문서/앱 호출부/QA 시나리오에서 경로 변경 반영 여부를 확인해 주세요.
106-108: 웨이블존 목록 조회(페이징) 설명 보강 LGTMplaceId 기준, page 0-base 안내가 명확합니다.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (3)
97-103: 빈 리스트/중복 placeId 입력 처리 및 0건 처리 메시지 개선 제안
- placeIds가 비어있거나 null인 경우를 사전에 방지(Validation)하거나, 컨트롤러에서 distinct 처리하면 불필요한 호출과 이중 저장 시도를 줄일 수 있습니다.
- 실제 추가 건수가 0건인 경우 현재 메시지("0개 리스트에 추가되었습니다.")는 사용자 경험상 혼동을 줄 수 있습니다. 별도 메시지로 분기하는 것을 권장합니다.
다음과 같이 컨트롤러에서 중복 제거 및 Null 필터링을 선제적으로 처리하고 메시지를 분기할 수 있습니다.
- int added = userPlaceService.addZoneToPlaces(userId, request.placeIds(), request.waybleZoneId()); - return CommonResponse.success(String.format("%d개 리스트에 추가되었습니다.", added)); + List<Long> placeIds = request.placeIds() == null + ? List.of() + : request.placeIds().stream() + .filter(java.util.Objects::nonNull) + .distinct() + .toList(); + int added = userPlaceService.addZoneToPlaces(userId, placeIds, request.waybleZoneId()); + String message = (added > 0) + ? String.format("%d개 리스트에 추가되었습니다.", added) + : "이미 추가된 항목이라 변경사항이 없습니다."; + return CommonResponse.success(message);추가로 DTO 레벨에서 아래 제약을 함께 권장합니다.
원하시면 DTO와 검증 애너테이션까지 포함한 패치를 만들어 드리겠습니다.
72-76: DELETE 요청에 RequestBody 사용은 일부 클라이언트/게이트웨이에서 호환성 이슈DELETE+Body는 명세상 금지는 아니지만, 일부 인프라/라이브러리가 바디를 무시하거나 차단하는 경우가 있습니다. PathVariable로 변경하면 상호운용성이 좋아집니다.
아래와 같이 경로 파라미터로 바꾸는 것을 제안합니다.
- @DeleteMapping("/zones") + @DeleteMapping("/zones/{placeId}/{waybleZoneId}") @Operation( - summary = "내가 저장한 리스트에서 웨이블존 제거", - description = "RequestBody로 placeId, waybleZoneId를 받아 지정한 장소에서 웨이블존을 제거합니다." + summary = "내가 저장한 리스트에서 웨이블존 제거", + description = "path로 전달된 placeId, waybleZoneId를 받아 지정한 장소에서 웨이블존을 제거합니다." ) @@ - public CommonResponse<String> removeZoneFromPlace( - @Parameter(hidden = true) @CurrentUser Long userId, - @RequestBody @Valid UserPlaceRemoveRequestDto request - ) { - userPlaceService.removeZoneFromPlace(userId, request.placeId(), request.waybleZoneId()); + public CommonResponse<String> removeZoneFromPlace( + @Parameter(hidden = true) @CurrentUser Long userId, + @PathVariable Long placeId, + @PathVariable Long waybleZoneId + ) { + userPlaceService.removeZoneFromPlace(userId, placeId, waybleZoneId); return CommonResponse.success("성공적으로 제거되었습니다."); }Also applies to: 83-87
58-58: sort 파라미터 허용값을 OpenAPI에 명시해 주세요현재 Service는 "name"/"title"만 특수 처리하고 그 외는 최신순으로 동작합니다. Swagger에 허용값을 명시하면 클라이언트 연동이 쉬워집니다.
다음과 같이 sort 파라미터에 @parameter + @Schema(allowableValues) 추가를 제안합니다.
- public CommonResponse<List<UserPlaceSummaryDto>> getMyPlaceSummaries( - @Parameter(hidden = true) @CurrentUser Long userId, - @RequestParam(name = "sort", defaultValue = "latest") String sort - ) { + public CommonResponse<List<UserPlaceSummaryDto>> getMyPlaceSummaries( + @Parameter(hidden = true) @CurrentUser Long userId, + @Parameter(description = "정렬 기준", schema = @Schema(allowableValues = {"latest", "name", "title"})) + @RequestParam(name = "sort", defaultValue = "latest") String sort + ) {추가 import 필요:
import io.swagger.v3.oas.annotations.media.Schema;Also applies to: 64-67
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (1)
src/main/java/com/wayble/server/user/service/UserPlaceService.java (2)
Service(27-137)z(103-114)
🔇 Additional comments (1)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (1)
101-101: 서비스 시그니처 및 DTO 제약 확인 완료
- src/main/java/com/wayble/server/user/service/UserPlaceService.java
• public int addZoneToPlaces(Long userId, List placeIds, Long waybleZoneId) – 반환 타입(int) 및 파라미터가 PR 의도(여러 리스트 추가)와 일치합니다.- src/main/java/com/wayble/server/user/dto/UserPlaceAddZonesRequestDto.java
• @notempty List<@NotNull Long> placeIds
• @NotNull Long waybleZoneId
→ validation 애노테이션이 모두 적용되어 있습니다.이대로 진행 부탁드립니다.
src/main/java/com/wayble/server/user/controller/UserPlaceController.java
Outdated
Show resolved
Hide resolved
src/main/java/com/wayble/server/user/controller/UserPlaceController.java
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/main/java/com/wayble/server/user/controller/UserPlaceController.java (1)
89-101: 페이지 번호 제약 조건 수정 필요Controller에 “page는 1부터 시작”이라 명시했지만, 제약이
@Min(0)으로 설정되어 있어page=0허용 시 서비스 내PageRequest.of(page - 1, …)호출 시 음수 페이지 인덱스로IllegalArgumentException이 발생합니다. 다음과 같이 수정해 주세요.• 파일:
src/main/java/com/wayble/server/user/controller/UserPlaceController.java
• 위치:getZonesInPlace메서드 파라미터- @RequestParam(defaultValue = "1") @Min(0) Integer page, + @RequestParam(defaultValue = "1") @Min(1) Integer page,
🧹 Nitpick comments (1)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (1)
106-122: DELETE에서 RequestBody 사용은 일부 클라이언트/프록시와의 호환성 이슈 가능Spring MVC에서는 동작하지만, 일부 클라이언트/프록시 환경은 DELETE 본문을 제대로 전달하지 못하는 경우가 있습니다. 큰 변경이 아니면 유지해도 되나, 가능하면 경로나 쿼리 파라미터로 식별자를 노출하는 것을 권장합니다.
대안 1(경로 변수 사용, RESTful):
- DELETE /api/v1/users/places/{placeId}/zones/{waybleZoneId}
대안 2(쿼리 파라미터):
- DELETE /api/v1/users/places/zones?placeId=...&waybleZoneId=...
또한, 바디가 없는 삭제라면 204 No Content를 사용하는 것도 고려해 볼 수 있습니다(현재 공통 응답 스펙 유지가 우선이면 200 유지 OK).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java(3 hunks)src/main/java/com/wayble/server/user/service/UserPlaceService.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/wayble/server/user/service/UserPlaceService.java
🔇 Additional comments (2)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (2)
58-58: 요약/설명 업데이트 적절요청 의도가 명확히 드러나도록 문구가 정리되었습니다. 별도 이슈 없습니다.
80-86: 검증 제약 어노테이션이 이미 적용되어 있습니다.
UserPlaceAddZonesRequestDto에서
- placeIds에
@NotEmpty- waybleZoneId에
@NotNull
이 선언되어 있으므로 추가 수정이 필요 없습니다.
| @Operation(summary = "웨이블존에 저장한 리스트 추가 (여러 개 가능)", | ||
| description = "웨이블존에 사용자가 요청한 리스트들을 추가합니다.") | ||
| @ApiResponses({ | ||
| @ApiResponse(responseCode = "200", description = "웨이블존 추가 성공"), | ||
| @ApiResponse(responseCode = "200", description = "웨이블존에 리스트 추가 성공"), | ||
| @ApiResponse(responseCode = "404", description = "유저/리스트/웨이블존을 찾을 수 없음") |
There was a problem hiding this comment.
API 문서 방향이 실제 동작과 반대입니다: ‘웨이블존에 리스트 추가’ → ‘리스트에 웨이블존 추가’로 수정 필요
서비스 호출은 웨이블존을 여러 리스트에 추가하는 동작(addZoneToPlaces)인데, 현재 문구는 “웨이블존에 리스트를 추가”로 반대로 서술되어 있어 오해의 소지가 큽니다. Swagger 문구를 아래처럼 교정해 주세요.
- @Operation(summary = "웨이블존에 저장한 리스트 추가 (여러 개 가능)",
- description = "웨이블존에 사용자가 요청한 리스트들을 추가합니다.")
+ @Operation(summary = "저장한 리스트에 웨이블존 추가 (여러 개 가능)",
+ description = "사용자가 지정한 웨이블존을 요청한 저장한 리스트들(placeIds)에 추가합니다.")
@@
- @ApiResponse(responseCode = "200", description = "웨이블존에 리스트 추가 성공"),
+ @ApiResponse(responseCode = "200", description = "리스트에 웨이블존 추가 성공"),📝 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.
| @Operation(summary = "웨이블존에 저장한 리스트 추가 (여러 개 가능)", | |
| description = "웨이블존에 사용자가 요청한 리스트들을 추가합니다.") | |
| @ApiResponses({ | |
| @ApiResponse(responseCode = "200", description = "웨이블존 추가 성공"), | |
| @ApiResponse(responseCode = "200", description = "웨이블존에 리스트 추가 성공"), | |
| @ApiResponse(responseCode = "404", description = "유저/리스트/웨이블존을 찾을 수 없음") | |
| @Operation(summary = "저장한 리스트에 웨이블존 추가 (여러 개 가능)", | |
| description = "사용자가 지정한 웨이블존을 요청한 저장한 리스트들(placeIds)에 추가합니다.") | |
| @ApiResponses({ | |
| @ApiResponse(responseCode = "200", description = "리스트에 웨이블존 추가 성공"), | |
| @ApiResponse(responseCode = "404", description = "유저/리스트/웨이블존을 찾을 수 없음") |
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/user/controller/UserPlaceController.java
around lines 74 to 78, the Swagger Operation summary/description currently reads
as if lists are being added to a zone, but the service actually adds a zone to
multiple lists (addZoneToPlaces); update the Operation.summary and
Operation.description text to reflect "리스트에 웨이블존 추가" (or similar phrasing: "사용자
요청한 리스트들에 웨이블존을 추가합니다") so the documentation matches the behavior; ensure any
related messages in ApiResponses remain accurate (change the 200 description to
indicate successful addition of a zone to lists if present).
There was a problem hiding this comment.
@coderabbitai 웨이블존에 리스트를 추가든 리스트에 웨이블존을 추가든 결국 디비에 placeId, waybleZoneId 매핑 레코드가 생겨서 결론은 둘이 동일한거 아니야? 서비스 로직에서 잘못된 부분 없는거같은데
There was a problem hiding this comment.
@seung-in-Yoo 맞습니다! 데이터베이스 관점에서는 결국 동일한 placeId, waybleZoneId 매핑 레코드가 생성되므로 기능적으로는 완전히 동일하네요.
제가 API 문서의 표현에 너무 세세하게 집착했던 것 같습니다. 현재 구현이 요구사항을 올바르게 충족하고 있고, 사용자 입장에서 혼란이 없다면 현재 상태로도 충분히 좋습니다.
서비스 로직도 정상적으로 구현되어 있어서 문제없어 보입니다! 🙂
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (4)
58-58: sort 파라미터 허용값을 문서에 명시해 주세요서비스는 sort 값으로 "name" 또는 "title"일 때만 제목 오름차순, 그 외에는 최신순을 적용합니다. Swagger 문서에 허용값과 기본값을 드러내면 혼선을 줄일 수 있습니다.
적용 예시(diff):
- @Operation(summary = "내가 저장한 리스트 요약 조회", description = "장소 관련 목록(리스트)만 반환합니다.") + @Operation(summary = "내가 저장한 리스트 요약 조회", description = "장소 관련 목록(리스트)만 반환합니다. sort: latest(기본) | name | title")
74-78: API 문서에 중복/부분성공 정책을 명시해 주세요여러 리스트에 추가 시 다음 정책이 문서화되면 클라이언트 구현이 쉬워집니다.
- placeIds 내 중복 ID는 허용/무시 여부
- 이미 매핑된 경우의 처리(건너뜀 vs 에러)
- 일부 실패 시 트랜잭션 롤백(전부/일부) 여부 및 응답의 의미(added는 “실제 신규 추가 수”인지)
현재 서비스 반환값이 int(added)라면 “신규 추가된 개수” 기준으로 보입니다. 문서에 해당 기준을 명확히 남겨 주세요.
106-122: DELETE 메서드에서 RequestBody 대신 경로/쿼리 파라미터 사용 권장Spring에서 DELETE+Body는 동작하지만, 일부 클라이언트/프록시가 본문을 제거/무시하는 사례가 있어 상호운용성 이슈가 납니다. REST 리소스 구조에도 경로 변수 방식이 더 자연스럽습니다.
적용 예시(diff):
- @DeleteMapping("/zones") - @Operation( - summary = "내가 저장한 리스트에서 웨이블존 제거", - description = "RequestBody로 placeId, waybleZoneId를 받아 지정한 장소에서 웨이블존을 제거합니다." - ) + @DeleteMapping("/{placeId}/zones/{waybleZoneId}") + @Operation( + summary = "내가 저장한 리스트에서 웨이블존 제거", + description = "경로 파라미터로 placeId, waybleZoneId를 받아 지정한 장소에서 웨이블존을 제거합니다." + ) @@ - public CommonResponse<String> removeZoneFromPlace( - @Parameter(hidden = true) @CurrentUser Long userId, - @RequestBody @Valid UserPlaceRemoveRequestDto request - ) { - userPlaceService.removeZoneFromPlace(userId, request.placeId(), request.waybleZoneId()); + public CommonResponse<String> removeZoneFromPlace( + @Parameter(hidden = true) @CurrentUser Long userId, + @PathVariable Long placeId, + @PathVariable Long waybleZoneId + ) { + userPlaceService.removeZoneFromPlace(userId, placeId, waybleZoneId); return CommonResponse.success("성공적으로 제거되었습니다."); }참고: 응답은 200 OK + 메시지로도 무방하지만, 완전한 REST 일관성을 원하시면 204 No Content로 단순화하는 것도 고려할 수 있습니다.
80-86: Optional: 컨트롤러 단에서 중복·null placeId 사전 제거로 불필요한 DB 호출 최소화서비스
addZoneToPlaces메서드에서는 이미 LinkedHashSet을 통한 중복 제거와
existsBy…체크로 기존 매핑을 건너뛰는 idempotent 동작을 보장합니다.
그렇지만 컨트롤러 단계에서 요청 DTO의placeIds를 가볍게 필터링하면
서비스 호출 전후로 발생하는 네트워크·메모리 오버헤드를 조금 더 줄일 수 있습니다.• 적용 위치
– 파일:src/main/java/com/wayble/server/user/controller/UserPlaceController.java
– 메서드:addZoneToPlaces(라인 80–86)• 선택적 리팩토링 예시(diff):
- int added = userPlaceService.addZoneToPlaces(userId, request.placeIds(), request.waybleZoneId()); + List<Long> placeIds = request.placeIds().stream() + .filter(Objects::nonNull) + .distinct() + .toList(); + int added = userPlaceService.addZoneToPlaces(userId, placeIds, request.waybleZoneId()); return CommonResponse.success(String.format("%d개 리스트에 추가되었습니다.", added));추가 import:
import java.util.Objects;서비스 레이어 구현:
Set<Long> uniquePlaceIds = new LinkedHashSet<>(placeIds); … boolean exists = mappingRepository.existsByUserPlace_IdAndWaybleZone_Id(placeId, waybleZoneId); if (exists) continue; …위와 같이 이미 중복·기존 매핑을 건너뛰고 있으므로, 컨트롤러 필터링은 선택적입니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (3)
src/main/java/com/wayble/server/user/service/UserPlaceService.java (3)
Service(27-137)z(103-114)Transactional(118-136)src/main/java/com/wayble/server/user/dto/UserPlaceRemoveRequestDto.java (1)
UserPlaceRemoveRequestDto(5-8)src/main/java/com/wayble/server/user/repository/UserPlaceWaybleZoneMappingRepository.java (1)
UserPlaceWaybleZoneMappingRepository(14-43)
🔇 Additional comments (1)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (1)
89-101: 1-based 페이징으로의 정합성 조정, 잘 반영되었습니다컨트롤러에서 page 기본값 1, @min(1)과 함께 문서에도 1부터 시작을 명시했고, 서비스에서 page-1로 0-based 변환하는 구현과 일치합니다. 프론트/백 간 오프바이원 이슈가 해소될 것입니다.
✔️ 연관 이슈
📝 작업 내용
스크린샷 (선택)
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Documentation