-
Notifications
You must be signed in to change notification settings - Fork 0
Feature / s3 / image path #128
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
Walkthrough새로운 Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 특별 검토 항목:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
🧹 Nitpick comments (2)
src/main/java/redot/redot_server/global/s3/util/ImageUrlResolver.java (1)
30-44: 중복 검사가 불필요합니다.Line 39-42의
isExternalUrl재검사는 불필요합니다. Line 36에서 이미 외부 URL이 아님을 확인했고,normalizePath는 외부 URL을 그대로 반환하거나 선행 슬래시만 추가하므로 외부 URL로 변환하지 않습니다.다음과 같이 리팩토링할 수 있습니다:
public String toPublicUrl(String pathOrUrl) { if (!StringUtils.hasText(pathOrUrl)) { return null; } String value = pathOrUrl.trim(); if (isExternalUrl(value) || !hasCdnBaseUrl) { return normalizePath(value); } - String normalizedPath = normalizePath(value); - if (isExternalUrl(normalizedPath)) { - return normalizedPath; - } - return cdnBaseUrl + normalizedPath; + return cdnBaseUrl + normalizePath(value); }src/main/java/redot/redot_server/global/s3/service/ImageStorageService.java (1)
34-40: 삭제 로직이 올바르지만 최적화 가능합니다.
toS3Key를 사용하여 URL을 S3 키로 변환하는 것은 올바릅니다. 하지만 Line 36의s3Manager.exists(key)검사는 불필요할 수 있습니다.S3Manager.deleteFile이 이미NoSuchKeyException을 처리하므로, 사전 존재 확인은 추가 S3 API 호출을 발생시킵니다.다음과 같이 리팩토링할 수 있습니다:
public void delete(String imageUrl) throws ImageUploadException { String key = imageUrlResolver.toS3Key(imageUrl); - if (!StringUtils.hasText(key) || !s3Manager.exists(key)) { + if (!StringUtils.hasText(key)) { return; } s3Manager.deleteFile(key); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/main/java/redot/redot_server/domain/admin/dto/response/AdminResponse.java(2 hunks)src/main/java/redot/redot_server/domain/admin/service/AdminService.java(5 hunks)src/main/java/redot/redot_server/domain/auth/service/AdminAuthService.java(3 hunks)src/main/java/redot/redot_server/domain/auth/service/CMSAuthService.java(3 hunks)src/main/java/redot/redot_server/domain/auth/service/RedotMemberAuthService.java(4 hunks)src/main/java/redot/redot_server/domain/cms/member/dto/response/CMSMemberResponse.java(2 hunks)src/main/java/redot/redot_server/domain/cms/member/service/CMSMemberService.java(5 hunks)src/main/java/redot/redot_server/domain/cms/site/setting/dto/response/SiteSettingResponse.java(2 hunks)src/main/java/redot/redot_server/domain/cms/site/setting/service/SiteSettingService.java(4 hunks)src/main/java/redot/redot_server/domain/redot/app/service/RedotAppCreationService.java(3 hunks)src/main/java/redot/redot_server/domain/redot/app/service/RedotAppService.java(4 hunks)src/main/java/redot/redot_server/domain/redot/member/dto/response/RedotMemberResponse.java(2 hunks)src/main/java/redot/redot_server/domain/redot/member/service/RedotMemberService.java(3 hunks)src/main/java/redot/redot_server/global/s3/service/ImageStorageService.java(2 hunks)src/main/java/redot/redot_server/global/s3/util/ImageUrlResolver.java(1 hunks)src/main/java/redot/redot_server/global/s3/util/S3Manager.java(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/redot/redot_server/global/s3/service/ImageStorageService.java (1)
src/main/java/redot/redot_server/global/s3/util/ImagePathGenerator.java (1)
ImagePathGenerator(8-35)
🔇 Additional comments (26)
src/main/java/redot/redot_server/global/s3/util/S3Manager.java (2)
59-62: 파라미터 이름 변경 및 검증 로직 개선이 적절합니다.
filePath에서key로 파라미터 이름을 변경한 것은 S3 API의 용어와 일치하며,StringUtils.hasText()사용은 Spring의 관례를 따릅니다.ImageUrlResolver.toS3Key()가 이미 선행 슬래시를 제거하므로 이 변경사항은 전체 설계와 잘 통합됩니다.
86-88: 방어적 프로그래밍이 적절하게 적용되었습니다.빈 키에 대한 조기 반환은 불필요한 S3 API 호출을 방지하며,
deleteFile의 검증 방식과 일관성을 유지합니다.src/main/java/redot/redot_server/global/s3/util/ImageUrlResolver.java (4)
16-25: 생성자 구현이 올바릅니다.CDN base URL의 선택적 설정을 적절히 처리하고 있으며, 후행 슬래시 제거와
hasCdnBaseUrl플래그를 통한 최적화가 잘 구현되어 있습니다.
49-58: 구현이 올바릅니다.CDN URL을 내부 저장 경로로 변환하는 로직이 적절하며, 외부 URL과 내부 경로를 올바르게 구분하여 처리합니다.
63-69: S3 키 변환 로직이 올바릅니다.
toStoredPath를 재사용하고 선행 슬래시를 제거하여 S3 API 규격에 맞는 키를 생성합니다. 외부 URL에 대해null을 반환하는 것도 적절합니다.
71-90: 헬퍼 메서드들이 올바르게 구현되었습니다.각 헬퍼 메서드가 명확한 책임을 가지며, 로직이 정확합니다.
isExternalUrl은 HTTP/HTTPS 프로토콜만 확인하지만 현재 요구사항에는 충분합니다.src/main/java/redot/redot_server/domain/auth/service/RedotMemberAuthService.java (1)
34-34: ImageUrlResolver 통합이 일관되게 적용되었습니다.
signUp과getCurrentMember메서드 모두에서imageUrlResolver를 응답 생성 시 전달하여 이미지 URL을 올바르게 변환합니다.Also applies to: 58-58, 112-112
src/main/java/redot/redot_server/domain/admin/dto/response/AdminResponse.java (1)
14-22: 팩토리 메서드 업데이트가 올바릅니다.
ImageUrlResolver를 파라미터로 받아 프로필 이미지 URL을 공개 URL로 변환하는 패턴이 일관되게 적용되었습니다.src/main/java/redot/redot_server/domain/auth/service/CMSAuthService.java (1)
35-35: ImageUrlResolver 통합이 적절합니다.
getCurrentCMSMemberInfo에서 응답 생성 시imageUrlResolver를 전달하여 이미지 URL 변환을 처리합니다.Also applies to: 81-81
src/main/java/redot/redot_server/domain/redot/member/dto/response/RedotMemberResponse.java (1)
14-26: 팩토리 메서드들이 일관되게 업데이트되었습니다.
fromEntity와fromNullable모두ImageUrlResolver를 받아 프로필 이미지 URL을 공개 URL로 변환하며,fromNullable이fromEntity를 재사용하는 구조가 깔끔합니다.src/main/java/redot/redot_server/global/s3/service/ImageStorageService.java (1)
26-32: 업로드 후 URL 변환이 올바르게 구현되었습니다.파일을 S3에 업로드한 후 저장 경로를 공개 URL로 변환하여 반환하는 흐름이 적절합니다.
src/main/java/redot/redot_server/domain/cms/member/dto/response/CMSMemberResponse.java (1)
17-28: 팩토리 메서드 업데이트가 일관됩니다.다른 응답 DTO들과 동일한 패턴으로
ImageUrlResolver를 받아 프로필 이미지 URL을 공개 URL로 변환합니다.src/main/java/redot/redot_server/domain/auth/service/AdminAuthService.java (1)
69-74: LGTM!
getCurrentAdminInfo메서드에서AdminResponse.from(admin, imageUrlResolver)를 사용하여 이미지 URL을 CDN URL로 변환하는 패턴이 올바르게 적용되었습니다.src/main/java/redot/redot_server/domain/cms/member/service/CMSMemberService.java (2)
74-85: LGTM!
updateCMSMember에서imageUrlResolver.toStoredPath()를 사용하여 입력 URL을 저장 경로로 변환하고, 저장된 경로와 비교하여 이전 이미지 삭제 여부를 판단하는 로직이 올바르게 구현되었습니다.
112-119: 이 주석은 제거 가능합니다
imageStorageService.upload()는 이미toPublicUrl()변환을 내부에서 처리하여 CDN URL을 반환합니다. ImageStorageService 구현(line 31)에서return imageUrlResolver.toPublicUrl(storedPath);로 확인됩니다. 따라서uploadProfileImage()에서 반환되는 URL은 이미 CDN URL이며, 추가 변환이 필요하지 않습니다.src/main/java/redot/redot_server/domain/redot/app/service/RedotAppCreationService.java (1)
87-92: LGTM!
initializeAppAssets에서SiteSettingResponse.fromEntity와RedotMemberResponse.fromNullable에imageUrlResolver를 올바르게 전달하여 이미지 URL 변환 패턴이 일관되게 적용되었습니다.src/main/java/redot/redot_server/domain/redot/app/service/RedotAppService.java (2)
76-82: LGTM!
getRedotAppInfo에서SiteSettingResponse와RedotMemberResponse생성 시imageUrlResolver를 올바르게 전달하고 있습니다.
97-103: LGTM!
getRedotAppList의 페이지네이션 매핑에서도 동일한 패턴으로imageUrlResolver가 일관되게 적용되었습니다.src/main/java/redot/redot_server/domain/cms/site/setting/service/SiteSettingService.java (2)
48-57: LGTM!
updateSiteSetting에서imageUrlResolver.toStoredPath()를 사용하여 로고 URL을 저장 경로로 변환하고, 이전 로고와 비교하여 삭제 이벤트를 발행하는 로직이 올바르게 구현되었습니다.
68-76:uploadLogoImage에서 반환되는 URL 형식 확인 필요
imageStorageService.upload()의 반환값이 직접UploadedImageUrlResponse에 사용됩니다. 다른 서비스의 유사한uploadProfileImage메서드들과 동일한 패턴인데, 이 반환값이 CDN URL인지 저장 경로인지 확인이 필요합니다. 프론트엔드에서 업로드 후 바로 이미지를 표시하려면 CDN URL이 반환되어야 합니다.src/main/java/redot/redot_server/domain/redot/member/service/RedotMemberService.java (2)
70-82: LGTM!
updateRedotMemberInfo에서imageUrlResolver.toStoredPath()로 입력 URL을 저장 경로로 변환하고, 이전 이미지와 비교 후 삭제 이벤트를 발행하며, 응답 생성 시imageUrlResolver를 전달하는 패턴이 올바르게 적용되었습니다.
47-48: 외부 URL은 올바르게 처리되고 있습니다.검증 결과,
profile.profileImageUrl()이 변환 없이 저장되는 것이 맞으며,ImageUrlResolver.toPublicUrl()에서도 외부 URL을 올바르게 처리합니다.
toPublicUrl()메서드는http://또는https://로 시작하는 외부 URL을 감지하여 그대로 반환합니다 (CDN 경로 추가 없음)- 소셜 프로바이더(Google, Kakao 등)의 외부 URL은
normalizePath()를 거쳐도 변환되지 않고 반환됩니다따라서 현재 구현은 안전하고 추가 조치가 필요하지 않습니다.
src/main/java/redot/redot_server/domain/admin/service/AdminService.java (3)
46-55: LGTM!
createAdmin에서imageUrlResolver.toStoredPath()를 사용하여 프로필 이미지 URL을 저장 경로로 변환한 후Admin.create()에 전달하고, 응답 생성 시imageUrlResolver를 전달하는 패턴이 올바르게 적용되었습니다.
84-89: LGTM!
updateAdmin에서 저장 경로 변환, 이전 이미지 삭제 확인, 엔티티 업데이트, 응답 생성의 전체 흐름이 일관되게 구현되었습니다.
126-133:ImageStorageService.upload()이미 CDN URL을 반환하므로 추가 조치 불필요
ImageStorageService.upload()메서드는 저장된 경로를imageUrlResolver.toPublicUrl()로 변환하여 CDN이 붙은 공개 URL을 반환합니다(줄 31). 따라서 프론트엔드가 업로드 응답을 받은 후 바로 이미지를 표시할 수 있으며, 이후 업데이트 요청 시 해당 URL을 다시 전송하는 예상 흐름이 올바르게 작동합니다. 모든 서비스(AdminService,CMSMemberService,SiteSettingService,RedotMemberService)가 일관되게 같은 방식으로 처리하고 있습니다.src/main/java/redot/redot_server/domain/cms/site/setting/dto/response/SiteSettingResponse.java (1)
14-23: LGTM!
fromEntity메서드에서imageUrlResolver.toPublicUrl()을 사용하여 저장된 로고 URL을 CDN URL로 변환하는 패턴이 올바르게 적용되었습니다.siteSetting.getLogoUrl()이 null일 경우에도toPublicUrl()메서드의 31-33줄에서StringUtils.hasText()검사를 통해 null을 안전하게 처리하고 null을 반환하므로 문제없습니다.
Overview
Related Issue
PR Checklist
Additional Information