[FEAT] 사진첩과 연결할 공연 조회 api 개발, 일반유저 접근 페이지 수정, specification 이용한 approval 조건 추가 #135
Conversation
…al 조건절 추가 및 공연 조회 쿼리 성능 최적화
📝 WalkthroughWalkthroughRefactors amateur-show and photo-album APIs to remove member-context from multiple endpoints, replaces in-memory filtering with Specification-driven repository queries (including date-round filters), and adds a performer-only photo-album endpoint to list a member's shows. Changes
Sequence Diagram(s)sequenceDiagram
%% Styling hint: controller/service/repo flow for show listing
participant Client as Client
participant Ctrl as AmateurController
participant Svc as AmateurServiceImpl
participant Repo as AmateurShowRepository
rect rgb(240,248,255)
Note right of Ctrl: Request without member context
end
Client->>Ctrl: GET /amateurShows/today?page...
Ctrl->>Svc: getShowToday(pageable)
Svc->>Svc: build Specification (isApproved ∧ hasRoundOn(today))
Svc->>Repo: findAll(spec, pageable)
Repo-->>Svc: Slice<AmateurShow>
Svc->>Svc: map -> DTO (mergeSchedule)
Svc-->>Ctrl: Slice<DTO>
Ctrl-->>Client: 200 OK (paged DTOs)
sequenceDiagram
%% PhotoAlbum: performer-only getMyShows flow
participant Client as Client
participant Ctrl as PhotoAlbumController
participant Svc as PhotoAlbumServiceImpl
participant Repo as AmateurShowRepository
Client->>Ctrl: GET /photoAlbums/getMyShows (authenticated PERFORMER)
Ctrl->>Svc: getMyShows(memberId)
Svc->>Svc: verify role == PERFORMER
Svc->>Repo: findAllByMemberId(memberId)
Repo-->>Svc: List<AmateurShow>
Svc->>Svc: map -> MyShowsForPhotoAlbumDTO (mergeScheduleWithoutDays)
Svc-->>Ctrl: List<MyShowsForPhotoAlbumDTO>
Ctrl-->>Client: 200 OK (list)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
![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
Fix all issues with AI Agents 🤖
In @src/main/java/cc/backend/amateurShow/controller/AmateurController.java:
- Around line 95-100: The getShowIncoming controller method currently calls
amateurService.getShowToday, which mismatches the operation summary for "임박한 공연
조회 API"; update AmateurController.getShowIncoming to call the correct service
method (e.g., amateurService.getShowIncoming or amateurService.getUpcomingShows)
that returns upcoming/imminent shows instead of today's shows — if that service
method does not exist, implement a new method in AmateurService (and its impl)
that queries for upcoming rounds and returns a Slice<AmateurShowList>, and
ensure the returned DTO/signature matches SliceResponse.of(...) used in the
controller.
In
@src/main/java/cc/backend/amateurShow/service/amateurShowService/AmateurServiceImpl.java:
- Around line 561-567: Using Integer.MAX_VALUE as the page size in
PageRequest.of risks loading an unbounded number of AmateurShow entities into
memory; replace this with a reasonable upper bound (e.g., a defined constant
MAX_PAGE_SIZE or a sensible cap like 1000) or switch to a streaming/batching
approach using repository methods that return a Stream or Pageable-backed
iteration; update the PageRequest.of call (currently constructing sortedPage)
and/or change the call path via amateurShowRepository.findAll(spec, ...) to use
streaming (e.g., a repository method returning Stream<AmateurShow>) or looped
Pageable requests to fetch results in chunks.
In @src/main/java/cc/backend/photoAlbum/service/PhotoAlbumServiceImpl.java:
- Around line 374-376: The if-check in PhotoAlbumServiceImpl that throws
GeneralException when member.getRole() != Role.PERFORMER is redundant because
the controller already enforces @PreAuthorize("hasRole('PERFORMER')"); remove
the role check (the if block referencing member.getRole() and Role.PERFORMER)
from PhotoAlbumServiceImpl so the service relies on controller/security
annotations (PhotoAlbumController's @PreAuthorize) and leave any necessary
null/ownership checks only.
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/cc/backend/amateurShow/service/amateurShowService/AmateurServiceImpl.java (1)
602-613:statusmay benullwhen passed to DTO builder.When
status == null, the query retrieves all shows regardless of status (line 596), but the DTO builder on line 611 setsstatus(status)which will benull. This could confuse API consumers expecting the actual status of each show.Consider using the show's actual status instead:
🔎 Proposed fix
return shows.map(show -> { String schedule = AmateurConverter.mergeSchedule(show.getStart(), show.getEnd()); return AmateurShowResponseDTO.MyShowAmateurShowList.builder() .amateurShowId(show.getId()) .name(show.getName()) //.place(show.getPlace()) .detailAddress(show.getDetailAddress()) .schedule(schedule) .posterImageUrl(show.getPosterImageUrl()) - .status(status) + .status(show.getStatus()) .build(); });
🧹 Nitpick comments (3)
src/main/java/cc/backend/photoAlbum/dto/PhotoAlbumResponseDTO.java (1)
76-84: Consider removing the redundantmemberIdfield.The
memberIdfield inMyShowsForPhotoAlbumDTOappears redundant since the service methodgetMyShowsalways sets it to the requesting user's ID (the samememberIdparameter passed to the method). The client already knows their own ID, and including it in every item of the returned list adds unnecessary data transfer overhead.🔎 Proposed refactor to remove memberId field
@Builder @Getter @NoArgsConstructor @AllArgsConstructor public static class MyShowsForPhotoAlbumDTO { - private Long memberId; private Long amateurShowId; private String schedule; }Then update the service implementation accordingly:
return amateurShows.stream() .map(show->{ String schedule = mergeScheduleWithoutDays(show.getStart(), show.getEnd()); return PhotoAlbumResponseDTO.MyShowsForPhotoAlbumDTO.builder() .amateurShowId(show.getId()) - .memberId(memberId) .schedule(schedule) .build(); }) .collect(Collectors.toList());src/main/java/cc/backend/amateurShow/service/amateurShowService/AmateurService.java (1)
30-30: Remove commented-out code.Commented-out code should be removed to keep the codebase clean. If the functionality is needed later, it can be retrieved from version control.
🔎 Proposed fix
- // ReserveListResponseDTO getReserveListDetail(Long amateurShowId, Long memberId);src/main/java/cc/backend/amateurShow/controller/AmateurController.java (1)
11-11: Unused import.
MemberServiceis imported but no longer used after removing the member-related field.🔎 Proposed fix
-import cc.backend.member.MemberService;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/cc/backend/amateurShow/controller/AmateurController.javasrc/main/java/cc/backend/amateurShow/repository/AmateurShowRepository.javasrc/main/java/cc/backend/amateurShow/repository/specification/AmateurShowSpecification.javasrc/main/java/cc/backend/amateurShow/service/amateurShowService/AmateurService.javasrc/main/java/cc/backend/amateurShow/service/amateurShowService/AmateurServiceImpl.javasrc/main/java/cc/backend/photoAlbum/controller/PhotoAlbumController.javasrc/main/java/cc/backend/photoAlbum/dto/PhotoAlbumResponseDTO.javasrc/main/java/cc/backend/photoAlbum/service/PhotoAlbumService.javasrc/main/java/cc/backend/photoAlbum/service/PhotoAlbumServiceImpl.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/cc/backend/photoAlbum/service/PhotoAlbumServiceImpl.java (1)
src/main/java/cc/backend/photoAlbum/dto/PhotoAlbumResponseDTO.java (1)
PhotoAlbumResponseDTO(11-87)
src/main/java/cc/backend/amateurShow/service/amateurShowService/AmateurServiceImpl.java (3)
src/main/java/cc/backend/amateurShow/converter/AmateurConverter.java (1)
AmateurConverter(17-310)src/main/java/cc/backend/amateurShow/repository/specification/AmateurShowSpecification.java (1)
AmateurShowSpecification(12-88)src/main/java/cc/backend/amateurShow/dto/AmateurShowResponseDTO.java (1)
AmateurShowResponseDTO(12-122)
⏰ 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). (1)
- GitHub Check: build-and-deploy
🔇 Additional comments (19)
src/main/java/cc/backend/photoAlbum/service/PhotoAlbumService.java (2)
22-22: LGTM!The new
getMyShowsmethod signature is clear and follows the existing patterns in the service interface.
20-20: All callers have been properly updated for the breaking signature change.The search for remaining calls using the old three-argument signature returned no results. The codebase consistently uses the new signature
getAllRecentPhotoAlbumList(Long cursorId, int size)across the interface definition, implementation, and the identified call site in PhotoAlbumController.src/main/java/cc/backend/photoAlbum/service/PhotoAlbumServiceImpl.java (2)
268-311: LGTM!The refactoring to remove member context from
getAllRecentPhotoAlbumListcorrectly enables visitor-first access. The cursor-based pagination logic is sound, and the N+1 query prevention with batch image retrieval is well-implemented.
378-378: The methodfindAllByMemberId(Long memberId)is properly declared in theAmateurShowRepositoryinterface at line 61 ofsrc/main/java/cc/backend/amateurShow/repository/AmateurShowRepository.java. The code at line 378 correctly calls this existing method.src/main/java/cc/backend/photoAlbum/controller/PhotoAlbumController.java (3)
32-38: LGTM!The new endpoint is well-structured with proper security authorization, clear documentation, and correct delegation to the service layer.
78-78: LGTM!Adding
@PreAuthorize("hasRole('PERFORMER')")to the delete endpoint ensures proper authorization and aligns with security best practices for mutating operations.
92-92: LGTM!The service call correctly updated to match the new signature without the
memberIdparameter, enabling visitor-first access to the photo album list.src/main/java/cc/backend/amateurShow/repository/AmateurShowRepository.java (2)
61-61: LGTM!The new
findAllByMemberIdmethod provides a simple member-based query for retrieving all shows, supporting the photo album API use case.
63-73: Good addition of approval status filter.Adding
AND s.approvalStatus = 'APPROVED'ensures only approved shows appear in the hot shows list, which aligns with the PR objective of restricting general user access to approved content only.src/main/java/cc/backend/amateurShow/repository/specification/AmateurShowSpecification.java (2)
22-25: LGTM!The
isApproved()specification correctly filters shows byApprovalStatus.APPROVED, providing a clean replacement for the previousisAudienceVisible()method.
50-61: No action required — application currently uses MySQL exclusively.Verification shows the application uses only MySQL across all environments (dev, prod, and tests). The build.gradle includes only the MySQL connector (
runtimeOnly 'com.mysql:mysql-connector-j') with no H2, PostgreSQL, TestContainers, or other database dependencies. Test configurations do not override the database setup. The MySQL-specificcb.function("DATE", ...)is not a portability concern in the current context since no multi-database support is required or planned.src/main/java/cc/backend/amateurShow/service/amateurShowService/AmateurService.java (1)
18-25: LGTM!The removal of
memberIdfrom these method signatures correctly reflects the shift to member-agnostic public endpoints. These audience-facing methods now rely on database-level filtering via Specifications rather than member-based authorization.src/main/java/cc/backend/amateurShow/controller/AmateurController.java (1)
59-61: LGTM!The endpoint correctly removes member context, making it a public API that relies on database-level filtering for approval status.
src/main/java/cc/backend/amateurShow/service/amateurShowService/AmateurServiceImpl.java (6)
6-6: LGTM!The new imports for
ApprovalStatus,AmateurShowSpecification,Specification, and Spring Data domain classes properly support the refactored Specification-based querying approach.Also applies to: 11-11, 29-29, 31-31
446-451: LGTM!The refactored
getAmateurShowmethod correctly removes the unusedmemberIdparameter. The method now simply fetches the show by ID and converts to DTO.
455-478: LGTM!The
getShowTodaymethod is cleanly refactored to use Specifications for database-level filtering. CombiningisApproved()andhasRoundOn(today)correctly filters for approved shows with rounds on the current date.
482-509: LGTM!The
getShowOngoingmethod correctly uses Specifications and creates a properly sortedPageablefor the query.
513-548: LGTM!The ranking query correctly uses Specifications with multi-field sorting (sold tickets descending, start date ascending) and limits to 10 results.
617-639: LGTM!The
getRecentlyHotShowmethod correctly uses the repository'sfindHotShowsmethod which already includes the approval status filter in the query.
| @GetMapping("/incoming") | ||
| @Operation(summary = "임박한 공연 조회 API") | ||
| public ApiResponse<SliceResponse<AmateurShowResponseDTO.AmateurShowList>> getShowIncoming( | ||
| @AuthenticationPrincipal(expression = "member") Member member, @ParameterObject Pageable pageable) { | ||
| Slice<AmateurShowList> sliceResult = amateurService.getShowToday(member.getId(), pageable); | ||
| public ApiResponse<SliceResponse<AmateurShowResponseDTO.AmateurShowList>> getShowIncoming(@ParameterObject Pageable pageable) { | ||
| Slice<AmateurShowList> sliceResult = amateurService.getShowToday(pageable); | ||
| return ApiResponse.onSuccess(SliceResponse.of(sliceResult)); | ||
| } |
There was a problem hiding this comment.
getShowIncoming delegates to getShowToday — likely incorrect.
The operation summary says "임박한 공연 조회 API" (upcoming/imminent shows), but the implementation calls getShowToday(pageable), which filters for shows with rounds happening today. These are semantically different:
- "Incoming" typically means upcoming/soon-to-happen shows
- "Today" means shows occurring today
This may be a copy-paste error or intentional reuse, but the API behavior doesn't match its description.
🤖 Prompt for AI Agents
In @src/main/java/cc/backend/amateurShow/controller/AmateurController.java
around lines 95-100, The getShowIncoming controller method currently calls
amateurService.getShowToday, which mismatches the operation summary for "임박한 공연
조회 API"; update AmateurController.getShowIncoming to call the correct service
method (e.g., amateurService.getShowIncoming or amateurService.getUpcomingShows)
that returns upcoming/imminent shows instead of today's shows — if that service
method does not exist, implement a new method in AmateurService (and its impl)
that queries for upcoming rounds and returns a Slice<AmateurShowList>, and
ensure the returned DTO/signature matches SliceResponse.of(...) used in the
controller.
src/main/java/cc/backend/amateurShow/repository/specification/AmateurShowSpecification.java
Show resolved
Hide resolved
| // Pageable로 start 기준 오름차순 정렬, limit 없이 전체 조회 | ||
| Pageable sortedPage = PageRequest.of( | ||
| 0, | ||
| Integer.MAX_VALUE, | ||
| Sort.by("start").ascending()); | ||
|
|
||
| result.add(AmateurShowResponseDTO.AmateurShowList.builder() | ||
| .amateurShowId(show.getId()) | ||
| .name(show.getName()) | ||
| //.place(show.getPlace()) | ||
| .detailAddress(show.getDetailAddress()) | ||
| .schedule(schedule) | ||
| .posterImageUrl(show.getPosterImageUrl()) | ||
| .build()); | ||
| } | ||
| } | ||
| List<AmateurShow> shows = amateurShowRepository.findAll(spec, sortedPage).getContent(); |
There was a problem hiding this comment.
Integer.MAX_VALUE as page size is risky.
Using Integer.MAX_VALUE as the page size could load an unbounded number of records into memory if many shows close on the same day. Consider using a reasonable upper bound or streaming/batching if truly unbounded results are needed.
🔎 Proposed fix
// Pageable로 start 기준 오름차순 정렬, limit 없이 전체 조회
Pageable sortedPage = PageRequest.of(
0,
- Integer.MAX_VALUE,
+ 1000, // Set a reasonable upper bound
Sort.by("start").ascending());📝 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.
| // Pageable로 start 기준 오름차순 정렬, limit 없이 전체 조회 | |
| Pageable sortedPage = PageRequest.of( | |
| 0, | |
| Integer.MAX_VALUE, | |
| Sort.by("start").ascending()); | |
| result.add(AmateurShowResponseDTO.AmateurShowList.builder() | |
| .amateurShowId(show.getId()) | |
| .name(show.getName()) | |
| //.place(show.getPlace()) | |
| .detailAddress(show.getDetailAddress()) | |
| .schedule(schedule) | |
| .posterImageUrl(show.getPosterImageUrl()) | |
| .build()); | |
| } | |
| } | |
| List<AmateurShow> shows = amateurShowRepository.findAll(spec, sortedPage).getContent(); | |
| // Pageable로 start 기준 오름차순 정렬, limit 없이 전체 조회 | |
| Pageable sortedPage = PageRequest.of( | |
| 0, | |
| 1000, // Set a reasonable upper bound | |
| Sort.by("start").ascending()); | |
| List<AmateurShow> shows = amateurShowRepository.findAll(spec, sortedPage).getContent(); |
🤖 Prompt for AI Agents
In
@src/main/java/cc/backend/amateurShow/service/amateurShowService/AmateurServiceImpl.java
around lines 561-567, Using Integer.MAX_VALUE as the page size in PageRequest.of
risks loading an unbounded number of AmateurShow entities into memory; replace
this with a reasonable upper bound (e.g., a defined constant MAX_PAGE_SIZE or a
sensible cap like 1000) or switch to a streaming/batching approach using
repository methods that return a Stream or Pageable-backed iteration; update the
PageRequest.of call (currently constructing sortedPage) and/or change the call
path via amateurShowRepository.findAll(spec, ...) to use streaming (e.g., a
repository method returning Stream<AmateurShow>) or looped Pageable requests to
fetch results in chunks.
| if(member.getRole() != Role.PERFORMER){ | ||
| throw new GeneralException(ErrorStatus.MEMBER_NOT_AUTHORIZED); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove redundant role check.
The role check here is redundant because the @PreAuthorize("hasRole('PERFORMER')") annotation on the controller endpoint (line 32 in PhotoAlbumController.java) already enforces this constraint. If a non-PERFORMER user reaches this service method, the security layer has failed.
🔎 Proposed fix to remove redundant check
public List<PhotoAlbumResponseDTO.MyShowsForPhotoAlbumDTO> getMyShows(Long memberId) {
Member member = memberRepository.findById(memberId)
.orElseThrow(() -> new GeneralException(ErrorStatus.MEMBER_NOT_FOUND));
- if(member.getRole() != Role.PERFORMER){
- throw new GeneralException(ErrorStatus.MEMBER_NOT_AUTHORIZED);
- }
-
List<AmateurShow> amateurShows = amateurShowRepository.findAllByMemberId(memberId);🤖 Prompt for AI Agents
In @src/main/java/cc/backend/photoAlbum/service/PhotoAlbumServiceImpl.java
around lines 374-376, The if-check in PhotoAlbumServiceImpl that throws
GeneralException when member.getRole() != Role.PERFORMER is redundant because
the controller already enforces @PreAuthorize("hasRole('PERFORMER')"); remove
the role check (the if block referencing member.getRole() and Role.PERFORMER)
from PhotoAlbumServiceImpl so the service relies on controller/security
annotations (PhotoAlbumController's @PreAuthorize) and leave any necessary
null/ownership checks only.
| private static String mergeScheduleWithoutDays(LocalDate start, LocalDate end) { | ||
| try { | ||
| if (start == null || end == null) { | ||
| return ""; | ||
| } | ||
|
|
||
| DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy.MM.dd"); | ||
|
|
||
| String startStr = start.format(formatter); | ||
| String endStr = end.format(formatter); | ||
|
|
||
| // 최종 결과: "2025.10.02~2025.10.05" | ||
| return startStr + "~" + endStr; | ||
| } catch (Exception e) { | ||
| e.printStackTrace(); | ||
| return ""; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Refactor to eliminate code duplication and improve error handling.
This method duplicates the logic of the existing mergeSchedule method (imported at line 33). The main differences are:
- Date format:
mergeScheduleincludes day-of-week; this method doesn't - Error handling: This method uses
printStackTrace(), which is not production-ready
Consider these improvements:
- Code duplication: Extract the format pattern as a parameter or create a variant of
mergeSchedulein the converter utility - Error handling: Replace
printStackTrace()with proper logging using a logger
🔎 Proposed refactor options
Option 1: Add format parameter to existing utility
In cc/backend/amateurShow/converter/AmateurConverter.java:
public static String mergeSchedule(LocalDate start, LocalDate end, String pattern) {
try {
if (start == null || end == null) {
return "";
}
DateTimeFormatter formatter = DateTimeFormatter.ofPattern(pattern);
String startStr = start.format(formatter);
String endStr = end.format(formatter);
return startStr + "~" + endStr;
} catch (Exception e) {
log.error("Failed to format schedule", e);
return "";
}
}Option 2: Use proper logging instead of printStackTrace()
If keeping the method separate:
+ private static final Logger log = LoggerFactory.getLogger(PhotoAlbumServiceImpl.class);
+
private static String mergeScheduleWithoutDays(LocalDate start, LocalDate end) {
try {
if (start == null || end == null) {
return "";
}
DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy.MM.dd");
String startStr = start.format(formatter);
String endStr = end.format(formatter);
// 최종 결과: "2025.10.02~2025.10.05"
return startStr + "~" + endStr;
} catch (Exception e) {
- e.printStackTrace();
+ log.error("Failed to format schedule without days for start={}, end={}", start, end, e);
return "";
}
}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/cc/backend/amateurShow/service/amateurShowService/AmateurServiceImpl.java (1)
552-575: Consider adding pagination or a reasonable size limit.The method calls
findAll(spec, Sort)without pagination, which could return an unbounded number of shows if many close on the same day. While this may be acceptable for typical use cases, consider adding a size limit for safety.🔎 Proposed fix with size limit
public List<AmateurShowResponseDTO.AmateurShowList> getShowClosing() { LocalDate today = LocalDate.now(); Specification<AmateurShow> spec = Specification .where(AmateurShowSpecification.isApproved()) .and(AmateurShowSpecification.hasLastRoundOn(today)); - List<AmateurShow> shows = amateurShowRepository.findAll(spec, Sort.by("start").ascending()); + Pageable pageable = PageRequest.of(0, 100, Sort.by("start").ascending()); + List<AmateurShow> shows = amateurShowRepository.findAll(spec, pageable).getContent(); + // DTO 변환 return shows.stream() .map(show -> { String schedule = AmateurConverter.mergeSchedule(show.getStart(), show.getEnd()); return AmateurShowResponseDTO.AmateurShowList.builder() .amateurShowId(show.getId()) .name(show.getName()) .detailAddress(show.getDetailAddress()) .schedule(schedule) .posterImageUrl(show.getPosterImageUrl()) .build(); }) .collect(Collectors.toList()); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/cc/backend/amateurShow/repository/specification/AmateurShowSpecification.javasrc/main/java/cc/backend/amateurShow/service/amateurShowService/AmateurServiceImpl.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/cc/backend/amateurShow/service/amateurShowService/AmateurServiceImpl.java (3)
src/main/java/cc/backend/amateurShow/converter/AmateurConverter.java (1)
AmateurConverter(17-310)src/main/java/cc/backend/amateurShow/repository/specification/AmateurShowSpecification.java (1)
AmateurShowSpecification(14-97)src/main/java/cc/backend/amateurShow/dto/AmateurShowResponseDTO.java (1)
AmateurShowResponseDTO(12-122)
⏰ 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). (1)
- GitHub Check: build-and-deploy
🔇 Additional comments (6)
src/main/java/cc/backend/amateurShow/service/amateurShowService/AmateurServiceImpl.java (5)
446-451: LGTM! Public endpoint pattern confirmed.The removal of
memberIdparameter aligns with making this a public endpoint. The implementation correctly fetches the show by ID with appropriate error handling.
455-478: Excellent refactoring to Specification-based querying.The migration from in-memory filtering to database-level Specification queries improves performance and scalability. The use of
Slicefor pagination is appropriate for this use case.
482-509: LGTM! Consistent Specification pattern.The explicit
PageRequestconstruction with sorting ensures predictable results. The implementation follows the same clean pattern as other refactored methods.
513-548: Well-implemented ranking query with appropriate limits.The use of
PageRequest.of(0, 10, ...)with multi-field sorting correctly implements top-10 ranking logic. Converting theSliceto aListis appropriate since this method returns a fixed-size ranking.
610-632: LGTM! Appropriate direct query for simple use case.The removal of
memberIdmakes this a public endpoint, and the direct repository call with a fixed limit of 3 is appropriate for a "hot shows" feature. The implementation is clean and consistent with other DTO mapping.src/main/java/cc/backend/amateurShow/repository/specification/AmateurShowSpecification.java (1)
48-96: This concern overstates the portability issue.Both methods use
cb.function("DATE", ...), which is appropriate for this MySQL-only application (as documented in the code comment "MySQL 기준"). PostgreSQL actually supports theDATE()function as a cast operator and would not fail. H2 has more limited support, but H2 is not part of this project's configuration (only MySQL is configured inbuild.gradleand application profiles).Refactoring to use
CASTorBETWEENapproaches would only be necessary if the application gains multi-database support in the future. For the current MySQL-only context, the existing implementation is idiomatic and correct.Likely an incorrect or invalid review comment.
#이슈번호,#이슈번호Summary by CodeRabbit
New Features
Improvements
Security
✏️ Tip: You can customize this high-level summary in your review settings.