-
Notifications
You must be signed in to change notification settings - Fork 0
[Refactor] Presigned URL 캐싱 #89
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
base: develop
Are you sure you want to change the base?
Conversation
""" Walkthrough이 변경사항은 Presigned URL을 S3에서 매번 요청하는 대신, Caffeine 기반의 캐시를 도입하여 URL을 재사용하도록 시스템을 리팩터링합니다. 이를 위해 캐시 설정, 저장소, 서비스, 테스트 코드 전반에 걸쳐 관련 클래스와 의존성을 추가 및 수정하였습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ImageRepository
participant CachePreSignedUrlRepository
participant S3ImageRepository
Client->>ImageRepository: getPresignedUrl(imageKey)
alt imageKey is null or empty
ImageRepository-->>Client: return null
else imageKey not null
ImageRepository->>CachePreSignedUrlRepository: get(imageKey)
alt cache hit
CachePreSignedUrlRepository-->>ImageRepository: Optional.of(url)
ImageRepository-->>Client: return url
else cache miss
CachePreSignedUrlRepository-->>ImageRepository: Optional.empty
ImageRepository->>S3ImageRepository: getPresignedUrl(imageKey)
S3ImageRepository-->>ImageRepository: url
ImageRepository->>CachePreSignedUrlRepository: put(imageKey, url)
ImageRepository-->>Client: return url
end
end
Suggested labels
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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: 5
🧹 Nitpick comments (2)
src/test/java/eatda/document/BaseDocumentTest.java (1)
56-58
: 불필요한 공백 라인 표시(~) 제거 권장라인 56에 변경 표시만 남아 있어 추후 diff 노이즈가 될 수 있습니다. 깔끔하게 정리해 주세요.
src/main/java/eatda/repository/CacheSetting.java (1)
16-17
: 생성자 매개변수명을 일반적인 이름으로 수정하세요.생성자의 첫 번째 매개변수가 "image"로 하드코딩되어 있습니다. 이는 다른 캐시 타입 추가 시 혼란을 야기할 수 있습니다.
- CacheSetting(String image, Duration timeToLive, int maximumSize) { - this.name = image; + CacheSetting(String name, Duration timeToLive, int maximumSize) { + this.name = name;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
build.gradle
(2 hunks)src/main/java/eatda/config/CacheConfig.java
(1 hunks)src/main/java/eatda/repository/CacheSetting.java
(1 hunks)src/main/java/eatda/repository/image/CachePreSignedUrlRepository.java
(1 hunks)src/main/java/eatda/repository/image/ImageDomain.java
(1 hunks)src/main/java/eatda/repository/image/ImageRepository.java
(1 hunks)src/main/java/eatda/repository/image/S3ImageRepository.java
(3 hunks)src/main/java/eatda/service/store/CheerService.java
(3 hunks)src/main/java/eatda/service/store/StoreService.java
(3 hunks)src/main/java/eatda/service/story/StoryService.java
(4 hunks)src/test/java/eatda/controller/BaseControllerTest.java
(3 hunks)src/test/java/eatda/controller/story/StoryControllerTest.java
(1 hunks)src/test/java/eatda/document/BaseDocumentTest.java
(1 hunks)src/test/java/eatda/document/story/StoryDocumentTest.java
(3 hunks)src/test/java/eatda/repository/BaseCacheRepositoryTest.java
(1 hunks)src/test/java/eatda/repository/BaseJpaRepositoryTest.java
(1 hunks)src/test/java/eatda/repository/image/ImageRepositoryTest.java
(1 hunks)src/test/java/eatda/repository/image/S3ImageRepositoryTest.java
(6 hunks)src/test/java/eatda/repository/store/CheerRepositoryTest.java
(1 hunks)src/test/java/eatda/service/BaseServiceTest.java
(3 hunks)src/test/java/eatda/service/story/StoryServiceTest.java
(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
src/test/java/eatda/controller/story/StoryControllerTest.java (1)
Learnt from: leegwichan
PR: YAPP-Github/26th-Web-Team-1-BE#60
File: src/test/java/eatda/controller/store/StoreControllerTest.java:10-32
Timestamp: 2025-07-09T07:56:50.612Z
Learning: 컨트롤러 테스트에서 MockitoBean으로 의존성을 모킹한 경우, 상세한 비즈니스 로직 검증보다는 컨트롤러 계층의 동작(라우팅, 파라미터 처리, 응답 구조 등)을 검증하는 것이 더 적절합니다. 모킹된 데이터에 대한 상세 검증은 의미가 없기 때문입니다.
src/test/java/eatda/controller/BaseControllerTest.java (1)
Learnt from: leegwichan
PR: YAPP-Github/26th-Web-Team-1-BE#60
File: src/test/java/eatda/controller/store/StoreControllerTest.java:10-32
Timestamp: 2025-07-09T07:56:50.612Z
Learning: 컨트롤러 테스트에서 MockitoBean으로 의존성을 모킹한 경우, 상세한 비즈니스 로직 검증보다는 컨트롤러 계층의 동작(라우팅, 파라미터 처리, 응답 구조 등)을 검증하는 것이 더 적절합니다. 모킹된 데이터에 대한 상세 검증은 의미가 없기 때문입니다.
🧬 Code Graph Analysis (2)
src/main/java/eatda/repository/image/S3ImageRepository.java (2)
src/main/java/eatda/repository/image/ImageRepository.java (1)
Component
(9-39)src/main/java/eatda/repository/image/CachePreSignedUrlRepository.java (1)
Component
(9-27)
src/main/java/eatda/repository/image/CachePreSignedUrlRepository.java (2)
src/main/java/eatda/repository/image/ImageRepository.java (1)
Component
(9-39)src/main/java/eatda/repository/image/S3ImageRepository.java (1)
Component
(20-97)
🔇 Additional comments (29)
src/main/java/eatda/repository/image/ImageDomain.java (1)
1-1
: 패키지 이동 후 전체 의존성 업데이트 확인 필요
eatda.service.common
→eatda.repository.image
로 패키지가 이동되었습니다. IDE 자동 리팩터링으로 누락된 import / FQN 참조가 없는지 전역 검색을 통해 한 번 더 확인해 주세요.src/test/java/eatda/repository/BaseJpaRepositoryTest.java (1)
16-16
:BaseJpaRepositoryTest
네이밍 변경 승인네이밍이 JPA 테스트 성격을 더 명확히 드러냅니다. 다른 테스트 클래스의 상속 선언도 모두 업데이트된 것 확인했습니다.
src/test/java/eatda/controller/story/StoryControllerTest.java (1)
43-44
:multiPart
가독성 개선 👍파라미터가 한눈에 들어와 유지보수가 용이해졌습니다. 기능-행위에는 영향 없습니다.
src/test/java/eatda/document/story/StoryDocumentTest.java (1)
64-66
: 코드 포맷팅 개선이 적절합니다.multiPart 메서드 호출을 여러 줄로 분할하여 가독성이 향상되었습니다.
build.gradle (2)
60-60
: 캐싱 기능을 위한 적절한 의존성 추가Spring Boot의 캐시 추상화를 위한 starter 의존성이 추가되었습니다.
71-71
: 고성능 인메모리 캐시 구현체 추가Caffeine은 높은 성능과 메모리 효율성을 제공하는 캐시 라이브러리로 25분 TTL 요구사항에 적합합니다.
src/main/java/eatda/service/store/StoreService.java (3)
12-12
: ImageService에서 ImageRepository로의 리팩터링이미지 관련 로직이 서비스 계층에서 리포지토리 계층으로 이동되어 아키텍처가 개선되었습니다.
29-29
: 의존성 필드명 업데이트ImageRepository로의 마이그레이션에 맞춰 필드명이 적절히 변경되었습니다.
41-41
: 메서드 호출 업데이트ImageRepository의 getPresignedUrl 메서드 호출로 변경되어 캐싱 기능이 활용됩니다.
src/test/java/eatda/repository/BaseCacheRepositoryTest.java (1)
1-13
: BaseCacheRepositoryTest의 CacheConfig 직접 호출 검증 완료 및 승인CacheConfig.cacheManager() 메서드는 SimpleCacheManager를 생성·설정·초기화까지 자체 처리하므로, 스프링 컨텍스트 없이도 프로덕션 환경과 동일한 캐시 설정을 재현합니다.
- src/main/java/eatda/config/CacheConfig.java:
- @bean으로 정의된 cacheManager() 내부에서
- SimpleCacheManager 생성
- setCaches(…) 호출
- afterPropertiesSet() 직접 호출
- 외부 의존성 없이 완전한 초기화 로직 수행
- BaseCacheRepositoryTest에서 new CacheConfig().cacheManager() 사용 시 설정 불일치 우려 없음
따라서 해당 테스트 기반 클래스 설계를 그대로 승인합니다.
src/test/java/eatda/controller/BaseControllerTest.java (3)
18-18
: ImageRepository로의 일관된 마이그레이션ImageService에서 ImageRepository로의 리팩터링이 테스트 코드에도 일관성 있게 적용되었습니다.
80-80
: MockitoBean 의존성 업데이트ImageRepository를 모킹하도록 변경되어 새로운 아키텍처와 일치합니다.
112-113
: 모킹 설정 업데이트가 적절합니다.ImageRepository의 메서드들에 대한 모킹 설정이 올바르게 업데이트되었으며, 컨트롤러 테스트에서는 상세한 비즈니스 로직보다는 컨트롤러 계층의 동작을 검증하는 것이 적절합니다.
src/main/java/eatda/service/store/CheerService.java (1)
6-6
: 의존성 변경이 깔끔하게 적용되었습니다.ImageService에서 ImageRepository로의 리팩터링이 일관성 있게 적용되었습니다. 기존 로직은 그대로 유지하면서 캐싱 기능이 추가된 새로운 구조로 전환되었습니다.
Also applies to: 19-19, 30-30
src/test/java/eatda/service/BaseServiceTest.java (1)
13-13
: 테스트 인프라가 새로운 아키텍처에 맞게 업데이트되었습니다.ImageRepository로의 변경사항과 StoryRepository 추가가 적절히 반영되었습니다. 모킹 설정도 새로운 구조에 맞게 올바르게 수정되었습니다.
Also applies to: 38-38, 58-64
src/test/java/eatda/repository/image/S3ImageRepositoryTest.java (1)
1-1
: 테스트 클래스가 새로운 구조에 맞게 성공적으로 마이그레이션되었습니다.ImageServiceTest에서 S3ImageRepositoryTest로의 변경이 올바르게 적용되었습니다. 패키지 이동, 클래스명 변경, 그리고 테스트 로직 업데이트가 일관성 있게 처리되었으며, 기존 테스트 커버리지가 유지되었습니다.
Also applies to: 30-30, 36-36, 40-42, 58-58, 81-81, 102-102, 125-125
src/test/java/eatda/service/story/StoryServiceTest.java (1)
16-16
: 테스트 코드가 새로운 아키텍처에 맞게 정확히 업데이트되었습니다.ImageRepository로의 변경사항이 일관성 있게 적용되었고, 불필요한 의존성도 정리되었습니다. 테스트 로직과 검증 내용은 그대로 유지되어 기능 테스트 커버리지가 보장됩니다.
Also applies to: 45-45, 93-94
src/main/java/eatda/repository/CacheSetting.java (1)
9-10
: 캐시 설정이 PR 목표에 부합하게 구성되었습니다.25분 TTL 설정이 30분보다 명확히 짧게 설정되어 PR 목표에 부합하며, 1000개 최대 크기도 이미지 URL 캐싱에 적절해 보입니다.
src/main/java/eatda/config/CacheConfig.java (2)
16-26
: 캐시 매니저 구성이 올바르게 구현되었습니다.CacheSetting enum을 기반으로 동적으로 캐시를 생성하는 방식은 유지보수성이 뛰어나며,
afterPropertiesSet()
호출도 적절합니다.
28-35
: Caffeine 캐시 설정이 적절합니다.TTL과 최대 크기 설정이 CacheSetting에서 올바르게 가져와지고 있으며, expireAfterWrite 정책이 적절합니다.
src/main/java/eatda/repository/image/CachePreSignedUrlRepository.java (1)
9-27
: 캐시 저장소 구현이 깔끔하고 안전합니다.
- Optional을 사용한 null-safe 반환
- 타입 안전한 캐시 조회
- 생성자 주입을 통한 의존성 관리
모든 부분이 적절하게 구현되었습니다.
src/main/java/eatda/service/story/StoryService.java (1)
11-12
: ImageRepository로의 의존성 변경이 올바르게 적용되었습니다.서비스 레이어에서 ImageService를 ImageRepository로 교체하여 캐싱 레이어를 투명하게 활용할 수 있게 되었습니다. 모든 메서드 호출이 일관되게 업데이트되었습니다.
Also applies to: 33-33, 42-42, 79-79
src/main/java/eatda/repository/image/ImageRepository.java (2)
17-23
: 업로드 시 캐시 예열 전략이 우수합니다.업로드 후 즉시 presigned URL을 생성하여 캐시에 저장하는 방식은, 사용자가 업로드한 이미지가 포함된 페이지로 이동할 가능성이 높다는 PR 목표와 잘 일치합니다.
25-38
: Cache-aside 패턴이 올바르게 구현되었습니다.
- 캐시 우선 조회
- 캐시 미스 시 S3에서 조회 후 캐시 저장
- null/empty 키에 대한 적절한 처리
구현이 안전하고 효율적입니다.
src/main/java/eatda/repository/image/S3ImageRepository.java (3)
1-1
: 서비스에서 리포지토리로의 아키텍처 개선이 적절합니다.
- 패키지 이동 및 어노테이션 변경이 레이어 책임에 맞게 수정됨
- @component 사용이 리포지토리 레이어에 적합함
Also applies to: 11-11, 20-21
27-28
: Presigned URL 유효기간 설정이 매우 적절합니다.캐시 TTL(25분)에 5분을 추가하여 30분으로 설정한 것은 캐시된 URL이 만료되기 전까지 항상 유효함을 보장하는 훌륭한 전략입니다.
80-80
: null 체크 제거가 아키텍처 개선과 일치합니다.ImageRepository에서 null 처리를 담당하므로 이 레이어에서는 null 체크가 불필요해졌습니다. 책임 분리가 잘 되었습니다.
src/test/java/eatda/repository/image/ImageRepositoryTest.java (2)
16-27
: 테스트 구조가 잘 설계되었습니다S3ImageRepository는 모킹하고 CachePreSignedUrlRepository는 실제 인스턴스를 사용하는 접근 방식이 적절합니다. 이를 통해 캐시 동작을 실제로 검증하면서 외부 의존성은 격리할 수 있습니다.
16-92
: 전반적으로 잘 구성된 테스트입니다테스트 구조와 시나리오 커버리지가 우수합니다. 다음과 같은 장점들이 있습니다:
- Nested 클래스를 활용한 명확한 테스트 구조
- 캐시 히트/미스 시나리오의 포괄적인 커버리지
- MockMultipartFile과 ParameterizedTest의 적절한 활용
- 한국어 테스트 메서드명으로 가독성 향상
제안된 Mock 검증 추가로 테스트의 완성도를 더욱 높일 수 있을 것입니다.
# Conflicts: # src/test/java/eatda/controller/story/StoryControllerTest.java # src/test/java/eatda/service/story/StoryServiceTest.java
|
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.
이번 PR도 고생하셨습니다! 🎉
이번 기능의 추가로 불필요한 S3 요청이 많이 줄어들겠네요
관련해 코멘트 남겼으니 확인해주세요
@Getter | ||
public enum CacheSetting { | ||
|
||
IMAGE("image", Duration.ofMinutes(25), 1_000), |
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.
[제안]
현재 개발, 프로덕션 인스턴스 메모리가 여유가 거의 없기에 1000개는 많을수도 있을것 같아요
우선 100개정도로만 조정해보는것은 어떨까요?
|
||
@Component | ||
@RequiredArgsConstructor | ||
public class ImageRepository { |
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.
[질문]
음.. 현재 ImageRepository
는 실질적으로 S3 업로드, Presigned URL 생성, 캐시 저장/탐색 등
도메인 수준의 정책 판단과 흐름 제어를 포함하고 있어 보이는데요
이런 역할은 보통 Service 계층에서 수행하는 책임으로 보고,
지금 구조는 이름만 Repository이지 실제로는 완전히 Service의 역할을 하고 있다고 느껴지는데요.
왜 계층을 바꾸신 건지 잘 이해가 되지 않아요 😿
혹시 단순히 기술적 의존성을 줄이려는 의도라면, Service 내부에서 cache + S3를 위임하는 구조로도 충분히 가능한데,
이걸 Repository로 옮긴 이유가 궁금합니다.
@RequiredArgsConstructor | ||
public class ImageRepository { | ||
|
||
private final S3ImageRepository s3Repository; |
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.
[질문]
클래스명과 변수명이 다른건 의도하신건가요?
Optional<String> cachedUrl = cachePreSignedUrlRepository.get(imageKey); | ||
if (cachedUrl.isPresent()) { | ||
return cachedUrl.get(); | ||
} |
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.
공백이 추가되면 가독성이 높아질것 같아요!
|
||
private static final Set<String> ALLOWED_CONTENT_TYPES = Set.of("image/jpg", "image/jpeg", "image/png"); | ||
private static final String DEFAULT_CONTENT_TYPE = "bin"; | ||
private static final String PATH_DELIMITER = "/"; | ||
private static final String EXTENSION_DELIMITER = "."; | ||
private static final Duration PRESIGNED_URL_DURATION = Duration.ofMinutes(30); | ||
private static final Duration PRESIGNED_URL_DURATION = CacheSetting.IMAGE.getTimeToLive() |
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.
[수정]
현재 PRESIGNED_URL_DURATION 설정이 CacheSetting의 TTL에 의존하게 바뀌었는데,
정책 상으로는 오히려 Presigned URL의 유효시간이 기준이 되고,
캐시 TTL은 그보다 짧게 설정되어야 하지 않을까요?
Duration.ofMinutes(30) 등으로 기준을 고정하고
캐시는 PRESIGNED_URL_DURATION 을 기준으로 바뀌는게 좋을것 같아요
.containsExactlyInAnyOrder( | ||
"https://s3.bucket.com/story/dummy/2.jpg", | ||
"https://s3.bucket.com/story/dummy/1.jpg" | ||
); |
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.
위에 when과 extracting 관련 검증이 사라진 이유는 무엇인가요?
✨ 개요
🧾 관련 이슈
closed #82
🔍 참고 사항 (선택)
캐싱 정책
구현 방향
Summary by CodeRabbit
신규 기능
리팩터링
테스트