Repository navigation
refactor: 사용하지 않는 의존성 정리 및 AWS SDK v2 전환 - #2521
Conversation
- AWS SDK v1 전체 묶음을 s3, ses 모듈로 교체 - 사용하지 않는 h2, opencsv, rest-assured와 중복 선언된 mysql-connector-j 제거 - opencsv 제거로 빠진 commons-collections 대신 Spring CollectionUtils 사용
- S3, SES 클라이언트를 SDK v2로 교체하고 v1 의존성 제거 - v2 S3Client와 이름이 겹쳐 기존 S3Client를 S3ClientWrapper로 변경 - v1을 통해 들어오던 joda-time 대신 java.time 사용
github-actions
Bot
added
공통
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Build dependencies and type updates build.gradle, src/main/java/in/koreatech/koin/domain/shop/repository/shop/ShopSearchQueryRepository.java, src/main/java/in/koreatech/koin/domain/student/service/StudentValidationService.java |
The MySQL connector becomes runtime-only. H2, OpenCSV, Rest Assured, and AWS SDK v1 are removed, and AWS SDK v2 SES is added. The two imports switch to Spring CollectionUtils and java.time.LocalDateTime. |
Configure and implement the SDK v2 S3 client src/main/java/in/koreatech/koin/infrastructure/config/S3Config.java, src/main/java/in/koreatech/koin/infrastructure/s3/client/S3ClientWrapper.java, src/test/java/in/koreatech/koin/unit/infrastructure/s3/client/S3ClientWrapperTest.java |
S3 configuration and the wrapper use SDK v2. The wrapper implements upload, deletion, download, and existence checks with SDK v2 requests. |
Wire S3 wrapper into services and tests src/main/java/in/koreatech/koin/domain/community/article/service/ArticleService.java, src/main/java/in/koreatech/koin/domain/community/article/service/summary/ArticleSummarySourceReader.java, src/main/java/in/koreatech/koin/domain/coop/service/CoopService.java, src/main/java/in/koreatech/koin/domain/dining/service/DiningReportService.java, src/main/java/in/koreatech/koin/infrastructure/s3/eventlistener/S3EventListener.java, src/main/java/in/koreatech/koin/infrastructure/s3/service/UploadService.java, src/test/java/in/koreatech/koin/acceptance/domain/*, src/test/java/in/koreatech/koin/unit/domain/community/article/service/summary/ArticleSummarySourceReaderTest.java, src/test/java/in/koreatech/koin/unit/domain/dining/service/DiningReportImageValidationTest.java |
The services and listener use S3ClientWrapper. Dining image validation catches SDK v2 exceptions, and affected tests use the wrapper and SDK v2 client requests. |
Configure SES and send email src/main/java/in/koreatech/koin/infrastructure/config/AwsSesConfig.java, src/main/java/in/koreatech/koin/infrastructure/email/client/SesMailSender.java, src/main/java/in/koreatech/koin/infrastructure/email/service/EmailService.java |
The SES bean and mail sender use SDK v2. EmailService builds the SDK v2 request while retaining its destination, source, rendered HTML, subject, and UTF-8 charsets. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Refactor
Possibly related issues
- BCSDLab/KOIN_API_V2#2520 — Its stated goals include removing unused dependencies and moving S3 and SES to AWS SDK v2, which this pull request does.
Suggested reviewers: taejinn
Merge Risk: 
2e611
-
The S3 and SES migrations have no identified merge-blocking regression. The change is ready to merge after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly summarizes the main changes: removing unused dependencies and migrating from AWS SDK v1 to v2. Linked Issues check Issue #2520requires two coding changes: (1) remove unused dependencies, and (2) move S3 and SES to AWS SDK v2 and remove the v1 SDK.
(1) Dependency removal: build.gradle at head removes the aws-java…Out of Scope Changes check Every change maps to the issue scope or supports it:
- Dependency removals in build.gradle (h2, opencsv, rest-assured, duplicate mysql-connector-j, aws-java-sdk v1) match the issue's unused-dependency…Full details: Linked Issues check
-
Explanation
Issue
#2520requires two coding changes: (1) remove unused dependencies, and (2) move S3 and SES to AWS SDK v2 and remove the v1 SDK.(1) Dependency removal: build.gradle at head removes the aws-java-sdk 1.12.672 dependency, the compile-scope mysql-connector-j duplicate (the runtimeOnly declaration remains), the h2 runtime dependency, opencsv, and rest-assured. The PR also replaces the Apache commons-collections and Joda-Time usages with Spring CollectionUtils and java.time in ShopSearchQueryRepository and StudentValidationService.
(2) SDK consolidation: build.gradle adds software.amazon.awssdk:ses and keeps the v2 BOM and s3 module. S3Config (verified at head) defines a v2 S3Client bean using InstanceProfileCredentialsProvider and AP_NORTHEAST_2. S3ClientWrapper (verified at head) uses v2 S3Client for upload, single and batch delete, download, and headObject-based existence checks. The summary reports that AwsSesConfig, SesMailSender, and EmailService use v2 SesClient and SendEmailRequest. Callers (ArticleService, CoopService, DiningReportService, S3EventListener, UploadService, and related tests) use the renamed S3ClientWrapper.
The issue contains no further acceptance criteria. No unmet coding requirement is found. The PR adds no new automated tests. It modifies existing tests for v2 instead. The issue does not require new tests. AwsSesConfig, SesMailSender, and EmailService were assessed from the change summary, not from source inspection at head.
Full details: Out of Scope Changes check
-
Explanation
Every change maps to the issue scope or supports it:
- Dependency removals in build.gradle (h2, opencsv, rest-assured, duplicate mysql-connector-j, aws-java-sdk v1) match the issue's unused-dependency and v1-removal goals.
- The commons-collections and Joda-Time replacements are needed because removing the dependencies that brought them in would otherwise break compilation. They are direct consequences of the cleanup.
- The S3Client to S3ClientWrapper rename is needed to avoid a type-name clash with the v2 S3Client. It follows the existing CloudFrontClientWrapper naming pattern.
- Test changes update existing tests to the v2 types and wrapper. They do not add unrelated behavior.
- The DiningReportService change replaces AmazonClientException handling with SdkException handling, which is required by the v1 removal.
- firebase-admin is not changed in build.gradle, which matches the PR description.
One side effect is documented in the PR: removing jackson-dataformat-cbor means application/cbor requests are no longer handled. This follows from removing an unused dependency, so it is connected to the issue scope. Reviewers should still confirm that no client sends CBOR requests.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 6 functions across 19 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
kongwoojin
left a comment
kongwoojin
left a comment
There was a problem hiding this comment.
어째 재학생이 아닌 취업하신 분이 열일하시는지는 모르겠지만... 고생하셨습니다!
📦 develop 배포 결과 비교머지 후 develop CD(run)가 성공했고, 직전 develop 배포 10회와 비교했습니다.
stage 앱 기동 시간 (
|
✅ stage 실제 동작 확인stage 서버(EC2 IAM Role)에서 배포된 jar의
|
🔍 개요
🚀 주요 변경 내용
h2,opencsv,rest-assured, 중복 선언된mysql-connector-jaws-java-sdk) 제거, S3와 SES를 v2로 전환S3Config: v2S3Client빈 추가. 인증은 presigner와 같은 EC2 IAM Role 방식AwsSesConfig,SesMailSender,EmailService: v2SesClient로 교체DiningReportService:AmazonClientException대신SdkException처리S3Client와 이름이 겹쳐 기존S3Client를S3ClientWrapper로 변경 (CloudFrontClientWrapper와 같은 규칙)commons-collections→ SpringCollectionUtilsjoda-time→java.time💬 참고 사항
doesObjectExist가 없어, v1처럼 HEAD 요청이 404면 false, 그 외 오류는 예외로 처리application/octet-stream)과 public-read 권한은 기존과 같음./gradlew build통과 (테스트 1,702개 실패 0,webAuthHttpTest통과)jackson-dataformat-cbor가 빠져application/cbor형식 요청은 더 이상 처리하지 않음 (사용처 없음)📌 후속 작업
1.21.4와junit-jupiter 1.19.3혼재)spring-security-crypto 6.2.2,jsoup 1.15.3,zip4j 2.6.1) — Spring Boot 버전업 때 함께SesMailSender의@Recover메서드 인자가sendMail과 맞지 않아 재시도 실패 시 호출되지 않음 (기존 문제)✅ Checklist (완료 조건)
Summary by CodeRabbit