-
Notifications
You must be signed in to change notification settings - Fork 1
[FIX] 방문자 0인 시간대 채우는 로직 추가 #126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
# Conflicts: # src/main/java/cc/backend/photoAlbum/service/PhotoAlbumServiceImpl.java
📝 WalkthroughWalkthroughReplaced hour-row direct mapping with a 24-hour accumulation and sort for hourly visitor counts; updated an image field comment to use "casting"; refactored photo-album image resolution to batch-fetch images and map presigned-URL DTOs centrally. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/cc/backend/photoAlbum/service/PhotoAlbumServiceImpl.java (1)
355-375: Consider batching image resolution for consistency.This helper method iterates and calls
imageService.getImages(List.of(img), ...)per image, which could be refactored to batch all images in one call, similar to the approach used ingetPhotoAlbumList(lines 151-163). This would improve performance and maintain consistency across the codebase.🔎 Proposed refactor
private Map<Long, String> getFirstImageMapForPhotoAlbums(List<Long> albumIds) { if (albumIds == null || albumIds.isEmpty()) { return Collections.emptyMap(); } // 배치로 대표 이미지 조회 List<Image> firstImages = imageRepository.findFirstByContentIds(albumIds, FilePath.photoAlbum); - Map<Long, String> result = new HashMap<>(); - - for (Image img : firstImages) { - String presignedUrl = imageService - .getImages(List.of(img), img.getMemberId()) - .get(0) - .getPresignedUrl(); - - result.put(img.getContentId(), presignedUrl); - } - return result; + if (firstImages.isEmpty()) { + return Collections.emptyMap(); + } + + // Batch fetch presigned URLs + Long memberId = firstImages.get(0).getMemberId(); + return imageService.getImages(firstImages, memberId) + .stream() + .collect(Collectors.toMap( + ImageResponseDTO.ImageResultWithPresignedUrlDTO::getContentId, + ImageResponseDTO.ImageResultWithPresignedUrlDTO::getPresignedUrl, + (a, b) -> a + )); }Note: The proposed refactor assumes all images share the same
memberIdfor authorization purposes. Verify this assumption holds for your use case.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/cc/backend/admin/dashboard/service/DashboardService.javasrc/main/java/cc/backend/image/entity/Image.javasrc/main/java/cc/backend/photoAlbum/service/PhotoAlbumServiceImpl.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/cc/backend/photoAlbum/service/PhotoAlbumServiceImpl.java (2)
src/main/java/cc/backend/image/DTO/ImageResponseDTO.java (1)
ImageResponseDTO(11-38)src/main/java/cc/backend/photoAlbum/dto/PhotoAlbumResponseDTO.java (1)
PhotoAlbumResponseDTO(11-89)
🪛 GitHub Actions: CI/CD with Blue-Green Deployment
src/main/java/cc/backend/admin/dashboard/service/DashboardService.java
[error] 55-55: incompatible types: Integer cannot be converted to String
[error] 56-56: incompatible types: List cannot be converted to List
🔇 Additional comments (5)
src/main/java/cc/backend/image/entity/Image.java (1)
35-35: LGTM!The comment terminology update from "actor" to "casting" aligns with the domain model naming conventions.
src/main/java/cc/backend/admin/dashboard/service/DashboardService.java (1)
43-51: LGTM on the 24-hour map initialization logic.The approach of pre-populating all 24 hours with zeros and then overwriting with actual data ensures consistent output regardless of which hours have visitor data. The sort operation ensures proper ordering.
src/main/java/cc/backend/photoAlbum/service/PhotoAlbumServiceImpl.java (3)
140-148: LGTM on batched image fetching.The merge function
(a, b) -> ais a good defensive measure to handle potential duplicate contentIds, ensuring deterministic behavior by keeping the first encountered image.
151-163: Good performance improvement with batched presigned URL generation.The ternary guard for empty
albumImageMapavoids an unnecessary service call, and batching all images into a singlegetImagescall reduces N+1 query overhead.
166-178: Verify null handling for albums without images.
imageDtoMap.get(album.getId())will returnnullif an album has no associated images. Confirm this is the intended behavior and that the frontend/consumers handlenullforimageResultWithPresignedUrlDTOgracefully.
#이슈번호,#이슈번호Summary by CodeRabbit
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.