Refactor/#63 s3 presigned#66
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds AWS S3 presigned upload support and related S3 beans/config; refactors job-posting upload/extract/ingest to JSON requests using S3 object keys; adds JobPostingImageStorageService, presign endpoint, S3 utilities, and an LLM-based analysis domain with controllers, services, DTOs, entities, repository methods, and tests. ChangesRepository-wide checkpoint
Sequence Diagram(s)sequenceDiagram
participant Client
participant PresignController as JobPostingUploadController
participant StorageService as JobPostingImageStorageService
participant S3ObjectUrlService
participant S3
Client->>PresignController: POST /api/job-postings/images/presign-upload
PresignController->>StorageService: createUploadPresignUrl(userId, request)
StorageService->>S3ObjectUrlService: createPresignedPutUrl(key, contentType, minutes)
S3ObjectUrlService->>S3: GeneratePresignedPutUrl
S3-->>S3ObjectUrlService: presigned URL
S3ObjectUrlService-->>StorageService: URL string
StorageService-->>PresignController: JobPostingImageUploadPresignResponse
PresignController-->>Client: ApiResponse with objectKey, uploadUrl, expiresInMinutes
Client->>S3: PUT file to presigned URL
S3-->>Client: 200 OK
sequenceDiagram
participant Client
participant ExtractController as JobPostingAiController
participant IngestService as JobPostingIngestService
participant AsyncFacade as JobPostingAsyncFacadeService
participant AiService as JobPostingAiService
participant StorageService as JobPostingImageStorageService
participant OpenAI
Client->>ExtractController: POST /api/job-postings/extract (rawText + imageObjectKey)
ExtractController->>AiService: extractJobPosting(userId, rawText, imageObjectKey)
AiService->>StorageService: createReadableImageUrl(userId, objectKey)
StorageService-->>AiService: presigned GET URL
AiService->>OpenAI: Extract with text + image URL
OpenAI-->>AiService: Job posting data
AiService-->>ExtractController: JobPostingExtractResponse
ExtractController-->>Client: ApiResponse with extracted fields
Client->>IngestService: POST /api/job-postings/ingest (rawText + imageObjectKey)
IngestService->>AiService: extractJobPosting(userId, rawText, imageObjectKey)
AiService->>OpenAI: Extract with text + image URL
OpenAI-->>AiService: Job posting data
AiService-->>IngestService: extracted response
IngestService->>AsyncFacade: submit(user, JobPostingIngestRequest)
AsyncFacade-->>Client: ApiResponse with jobId
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build.gradle`:
- Around line 52-53: The build.gradle currently pins
io.awspring.cloud:spring-cloud-aws-starter-s3 to 3.1.1 which is incompatible
with Spring Boot 3.5.14; update the dependency to the 3.4.x release train
(replace version 3.1.1 with a 3.4.x version) and preferably manage Spring Cloud
AWS artifacts via the spring-cloud-aws-dependencies BOM by adding the BOM to
dependencyManagement and removing hardcoded versions so that
io.awspring.cloud:spring-cloud-aws-starter-s3 and related AWS starters are
resolved consistently for Spring Boot 3.5.14.
In `@ops/s3/job-posting-image-lifecycle.json`:
- Around line 7-10: The lifecycle policy currently expires every object with
Prefix "job-postings/" after 1 day which will delete all images because
JobPostingImageStorageService stores uploads under
job-postings/{userId}/{UUID}.{ext} (see buildObjectKey()) and
createReadableImageUrl() only presigns GETs and does not move files; either
narrow the lifecycle to a true temporary prefix (e.g., change the policy Prefix
to "job-postings/tmp/") or modify JobPostingImageStorageService.buildObjectKey()
to place temporary uploads under "job-postings/tmp/" (and ensure any
promotion/copy flow moves promoted files out of the tmp prefix), so permanent
images are not swept by the 1-day expiration.
In
`@src/main/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingImageStorageService.java`:
- Around line 75-78: In createReadableImageUrl, check for a blank objectKey
before performing any S3 lookups: at the top of the method (before
validateOwnership and validateUploadedObject) return null (or throw the
INVALID_PARAMETER error used by your service) when objectKey is null/empty/blank
(e.g. if (objectKey == null || objectKey.trim().isEmpty()) return null;). Apply
the same early-blank-key guard to the other methods that call
validateUploadedObject/validateOwnership (the blocks referenced around lines
81-84 and 107-113) to avoid invoking s3ObjectUrlService.createPresignedGetUrl or
validateUploadedObject with a blank key.
- Around line 107-113: The call to s3Client.headObject in
validateUploadedObject(String objectKey) can throw S3 SDK exceptions that
currently bubble up as 500; wrap the headObject(...) invocation in a try/catch,
catch software.amazon.awssdk.services.s3.model.NoSuchKey / S3Exception (or the
broad S3Exception) and translate S3 404 to a domain NotFound/ClientError (e.g.,
throw a custom JobPostingImageNotFoundException or BadRequest/NotFound-specific
runtime exception), translate 403 to a Forbidden domain exception, and rethrow
or wrap other S3Exceptions as a mapped domain/internal exception; keep the
bucket/key lookup using s3ObjectUrlService.getBucket() and preserve the method
validateUploadedObject signature.
In `@src/main/java/com/jobdri/jobdri_api/global/config/s3/S3Config.java`:
- Around line 16-23: Remove the hard-coded AWS static credentials and switch to
the SDK's default credential chain: delete the accessKey and secretKey fields in
S3Config and stop constructing AwsBasicCredentials; in s3Client() and
s3Presigner() either omit setting credentialsProvider or set it to
DefaultCredentialsProvider.create(), and update the builder calls accordingly;
also remove the now-unused AwsBasicCredentials import. Ensure
S3Config.s3Client() and S3Config.s3Presigner() still set the region but rely on
the DefaultCredentialsProvider so IAM roles/environment variables/EC2 metadata
work.
In
`@src/main/java/com/jobdri/jobdri_api/global/config/s3/S3ObjectUrlService.java`:
- Around line 33-34: Validate presigned URL expiration minutes before calling
signatureDuration in S3ObjectUrlService: ensure presignGetExpirationMinutes and
expiresInMinutes are within the SigV4 bounds (1..10080 minutes); if out of
range, either clamp to the nearest valid value or throw a clear
IllegalArgumentException. Add the checks right before the calls that build the
Get/Put presigners (the code that uses getObjectRequest and putObjectRequest and
calls .signatureDuration(Duration.ofMinutes(...))) so invalid config/input
cannot reach AwsPresigner and cause 500s.
In `@src/main/java/com/jobdri/jobdri_api/global/config/s3/S3Uploader.java`:
- Around line 36-41: The upload(File uploadFile, String dirName) method
currently only calls removeNewFile(uploadFile) after a successful putS3 call,
leaving temp files if putS3 throws; update upload to ensure
removeNewFile(uploadFile) is executed regardless of success by wrapping the
putS3 call in a try/finally (or try/catch/finally) so the temp file is always
deleted, rethrow or propagate the original exception from putS3 unchanged (or
return the upload URL on success), and keep references to the methods
upload(File,String), putS3(File,String) and removeNewFile(File) to locate the
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ef07c63c-5503-47b4-91d2-1a0f08001c7f
📒 Files selected for processing (25)
build.gradleops/s3/README.mdops/s3/job-posting-image-cors.jsonops/s3/job-posting-image-lifecycle.jsonsrc/main/java/com/jobdri/jobdri_api/domain/jobposting/controller/JobPostingAiController.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/controller/JobPostingUploadController.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/dto/request/JobPostingExtractMultipartRequest.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/dto/request/JobPostingExtractRequest.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/dto/request/JobPostingImageUploadPresignRequest.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/dto/request/JobPostingIngestCommand.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/dto/request/JobPostingIngestMultipartRequest.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/dto/request/JobPostingIngestRequest.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/dto/response/JobPostingImageUploadPresignResponse.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAiService.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAsyncFacadeService.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingImageStorageService.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingIngestService.javasrc/main/java/com/jobdri/jobdri_api/global/config/s3/S3Config.javasrc/main/java/com/jobdri/jobdri_api/global/config/s3/S3ObjectUrlService.javasrc/main/java/com/jobdri/jobdri_api/global/config/s3/S3Uploader.javasrc/main/resources/application-dev.yamlsrc/main/resources/application-prod.yamlsrc/test/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAiServiceTest.javasrc/test/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingIngestServiceTest.javasrc/test/resources/application-test.yaml
💤 Files with no reviewable changes (2)
- src/main/java/com/jobdri/jobdri_api/domain/jobposting/dto/request/JobPostingExtractMultipartRequest.java
- src/main/java/com/jobdri/jobdri_api/domain/jobposting/dto/request/JobPostingIngestMultipartRequest.java
- 자소서 분석 실행 API 추가 - 자소서 분석 결과 조회 API 추가 - 저장된 문항 답변과 공고 정보를 기반으로 LLM 분석 프롬프트 구성 - LLM JSON 응답을 Analysis 및 QuestionAnalysis 엔티티로 저장 - 재분석 시 기존 분석 결과와 문항 분석 결과를 교체하도록 처리 - 분석 완료 시 MockApply 상태를 COMPLETED로 변경 - 원문에 존재하지 않는 분석 sentence는 저장하지 않도록 검증 - start/end index를 서버에서 answer 기준으로 계산 - 점수를 0~100 범위로 보정 - 분석 결과 응답 DTO 및 LLM 응답 DTO 추가 - 분석 결과 없음 예외 코드 추가 - 로컬 실행 기본 프로필을 dev로 설정하고 dev JWT 기본값 추가 - 자소서 분석 서비스 테스트 추가
- 첨삭 문장 상태 enum 추가 - 분석 LLM 응답에 status 필드 반영 - 문항 분석 엔티티와 응답 DTO에 status 추가 - status 값 소문자 응답 및 기본값 보정 처리 - 분석 서비스 테스트에 status 검증 추가
- 문항 후보 조회 응답에 questionId 필드 추가 - 기본 문항 후보에 고정 ID 부여 - 문항 후보 조회 테스트에 ID 검증 추가
✨ 어떤 이유로 PR를 하셨나요?
📋 세부 내용 - 왜 해당 PR이 필요한지 작업 내용을 자세하게 설명해주세요
📸 작업 화면 스크린샷
🚨 관련 이슈 번호 [#63]
Summary by CodeRabbit
New Features
UX / API Changes
Documentation
Tests