[FIX] presignedUrl 발급 로직 성능 개선 #125
Conversation
# Conflicts: # src/main/java/cc/backend/photoAlbum/service/PhotoAlbumServiceImpl.java
📝 WalkthroughWalkthroughThe PR updates a documentation comment in the Image entity and introduces batched image retrieval optimization in PhotoAlbumServiceImpl's getPhotoAlbumList method, replacing individual per-album image lookups with a single bulk fetch operation followed by DTO conversion. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Service as PhotoAlbumService
participant DB as Database
participant ImageService
participant DTO as DTO Assembly
Client->>Service: getPhotoAlbumList(pageable)
rect rgb(200, 220, 250)
Note over Service: Fetch Albums (Single Page)
Service->>DB: findAll(pageable)
DB-->>Service: List<Album>
end
rect rgb(220, 250, 200)
Note over Service: Batch Image Retrieval (New Optimization)
Service->>Service: Extract albumIds from page
Service->>DB: findFirstByContentIds(albumIds)
DB-->>Service: List<Image>
Service->>Service: Build albumImageMap (merge first)
end
rect rgb(250, 230, 200)
Note over Service: Bulk DTO Conversion
Service->>ImageService: Convert all images to ImageResultWithPresignedUrlDTO
ImageService-->>Service: imageDtoMap
end
rect rgb(240, 240, 240)
Note over DTO: Assemble Response
Service->>DTO: Build SinglePhotoAlbumDTO list
DTO->>DTO: imageDtoMap.get(albumId) per album
DTO-->>Service: List<SinglePhotoAlbumDTO>
end
Service-->>Client: Page<SinglePhotoAlbumDTO>
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 (2)
🔇 Additional comments (5)
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: 0
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/photoAlbum/service/PhotoAlbumServiceImpl.java (1)
366-373: Apply the same batching optimization here.The loop calls
imageService.getImages()individually for each image, creating an N+1 query pattern. This is inconsistent with the optimization just implemented ingetPhotoAlbumList(lines 151-163), where presigned URLs are generated in bulk.🔎 Proposed refactor to batch presigned URL generation
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 convert all images to DTOs with presigned URLs + // Note: Assumes all images belong to the same member, or adjust as needed + Long memberId = firstImages.get(0).getMemberId(); + List<ImageResponseDTO.ImageResultWithPresignedUrlDTO> imageDtos = + imageService.getImages(firstImages, memberId); + + return imageDtos.stream() + .collect(Collectors.toMap( + ImageResponseDTO.ImageResultWithPresignedUrlDTO::getContentId, + ImageResponseDTO.ImageResultWithPresignedUrlDTO::getPresignedUrl, + (a, b) -> a + )); }Note: This assumes images from the same batch can share a
memberIdfor presigned URL generation. If different images require differentmemberIdvalues for authorization, you'll need to adjust the batching strategy accordingly.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/cc/backend/image/entity/Image.javasrc/main/java/cc/backend/photoAlbum/service/PhotoAlbumServiceImpl.java
🔇 Additional comments (5)
src/main/java/cc/backend/image/entity/Image.java (1)
35-35: LGTM! Documentation clarification.The terminology update from "actor" to "casting" improves clarity in the theater/performance domain context.
src/main/java/cc/backend/photoAlbum/service/PhotoAlbumServiceImpl.java (4)
135-138: LGTM! Clean preparation for batch query.Extracting album IDs upfront is the correct approach for the N+1 prevention strategy implemented below.
151-163: LGTM! Efficient bulk DTO conversion with presigned URLs.The empty map guard and batch conversion of images to DTOs with presigned URLs is well-implemented. This centralizes the presigned URL generation instead of calling it per-album.
166-178: LGTM! Clean DTO assembly using pre-fetched data.The mapping correctly uses the pre-fetched
imageDtoMap. Note thatimageDtoMap.get(album.getId())will returnnullfor albums without images, which is passed toimageResultWithPresignedUrlDTO. This appears intentional based on the DTO structure.
140-148: Batch optimization is sound; merge function is defensive but unnecessary.The query
findFirstByContentIdsuses a subqueryi.id = (SELECT MIN(i2.id) FROM Image i2 WHERE i2.contentId = i.contentId)to guarantee exactly one Image per contentId. The merge function(a, b) -> ais therefore redundant and can be removed in favor of the more standard pattern used elsewhere (e.g., line 362), which collects directly to aMap<Long, Image>without mapping.
#이슈번호,#이슈번호Summary by CodeRabbit
Performance Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.