Skip to content

[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

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

Conversation

leegwichan
Copy link
Member

@leegwichan leegwichan commented Jul 19, 2025

✨ 개요

  • 문제점 : 이미지가 들어있는 모든 요소마다 S3에 요청하여 PreSigned Url 을 받아오는 비용이 비쌈
  • 해결 방안 : Spring 내부에서 사용할 수 있는 캐시를 이용하여, PreSigned Url 을 25분간 캐싱함

🧾 관련 이슈

closed #82

🔍 참고 사항 (선택)

캐싱 정책

  • TTL : 25분 (30분 보다는 명확하게 짧아야 할 것 같아 설정함)
  • 캐시에 URL을 넣어주는 경우
    • 캐시에 URL이 없어 s3에서 URL을 만든 경우
    • 이미지를 업로드 한 이후 (비즈니스 플로우 상, 자신이 이미지를 넣은 스토리/응원 을 만들면 해당 페이지로 이동하기 때문에)

구현 방향

  • 기존 ImageService의 로직을 S3ImageRepository 로 옮김
    • 이 부분에 대해 Repository 라고 부를 수 있을까 고민했지만, 일단 구현의 용이성을 위해 옮겼습니다.
    • 다른 한편으로는 이미지를 저장/조회 하는 기능이니까 하나의 저장소라고 볼 수도 있다고 생각했습니다. (물론 각종 유효성 검사 로직이 있지는 하지만...)
  • ImageRepository 에서 S3ImageRepository와 CachePreSignedUrlRepository 를 통해 전반적으로 조회/저장 시 캐싱 로직을 관리함

Summary by CodeRabbit

  • 신규 기능

    • 이미지의 사전 서명 URL을 메모리 캐시에 저장하여 반복 요청 시 성능을 향상시키는 캐싱 기능이 추가되었습니다.
  • 리팩터링

    • 이미지 관련 서비스 구조가 개선되어, 이미지 업로드 및 URL 조회 로직이 새로운 저장소 클래스로 이전되었습니다.
    • 기존 이미지 서비스가 리포지토리 구조로 변경되고, 관련 의존성 주입 및 호출 부분이 모두 업데이트되었습니다.
  • 테스트

    • 이미지 캐싱 및 저장소 관련 단위 테스트와 통합 테스트가 추가 및 개선되었습니다.
    • 기존 테스트 코드가 새로운 이미지 저장소 구조에 맞게 수정되었습니다.
    • 캐시 설정 및 동작 검증을 위한 테스트 기반 클래스가 추가되었습니다.

Copy link

coderabbitai bot commented Jul 19, 2025

"""

Walkthrough

이 변경사항은 Presigned URL을 S3에서 매번 요청하는 대신, Caffeine 기반의 캐시를 도입하여 URL을 재사용하도록 시스템을 리팩터링합니다. 이를 위해 캐시 설정, 저장소, 서비스, 테스트 코드 전반에 걸쳐 관련 클래스와 의존성을 추가 및 수정하였습니다.

Changes

파일/그룹 변경 요약
build.gradle Spring Cache 및 Caffeine 의존성 추가
src/main/java/eatda/config/CacheConfig.java Caffeine 기반 캐시 매니저 설정 클래스 추가
src/main/java/eatda/repository/CacheSetting.java 캐시 설정용 Enum 추가 (캐시 이름, TTL, 최대 크기 등)
src/main/java/eatda/repository/image/CachePreSignedUrlRepository.java Presigned URL 캐싱용 저장소 클래스 추가
src/main/java/eatda/repository/image/ImageDomain.java 패키지 변경 (service.commonrepository.image)
src/main/java/eatda/repository/image/ImageRepository.java Presigned URL 캐싱 및 업로드 로직을 담당하는 저장소 클래스 추가
src/main/java/eatda/repository/image/S3ImageRepository.java 클래스명 및 패키지 변경, Presigned URL 만료시간 설정, 널 체크 제거 등 리팩터링
src/main/java/eatda/service/store/CheerService.java
src/main/java/eatda/service/store/StoreService.java
src/main/java/eatda/service/story/StoryService.java
ImageServiceImageRepository로 의존성 및 호출 변경
src/test/java/eatda/controller/BaseControllerTest.java
src/test/java/eatda/service/BaseServiceTest.java
테스트에서 ImageServiceImageRepository로 변경, 목킹 로직 수정
src/test/java/eatda/document/BaseDocumentTest.java
src/test/java/eatda/document/story/StoryDocumentTest.java
src/test/java/eatda/controller/story/StoryControllerTest.java
불필요한 ImageService 관련 코드 및 import 제거, 포맷팅 개선
src/test/java/eatda/repository/BaseCacheRepositoryTest.java 캐시 관련 테스트 베이스 클래스 추가
src/test/java/eatda/repository/BaseJpaRepositoryTest.java
src/test/java/eatda/repository/store/CheerRepositoryTest.java
테스트 베이스 클래스명 및 상속 구조 변경
src/test/java/eatda/repository/image/ImageRepositoryTest.java ImageRepository 캐싱 동작 테스트 신규 추가
src/test/java/eatda/repository/image/S3ImageRepositoryTest.java S3 저장소 테스트 클래스명 및 패키지 변경, 테스트 대상 클래스 변경
src/test/java/eatda/service/story/StoryServiceTest.java imageServiceimageRepository로 테스트 전환, 불필요한 필드/설정 제거

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
Loading

Suggested labels

released on @beta

Poem

🐇
캐시 속에 URL을 숨겨
S3에 덜 물어보는 오늘,
토끼는 뛴다, 빨라진 이미지처럼!
캐피인과 스프링이 손을 잡고
프리사인드 URL도 이제는 똑똑하게,
한 번만 꺼내 쓰고 또 쓰고,
성능 향상에 깡총깡총!
🥕✨
"""


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1310295 and 34f60ed.

📒 Files selected for processing (2)
  • src/main/java/eatda/config/CacheConfig.java (1 hunks)
  • src/test/java/eatda/repository/image/ImageRepositoryTest.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/main/java/eatda/config/CacheConfig.java
  • src/test/java/eatda/repository/image/ImageRepositoryTest.java
✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 65390cc and 57ec26d.

📒 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.commoneatda.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
Copy link

@leegwichan leegwichan self-assigned this Jul 20, 2025
@leegwichan leegwichan requested a review from lvalentine6 July 20, 2025 03:18
Copy link
Member

@lvalentine6 lvalentine6 left a 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),
Copy link
Member

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 {
Copy link
Member

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;
Copy link
Member

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();
}
Copy link
Member

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()
Copy link
Member

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"
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위에 when과 extracting 관련 검증이 사라진 이유는 무엇인가요?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants