[Refactor] LLM 호출 동시성 및 중복 작업 제어 개선 (#113)#114
Conversation
📝 WalkthroughWalkthroughThis PR adds an ChangesLLM concurrency limiting and duplicate task/cache guards
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
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/jobposting/service/JobPostingAiService.java (1)
79-82: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDon’t cache limiter fallbacks as generated questions
generateMockRecommendedQuestionsand the other LLM entrypoints swallowGeneralExceptionfromllmConcurrencyLimiter.execute(...)and return fallback payloads.MockQuestionCacheService.createAndCacheQuestionsInternalthen persists that response verbatim, so a limiter timeout can be stored as the cached questions. Re-throw the limiter exception (likeAnalysisAiClient) or return a flag so callers skip caching degraded output.🤖 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/jobposting/service/JobPostingAiService.java` around lines 79 - 82, The fallback handling in JobPostingAiService is treating limiter failures as successful AI output, which lets degraded responses get cached. Update generateMockRecommendedQuestions and the other LLM entrypoints around llmConcurrencyLimiter.execute(...) to not swallow GeneralException; either re-throw it like AnalysisAiClient or return an explicit failure signal so MockQuestionCacheService.createAndCacheQuestionsInternal can skip persisting fallback payloads. Ensure the catch block in JobPostingAiService only falls back for true API errors, not limiter timeouts.
🧹 Nitpick comments (9)
src/test/java/com/jobdri/jobdri_api/domain/jobposting/service/MockQuestionCacheServiceTest.java (1)
46-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCoverage gap: the new in-flight dedup path is never exercised.
getRecommendedQuestionsUsesCachehits the cache and returns beforemockQuestionInflightRegistry.execute(...)is reached, and the twocreateAndCacheQuestionstests call that method directly (bypassing the registry). No test verifies that a cache miss ingetRecommendedQuestionsroutes throughmockQuestionInflightRegistry.execute(...). Consider adding a case that stubs the registry mock to run the supplier and asserts the miss path.🤖 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/jobposting/service/MockQuestionCacheServiceTest.java` around lines 46 - 59, The new in-flight dedup path in MockQuestionCacheService is not covered because the current cache-hit test returns before MockQuestionInflightRegistry.execute is called, and the createAndCacheQuestions tests bypass getRecommendedQuestions entirely. Add a test around getRecommendedQuestions that forces a cache miss, stubs mockQuestionInflightRegistry.execute to invoke the supplied action, and asserts the method routes through that registry path before creating and caching questions.src/main/java/com/jobdri/jobdri_api/domain/jobposting/service/MockQuestionInflightRegistry.java (1)
16-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSingle-flight logic is correct; add a dedicated test for this concurrency primitive.
The leader/follower coordination via
putIfAbsent, synchronoustask.run(), and atomicremove(key, task)is sound. Two things worth noting:
- Followers waiting on
existingTask.get()inherit the leader's failure — a transient failure in the single leader propagates to all concurrent callers rather than letting them retry. This is acceptable single-flight behavior, but intentional.- There is no test covering this component directly (concurrent callers collapsing to one supplier invocation, exception propagation, and key cleanup). A concurrency primitive like this benefits from an explicit test.
🤖 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/jobposting/service/MockQuestionInflightRegistry.java` around lines 16 - 42, Add a dedicated concurrency test for MockQuestionInflightRegistry.execute to cover the single-flight behavior explicitly. Verify that concurrent callers with the same key collapse into one supplier invocation, that followers on existingTask.get() receive the same failure as the leader when supplier::get throws, and that inflightTasks is cleaned up after completion via the remove(key, task) path. Use the execute method and the MockQuestionInflightRegistry class as the anchors for the new test.src/main/java/com/jobdri/jobdri_api/domain/jobposting/service/MockQuestionCacheService.java (1)
33-36: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffTransaction stays open while blocking on the in-flight leader and the LLM call.
getRecommendedQuestionsis covered by the class-level@Transactional, so on a cache miss each concurrent follower holds its DB connection open while blocked inmockQuestionInflightRegistry.execute(...)waiting for the leader's (slow, semaphore-throttled) OpenAI generation to finish. Under a burst of concurrent requests for the same key this can pin connections and pressure the pool for the duration of the external call.Consider resolving the cache read/generation outside a long-lived transaction (e.g., non-transactional coordination + a short transaction only around the persistence step).
🤖 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/jobposting/service/MockQuestionCacheService.java` around lines 33 - 36, The class-level transactional scope in MockQuestionCacheService#getRecommendedQuestions keeps DB connections open while threads wait inside mockQuestionInflightRegistry.execute and while createAndCacheQuestions calls the LLM. Refactor this flow so cache lookup and in-flight coordination happen outside a long-lived transaction, and keep only the persistence portion in a short transactional boundary. Update the relevant methods in MockQuestionCacheService (especially getRecommendedQuestions and createAndCacheQuestions) to avoid holding the transaction across the blocking leader/follower path.src/main/resources/application-dev.yaml (1)
143-151: 🚀 Performance & Scalability | 🔵 TrivialAsync pool size exceeds global LLM concurrency budget.
async.llm.max-pool-sizeis 6 whilellm.concurrency.max-concurrent-requestsis 4. Since the limiter is shared globally across all OpenAI call sites (AnalysisAiClient,JobPostingAiService), up to 2 of the async worker threads will routinely block ontryAcquireand fail withSERVICE_UNAVAILABLEafter the 3s timeout once the pool saturates, rather than being naturally queued by the executor. Worth confirming this is the intended backpressure behavior, or sizingmax-concurrent-requeststo match/exceedasync.llm.max-pool-size.🤖 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/resources/application-dev.yaml` around lines 143 - 151, The async LLM executor can outgrow the shared global concurrency limit, causing worker threads to block and time out in the OpenAI call path. Review the `llm.core-pool-size`, `llm.max-pool-size`, and `llm.concurrency.max-concurrent-requests` settings in `application-dev.yaml`, and either align `max-concurrent-requests` with or above the async pool size or intentionally document the desired backpressure behavior for `AnalysisAiClient` and `JobPostingAiService`.src/main/resources/application-prod.yaml (1)
155-163: 🚀 Performance & Scalability | 🔵 TrivialSame async-pool vs. concurrency-limit mismatch as
application-dev.yaml.
async.llm.max-pool-size(6) exceedsllm.concurrency.max-concurrent-requests(4) here too. In production this means the executor can dispatch more concurrent LLM-related tasks than the semaphore will admit, so some requests will wait outacquire-timeout-millisand surface asSERVICE_UNAVAILABLEunder moderate concurrency rather than actual OpenAI overload. Recommend aligning these two knobs (or documenting the intentional backpressure) before rollout.🤖 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/resources/application-prod.yaml` around lines 155 - 163, The production LLM async executor and concurrency limit are mismatched, so the pool can queue more work than `llm.concurrency.max-concurrent-requests` allows. Update the `llm` settings in `application-prod.yaml` so `core-pool-size`/`max-pool-size` are aligned with `llm.concurrency.max-concurrent-requests`, or explicitly document the intentional backpressure behavior. Use the `llm` and `llm.concurrency` sections to locate the settings.src/test/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAiServiceTest.java (1)
56-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo coverage verifies the limiter interaction or operation keys.
Tests only add the mock for constructor wiring; none verify
llmConcurrencyLimiter.execute(...)is invoked with the correct per-operation key (e.g."job-posting-generate","mock-question-generate"), nor that a limiter-thrown exception is handled as expected. Since operation keys are plain string literals with no compile-time check, a copy-paste mistake across the 5 call sites would go undetected.Consider adding
verify(llmConcurrencyLimiter).execute(eq("mock-question-generate"), any())-style assertions, and/or stubbingexecuteto invoke the supplied lambda (thenAnswer(inv -> ((LlmConcurrencyLimiter.CheckedSupplier<?>) inv.getArgument(1)).get())) to exercise the real success path.🤖 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/jobposting/service/JobPostingAiServiceTest.java` around lines 56 - 69, The current tests only wire in llmConcurrencyLimiter but do not verify its execute interaction or the per-operation key literals used by JobPostingAiService. Update JobPostingAiServiceTest to assert that llmConcurrencyLimiter.execute is called with the expected keys for each relevant path (for example the generate/question flows), and add a stub on execute that runs the supplied CheckedSupplier so the real success path is exercised. Also add a case that makes execute throw to confirm JobPostingAiService handles limiter failures as intended.src/main/java/com/jobdri/jobdri_api/global/config/LlmConcurrencyLimiter.java (1)
55-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnwrap already-unchecked exceptions instead of re-wrapping.
catch (Exception e) { throw new RuntimeException(e); }re-wraps exceptions that are alreadyRuntimeExceptions (e.g. OpenAI SDK errors), losing the original type and muddyinge.getMessage()for callers that log it (e.g.AnalysisAiClient/JobPostingAiServiceboth dolog.error("...: {}", e.getMessage(), e)). During incident diagnosis this matters.♻️ Proposed fix
- } catch (Exception e) { - throw new RuntimeException(e); - } + } catch (RuntimeException e) { + throw e; + } catch (Exception e) { + throw new RuntimeException(e); + }🤖 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/global/config/LlmConcurrencyLimiter.java` around lines 55 - 58, The exception handling in LlmConcurrencyLimiter is re-wrapping already-unchecked errors in a new RuntimeException, which hides the original exception type and message. Update the catch(Exception e) path so RuntimeException instances are propagated as-is, and only wrap checked exceptions; keep GeneralException handling unchanged and preserve the original throwable for callers like AnalysisAiClient and JobPostingAiService that log e.getMessage() and the stack trace.src/main/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisAsyncFacadeService.java (1)
60-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate "already in progress" response construction.
This block duplicates the response-building logic already present in
submit()(Lines 27-33). Extracting a small helper (e.g.toInProgressResponse(AnalysisAsyncTask task)) would keep both paths' status/message in sync if either changes later.🤖 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/analysis/service/AnalysisAsyncFacadeService.java` around lines 60 - 67, The in-progress response is built twice in AnalysisAsyncFacadeService, once in submit() and again in the createPendingTask() failure path, which risks them drifting apart. Extract the shared response construction into a helper such as toInProgressResponse(AnalysisAsyncTask task) and use it from both submit() and the pendingTaskResult.created() false branch so the taskId, status, and message stay consistent.src/test/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisAsyncFacadeServiceTest.java (1)
43-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood coverage of the conflict/fallback path; consider an edge-case test too.
This test correctly verifies the primary conflict path where
findActiveTaskfinds the existing task aftercreatePendingTaskthrows. Consider adding a companion test wherefindActiveTaskreturns empty on the retry (simulating the task completing/being deleted in the race window), asserting that the originalDataIntegrityViolationExceptionpropagates instead of being swallowed.🤖 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/analysis/service/AnalysisAsyncFacadeServiceTest.java` around lines 43 - 62, The current test only covers the fallback path where AnalysisAsyncFacadeService.submit recovers by re-reading an existing task after createPendingTask fails. Add a companion test around submit, findActiveTask, and createPendingTask that simulates the retry still returning empty (the race window where the task disappears/completes), and assert that the original DataIntegrityViolationException is propagated instead of returning a response or swallowing the error.
🤖 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/analysis/service/AnalysisAsyncFacadeService.java`:
- Around line 86-98: The catch in createPendingTask is too broad and treats
every DataIntegrityViolationException as a duplicate active task. Narrow the
handling in AnalysisAsyncFacadeService#createPendingTask by checking that the
failure is specifically the unique-constraint case for the pending task creation
(for example via the constraint name from the most specific cause) before
falling back to findActiveTask. Let unrelated integrity violations propagate so
only the intended duplicate-task path is converted into a PendingTaskResult with
false.
In
`@src/main/java/com/jobdri/jobdri_api/domain/jobposting/service/MockQuestionCacheService.java`:
- Around line 59-72: The fallback in MockQuestionCacheService cannot reliably
read the cached row after a DataIntegrityViolationException because the
surrounding `@Transactional` scope is still marked rollback-only after
mockQuestionCacheRepository.save(...) fails. Update the retry path in
MockQuestionCacheService, especially getCachedQuestions(request) and the
save/catch block around MockQuestionCache.create(...), so the re-read happens in
a fresh transaction or only after the failed write has been rolled back,
ensuring the existing row can be returned consistently.
In
`@src/test/java/com/jobdri/jobdri_api/domain/jobposting/service/MockQuestionCacheServiceTest.java`:
- Around line 133-152: The mocked save() conflict path in
MockQuestionCacheServiceTest does not reproduce the real transaction/session
failure behavior, so the fallback query is not being validated accurately. Move
this coverage from a pure Mockito unit test to a `@DataJpa-like` or integration
test that uses a real datasource and exercises
MockQuestionCacheService.createAndCacheQuestions with the repository conflict
path, so the retry/fallback behavior after DataIntegrityViolationException is
tested under real transaction semantics.
---
Outside diff comments:
In
`@src/main/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAiService.java`:
- Around line 79-82: The fallback handling in JobPostingAiService is treating
limiter failures as successful AI output, which lets degraded responses get
cached. Update generateMockRecommendedQuestions and the other LLM entrypoints
around llmConcurrencyLimiter.execute(...) to not swallow GeneralException;
either re-throw it like AnalysisAiClient or return an explicit failure signal so
MockQuestionCacheService.createAndCacheQuestionsInternal can skip persisting
fallback payloads. Ensure the catch block in JobPostingAiService only falls back
for true API errors, not limiter timeouts.
---
Nitpick comments:
In
`@src/main/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisAsyncFacadeService.java`:
- Around line 60-67: The in-progress response is built twice in
AnalysisAsyncFacadeService, once in submit() and again in the
createPendingTask() failure path, which risks them drifting apart. Extract the
shared response construction into a helper such as
toInProgressResponse(AnalysisAsyncTask task) and use it from both submit() and
the pendingTaskResult.created() false branch so the taskId, status, and message
stay consistent.
In
`@src/main/java/com/jobdri/jobdri_api/domain/jobposting/service/MockQuestionCacheService.java`:
- Around line 33-36: The class-level transactional scope in
MockQuestionCacheService#getRecommendedQuestions keeps DB connections open while
threads wait inside mockQuestionInflightRegistry.execute and while
createAndCacheQuestions calls the LLM. Refactor this flow so cache lookup and
in-flight coordination happen outside a long-lived transaction, and keep only
the persistence portion in a short transactional boundary. Update the relevant
methods in MockQuestionCacheService (especially getRecommendedQuestions and
createAndCacheQuestions) to avoid holding the transaction across the blocking
leader/follower path.
In
`@src/main/java/com/jobdri/jobdri_api/domain/jobposting/service/MockQuestionInflightRegistry.java`:
- Around line 16-42: Add a dedicated concurrency test for
MockQuestionInflightRegistry.execute to cover the single-flight behavior
explicitly. Verify that concurrent callers with the same key collapse into one
supplier invocation, that followers on existingTask.get() receive the same
failure as the leader when supplier::get throws, and that inflightTasks is
cleaned up after completion via the remove(key, task) path. Use the execute
method and the MockQuestionInflightRegistry class as the anchors for the new
test.
In
`@src/main/java/com/jobdri/jobdri_api/global/config/LlmConcurrencyLimiter.java`:
- Around line 55-58: The exception handling in LlmConcurrencyLimiter is
re-wrapping already-unchecked errors in a new RuntimeException, which hides the
original exception type and message. Update the catch(Exception e) path so
RuntimeException instances are propagated as-is, and only wrap checked
exceptions; keep GeneralException handling unchanged and preserve the original
throwable for callers like AnalysisAiClient and JobPostingAiService that log
e.getMessage() and the stack trace.
In `@src/main/resources/application-dev.yaml`:
- Around line 143-151: The async LLM executor can outgrow the shared global
concurrency limit, causing worker threads to block and time out in the OpenAI
call path. Review the `llm.core-pool-size`, `llm.max-pool-size`, and
`llm.concurrency.max-concurrent-requests` settings in `application-dev.yaml`,
and either align `max-concurrent-requests` with or above the async pool size or
intentionally document the desired backpressure behavior for `AnalysisAiClient`
and `JobPostingAiService`.
In `@src/main/resources/application-prod.yaml`:
- Around line 155-163: The production LLM async executor and concurrency limit
are mismatched, so the pool can queue more work than
`llm.concurrency.max-concurrent-requests` allows. Update the `llm` settings in
`application-prod.yaml` so `core-pool-size`/`max-pool-size` are aligned with
`llm.concurrency.max-concurrent-requests`, or explicitly document the
intentional backpressure behavior. Use the `llm` and `llm.concurrency` sections
to locate the settings.
In
`@src/test/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisAsyncFacadeServiceTest.java`:
- Around line 43-62: The current test only covers the fallback path where
AnalysisAsyncFacadeService.submit recovers by re-reading an existing task after
createPendingTask fails. Add a companion test around submit, findActiveTask, and
createPendingTask that simulates the retry still returning empty (the race
window where the task disappears/completes), and assert that the original
DataIntegrityViolationException is propagated instead of returning a response or
swallowing the error.
In
`@src/test/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAiServiceTest.java`:
- Around line 56-69: The current tests only wire in llmConcurrencyLimiter but do
not verify its execute interaction or the per-operation key literals used by
JobPostingAiService. Update JobPostingAiServiceTest to assert that
llmConcurrencyLimiter.execute is called with the expected keys for each relevant
path (for example the generate/question flows), and add a stub on execute that
runs the supplied CheckedSupplier so the real success path is exercised. Also
add a case that makes execute throw to confirm JobPostingAiService handles
limiter failures as intended.
In
`@src/test/java/com/jobdri/jobdri_api/domain/jobposting/service/MockQuestionCacheServiceTest.java`:
- Around line 46-59: The new in-flight dedup path in MockQuestionCacheService is
not covered because the current cache-hit test returns before
MockQuestionInflightRegistry.execute is called, and the createAndCacheQuestions
tests bypass getRecommendedQuestions entirely. Add a test around
getRecommendedQuestions that forces a cache miss, stubs
mockQuestionInflightRegistry.execute to invoke the supplied action, and asserts
the method routes through that registry path before creating and caching
questions.
🪄 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: 948feb52-c501-404d-b4f7-9420585bfd69
📒 Files selected for processing (14)
ops/db/migrations/20260704_analysis_async_tasks_active_unique.sqlsrc/main/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisAiClient.javasrc/main/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisAsyncFacadeService.javasrc/main/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisAsyncTaskService.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAiService.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/service/MockQuestionCacheService.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/service/MockQuestionInflightRegistry.javasrc/main/java/com/jobdri/jobdri_api/global/config/LlmConcurrencyLimiter.javasrc/main/resources/application-dev.yamlsrc/main/resources/application-prod.yamlsrc/test/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisAsyncFacadeServiceTest.javasrc/test/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingAiServiceTest.javasrc/test/java/com/jobdri/jobdri_api/domain/jobposting/service/MockQuestionCacheServiceTest.javasrc/test/resources/application-test.yaml
| when(mockQuestionCacheRepository.findByCompany_IdAndDetailClassification_IdAndPromptVersion( | ||
| 1L, | ||
| 100L, | ||
| MockQuestionCacheService.PROMPT_VERSION | ||
| )) | ||
| .thenReturn(Optional.empty()) | ||
| .thenReturn(Optional.of(savedCache)); | ||
| when(detailClassificationRepository.findById(100L)).thenReturn(Optional.of(detailClassification)); | ||
| when(companyRepository.findById(1L)).thenReturn(Optional.of(company)); | ||
| when(jobPostingAiService.generateMockRecommendedQuestions( | ||
| org.mockito.ArgumentMatchers.eq(request), | ||
| org.mockito.ArgumentMatchers.eq(company) | ||
| )).thenReturn(aiResponse); | ||
| when(mockQuestionCacheRepository.save(org.mockito.ArgumentMatchers.any(MockQuestionCache.class))) | ||
| .thenThrow(new DataIntegrityViolationException("duplicate cache")); | ||
|
|
||
| List<String> questions = mockQuestionCacheService.createAndCacheQuestions(request); | ||
|
|
||
| assertThat(questions).containsExactly("질문 A", "질문 B"); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
This test cannot validate the real conflict-recovery behavior.
Because the repository is a mock, save(...) throwing DataIntegrityViolationException does not abort a transaction or invalidate a Hibernate session, so the stubbed second findByCompany_IdAndDetailClassification_IdAndPromptVersion(...) returns cleanly. In production the fallback query runs inside the aborted transaction (see the service comment) and would fail, so this green test gives false confidence in the recovery path. A @DataJpalike/integration test hitting a real datasource is needed to actually cover the constraint-conflict fallback.
🤖 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/jobposting/service/MockQuestionCacheServiceTest.java`
around lines 133 - 152, The mocked save() conflict path in
MockQuestionCacheServiceTest does not reproduce the real transaction/session
failure behavior, so the fallback query is not being validated accurately. Move
this coverage from a pure Mockito unit test to a `@DataJpa-like` or integration
test that uses a real datasource and exercises
MockQuestionCacheService.createAndCacheQuestions with the repository conflict
path, so the retry/fallback behavior after DataIntegrityViolationException is
tested under real transaction semantics.
1 participant
✨ 어떤 이유로 PR를 하셨나요?
📋 세부 내용 - 왜 해당 PR이 필요한지 작업 내용을 자세하게 설명해주세요
📸 작업 화면 스크린샷
🚨 관련 이슈 번호 [ ]
Summary by CodeRabbit
New Features
Bug Fixes