Repository navigation
feat: 매거진, 투데이 2차 api 구현 - #43
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change adds tourism magazine and bookmark APIs, tourism API integration, and authenticated course-place addition and reordering. It also replaces legacy course DTOs and updates persistence, security, configuration, and tests. ChangesCourse APIs
Place APIs
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant PlaceController
participant MagazineService
participant TourApiClient
participant PlaceBookmarkService
participant BookmarkPlaceRepository
Client->>PlaceController: request magazine or bookmark operation
PlaceController->>MagazineService: retrieve magazine data
MagazineService->>TourApiClient: query tourism API
TourApiClient-->>MagazineService: return typed tourism data
PlaceController->>PlaceBookmarkService: create or delete bookmark
PlaceBookmarkService->>BookmarkPlaceRepository: validate ownership or persist bookmark
BookmarkPlaceRepository-->>PlaceBookmarkService: return bookmark state
PlaceBookmarkService-->>Client: return operation response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
5969e32 to
3fa3bd4
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
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/daytodo/domain/region/entity/Region.java (1)
29-40: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPopulate KorService2 region codes before relying on them.
RegionexposesareaCodeandsigunguCode, but the constructor leaves them unset and no code populates existing rows. In production withddl-auto: update,MagazineService.resolveInterestRegionfilters out interest regions because their regions lack non-null codes, so magazine listings use the nationwide query.PlaceBookmarkService.resolveRegionalso fails to match bookmarked tourism places againstRegionRepository.findFirstByAreaCodeAndSigunguCode. Add a migration or seed step for existing regions and set these fields on newRegioncreation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/daytodo/domain/region/entity/Region.java` around lines 29 - 40, Populate areaCode and sigunguCode for existing Region records through a migration or seed step, and update the Region constructor to assign these values when creating new regions. Ensure the values match KorService2 mappings so MagazineService.resolveInterestRegion and PlaceBookmarkService.resolveRegion can use non-null codes.
🧹 Nitpick comments (4)
src/main/java/com/daytodo/domain/place/service/PlaceBookmarkService.java (1)
141-150: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winLog when coordinate parsing falls back to (0.0, 0.0).
parseCoordinatereturns0.0whenmapx/mapyis null, blank, or unparsable. Latitude and longitude0.0point to a real location, not to a "missing data" marker. A place created with a missing or malformed coordinate from the tourism API silently gets a valid-looking but wrong position. Add a log statement when parsing fails, so bad upstream data is visible in operational logs instead of only appearing as a misplaced marker later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/daytodo/domain/place/service/PlaceBookmarkService.java` around lines 141 - 150, Update PlaceBookmarkService.parseCoordinate to log when the input is null, blank, or causes NumberFormatException before returning the 0.0 fallback. Include the supplied value and enough context to identify coordinate parsing failures, while preserving the existing successful parsing and fallback behavior.src/main/java/com/daytodo/global/config/RestClientConfig.java (1)
13-13: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMark the
RestClientbeans with@Qualifierto make injection explicit.Two
RestClientbeans now exist.TourApiClientinjectsRestClient tourRestClientthrough a Lombok-generated constructor, so resolution depends on the parameter name matching the bean name. That match requires compilation with-parameters. Add@Qualifier("tourRestClient")at the injection point, or annotate the beans, so injection does not depend on parameter-name retention.Also applies to: 23-23
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/daytodo/global/config/RestClientConfig.java` at line 13, Add explicit `@Qualifier` annotations for both RestClient beans in RestClientConfig, using their bean names such as “naverRestClient” and “tourRestClient”, and ensure TourApiClient’s RestClient injection point explicitly uses `@Qualifier`("tourRestClient"). Do not rely on constructor parameter names or -parameters for bean resolution.src/main/java/com/daytodo/domain/place/service/MagazineService.java (1)
1-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit tests for
MagazineService's branch logic.Cover
resolveInterestRegionwith multiple interest regions where only a non-first one has an area code, and coverfetchOverviewwith a simulated external failure other thanProjectException. These paths hide the two issues raised above.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/daytodo/domain/place/service/MagazineService.java` around lines 1 - 140, Add unit tests for MagazineService covering resolveInterestRegion with multiple interest regions where only a later region has an area code, and fetchOverview when tourApiClient.detailCommon throws a non-ProjectException failure. Assert the expected region selection and overview fallback behavior, using mocks for the repositories and TourApiClient.src/test/java/com/daytodo/domain/course/controller/TodayCourseControllerTest.java (1)
36-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd test coverage for the new place-management endpoints.
This file has tests for
getTodayCourseandsaveMemoryPhotos, but no tests for the newPOST /courses/{courseId}/today-places(addPlaceToCourse) orPATCH /courses/{courseId}/today-places/order(reorderCoursePlaces) endpoints added in this cohort. Add WebMvc tests covering at least the success path and theMISSING_PLACE_ID/INVALID_PLACE_ORDER/COURSE_PLACE_NOT_FOUNDerror mappings, mirroring the existingemptyImageUrlstest pattern.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/daytodo/domain/course/controller/TodayCourseControllerTest.java` around lines 36 - 62, Extend TodayCourseControllerTest with WebMvc tests for addPlaceToCourse and reorderCoursePlaces, covering successful responses and exception mappings for MISSING_PLACE_ID, INVALID_PLACE_ORDER, and COURSE_PLACE_NOT_FOUND. Mirror the existing emptyImageUrls setup with mocked service calls, endpoint requests, and assertions for HTTP status, error code, and message; use the controller’s request payloads and response contracts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/com/daytodo/domain/course/service/CourseService.java`:
- Around line 529-544: Prevent concurrent place_order updates by locking the
course row before any read-modify-write operations: in
src/main/java/com/daytodo/domain/course/service/CourseService.java lines
529-544, update appendPlaceToCourse to call
courseRepository.findByIdForUpdate(course.getCourseId()) before the duplicate
check and maximum-order lookup; in
src/main/java/com/daytodo/domain/course/service/TodayCourseService.java lines
158-199, call courseRepository.findByIdForUpdate(courseId) before loading and
reassigning placeOrder.
In `@src/main/java/com/daytodo/domain/place/dto/request/PlaceReqDTO.java`:
- Around line 16-20: The CreateBookmark request field currently mislabels the
tourism content ID as placeId. Rename CreateBookmark.placeId to contentId,
update its validation/accessors and all request-handling references to use
contentId, while preserving the response DTO’s internal Place identifier.
In `@src/main/java/com/daytodo/domain/place/dto/response/PlaceResDTO.java`:
- Around line 24-29: Rename the response record field CreateBookmark.placeId to
distinguish the internal Place primary key from the request’s KorService2
contentId, and update its builder/accessor usages consistently. Preserve the
value sourced from place.getPlaceId() while aligning the response naming with
the request/response domain terminology.
In `@src/main/java/com/daytodo/domain/place/infra/TourApiClient.java`:
- Around line 81-91: Update TourApiClient.call to reject both a null response
body and a non-success KorService2 resultCode before returning the response. Use
the existing TourApiResponse/Header symbols to validate the result code, and
throw ProjectException with PlaceErrorCode.TOUR_API_ERROR for either failure
while preserving the existing RestClientException handling and logging.
- Around line 98-107: Update the URI construction in buildUri so the
request-supplied contentId is encoded or validated as an allowed content ID
before it reaches the builder, while preserving the existing query parameters
and call flow. Avoid treating unencoded client values as pre-encoded when
invoking builder.build(true), or reject invalid contentId values before URI
creation so reserved characters cannot alter the query or trigger an uncaught
IllegalArgumentException.
In `@src/main/java/com/daytodo/domain/place/service/MagazineService.java`:
- Around line 33-63: Update getMagazineList so the per-item fetchOverview calls
no longer run sequentially inside the stream; dispatch them concurrently using
CompletableFuture with a bounded executor, gather each result before
constructing MagazineItem values, and preserve item ordering and ad sorting.
Keep fetchOverview’s per-item failure isolation so one failed detailCommon call
does not fail the entire list response.
- Around line 89-101: Update resolveInterestRegion to preserve interest-region
priority: index the findAllByRegionIdIn(regionIds) results by region ID, then
iterate the already ordered regionIds and return the first mapped Region with a
non-null areaCode. Keep returning null when no ordered region has a valid area
code.
In `@src/main/java/com/daytodo/domain/place/service/PlaceBookmarkService.java`:
- Around line 58-76: Update createBookmark to handle
DataIntegrityViolationException from both the lazy Place upsert and
BookmarkPlace insertion: after a place insert conflict, retrieve the existing
Place by tour content ID, and after a bookmark insert conflict, raise the
existing DUPLICATE_BOOKMARK business error. Preserve the normal lookup,
duplicate pre-check, and successful response paths while ensuring concurrent
requests receive controlled outcomes.
In `@src/main/java/com/daytodo/domain/region/repository/RegionRepository.java`:
- Around line 15-16: Update PlaceBookmarkService.resolveRegion before calling
RegionRepository.findFirstByAreaCodeAndSigunguCode so a null sigunguCode returns
Optional.empty() instead of executing the derived query; retain the existing
lookup for non-null values. Also ensure the repository lookup has deterministic
ordering when multiple regions share the codes, using the established ordering
field or query mechanism.
In `@src/main/java/com/daytodo/global/config/RestClientConfig.java`:
- Around line 20-27: The tourRestClient bean currently relies on default request
timeouts. Update tourRestClient in RestClientConfig to build and configure a
RestClient request factory using the Spring Boot 4 timeout settings, applying
both connect and read timeouts while preserving the existing baseUrl
configuration used by TourApiClient, MagazineService, and PlaceBookmarkService.
In `@src/main/resources/application.yml`:
- Around line 5-8: Remove the global
jackson.deserialization.accept-empty-string-as-null-object setting from
application.yml. Configure a dedicated JsonMapper and
JacksonJsonHttpMessageConverter for the KorService2 REST client or flow,
enabling empty-string-as-null-object only there while leaving the
application-wide Jackson mapper unchanged.
---
Outside diff comments:
In `@src/main/java/com/daytodo/domain/region/entity/Region.java`:
- Around line 29-40: Populate areaCode and sigunguCode for existing Region
records through a migration or seed step, and update the Region constructor to
assign these values when creating new regions. Ensure the values match
KorService2 mappings so MagazineService.resolveInterestRegion and
PlaceBookmarkService.resolveRegion can use non-null codes.
---
Nitpick comments:
In `@src/main/java/com/daytodo/domain/place/service/MagazineService.java`:
- Around line 1-140: Add unit tests for MagazineService covering
resolveInterestRegion with multiple interest regions where only a later region
has an area code, and fetchOverview when tourApiClient.detailCommon throws a
non-ProjectException failure. Assert the expected region selection and overview
fallback behavior, using mocks for the repositories and TourApiClient.
In `@src/main/java/com/daytodo/domain/place/service/PlaceBookmarkService.java`:
- Around line 141-150: Update PlaceBookmarkService.parseCoordinate to log when
the input is null, blank, or causes NumberFormatException before returning the
0.0 fallback. Include the supplied value and enough context to identify
coordinate parsing failures, while preserving the existing successful parsing
and fallback behavior.
In `@src/main/java/com/daytodo/global/config/RestClientConfig.java`:
- Line 13: Add explicit `@Qualifier` annotations for both RestClient beans in
RestClientConfig, using their bean names such as “naverRestClient” and
“tourRestClient”, and ensure TourApiClient’s RestClient injection point
explicitly uses `@Qualifier`("tourRestClient"). Do not rely on constructor
parameter names or -parameters for bean resolution.
In
`@src/test/java/com/daytodo/domain/course/controller/TodayCourseControllerTest.java`:
- Around line 36-62: Extend TodayCourseControllerTest with WebMvc tests for
addPlaceToCourse and reorderCoursePlaces, covering successful responses and
exception mappings for MISSING_PLACE_ID, INVALID_PLACE_ORDER, and
COURSE_PLACE_NOT_FOUND. Mirror the existing emptyImageUrls setup with mocked
service calls, endpoint requests, and assertions for HTTP status, error code,
and message; use the controller’s request payloads and response contracts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9567706e-48ec-41b2-a6ee-47fccc6958ff
📒 Files selected for processing (36)
.env.examplesrc/main/java/com/daytodo/domain/course/controller/TodayCourseController.javasrc/main/java/com/daytodo/domain/course/converter/MemoryPhotoConverter.javasrc/main/java/com/daytodo/domain/course/converter/TodayCourseConverter.javasrc/main/java/com/daytodo/domain/course/dto/request/CourseReqDTO.javasrc/main/java/com/daytodo/domain/course/dto/request/TodayCourseRequest.javasrc/main/java/com/daytodo/domain/course/dto/response/TodayCourseResponse.javasrc/main/java/com/daytodo/domain/course/entity/CoursePlace.javasrc/main/java/com/daytodo/domain/course/exception/code/CourseErrorCode.javasrc/main/java/com/daytodo/domain/course/service/CourseService.javasrc/main/java/com/daytodo/domain/course/service/TodayCourseService.javasrc/main/java/com/daytodo/domain/place/controller/PlaceController.javasrc/main/java/com/daytodo/domain/place/converter/MagazineConverter.javasrc/main/java/com/daytodo/domain/place/dto/request/PlaceReqDTO.javasrc/main/java/com/daytodo/domain/place/dto/response/MagazineResDTO.javasrc/main/java/com/daytodo/domain/place/dto/response/PlaceResDTO.javasrc/main/java/com/daytodo/domain/place/entity/Magazine.javasrc/main/java/com/daytodo/domain/place/entity/Place.javasrc/main/java/com/daytodo/domain/place/enums/MagazineStatus.javasrc/main/java/com/daytodo/domain/place/exception/code/PlaceErrorCode.javasrc/main/java/com/daytodo/domain/place/infra/TourApiClient.javasrc/main/java/com/daytodo/domain/place/infra/TourApiResponse.javasrc/main/java/com/daytodo/domain/place/repository/BookmarkPlaceRepository.javasrc/main/java/com/daytodo/domain/place/repository/MagazineRepository.javasrc/main/java/com/daytodo/domain/place/repository/PlaceRepository.javasrc/main/java/com/daytodo/domain/place/service/MagazineService.javasrc/main/java/com/daytodo/domain/place/service/PlaceBookmarkService.javasrc/main/java/com/daytodo/domain/region/entity/Region.javasrc/main/java/com/daytodo/domain/region/repository/RegionRepository.javasrc/main/java/com/daytodo/domain/user/repository/WithdrawnUserCleanupRepository.javasrc/main/java/com/daytodo/global/config/RestClientConfig.javasrc/main/java/com/daytodo/global/config/SecurityConfig.javasrc/main/java/com/daytodo/global/config/TourApiProperties.javasrc/main/resources/application.ymlsrc/test/java/com/daytodo/domain/course/controller/TodayCourseControllerTest.javasrc/test/java/com/daytodo/domain/course/service/CourseServiceTest.java
💤 Files with no reviewable changes (1)
- src/main/java/com/daytodo/domain/course/dto/request/CourseReqDTO.java
kwonwnsduf
left a comment
kwonwnsduf
left a comment
There was a problem hiding this comment.
appendPlaceToCourse()는 현재 placeOrder의 최대값을 조회한 뒤 +1을 해서 저장하고 있습니다. 동시에 여러 요청이 들어오면 동일한 placeOrder가 할당될 수 있습니다.
saveMemoryPhotos()에서 사용한 것처럼 Course를 FOR UPDATE로 조회한 뒤 placeOrder를 계산하도록 변경하는 것이 안전해 보입니다.
2.
현재는 존재 여부를 확인한 뒤 insert하는(check-then-insert) 구조입니다.
동시에 동일한 요청이 들어오면 Place 생성이나 Bookmark 생성 시 Unique Constraint 예외가 발생할 수 있으므로 DataIntegrityViolationException을 처리해 Place는 재조회하고, Bookmark는 DUPLICATE_BOOKMARK로 변환하는 처리가 필요해 보입니다.
3.
CreateBookmark의 placeId는 실제로는 내부 Place PK가 아니라 KorService2의 contentId를 의미합니다.
placeId라는 이름은 혼동의 여지가 있으므로 contentId로 변경하는 것이 API 의미를 더 명확하게 전달할 수 있을 것 같습니다.
4.
요청에서는 관광 콘텐츠의 contentId를 받고, 응답에서는 내부 Place PK를 반환하고 있는데 둘 다 placeId라는 이름을 사용하고 있습니다.
요청과 응답의 의미가 다르므로 필드명을 구분하는 것이 API 사용 시 혼동을 줄일 수 있을 것 같습니다.
5.
body()는 응답 본문이 없으면 null을 반환할 수 있는데 현재는 이에 대한 처리가 없습니다.
또한 KorService2는 HTTP 200이어도 resultCode가 0000이 아닌 경우가 있으므로 응답 body의 null 여부와 resultCode를 함께 검증하는 것이 필요해 보입니다.
6.
build(true)는 모든 쿼리 파라미터가 이미 인코딩되어 있다고 가정합니다.
contentId는 외부 입력값이므로 적절한 인코딩 또는 값 검증을 수행한 뒤 URI를 생성하는 것이 안전해 보입니다.
7.
현재 RestClient는 timeout 설정 없이 기본값을 사용하고 있습니다.
외부 API 응답이 지연될 경우 요청 스레드가 장시간 점유될 수 있으므로 connect timeout과 read timeout을 명시적으로 설정하는 것을 고려해 주시면 감사하겠습니다!
한 번 참고해주세요!
There was a problem hiding this comment.
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/daytodo/domain/course/exception/code/CourseErrorCode.java (1)
40-47: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winFix the enum declaration syntax before merging.
The last enum constant before
COURSE_RECOMMENDATION_FAILEDstill ends with;. Replace it with,unless it is explicitly the final constant, then run the project compile check.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/daytodo/domain/course/exception/code/CourseErrorCode.java` around lines 40 - 47, Fix the enum declaration in CourseErrorCode by replacing the semicolon after COURSE_PLACE_NOT_FOUND with a comma so COURSE_RECOMMENDATION_FAILED remains part of the enum constants, then run the project compile check.Source: Linters/SAST tools
🧹 Nitpick comments (1)
src/test/java/com/daytodo/domain/course/service/CourseServiceTest.java (1)
28-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsolidate the duplicate repository imports.
CourseServiceTest.javaimportsPlaceRecommendationRepository,RecommendationLikeRepository,PlaceRepository, andRecommendationCommentRepositorytwice. Remove one set of these declarations to avoid redundant imports.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/daytodo/domain/course/service/CourseServiceTest.java` around lines 28 - 31, Remove the duplicate imports for PlaceRecommendationRepository, RecommendationLikeRepository, PlaceRepository, and RecommendationCommentRepository in CourseServiceTest, retaining exactly one declaration of each.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/main/java/com/daytodo/domain/course/exception/code/CourseErrorCode.java`:
- Around line 40-47: Fix the enum declaration in CourseErrorCode by replacing
the semicolon after COURSE_PLACE_NOT_FOUND with a comma so
COURSE_RECOMMENDATION_FAILED remains part of the enum constants, then run the
project compile check.
---
Nitpick comments:
In `@src/test/java/com/daytodo/domain/course/service/CourseServiceTest.java`:
- Around line 28-31: Remove the duplicate imports for
PlaceRecommendationRepository, RecommendationLikeRepository, PlaceRepository,
and RecommendationCommentRepository in CourseServiceTest, retaining exactly one
declaration of each.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 113002dd-9786-4953-9bb8-24cdbc56875d
📒 Files selected for processing (4)
src/main/java/com/daytodo/domain/course/exception/code/CourseErrorCode.javasrc/main/java/com/daytodo/domain/place/entity/Place.javasrc/main/java/com/daytodo/domain/place/repository/PlaceRepository.javasrc/test/java/com/daytodo/domain/course/service/CourseServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/daytodo/domain/place/entity/Place.java
- src/main/java/com/daytodo/domain/place/repository/PlaceRepository.java
2 participants
📌 관련 이슈
🔎 What is this PR?
매거진(한국관광공사 KorService2 연동) 조회·장소 저장과 투데이 코스 장소 추가/순서 변경 API를 구현했습니다.
✨ Changes
Magazine경량 테이블로 광고 오버레이,Region에 관광 지역코드 매핑 추가Placelazy upsert 후BookmarkPlace생성X-User-Id헤더 → JWT(@AuthenticationPrincipal)📷 Result
💬 To. Reviewer
✅ 체크 리스트
Summary by CodeRabbit