refactor: s3 버전 업그레이드 및 로직 수정 #608
Conversation
- 의존성 수정 - 버전 업그레이드에 따른 코드 수정
- ImgType에서 UploadType으로 변경 - 해당되는 파일 모두 수정
lsy1307
requested review from
Hexeong,
JAEHEE25 and
sukangpunch
as code owners
Walkthrough
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. 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: 6
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/com/example/solidconnection/s3/service/S3Service.java (1)
93-93: "word" 확장자가 올바르지 않습니다.
"word"는 표준 파일 확장자가 아닙니다. Microsoft Word 97-2003 문서의 확장자는"doc"입니다.🐛 제안하는 수정
- List<String> allowedExtensions = Arrays.asList("jpg", "jpeg", "png", "webp", "pdf", "word", "docx"); + List<String> allowedExtensions = Arrays.asList("jpg", "jpeg", "png", "webp", "pdf", "doc", "docx");
🤖 Fix all issues with AI agents
In `@src/main/java/com/example/solidconnection/s3/service/FileUploadService.java`:
- Around line 41-43: In the S3Exception catch block inside FileUploadService
(catch (S3Exception e)), make the log.error call null-safe by not calling
e.awsErrorDetails().errorMessage() directly; instead check e.awsErrorDetails()
for null and fallback to e.getMessage() or e.toString() (or use
Optional.ofNullable(e.awsErrorDetails()).map(d ->
d.errorMessage()).orElse(e.getMessage())) and log that string, then rethrow the
same CustomException(S3_SERVICE_EXCEPTION) (optionally include the original
exception as the cause).
- Around line 27-28: You currently have `@EnableAsync` scattered across components
(e.g., UpdateViewCountScheduler, UpdateViewCountService) while methods like
uploadFile(String bucket, String fileName, MultipartFile multipartFile) rely on
`@Async`; create a single `@Configuration` class (e.g., AsyncConfig) and place
`@EnableAsync` there, remove all other `@EnableAsync` annotations from
components/services, ensure Spring picks up the config via component scanning or
`@Import` so `@Async` on uploadFile continues to work.
In `@src/main/java/com/example/solidconnection/s3/service/S3Service.java`:
- Around line 68-70: The code is invoking FileUploadService.uploadFile()
asynchronously twice (asyncExecutor.submit(...) wrapping a method already
annotated with `@Async`), causing redundant threading and unpredictable behavior;
remove the outer asyncExecutor.submit(...) wrapper and call
fileUploadService.uploadFile(bucket, uploadPath, multipartFile) directly so the
`@Async` on FileUploadService.uploadFile handles asynchrony, or alternatively
remove the `@Async` annotation from FileUploadService.uploadFile if you prefer
using asyncExecutor.submit() as the single async mechanism — choose one approach
and eliminate the other to avoid double asynchronous execution.
In `@src/main/resources/secret`:
- Line 1: Add an explicit exclusion for the secret submodule directory in the
Gradle bootJar configuration to prevent accidental packaging of
src/main/resources/secret; update the bootJar task configuration
(tasks.named('bootJar') / bootJar closure) in build.gradle to exclude the
secret/** path so the secret submodule is never included in built jars even if
the submodule becomes initialized.
In
`@src/test/java/com/example/solidconnection/news/service/NewsCommandServiceTest.java`:
- Around line 186-190: The assertion in NewsCommandServiceTest uses
then(s3Service).should(never()).uploadFile(null, UploadType.NEWS), which only
prevents a null argument and can miss calls with non-null files; update the
verification to assert no uploadFile call with any file for UploadType.NEWS
(e.g., use any(File.class) or Mockito.any() for the first argument and
eq(UploadType.NEWS) for the second) so the mock truly verifies that no upload
was invoked, and apply the same replacement for the similar verification around
lines 248-253.
In `@src/test/java/com/example/solidconnection/s3/service/S3ServiceTest.java`:
- Around line 21-36: The test mixes Mockito unit setup and a Spring integration
context: in S3ServiceTest you must choose one approach—either convert to a pure
unit test by removing `@TestContainerSpringBootTest` and adding
`@ExtendWith`(MockitoExtension.class) (keep `@InjectMocks` private S3Service
s3Service and `@Mock` S3Client, FileUploadService, ThreadPoolTaskExecutor), or
convert to a Spring integration test by removing `@InjectMocks` and replacing
`@Mock` with `@MockBean` and injecting the real bean (use `@Autowired` private
S3Service s3Service) so Spring wiring is used and only S3Client (and other
external collaborators) are mocked; update the test class annotations and field
annotations accordingly to match the chosen mode.
🧹 Nitpick comments (6)
src/main/java/com/example/solidconnection/s3/service/FileUploadService.java (1)
16-16: 미사용 import를 제거해 주세요.
ObjectCannedACL이 import되었지만 코드에서 사용되지 않습니다. 불필요한 import는 코드 가독성을 해칠 수 있습니다.♻️ 제안하는 수정
-import software.amazon.awssdk.services.s3.model.ObjectCannedACL;build.gradle (1)
68-69: AWS SDK v2로의 깔끔한 마이그레이션이 잘 진행되고 있습니다.
BOM 패턴 활용:
platform('software.amazon.awssdk:bom:2.41.4')를 통해 AWS SDK v2의 버전을 일관되게 관리하는 올바른 접근 방식입니다.S3 모듈 선언: BOM이 버전을 관리하므로
software.amazon.awssdk:s3만 명시하면 되며, 이는 깔끔하고 유지보수하기 좋은 구조입니다.버전 업그레이드 권장: 현재 AWS SDK for Java v2의 최신 버전은 2.41.7입니다 (2026년 1월 13일 릴리스). 2.41.4는 안정적이지만, 최신 버전으로 업데이트하면 최근 버그 수정 및 개선 사항을 반영할 수 있습니다.
src/main/java/com/example/solidconnection/chat/service/ChatService.java (1)
247-258: 이미지 확장자 리스트를 상수로 추출하는 것을 권장합니다.현재 로직은 정확하게 동작하지만, 몇 가지 개선점이 있습니다:
- 상수 추출:
imageExtensions리스트가 메서드 내부에 하드코딩되어 있어 재사용성이 떨어집니다.- 확장자 누락 가능성:
gif,bmp,svg등 다른 이미지 형식도 고려해볼 수 있습니다.♻️ 상수로 추출하는 리팩터링 제안
+ private static final Set<String> IMAGE_EXTENSIONS = Set.of("jpg", "jpeg", "png", "webp", "gif"); + `@Transactional` public void sendChatImage(ChatImageSendRequest chatImageSendRequest, long siteUserId, long roomId) { // ... existing code ... ChatMessage chatMessage = new ChatMessage("", senderId, chatRoom); - // 이미지 판별을 위한 확장자 리스트 - List<String> imageExtensions = Arrays.asList("jpg", "jpeg", "png", "webp"); for (String imageUrl : chatImageSendRequest.imageUrls()) { String extension = StringUtils.getFilenameExtension(imageUrl); - boolean isImage = extension != null && imageExtensions.contains(extension.toLowerCase()); + boolean isImage = extension != null && IMAGE_EXTENSIONS.contains(extension.toLowerCase());src/test/java/com/example/solidconnection/s3/service/S3ServiceTest.java (1)
37-41: 테스트 헬퍼 메서드 개선을 권장합니다.
createMockFile에서 content type이image/jpeg로 고정되어 있어 PDF/DOCX 테스트(Line 111-112)와 일치하지 않습니다.♻️ Content type을 파라미터로 받는 오버로드 추가
private MockMultipartFile createMockFile(String originalName, long size) { - return new MockMultipartFile("file", originalName, "image/jpeg", new byte[(int) size]); + String contentType = guessContentType(originalName); + return new MockMultipartFile("file", originalName, contentType, new byte[(int) size]); } + +private String guessContentType(String filename) { + if (filename.endsWith(".pdf")) return "application/pdf"; + if (filename.endsWith(".docx")) return "application/vnd.openxmlformats-officedocument.wordprocessingml.document"; + if (filename.endsWith(".png")) return "image/png"; + return "image/jpeg"; +}src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java (1)
186-193: 1) “증빙서류” 픽스처가 jpg라서, 확장자/이미지 분기 로직에 휘말릴 수 있습니다.
- PR 목표가 “확장자 포함” 및 “파일 종류 분기”라서, 여기서는 pdf로 맞추는 게 더 안전합니다.제안 변경(diff)
private MockMultipartFile createMentorProofFile() { return new MockMultipartFile( "image", - "test.jpg", - "image/jpeg", + "test.pdf", + "application/pdf", "test image content".getBytes() ); }src/main/java/com/example/solidconnection/news/service/NewsCommandService.java (1)
71-80: 1) 업로드 응답 null 가능성이 “정말 0%”가 아니라면, 여기서 한 번 더 못 박는 게 안전합니다.
-uploadedFile또는fileUrl()이 null이면 NPE로 터질 수 있습니다(계약이 강하면 스킵 가능).제안 변경(diff)
} else if (imageFile != null && !imageFile.isEmpty()) { UploadedFileUrlResponse uploadedFile = s3Service.uploadFile(imageFile, UploadType.NEWS); + if (uploadedFile == null || uploadedFile.fileUrl() == null) { + throw new CustomException(/* 적절한 에러코드 */); + } deleteCustomImage(news.getThumbnailUrl()); news.updateThumbnailUrl(uploadedFile.fileUrl()); }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
build.gradlesrc/main/java/com/example/solidconnection/chat/service/ChatService.javasrc/main/java/com/example/solidconnection/community/post/service/PostCommandService.javasrc/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.javasrc/main/java/com/example/solidconnection/news/service/NewsCommandService.javasrc/main/java/com/example/solidconnection/s3/config/AmazonS3Config.javasrc/main/java/com/example/solidconnection/s3/controller/S3Controller.javasrc/main/java/com/example/solidconnection/s3/domain/UploadType.javasrc/main/java/com/example/solidconnection/s3/service/FileUploadService.javasrc/main/java/com/example/solidconnection/s3/service/S3Service.javasrc/main/java/com/example/solidconnection/score/service/ScoreService.javasrc/main/java/com/example/solidconnection/siteuser/service/MyPageService.javasrc/main/resources/secretsrc/test/java/com/example/solidconnection/community/post/service/PostCommandServiceTest.javasrc/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.javasrc/test/java/com/example/solidconnection/news/service/NewsCommandServiceTest.javasrc/test/java/com/example/solidconnection/s3/service/S3ServiceTest.javasrc/test/java/com/example/solidconnection/score/service/ScoreServiceTest.javasrc/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-02T08:56:21.107Z
Learnt from: whqtker
Repo: solid-connection/solid-connect-server PR: 602
File: build.gradle:72-74
Timestamp: 2026-01-02T08:56:21.107Z
Learning: In the solid-connect-server project, when using Spring Boot 3.1.5, constrain datasource-micrometer to 1.2.x in Gradle dependencies. Do not use datasource-micrometer 2.x with Spring Boot 3.x, as 2.x targets Spring Boot 4.x. Ensure dependency declarations specify the 1.2.x version for compatibility.
Applied to files:
build.gradle
📚 Learning: 2026-01-02T08:56:21.107Z
Learnt from: whqtker
Repo: solid-connection/solid-connect-server PR: 602
File: build.gradle:72-74
Timestamp: 2026-01-02T08:56:21.107Z
Learning: In Gradle build files, avoid declaring datasource-proxy explicitly when using net.ttddyy.observation:datasource-micrometer, because it is a transitive dependency of datasource-micrometer. Remove the explicit datasource-proxy dependency declaration across Gradle scripts to prevent duplication. After changes, run a clean build to ensure no unresolved or duplicated classes and verify that datasource-proxy is still available transitively.
Applied to files:
build.gradle
📚 Learning: 2025-11-17T06:30:49.502Z
Learnt from: sukangpunch
Repo: solid-connection/solid-connect-server PR: 562
File: src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java:10-23
Timestamp: 2025-11-17T06:30:49.502Z
Learning: MentorApplication 도메인에서 universityId는 null일 수 있으며, MentorApplicationRequest에서도 이 필드에 대한 NotNull validation을 추가하지 않아야 한다.
Applied to files:
src/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.javasrc/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java
🧬 Code graph analysis (1)
src/main/java/com/example/solidconnection/s3/service/FileUploadService.java (1)
src/main/java/com/example/solidconnection/common/exception/CustomExceptionHandler.java (1)
Slf4j(22-88)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (25)
src/main/java/com/example/solidconnection/s3/domain/UploadType.java (1)
6-20: LGTM! 명확한 네이밍 개선입니다.
- Enum 이름 변경:
ImgType→UploadType으로 변경하여 이미지 외 파일도 관리할 수 있음을 명확히 표현했습니다.- CHAT 경로 변경:
"chat"→"chat/files"로 변경하여 채팅 파일의 저장 경로가 더 명확해졌습니다.- 생성자 일관성: 생성자 이름도 enum 이름에 맞게 적절히 변경되었습니다.
src/main/java/com/example/solidconnection/s3/config/AmazonS3Config.java (1)
24-31: AWS SDK v2 클라이언트 설정이 올바르게 마이그레이션되었습니다.
- 빌더 패턴 사용:
S3Client.builder()를 사용하여 클라이언트를 구성합니다.- 자격 증명 설정:
AwsBasicCredentials와StaticCredentialsProvider를 사용하여 고정 자격 증명을 제공합니다.- 리전 설정:
Region.of(region)으로 문자열 리전을 SDK Region 객체로 변환합니다.src/main/java/com/example/solidconnection/s3/service/S3Service.java (2)
124-136: 삭제 로직이 AWS SDK v2로 올바르게 마이그레이션되었습니다.
- 빌더 패턴:
DeleteObjectRequest.builder()를 사용하여 요청 객체를 구성합니다.- 예외 처리:
S3Exception과SdkException을 분리하여 서비스/클라이언트 오류를 구분합니다.
61-66: 파일 크기와 타입에 따른 업로드 경로 분기가 명확합니다.
- 대용량 파일 처리: 5MB 이상 파일은
original/경로로 업로드하여 Lambda 리사이징을 활용합니다.- CHAT 타입 예외: 채팅 파일은 다양한 형식을 지원하므로 리사이징을 건너뜁니다.
- 반환 경로: 리사이징 대상은
.webp확장자로 변환된 경로를 반환합니다.src/main/java/com/example/solidconnection/chat/service/ChatService.java (2)
30-30: LGTM! 새로운 import 추가가 적절합니다.
Arrays- 이미지 확장자 리스트 생성에 사용StringUtils- 파일 확장자 추출에 사용두 import 모두 변경된 로직에서 필요한 유틸리티입니다.
Also applies to: 43-43
267-282: LGTM! 썸네일 URL 생성 로직이 적절합니다.
- 경로 변환:
chat/files/→chat/thumbnails/변환이 새로운 업로드 구조와 일치합니다.- 안전한 폴백: 예외 발생 시 원본 URL을 반환하여 서비스 중단을 방지합니다.
- 파일명 처리:
_thumb접미사 추가 로직이 명확합니다.src/test/java/com/example/solidconnection/s3/service/S3ServiceTest.java (2)
43-91: LGTM! 테스트 케이스 구성이 명확합니다.테스트가 잘 구조화되어 있습니다:
- 5MB 미만 파일: 원본 확장자 유지 및 직접 경로 사용 검증
- 5MB 이상 파일: resize 경로 및 webp 변환 검증
- 채팅 파일: 리사이징 바이패스 및
chat/files/경로 검증Mock 설정 문제(앞서 언급)가 해결되면 테스트가 의도대로 동작할 것입니다.
94-122: LGTM! 파일 검증 테스트가 적절합니다.
- 허용되지 않은 확장자 테스트:
.exe파일 업로드 시 예외 발생 검증- 채팅 문서 허용 테스트:
.docx파일이 CHAT 타입에서 허용되는지 검증
assertThatCode(...).doesNotThrowAnyException()패턴을 잘 활용했습니다.src/main/java/com/example/solidconnection/s3/controller/S3Controller.java (3)
4-4: LGTM! Import 변경이 올바릅니다.
ImgType→UploadTypeenum 마이그레이션의 일부로 import가 정확히 업데이트되었습니다.
38-70: LGTM! 모든 업로드 엔드포인트에서 UploadType이 일관되게 사용됩니다.변경 사항 정리:
/profile/pre:UploadType.PROFILE사용/profile/post:UploadType.PROFILE사용/gpa:UploadType.GPA사용/language-test:UploadType.LANGUAGE_TEST사용enum 마이그레이션이 일관되게 적용되었습니다.
72-78: LGTM! 채팅 파일 업로드 엔드포인트 개선이 적절합니다.변경 사항:
- 메서드명:
uploadChatImage→uploadChatFile(다양한 파일 형식 지원 반영)- 파라미터명:
imageFiles→files(의미 명확화)- 타입:
UploadType.CHAT사용채팅에서 이미지 외에도 문서 파일을 전송할 수 있다는 요구사항을 잘 반영했습니다.
src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java (2)
17-17: LGTM! Import 업데이트 완료.
ImgType→UploadType마이그레이션에 따른 import 변경입니다.
93-100: LGTM! 프로필 이미지 업로드 로직이 올바르게 업데이트되었습니다.
UploadType.PROFILE을 사용하여 S3Service에 파일 업로드를 요청합니다. 기존 로직은 그대로 유지되며, enum 타입만 변경되었습니다.src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java (2)
28-28: LGTM! 테스트 파일의 Import 업데이트 완료.
UploadTypeimport로 변경되어 프로덕션 코드와 일치합니다.
212-212: LGTM! 모든 Mock 설정이 일관되게 업데이트되었습니다.변경된 Mock 설정 목록:
- Line 212: 새 이미지 업데이트 테스트
- Line 227: 첫 프로필 수정 테스트
- Line 242: 기존 프로필 삭제 테스트
- Line 258: 닉네임 수정 테스트 setUp
모든 위치에서
eq(UploadType.PROFILE)로 일관되게 변경되었습니다.Also applies to: 227-227, 242-242, 258-258
src/main/java/com/example/solidconnection/score/service/ScoreService.java (1)
8-8: LGTM! ImgType → UploadType 리팩토링이 올바르게 적용되었습니다.
Import 변경
ImgType→UploadType으로 일관되게 업데이트됨GPA 점수 제출 (Line 43)
UploadType.GPA사용으로 변경 완료어학 점수 제출 (Line 54)
UploadType.LANGUAGE_TEST사용으로 변경 완료서비스 로직에 영향 없이 깔끔하게 enum 이름만 변경되었습니다.
Also applies to: 43-43, 54-54
src/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.java (1)
8-8: LGTM! 멘토 신청 서비스의 UploadType 적용이 완료되었습니다.
Import 변경
UploadTypeimport로 업데이트됨멘토 증빙 파일 업로드 (Line 48)
UploadType.MENTOR_PROOF로 적절하게 변경됨기존 비즈니스 로직에 영향 없이 enum 참조만 업데이트되었습니다.
Also applies to: 48-48
src/test/java/com/example/solidconnection/community/post/service/PostCommandServiceTest.java (1)
28-28: LGTM! 테스트 코드가 프로덕션 변경과 일관되게 업데이트되었습니다.
Import 변경 (Line 28)
UploadTypeimport로 업데이트됨게시글 생성 테스트 Mock (Line 112-113)
UploadType.COMMUNITY사용으로 변경 완료게시글 수정 테스트 Mock (Line 182-183)
UploadType.COMMUNITY사용으로 변경 완료Mock 설정이
PostCommandService의 실제 구현과 일치합니다.Also applies to: 112-113, 182-183
src/main/java/com/example/solidconnection/community/post/service/PostCommandService.java (1)
21-21: LGTM! 커뮤니티 게시글 이미지 업로드 로직이 올바르게 업데이트되었습니다.
Import 변경 (Line 21)
UploadTypeimport로 업데이트됨이미지 업로드 호출 (Line 91)
UploadType.COMMUNITY로 변경되어 게시글 이미지 업로드 시 올바른 타입 사용
createPost와updatePost모두savePostImages메서드를 통해 일관된 업로드 타입을 사용합니다.Also applies to: 91-91
src/test/java/com/example/solidconnection/score/service/ScoreServiceTest.java (1)
7-7: LGTM! 점수 서비스 테스트가 프로덕션 변경과 일관되게 업데이트되었습니다.
Import 변경 (Line 7)
UploadTypeimport로 업데이트됨GPA 점수 등록 테스트 Mock (Line 118)
UploadType.GPA사용으로 변경 완료어학 점수 등록 테스트 Mock (Line 134)
UploadType.LANGUAGE_TEST사용으로 변경 완료Mock 설정이
ScoreService의 실제 구현과 정확히 일치합니다.Also applies to: 118-118, 134-134
src/test/java/com/example/solidconnection/news/service/NewsCommandServiceTest.java (2)
21-22: 1) UploadType 전환 정리 깔끔합니다.
- import 교체가 테스트 의도를 잘 따라갑니다.
74-76: 1) UploadType.NEWS로 스텁/호출 정합성 좋습니다.
- stubbing이uploadFile(..., UploadType.NEWS)로 통일돼서 읽기 편합니다.Also applies to: 113-115, 197-198, 260-262
src/test/java/com/example/solidconnection/mentor/service/MentorApplicationServiceTest.java (2)
16-18: 1) UploadType.MENTOR_PROOF로의 마이그레이션 일관성 좋습니다.
- 스텁이 전부 같은 enum을 바라봐서 유지보수가 편해집니다.Also applies to: 72-74, 90-92, 108-110, 125-127, 176-178
82-98: 1)universityId = null케이스 유지가 맞습니다.
- OTHER 선택에서 null 허용 테스트가 살아있어서 회귀 방지에 도움 됩니다. (Based on learnings, universityId는 null 가능)src/main/java/com/example/solidconnection/news/service/NewsCommandService.java (1)
14-16: 1) NEWS 썸네일 업로드 타입 전환(UploadType.NEWS) 자연스럽습니다.
-getImageUrl()에서 분기 + 업로드 흐름이 깔끔하게 유지됩니다.Also applies to: 42-48
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| @Async | ||
| public void uploadFile(String bucket, String fileName, MultipartFile multipartFile) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# `@EnableAsync` 설정 확인
rg -n "@EnableAsync" --type javaRepository: solid-connection/solid-connect-server
Length of output: 287
🏁 Script executed:
#!/bin/bash
# 두 파일의 내용 확인 - `@EnableAsync` 주변 컨텍스트
echo "=== UpdateViewCountScheduler.java ==="
sed -n '1,30p' src/main/java/com/example/solidconnection/scheduler/UpdateViewCountScheduler.java
echo -e "\n=== UpdateViewCountService.java ==="
sed -n '1,30p' src/main/java/com/example/solidconnection/community/post/service/UpdateViewCountService.javaRepository: solid-connection/solid-connect-server
Length of output: 2450
@EnableAsync는 선언되어 있지만, 설정 위치를 개선하세요.
다음과 같은 사항을 확인했습니다:
-
현재 상태
@EnableAsync가 코드베이스에 존재하므로uploadFile메서드의@Async애노테이션은 비동기로 작동합니다.
-
개선 필요 사항
@EnableAsync가UpdateViewCountScheduler(@Component) 와UpdateViewCountService(@Service) 등 여러 클래스에 분산되어 있습니다.- 모범 사례는 하나의 전용
@Configuration클래스에만@EnableAsync를 선언하는 것입니다.
-
권장 조치
- 중앙 집중식 설정 클래스를 만들고 그곳에
@EnableAsync를 옮기세요. (예:AsyncConfig.java) - 불필요한 중복 선언을 제거하세요.
- 중앙 집중식 설정 클래스를 만들고 그곳에
🤖 Prompt for AI Agents
In `@src/main/java/com/example/solidconnection/s3/service/FileUploadService.java`
around lines 27 - 28, You currently have `@EnableAsync` scattered across
components (e.g., UpdateViewCountScheduler, UpdateViewCountService) while
methods like uploadFile(String bucket, String fileName, MultipartFile
multipartFile) rely on `@Async`; create a single `@Configuration` class (e.g.,
AsyncConfig) and place `@EnableAsync` there, remove all other `@EnableAsync`
annotations from components/services, ensure Spring picks up the config via
component scanning or `@Import` so `@Async` on uploadFile continues to work.
There was a problem hiding this comment.
이건 나중에 한 번에 바꿔야 할 것 같네
There was a problem hiding this comment.
@lsy1307, 알겠습니다! @EnableAsync 설정 통합은 나중에 일괄적으로 정리하는 것이 좋겠네요.
필요하시다면 추적을 위한 GitHub 이슈를 생성해 드릴까요? 이슈에 현재 상황과 개선 방향을 정리해 두면 나중에 작업하실 때 도움이 될 것 같습니다.
src/main/java/com/example/solidconnection/s3/service/FileUploadService.java
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/s3/service/S3Service.java
Outdated
Show resolved
Hide resolved
| @@ -1 +1 @@ | |||
| Subproject commit 29524e2d6dad2042400de0370a11893029aacff2 | |||
| Subproject commit 1f93968a8475d4545d90e8f681b96382d25586af | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Submodule status ==="
git config --file .gitmodules --get-regexp path || echo "No .gitmodules found"
git submodule status 2>/dev/null || echo "No submodules configured"
echo
echo "=== Git log for src/main/resources/secret ==="
git log --oneline -n 10 -- src/main/resources/secret 2>/dev/null || echo "No history for this path"
echo
echo "=== Repository structure (relevant directories) ==="
ls -la src/main/resources/ 2>/dev/null || echo "Directory not found"
ls -la src/main/ 2>/dev/null || echo "src/main not found"
echo
echo "=== Build configuration files ==="
find . -maxdepth 2 -type f \( -name "pom.xml" -o -name "build.gradle" -o -name "settings.gradle" \) 2>/dev/null | head -20
echo
echo "=== Quick secret pattern scan (sample) ==="
rg -n 'AKIA[0-9A-Z]{16}|aws_secret_access_key|aws_access_key_id|SECRET_ACCESS_KEY|ACCESS_KEY_ID|secret[-_ ]?key|access[-_ ]?key' \
--max-count=20 2>/dev/null || echo "No patterns matched"
echo
echo "=== Checking for .git/config submodule entries ==="
git config --file .git/config --get-regexp '^submodule\.' 2>/dev/null || echo "No submodule configs"Repository: solid-connection/solid-connect-server
Length of output: 1835
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== build.gradle (resource management) ==="
cat build.gradle 2>/dev/null | head -100
echo
echo "=== .gitignore (secret patterns) ==="
cat .gitignore 2>/dev/null | grep -i 'secret\|key\|aws\|resource' || echo "No secret patterns in .gitignore"
echo
echo "=== Recent git commits touching sensitive files ==="
git log --name-only --oneline -n 20 2>/dev/null | grep -E 'secret|key|aws|credential' || echo "No sensitive file changes in recent history"
echo
echo "=== Git history check for exposed credentials ==="
git log -p -S 'AKIA' --all 2>/dev/null | head -50 || echo "No AKIA pattern in history"
git log -p -S 'aws_secret_access_key' --all 2>/dev/null | head -50 || echo "No aws_secret_access_key in history"
echo
echo "=== Submodule url and path ==="
cat .gitmodules 2>/dev/null || echo "No .gitmodules file"
echo
echo "=== Check src/main/java for environment-based secret injection ==="
rg -A 3 '@Value.*secret|@Value.*access' src/main/java 2>/dev/null | head -30Repository: solid-connection/solid-connect-server
Length of output: 4416
src/main/resources/secret 서브모듈은 올바르게 구성되어 있으나, 빌드 설정에서 명시적 제외 추가 권장
검증 결과 주요 보안 사항은 양호합니다:
-
서브모듈 설정 점검 완료
- 별도 프라이빗 저장소(
solid-connect-secret)로 분리 구성 ✓ - 현재 서브모듈이 uninitialized 상태라 빌드에 포함되지 않음 ✓
- 별도 프라이빗 저장소(
-
자격증명 노출 점검 완료
- 깃 히스토리에 노출된 AWS 키(AKIA 패턴) 없음 ✓
- 테스트 리소스는 플레이스홀더/임의값만 포함 ✓
- 환경변수 기반 주입(
@Value어노테이션) 적용됨 ✓
-
빌드 안전성 강화 제안
build.gradle에서src/main/resources/secret디렉토리 명시적 제외 추가- (현재는 서브모듈이 미초기화된 상태라 자동으로 제외되지만, 향후 실수 방지용)
tasks.named('bootJar') {
exclude 'secret/**'
}현재는 운영 상 문제 없으나, 실수 방지 차원에서 위 설정을 추가하면 더욱 견고합니다.
🤖 Prompt for AI Agents
In `@src/main/resources/secret` at line 1, Add an explicit exclusion for the
secret submodule directory in the Gradle bootJar configuration to prevent
accidental packaging of src/main/resources/secret; update the bootJar task
configuration (tasks.named('bootJar') / bootJar closure) in build.gradle to
exclude the secret/** path so the secret submodule is never included in built
jars even if the submodule becomes initialized.
| () -> assertThat(savedNews.getThumbnailUrl()).isEqualTo(newsProperties.defaultThumbnailUrl()), | ||
| () -> then(s3Service).should().deletePostImage(CUSTOM_IMAGE_URL), | ||
| () -> then(s3Service).should(never()).uploadFile(null, ImgType.NEWS) | ||
| () -> then(s3Service).should(never()).uploadFile(null, UploadType.NEWS) | ||
| ); | ||
| } |
There was a problem hiding this comment.
1) never().uploadFile(null, UploadType.NEWS)는 테스트를 “너무 봐줍니다” (false negative 위험).
- 지금 검증은 “null로 업로드 안 함”만 보장합니다.
- 만약 실수로 non-null 파일로 업로드가 호출돼도 이 검증은 통과할 수 있습니다.
- UploadType.NEWS 업로드 자체가 없음을 보려면 “any 파일”로 막는 쪽이 안전합니다.
제안 변경(diff)
- () -> then(s3Service).should(never()).uploadFile(null, UploadType.NEWS)
+ () -> then(s3Service).should(never()).uploadFile(any(), eq(UploadType.NEWS))Also applies to: 248-253
🤖 Prompt for AI Agents
In
`@src/test/java/com/example/solidconnection/news/service/NewsCommandServiceTest.java`
around lines 186 - 190, The assertion in NewsCommandServiceTest uses
then(s3Service).should(never()).uploadFile(null, UploadType.NEWS), which only
prevents a null argument and can miss calls with non-null files; update the
verification to assert no uploadFile call with any file for UploadType.NEWS
(e.g., use any(File.class) or Mockito.any() for the first argument and
eq(UploadType.NEWS) for the second) so the mock truly verifies that no upload
was invoked, and apply the same replacement for the similar verification around
lines 248-253.
- images->files로 디렉토리 경로 수정
- @async에 전적으로 위임
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/com/example/solidconnection/s3/service/S3Service.java (1)
91-91: 4. 허용된 확장자 목록에"word"가 잘못 포함되어 있습니다.
"word"는 실제 파일 확장자가 아닙니다. Microsoft Word 문서의 표준 확장자는"doc"(구버전) 또는"docx"(신버전)입니다."docx"는 이미 포함되어 있으므로,"word"를"doc"으로 변경해야 합니다.🐛 제안하는 수정 사항
-List<String> allowedExtensions = Arrays.asList("jpg", "jpeg", "png", "webp", "pdf", "word", "docx"); +List<String> allowedExtensions = Arrays.asList("jpg", "jpeg", "png", "webp", "pdf", "doc", "docx");
🧹 Nitpick comments (1)
src/main/java/com/example/solidconnection/s3/service/S3Service.java (1)
40-40: 1. 사용되지 않는asyncExecutor필드가 남아있습니다.이중 비동기 실행 문제를 해결하면서
asyncExecutor.submit()호출을 제거하셨는데,ThreadPoolTaskExecutor asyncExecutor필드 자체는 그대로 남아 있습니다. 현재 코드에서 이 필드를 사용하는 곳이 없으므로 제거하시는 것이 좋겠습니다.♻️ 제안하는 수정 사항
private final S3Client s3Client; private final SiteUserRepository siteUserRepository; private final FileUploadService fileUploadService; -private final ThreadPoolTaskExecutor asyncExecutor;
ThreadPoolTaskExecutorimport도 함께 제거해 주세요:-import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor;
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/example/solidconnection/s3/service/S3Service.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/example/solidconnection/s3/service/S3Service.java (1)
src/main/java/com/example/solidconnection/s3/controller/S3Controller.java (1)
RequiredArgsConstructor(19-84)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
src/main/java/com/example/solidconnection/s3/service/S3Service.java (3)
55-71: 2.uploadFile메서드 리팩터링이 잘 되었습니다.변경 사항을 정리하면:
UploadType을 활용하여 업로드 경로를 명확하게 구분- 5MB 이상 파일은
original/경로로 업로드하여 Lambda 리사이징 처리- CHAT 타입은 리사이징 로직을 우회하도록 처리
- 이중 비동기 실행 문제가 해결됨
73-81: 3.uploadFiles메서드가 올바르게 업데이트되었습니다.각 파일에 대해
uploadFile을 호출하여 일관된 처리를 보장합니다.
122-134: 5.deleteFile메서드가 SDK v2로 올바르게 마이그레이션되었습니다.변경 사항을 정리하면:
DeleteObjectRequest.builder()패턴으로 요청 객체 생성s3Client.deleteObject()호출로 삭제 수행- 예외 처리가
S3Exception→SdkException순서로 올바르게 구성됨 (구체적인 예외를 먼저 catch)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Hexeong
left a comment
Hexeong
left a comment
There was a problem hiding this comment.
고생하셨습니다! 몇 가지 내용에 대해 리뷰 달아뒀으니 확인 부탁드립니다 :)
| ChatMessage chatMessage = new ChatMessage("", senderId, chatRoom); | ||
|
|
||
| // 이미지 판별을 위한 확장자 리스트 | ||
| List<String> imageExtensions = Arrays.asList("jpg", "jpeg", "png", "webp"); |
There was a problem hiding this comment.
이 부분은 enum으로 관리하는 건 어떻게 생각하시나요?
|
|
||
| @Getter | ||
| public enum ImgType { | ||
| public enum UploadType { |
There was a problem hiding this comment.
저에게는 UploadType이 직관적으로는 업로드한 파일이 어떤 타입인지, 즉 파일 확장자를 구별하는 걸로 느껴집니다..!
해당 enum의 목적이 어떤 기능(도메인)에서 사용되는 기능인지를 구분하고, 저장하는 파일의 basePath를 결정지어야 하는 책임이 있다고 생각해서 다른 네이밍이 좋을 것 같다는 조심스런 의견을 드립니다 ㅎㅎ
(추신 : 제미나이가 추천한 이름은 UploadPath, UploadCategory였습니다..!)
|
|
||
| String thumbnailUrl = isImage ? generateThumbnailUrl(imageUrl) : null; | ||
|
|
||
| ChatAttachment attachment = new ChatAttachment(isImage, imageUrl, thumbnailUrl, null); |
There was a problem hiding this comment.
thumbnailUrl이 null인 경우에 대해 방어 로직이 존재하나요?? 이번 pr 상에서는 안보여서...
(개인적으로 gif나 avif 이미지 파일 확장자도 잘 쓰인다고 생각해서 확인 한 번 해주시면 감사드리겠습니다!)
whqtker
left a comment
whqtker
left a comment
There was a problem hiding this comment.
고생하셨습니다 ~ 이상 없어 보이지만, 조금 더 개선하면 좋을 것 같은 점 위주로 코멘트 남겼습니다
| COMMUNITY("community"), | ||
| NEWS("news"), | ||
| CHAT("chat"), | ||
| CHAT("chat/files"), |
There was a problem hiding this comment.
이미 chat 을 통해 업로드된 파일이 있나요 ? 그렇다면 관련해서 데이터 마이그레이션이 필요한 것 같습니다 !
| @@ -1,51 +1,49 @@ | |||
| package com.example.solidconnection.s3.service; | |||
| package com.example.solidconnection.s3.service; | |||
There was a problem hiding this comment.
FileUploadService가 전부 한 칸씩 들여쓰기 되었어요 ㅠㅠ
| ChatMessage chatMessage = new ChatMessage("", senderId, chatRoom); | ||
|
|
||
| // 이미지 판별을 위한 확장자 리스트 | ||
| List<String> imageExtensions = Arrays.asList("jpg", "jpeg", "png", "webp"); |
| class 파일_업로드_경로_및_리사이징_로직 { | ||
|
|
||
| @Test | ||
| void O5MB_미만의_이미지는_원본_확장자를_유지하며_업로드된다() { |
| private FileUploadService fileUploadService; | ||
|
|
||
| @Mock | ||
| private ThreadPoolTaskExecutor asyncExecutor; |
There was a problem hiding this comment.
s3Client, fileUploadService, asyncExecutor는 현재 사용되지 않고 있는데, 미사용 예정이라면 지워주세요 !
| public UploadedFileUrlResponse uploadFile(MultipartFile multipartFile, ImgType imageFile) { | ||
| // 파일 검증 | ||
| public UploadedFileUrlResponse uploadFile(MultipartFile multipartFile, UploadType uploadType) { | ||
| validateImgFile(multipartFile); |
There was a problem hiding this comment.
현재 PR의 작업 내용은 아니지만, validateImgFile가 이미지만 검증하는 것이 아닌 거 같습니다. validateFile 등과 같이 변경하는 게 좋을 거 같네요 !
|
|
||
| @DisplayName("S3 서비스 테스트") | ||
| @ExtendWith(MockitoExtension.class) | ||
| public class S3ServiceTest { |
There was a problem hiding this comment.
현재 테스트에서 S3Service의 deleteExProfile 메서드에 대한 테스트가 없는 거 같습니다. 해당 테스트도 추가해주세요 !
3 participants
관련 이슈
작업 내용
특이 사항
리뷰 요구사항 (선택)