feat: 이미지 로딩 시에 webp가 존재하면 로딩 아니면 원본 로딩 기능 추가 #1134
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
Image Resolution Abstraction backend/src/main/java/moadong/media/resolver/ImageDisplayUrlResolver.java, backend/src/main/java/moadong/media/resolver/NoOpImageDisplayUrlResolver.java |
새 인터페이스 ImageDisplayUrlResolver 추가 및 입력을 변경하지 않는 기본 구현 NoOpImageDisplayUrlResolver 추가. |
WebP Conversion Implementation backend/src/main/java/moadong/media/resolver/PreferWebpImageDisplayUrlResolver.java |
S3 키 추출, .webp 키 생성, HEAD 요청으로 .webp 존재 확인 후 가능하면 WebP URL 반환하는 구현 추가. 초기화(normalize viewEndpoint) 로직 포함. |
Service Integration backend/src/main/java/moadong/club/service/ClubProfileService.java, backend/src/main/java/moadong/club/service/ClubSearchService.java |
서비스에 ImageDisplayUrlResolver 주입 추가. ClubDetailedResult/ClubSearchResult 생성 시 이미지 URL을 resolver로 변환하여 사용하도록 호출부 변경(검색 메서드 시그니처 확장 포함). |
DTO Factory Method Enhancement backend/src/main/java/moadong/club/payload/dto/ClubDetailedResult.java |
새 정적 팩토리 of(Club club, ImageDisplayUrlResolver resolver) 추가. 기존 of(Club)는 새 오버로드로 위임하며 로고/커버/피드 이미지 URL을 선택적으로 변환하도록 수정. |
Tests backend/src/test/java/moadong/club/service/ClubSearchServiceTest.java |
테스트에 ImageDisplayUrlResolver 목 추가 및 resolveDisplayUrl 동작 스텁으로 설정. |
Sequence Diagram(s)
sequenceDiagram
participant Client
participant ClubService as ClubProfileService/<br/>ClubSearchService
participant Resolver as ImageDisplayUrlResolver
participant S3 as S3Client
participant DTO as ClubDetailedResult/<br/>ClubSearchResult
Client->>ClubService: 클럽 정보 요청
ClubService->>ClubService: Club 엔티티 조회
ClubService->>Resolver: resolveDisplayUrl(logoUrl)
alt PreferWebp 구현
Resolver->>S3: HEAD 요청 (.webp 존재 확인)
S3-->>Resolver: 200 또는 404 응답
alt .webp 존재
Resolver-->>ClubService: WebP URL 반환
else .webp 없음
Resolver-->>ClubService: 원본 URL 반환
end
else NoOp 구현
Resolver-->>ClubService: 원본 URL 반환
end
ClubService->>DTO: 해석된 URL로 DTO 생성
DTO-->>ClubService: ClubDetailedResult/ClubSearchResult 반환
ClubService-->>Client: 응답 반환
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- [feature] cover image 업로드 및 삭제 기능 구현 #537: ClubDetailedResult 변경 관련 — 커버 필드 추가 및 팩토리 수정과 중첩되는 변경점이 있음.
- [refactor] 동아리 수정 API 명세 변경 #961: ClubDetailedResult 팩토리 메서드 및 DTO 구성 관련 변경으로 코드 레벨 연관성 있음.
- [Release] BE v1.0.6 배포 #675: ClubDetailedResult 팩토리 및 ClubProfileService 호출부 변경을 다룸 — 서비스 통합 지점이 겹침.
Suggested labels
✨ Feature, 💾 BE
Suggested reviewers
- lepitaaar
- seongwon030
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 30.77% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR 제목은 WebP 이미지 우선 로딩 기능을 명확히 설명하며, 변경사항의 핵심을 정확히 나타냅니다. |
| Linked Issues check | ✅ Passed | 코드 변경사항이 이미지 컨버팅 서버 구축(MOA-593) 목표와 일치하며, WebP 우선 로딩 로직을 성공적으로 구현했습니다. |
| Out of Scope Changes check | ✅ Passed | 모든 변경사항은 ImageDisplayUrlResolver를 통한 WebP 우선 로딩 기능 구현으로 범위 내에 있습니다. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feature/#1133-image-convert-MOA-593
Important
Action Needed: IP Allowlist Update
If your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
- ✨
136.113.208.247/32(new) 34.170.211.100/3235.222.179.152/32
Failure to add the new IP will result in interrupted reviews.
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 help to get the list of available commands and usage tips.
Test Results72 tests 72 ✅ 20s ⏱️ Results for commit 178d2eb. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/src/main/java/moadong/club/service/ClubSearchService.java`:
- Around line 58-68: The mapping in ClubSearchService that calls
imageDisplayUrlResolver.resolveDisplayUrl(...) for each result causes
synchronous S3 HEAD calls (PreferWebpImageDisplayUrlResolver.resolveDisplayUrl)
and creates a performance bottleneck; fix by avoiding per-item synchronous HEADs
— either add Spring `@Cacheable` to
PreferWebpImageDisplayUrlResolver.resolveDisplayUrl to cache WebP existence, or
change the flow in the ClubSearchService mapping to resolve URLs in
batch/parallel (e.g., collect all logo keys, perform parallel/batched HeadObject
requests, then map results back to ClubSearchResult), or precompute/store the
WebP URL at upload time and have imageDisplayUrlResolver return the stored value
(or swap to NoOpImageDisplayUrlResolver if WebP is not needed).
🧹 Nitpick comments (3)
backend/src/main/java/moadong/media/resolver/PreferWebpImageDisplayUrlResolver.java (3)
81-90:toWebpKey에서 dot으로 시작하는 파일명 처리 확인 필요
lastDot > 0조건은.hidden같은 파일명에서 dot 인덱스가 0이 되어 확장자가 없는 것으로 처리됩니다. 이 경우.hidden.webp가 됩니다.실제로 이런 파일명이 사용되지 않는다면 문제없지만, 경로 구분자를 고려한 조건이 더 안전할 수 있습니다:
♻️ 제안된 수정 (선택적)
private static String toWebpKey(String key) { if (key == null || key.isEmpty()) { return key; } + int lastSlash = key.lastIndexOf('/'); int lastDot = key.lastIndexOf('.'); - if (lastDot > 0) { + if (lastDot > lastSlash + 1) { return key.substring(0, lastDot) + ".webp"; } return key + ".webp"; }
48-63: S3 HEAD 요청에 대한 캐싱 고려WebP 존재 여부 확인을 위해 매번 S3 HEAD 요청을 수행합니다. 이미지 URL은 자주 변경되지 않으므로, 결과를 캐싱하면 S3 호출 횟수와 응답 시간을 크게 줄일 수 있습니다.
예를 들어, Caffeine 또는 Spring Cache를 사용한 TTL 기반 캐시를 고려해 보세요:
// Example with Caffeine cache private final Cache<String, Boolean> webpExistsCache = Caffeine.newBuilder() .expireAfterWrite(Duration.ofHours(1)) .maximumSize(10_000) .build();
30-36:@PostConstruct메서드는private보다package-private또는protected권장
@PostConstruct가 private 메서드에서 동작하지만, 일부 프록시 기반 AOP 환경에서는 문제가 될 수 있습니다. 또한 테스트 시 접근성 문제가 있을 수 있습니다.♻️ 제안된 수정
`@PostConstruct` -private void init() { +void init() { if (viewEndpoint == null || viewEndpoint.isEmpty()) { throw new IllegalStateException("cloud.aws.s3.view-endpoint must be configured"); } normalizedViewEndpoint = viewEndpoint.replaceAll("/+$", ""); }
| .map(r -> new ClubSearchResult( | ||
| r.id(), | ||
| r.name(), | ||
| imageDisplayUrlResolver.resolveDisplayUrl(r.logo()), | ||
| r.tags(), | ||
| r.state(), | ||
| r.category(), | ||
| r.division(), | ||
| r.introduction(), | ||
| r.recruitmentStatus())) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the ImageDisplayUrlResolver class
find . -type f -name "*ImageDisplayUrlResolver*" | head -20Repository: Moadong/moadong
Length of output: 300
🏁 Script executed:
# Check the structure of the media/service directory
git ls-files | grep -i "media" | grep -E "\.(java|kt)$" | head -30Repository: Moadong/moadong
Length of output: 1105
🏁 Script executed:
# Look at the ClubSearchService file to understand the context better
cat -n backend/src/main/java/moadong/club/service/ClubSearchService.java | head -80Repository: Moadong/moadong
Length of output: 3705
🏁 Script executed:
# Check the ImageDisplayUrlResolver interface and implementations
cat -n backend/src/main/java/moadong/media/resolver/ImageDisplayUrlResolver.javaRepository: Moadong/moadong
Length of output: 597
🏁 Script executed:
# Check PreferWebpImageDisplayUrlResolver implementation
cat -n backend/src/main/java/moadong/media/resolver/PreferWebpImageDisplayUrlResolver.javaRepository: Moadong/moadong
Length of output: 3680
🏁 Script executed:
# Check for any caching mechanisms related to S3 or image resolution
rg -n "cache|Cache|@Cache" --type java -i backend/src/main/java/moadong/media/ -A 3Repository: Moadong/moadong
Length of output: 41
🏁 Script executed:
# Check if there are any `@Cacheable`, `@CacheEvict`, or caching-related annotations
rg -n "@Cacheable|@CacheEvict|@Cache|CacheManager|caffeine|ehcache" --type java backend/src/main/java/moadong/media/ backend/src/main/java/moadong/club/Repository: Moadong/moadong
Length of output: 41
🏁 Script executed:
# Look for any async processing or batch operations in media service
rg -n "@Async|async|CompletableFuture|batch" --type java -i backend/src/main/java/moadong/media/resolver/ backend/src/main/java/moadong/media/service/Repository: Moadong/moadong
Length of output: 41
🏁 Script executed:
# Check NoOpImageDisplayUrlResolver to see if there's an alternative
cat -n backend/src/main/java/moadong/media/resolver/NoOpImageDisplayUrlResolver.javaRepository: Moadong/moadong
Length of output: 560
🏁 Script executed:
# Verify search result size to understand scale of the problem
rg -n "LIMIT|limit|pageable" --type java backend/src/main/java/moadong/club/repository/ | head -20Repository: Moadong/moadong
Length of output: 41
🏁 Script executed:
# Check if there's any recent performance monitoring or discussions about this
rg -n "performance|latency|timeout|slow" --type java -i backend/src/main/java/moadong/club/service/ClubSearchService.javaRepository: Moadong/moadong
Length of output: 41
검색 결과마다 S3 HEAD 요청 발생 — 동기적 성능 병목 확인됨
현재 구현에서 PreferWebpImageDisplayUrlResolver.resolveDisplayUrl()은 각 검색 결과의 로고 URL에 대해 S3 HEAD 요청을 동기적으로 실행합니다(line 61). 검색 결과 10개 = 10개의 순차 S3 호출로 응답 시간이 크게 증가할 수 있습니다.
해결 방안:
- Spring의
@Cacheable어노테이션을resolveDisplayUrl()에 추가하여 WebP 존재 여부 캐싱 - 또는 배치 HeadObject 요청으로 병렬화
- 또는 이미지 업로드 시점에 WebP URL 사전 계산 후 DB에 저장
- WebP 기능이 필요 없다면
NoOpImageDisplayUrlResolver사용
🤖 Prompt for AI Agents
In `@backend/src/main/java/moadong/club/service/ClubSearchService.java` around
lines 58 - 68, The mapping in ClubSearchService that calls
imageDisplayUrlResolver.resolveDisplayUrl(...) for each result causes
synchronous S3 HEAD calls (PreferWebpImageDisplayUrlResolver.resolveDisplayUrl)
and creates a performance bottleneck; fix by avoiding per-item synchronous HEADs
— either add Spring `@Cacheable` to
PreferWebpImageDisplayUrlResolver.resolveDisplayUrl to cache WebP existence, or
change the flow in the ClubSearchService mapping to resolve URLs in
batch/parallel (e.g., collect all logo keys, perform parallel/batched HeadObject
requests, then map results back to ClubSearchResult), or precompute/store the
WebP URL at upload time and have imageDisplayUrlResolver return the stored value
(or swap to NoOpImageDisplayUrlResolver if WebP is not needed).
lepitaaar
left a comment
lepitaaar
left a comment
There was a problem hiding this comment.
webp 파일 존재할시 webp로 변환해서 보내주는작업 좋습니다~ 다만 매 요청마다 이미지 서버에 요청을 날리니 코드래빗 말대로 성능적인 이슈가 발생할꺼같습니다. 캐싱이던 초기 요청 이후로 이미지 주소를 변경해서 요청 횟수를 줄이는게 좋을꺼같아요
#️⃣연관된 이슈
#1133
📝작업 내용
방어 로직이라고 보시면 됩니다.
어차피 디비에 저장된 url 그대로 불러오니까요.
참고: Zepelown/image-converting-server
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit