[Fix] 재지원 플로우 지원 순번 저장 방식 수정#82
Merged
Merged
Conversation
- mock_applies에 sequence 필드 추가 - 모의 서류 지원 생성 request에서 sequence를 받을 수 있도록 수정 - 요청 sequence가 있으면 저장하고 없으면 기존 공고 기준 순번을 자동 계산 - 답변 저장 및 분석 응답에서 저장된 sequence를 우선 반환하도록 수정 - 기존 sequence null 데이터는 기존 계산 방식으로 fallback 처리 - 재지원 플로우의 sequence 반환 테스트 추가
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds optional validated ChangesMock Application Explicit Sequence Support
Sequence DiagramsequenceDiagram
participant Controller as MockApplyController
participant Service as MockApplyService
participant Persistence as MockApplyPersistenceService
participant Repo as MockApplyRepository
participant DB as Database
Controller->>Service: createActualApply(user, jobPostingId, sequence?)
Service->>Repo: resolveSequence(userId, jobPostingId, sequence?)
Repo->>DB: countByUserIdAndJobPostingId(userId, jobPostingId) (if needed)
DB-->>Repo: count
Repo-->>Service: computedSequence
Service->>Persistence: saveAndFlush(MockApply with sequence)
Persistence->>Repo: saveAndFlush(MockApply)
Repo->>DB: insert MockApply
DB-->>Repo: success / DataIntegrityViolation
Repo-->>Persistence: result / exception
Persistence-->>Service: saved MockApply or exception
Service->>Service: on conflict -> increment sequence and retry (bounded)
Service-->>Controller: MockApplyCreateResponse(jobPostingId, mockApplyId, applyType, sequence)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
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/jobdri/jobdri_api/domain/mockapply/controller/MockApplyController.java (1)
85-93:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSwagger success examples are stale after adding
sequenceto create responses.These endpoints now return
MockApplyCreateResponsewithsequence, but the documented200examples still omit it. Update the example JSON to include"sequence": <number>so API docs match runtime behavior.Also applies to: 129-141
🤖 Prompt for 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. In `@src/main/java/com/jobdri/jobdri_api/domain/mockapply/controller/MockApplyController.java` around lines 85 - 93, Update the Swagger 200 response examples for the endpoints that return MockApplyCreateResponse (notably the controller methods createActualApply and the other create endpoint around lines 129-141) so the example JSON includes the new "sequence" integer field; locate the `@ApiResponse/`@Operation example JSON tied to MockApplyCreateResponse and add "sequence": <number> (e.g., 1) to the example payload so the documented 200 response matches the runtime MockApplyCreateResponse structure returned by mockApplyService.createActualApply and the corresponding create method.
🤖 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
`@src/main/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyService.java`:
- Around line 191-199: The current resolveSequence(User, JobPosting, Integer)
uses count+1 which races under concurrent requests; modify
MockApplyService.resolveSequence to allocate sequences atomically by adding a DB
unique constraint on (user_id, job_posting_id, sequence) and changing the
allocation path to either (A) perform the allocation inside a DB
lock/transaction (e.g., SELECT MAX(sequence) FOR UPDATE on the relevant rows and
return max+1) or (B) implement a retry-on-conflict loop around
mockApplyRepository.save/create that catches the
unique-constraint/DataIntegrityViolationException and retries with an
incremented sequence; ensure the repository/entity (mockApplyRepository /
MockApply entity) has the new unique index so conflicts surface for retry.
---
Outside diff comments:
In
`@src/main/java/com/jobdri/jobdri_api/domain/mockapply/controller/MockApplyController.java`:
- Around line 85-93: Update the Swagger 200 response examples for the endpoints
that return MockApplyCreateResponse (notably the controller methods
createActualApply and the other create endpoint around lines 129-141) so the
example JSON includes the new "sequence" integer field; locate the
`@ApiResponse/`@Operation example JSON tied to MockApplyCreateResponse and add
"sequence": <number> (e.g., 1) to the example payload so the documented 200
response matches the runtime MockApplyCreateResponse structure returned by
mockApplyService.createActualApply and the corresponding create method.
🪄 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: 4d63d049-8fa4-49a2-86bf-9f61e130fe0f
📒 Files selected for processing (11)
src/main/java/com/jobdri/jobdri_api/domain/mockapply/controller/MockApplyController.javasrc/main/java/com/jobdri/jobdri_api/domain/mockapply/dto/request/MockApplyCreateActualRequest.javasrc/main/java/com/jobdri/jobdri_api/domain/mockapply/dto/request/MockApplyCreateMockFromJobPostingRequest.javasrc/main/java/com/jobdri/jobdri_api/domain/mockapply/dto/request/MockApplyCreateMockRequest.javasrc/main/java/com/jobdri/jobdri_api/domain/mockapply/dto/response/MockApplyCreateResponse.javasrc/main/java/com/jobdri/jobdri_api/domain/mockapply/entity/MockApply.javasrc/main/java/com/jobdri/jobdri_api/domain/mockapply/repository/MockApplyRepository.javasrc/main/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyService.javasrc/test/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisServiceTest.javasrc/test/java/com/jobdri/jobdri_api/domain/analysis/service/QuestionServiceTest.javasrc/test/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyServiceTest.java
- mock_applies에 사용자, 공고, 순번 기준 유니크 제약 추가 - 자동 순번 생성 중 중복 충돌이 발생하면 다음 순번으로 재시도하도록 수정 - 요청으로 전달된 sequence가 이미 사용 중이면 잘못된 요청으로 예외 처리 - 모의 서류 지원 생성 응답 Swagger 예시에 sequence 필드 추가 - 중복 sequence 요청 예외 테스트 추가
There was a problem hiding this comment.
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/jobdri/jobdri_api/domain/mockapply/service/MockApplyService.java (1)
194-197:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEnforce positive
sequencein service-level validation.Line 195 currently accepts caller-provided
sequenceas-is. Since these are public service overloads,0/negative values can bypass controller DTO validation and be stored.🔧 Suggested fix
private int resolveSequence(User user, JobPosting jobPosting, Integer requestedSequence) { if (requestedSequence != null) { + if (requestedSequence < 1) { + throw new GeneralException( + GeneralErrorCode.INVALID_PARAMETER, + "지원 순번은 1 이상이어야 합니다. sequence=" + requestedSequence + ); + } return requestedSequence; } return Math.toIntExact(mockApplyRepository.countByUserIdAndJobPostingId( user.getId(), jobPosting.getId() )) + 1; }🤖 Prompt for 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. In `@src/main/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyService.java` around lines 194 - 197, The resolveSequence method currently returns any non-null requestedSequence; change it to only accept and return requestedSequence when it's positive (requestedSequence != null && requestedSequence > 0), otherwise ignore it and compute the next valid sequence number (e.g., query the existing max sequence for the JobPosting via the repository/service and return max+1). Update resolveSequence(User user, JobPosting jobPosting, Integer requestedSequence) to enforce this check and fallback to the existing sequence-allocation logic so 0 or negative values are never persisted.
🤖 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.
Outside diff comments:
In
`@src/main/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyService.java`:
- Around line 194-197: The resolveSequence method currently returns any non-null
requestedSequence; change it to only accept and return requestedSequence when
it's positive (requestedSequence != null && requestedSequence > 0), otherwise
ignore it and compute the next valid sequence number (e.g., query the existing
max sequence for the JobPosting via the repository/service and return max+1).
Update resolveSequence(User user, JobPosting jobPosting, Integer
requestedSequence) to enforce this check and fallback to the existing
sequence-allocation logic so 0 or negative values are never persisted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fb5543fb-3673-4d08-a49e-a2e63442eb8a
📒 Files selected for processing (4)
src/main/java/com/jobdri/jobdri_api/domain/mockapply/controller/MockApplyController.javasrc/main/java/com/jobdri/jobdri_api/domain/mockapply/entity/MockApply.javasrc/main/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyService.javasrc/test/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyServiceTest.java
✅ Files skipped from review due to trivial changes (1)
- src/test/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyServiceTest.java
- 요청 sequence가 양수일 때만 명시 순번으로 사용하도록 수정 - 0 또는 음수 sequence 요청은 자동 순번 할당 로직으로 처리 - 중복 순번 충돌 처리 기준을 양수 sequence 요청으로 제한 - 0 이하 sequence 요청 시 다음 유효 순번이 저장되는 테스트 추가
There was a problem hiding this comment.
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/jobdri/jobdri_api/domain/mockapply/service/MockApplyService.java (1)
214-225:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftFix unsafe retry loop: don’t continue using the same transaction/session after
saveAndFlushthrows unique-constraintDataIntegrityViolationException.When the insert fails on the unique constraint, Spring/JPA/Hibernate typically marks the current transaction and persistence context as rollback-only/invalid, so the later
sequence++retry attempts inside the same@Transactionalflow are unreliable and may still end up failing/rolling back. Move each attempt into a fresh transaction boundary (e.g., extract the save into a@Transactional(propagation = REQUIRES_NEW)method called per retry), or use an atomic/locking-based sequence allocation approach to avoid the constraint violation in the first place. (src/main/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyService.java: 214-225)🤖 Prompt for 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. In `@src/main/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyService.java` around lines 214 - 225, The retry loop in MockApplyService that calls mockApplyRepository.saveAndFlush(MockApply.create(...)) inside the same transaction is unsafe because a DataIntegrityViolationException leaves the transaction/persistence context invalid; extract the save-and-flush into a new transactional boundary (create a helper method, e.g., saveMockApplyInNewTx(MockApply) annotated with `@Transactional`(propagation = REQUIRES_NEW)) and call that on each retry so each attempt uses a fresh transaction, or alternatively implement an atomic sequence allocator/DB lock to claim sequence numbers before persisting; update the loop to call the new method instead of saveAndFlush directly and increment sequence between new-transaction attempts.
🧹 Nitpick comments (1)
src/test/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyServiceTest.java (1)
113-129: ⚡ Quick winThis doesn't exercise the new retry path.
Both calls still take the happy path where
count + 1is available, so this only proves that non-positive inputs fall back to auto-allocation. Please add one deterministic case wherecount + 1is already occupied by an explicit stored sequence and assert that auto-allocation advances to the next free value.🤖 Prompt for 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. In `@src/test/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyServiceTest.java` around lines 113 - 129, Add a deterministic case that exercises the retry path by pre-inserting a MockApply with the "count + 1" sequence before calling mockApplyService.createActualApply with a non-positive requested sequence; specifically, in MockApplyServiceTest createActualApplyIgnoresNonPositiveRequestedSequence, save a MockApply (via the repository or existing test helper) for the same jobPosting with sequence equal to the current count + 1 so the first auto-allocated value is occupied, then call createActualApply(user, jobPosting.getId(), 0) (or -1) and assert the returned sequence and the persisted MockApply sequence advance to the next free value (e.g., count + 2). Ensure you reference mockApplyService.createActualApply and mockApplyRepository.findById/mockApplyRepository.save to locate where to insert the setup and assertions.
🤖 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.
Outside diff comments:
In
`@src/main/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyService.java`:
- Around line 214-225: The retry loop in MockApplyService that calls
mockApplyRepository.saveAndFlush(MockApply.create(...)) inside the same
transaction is unsafe because a DataIntegrityViolationException leaves the
transaction/persistence context invalid; extract the save-and-flush into a new
transactional boundary (create a helper method, e.g.,
saveMockApplyInNewTx(MockApply) annotated with `@Transactional`(propagation =
REQUIRES_NEW)) and call that on each retry so each attempt uses a fresh
transaction, or alternatively implement an atomic sequence allocator/DB lock to
claim sequence numbers before persisting; update the loop to call the new method
instead of saveAndFlush directly and increment sequence between new-transaction
attempts.
---
Nitpick comments:
In
`@src/test/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyServiceTest.java`:
- Around line 113-129: Add a deterministic case that exercises the retry path by
pre-inserting a MockApply with the "count + 1" sequence before calling
mockApplyService.createActualApply with a non-positive requested sequence;
specifically, in MockApplyServiceTest
createActualApplyIgnoresNonPositiveRequestedSequence, save a MockApply (via the
repository or existing test helper) for the same jobPosting with sequence equal
to the current count + 1 so the first auto-allocated value is occupied, then
call createActualApply(user, jobPosting.getId(), 0) (or -1) and assert the
returned sequence and the persisted MockApply sequence advance to the next free
value (e.g., count + 2). Ensure you reference mockApplyService.createActualApply
and mockApplyRepository.findById/mockApplyRepository.save to locate where to
insert the setup and assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 98bf8ebf-b116-43ed-a81a-9a8f279a0ee4
📒 Files selected for processing (2)
src/main/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyService.javasrc/test/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyServiceTest.java
- 순번 중복 충돌 후 재시도 시 새 트랜잭션에서 저장하도록 PersistenceService 추가 - 모의 서류 지원 생성 흐름의 외부 트랜잭션을 비활성화해 공고 저장 후 지원 저장 FK 참조 안정화 - 자동 순번 저장 중 충돌이 발생하면 다음 순번으로 재시도하도록 유지 - 0 이하 sequence 요청 시 중복 충돌을 거쳐 다음 유효 순번으로 저장되는 테스트 보강
There was a problem hiding this comment.
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/jobdri/jobdri_api/domain/mockapply/service/MockApplyService.java (1)
217-228:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNarrow DataIntegrityViolationException handling to the sequence-unique constraint only
MockApplyService.javalines 220-227 catch allDataIntegrityViolationExceptionand treat them as sequence conflicts (either throwingINVALID_PARAMETERfor explicit sequences or retrying/incrementing for auto-assigned ones). This can misreport unrelated integrity failures and mask real DB/data issues.Handle only the
uk_mock_apply_user_posting_sequence(i.e., detect the underlying unique-constraint violation via the most specific cause/constraint name/SQLState) and rethrow for otherDataIntegrityViolationExceptioncauses.🤖 Prompt for 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. In `@src/main/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyService.java` around lines 217 - 228, The catch for DataIntegrityViolationException in MockApplyService around mockApplyPersistenceService.saveAndFlush is too broad; change it to inspect the exception cause (e.g., ConstraintViolationException / SQLException) and only treat it as a sequence-unique conflict when the underlying constraint name or SQL state indicates the uk_mock_apply_user_posting_sequence violation—on that specific match keep the current behavior (throw INVALID_PARAMETER when requestedSequence is positive, otherwise increment and retry); for any other DataIntegrityViolationException causes rethrow the original exception so unrelated integrity errors are not masked.
🤖 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.
Outside diff comments:
In
`@src/main/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyService.java`:
- Around line 217-228: The catch for DataIntegrityViolationException in
MockApplyService around mockApplyPersistenceService.saveAndFlush is too broad;
change it to inspect the exception cause (e.g., ConstraintViolationException /
SQLException) and only treat it as a sequence-unique conflict when the
underlying constraint name or SQL state indicates the
uk_mock_apply_user_posting_sequence violation—on that specific match keep the
current behavior (throw INVALID_PARAMETER when requestedSequence is positive,
otherwise increment and retry); for any other DataIntegrityViolationException
causes rethrow the original exception so unrelated integrity errors are not
masked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4f239bfb-7e22-4a30-af94-9bb367cee982
📒 Files selected for processing (3)
src/main/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyPersistenceService.javasrc/main/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyService.javasrc/test/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyServiceTest.java
- DataIntegrityViolationException 발생 시 순번 유니크 제약 위반 여부를 확인하도록 수정 - uk_mock_apply_user_posting_sequence 제약 위반일 때만 순번 재시도 로직 수행 - 명시 sequence 중복 요청은 기존처럼 잘못된 요청 예외로 처리 - FK 오류 등 다른 무결성 오류는 원본 예외를 그대로 전파하도록 변경
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✨ 어떤 이유로 PR를 하셨나요?
📋 세부 내용 - 왜 해당 PR이 필요한지 작업 내용을 자세하게 설명해주세요
📸 작업 화면 스크린샷
🚨 관련 이슈 번호 [ ]
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation