Dating Exam Answer Personality type 추가#437
Merged
Merged
Conversation
|
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:
Walkthrough이 PR은 사용자별 dominant personality type 집계 및 조회 기능을 추가합니다. AnswerPersonalityType enum과 DatingExamAnswer.personalityType 필드를 도입하고, 사용자 집계용 JPA entity DatingExamSubmitResult 및 관련 Repository(DatingExamSubmitResultRepository, DatingExamAnswerRepository)를 추가했습니다. submit 흐름에서 제출된 answerId로 Answer를 조회해 personality별 counts를 집계하고, 비관적 락(findByMemberIdForUpdate)으로 DatingExamSubmitResult를 생성·갱신(updateSubmitResult)합니다. Query layer(DatingExamQueryService, DatingExamFinder)에 findDominantPersonalityType(memberId)를 추가하고, API(DatingExamApi)에 DominantPersonalityTypeResponse DTO를 반환하는 엔드포인트를 노출합니다. Flyway migrations(V11–V14)과 seed 데이터도 포함됩니다. Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as DatingExamApi
participant ModifyService as DatingExamModifyService
participant AnswerRepo as DatingExamAnswerRepository
participant ResultRepo as DatingExamSubmitResultRepository
participant Database as Database
Client->>API: POST /dating-exam/submit
API->>ModifyService: submitDatingExam(request, memberId)
ModifyService->>AnswerRepo: findAllByIdIn(answerIds)
AnswerRepo->>Database: SELECT answers (includes personality_type)
Database-->>AnswerRepo: answers
AnswerRepo-->>ModifyService: answers
ModifyService->>ModifyService: aggregate counts by AnswerPersonalityType
ModifyService->>ResultRepo: findByMemberIdForUpdate(memberId)
ResultRepo->>Database: SELECT ... FOR UPDATE
Database-->>ResultRepo: existing result or empty
ResultRepo-->>ModifyService: result
ModifyService->>ModifyService: create/update DatingExamSubmitResult.addCounts(...)
ModifyService->>ResultRepo: save(result)
ResultRepo->>Database: INSERT/UPDATE dating_exam_submit_result
Database-->>ResultRepo: OK
ResultRepo-->>ModifyService: saved result
ModifyService-->>API: submission complete
API-->>Client: 200 OK
Client->>API: GET /dating-exam/dominant-personality
API->>QueryService: findDominantPersonalityType(memberId)
participant QueryService as DatingExamQueryService
QueryService->>ResultRepo: findByMemberId(memberId)
ResultRepo->>Database: SELECT *
Database-->>ResultRepo: submit result
ResultRepo-->>QueryService: submit result
QueryService->>API: DominantPersonalityTypeResponse
API-->>Client: BaseResponse<DominantPersonalityTypeResponse>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present. |
Test results780 tests 780 ✅ 16s ⏱️ Results for commit b2a52bf. ♻️ This comment has been updated with latest results. |
![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
🧹 Nitpick comments (3)
src/main/java/deepple/deepple/datingexam/domain/AnswerPersonalityType.java (1)
3-8: LGTM!Enum이 domain layer에 적절히 위치하고, 명확한 naming convention을 따르고 있습니다. Korean comment로 각 type의 의미가 잘 문서화되어 있습니다.
단,
DatingExamSubmitResult.recalculateDominant()에서 ordinal 순서를 tie-breaking 우선순위로 사용하고 있으므로, enum 선언 순서가 business logic에 영향을 준다는 점을 Javadoc으로 명시하면 향후 유지보수 시 실수를 방지할 수 있습니다.,
📝 Optional: Javadoc 추가 제안
package deepple.deepple.datingexam.domain; +/** + * 답변에 연결된 성격 유형. + * <p> + * 선언 순서(ordinal)가 동점 시 우선순위로 사용됩니다: A > B > C > D + */ public enum AnswerPersonalityType { DECISIVE_INDEPENDENT, // (A) 단호한 독립주의자 GROWING_RUNNING_MATE, // (B) 성장하는 러닝메이트 DEVOTED_ROMANTIC, // (C) 헌신적인 로맨티스트 REALISTIC_SHELTER // (D) 현실적인 안식처 }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/deepple/deepple/datingexam/domain/AnswerPersonalityType.java` around lines 3 - 8, Add a Javadoc to the AnswerPersonalityType enum explaining that the declaration order is significant because DatingExamSubmitResult.recalculateDominant() uses enum ordinal() as a tie-breaking priority; mention that changing the enum order will affect business logic and that any reordering must be reflected in the tie-breaker logic or tests.src/main/java/deepple/deepple/datingexam/application/dto/DominantPersonalityTypeResponse.java (1)
6-10: Type safety 관련 고려 사항
personalityTypefield가String으로 정의되어 있습니다. API 응답에서 유연성을 위해 String을 사용하는 것은 유효한 선택이지만,AnswerPersonalityTypeenum을 직접 사용하면 type safety와 serialization 일관성을 보장할 수 있습니다.현재 구현이 의도된 것이라면 문제없습니다. 다만,
AnswerPersonalityTypeenum 값이 변경될 경우 이 DTO를 생성하는 코드에서 compile-time 검증이 이루어지지 않는다는 점을 유의하세요.💡 Alternative: enum type 직접 사용
public record DominantPersonalityTypeResponse( - `@Schema`(implementation = AnswerPersonalityType.class) - String personalityType + AnswerPersonalityType personalityType ) { }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/deepple/deepple/datingexam/application/dto/DominantPersonalityTypeResponse.java` around lines 6 - 10, The DTO DominantPersonalityTypeResponse currently declares personalityType as String which loses compile-time safety; change its type to the enum AnswerPersonalityType (i.e., public record DominantPersonalityTypeResponse(AnswerPersonalityType personalityType)) and update the `@Schema` annotation to reference the enum (or remove if redundant), ensure imports for AnswerPersonalityType are added, and adjust any callers/serializers that construct this record so they pass the enum value instead of a raw String to preserve type safety and consistent serialization.src/test/java/deepple/deepple/datingexam/adapter/database/DatingExamQueryRepositoryImplTest.java (1)
9-9: Wildcard import 사용에 대한 선택적 개선 제안Wildcard import (
import deepple.deepple.datingexam.domain.*;)는 명시적인 import에 비해 어떤 class가 사용되는지 파악하기 어렵습니다. 다만, 같은 module 내 domain type들이므로 critical한 issue는 아닙니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/deepple/deepple/datingexam/adapter/database/DatingExamQueryRepositoryImplTest.java` at line 9, Replace the wildcard import in DatingExamQueryRepositoryImplTest (import deepple.deepple.datingexam.domain.*) with explicit imports of the specific domain classes referenced by that test; open the test, identify the exact types used from package deepple.deepple.datingexam.domain and add individual import statements for each (e.g., the concrete classes referenced in the test) to improve clarity and maintainability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/deepple/deepple/datingexam/application/DatingExamModifyService.java`:
- Around line 81-84: The read-modify-write in DatingExamModifyService (calls
datingExamSubmitResultRepository.findByMemberId -> result.addCounts -> save) can
race; replace it with an atomic update: either implement a DB upsert (e.g.,
repository method using a single ON CONFLICT / MERGE native query or `@Modifying`
JPQL that increments counts by member_id) or acquire a row-level lock in a
transactional boundary (add `@Transactional` on the service method and use a
repository method like findByMemberIdForUpdate annotated with
`@Lock`(PESSIMISTIC_WRITE) to load the row or create it if absent before calling
DatingExamSubmitResult.addCounts and save). Ensure the chosen approach operates
on member_id atomically so concurrent submits don’t overwrite increments.
In
`@src/main/resources/db/migration/V11__reset_dating_exams_and_add_personality_type.sql`:
- Around line 4-11: This migration unconditionally deletes all rows from
dating_exam_submit, dating_exam_answer, dating_exam_question, and
dating_exam_subject which will remove production users' exam history; modify the
migration to be safe by either (a) adding a pre-delete backup step that copies
data from those tables into archive/backup tables (e.g.,
dating_exam_submit_backup, etc.) before DELETE, or (b) gating execution with an
environment-specific conditional/placeholder so it only runs in non-production,
or (c) restrict deletes with an explicit WHERE targeting only test/staging data;
update the migration to include the chosen safety mechanism and a clear comment
describing the backup/condition and reference the tables dating_exam_submit,
dating_exam_answer, dating_exam_question, dating_exam_subject.
- Around line 22-23: ALTER TABLE statement adding personality_type as "ALTER
TABLE dating_exam_answer ADD COLUMN personality_type VARCHAR(50) NOT NULL" is
unsafe under MySQL implicit DDL commits; change the migration so the NOT NULL
column is added with a safe default (e.g., ADD COLUMN ... VARCHAR(50) NOT NULL
DEFAULT '') or split into two migrations: first ADD COLUMN nullable (ALTER TABLE
dating_exam_answer ADD COLUMN personality_type VARCHAR(50)), backfill values,
then ALTER TABLE ... SET NOT NULL; also review Flyway transaction settings for
DDL and ensure migrations are idempotent when run after a partial failure.
---
Nitpick comments:
In
`@src/main/java/deepple/deepple/datingexam/application/dto/DominantPersonalityTypeResponse.java`:
- Around line 6-10: The DTO DominantPersonalityTypeResponse currently declares
personalityType as String which loses compile-time safety; change its type to
the enum AnswerPersonalityType (i.e., public record
DominantPersonalityTypeResponse(AnswerPersonalityType personalityType)) and
update the `@Schema` annotation to reference the enum (or remove if redundant),
ensure imports for AnswerPersonalityType are added, and adjust any
callers/serializers that construct this record so they pass the enum value
instead of a raw String to preserve type safety and consistent serialization.
In `@src/main/java/deepple/deepple/datingexam/domain/AnswerPersonalityType.java`:
- Around line 3-8: Add a Javadoc to the AnswerPersonalityType enum explaining
that the declaration order is significant because
DatingExamSubmitResult.recalculateDominant() uses enum ordinal() as a
tie-breaking priority; mention that changing the enum order will affect business
logic and that any reordering must be reflected in the tie-breaker logic or
tests.
In
`@src/test/java/deepple/deepple/datingexam/adapter/database/DatingExamQueryRepositoryImplTest.java`:
- Line 9: Replace the wildcard import in DatingExamQueryRepositoryImplTest
(import deepple.deepple.datingexam.domain.*) with explicit imports of the
specific domain classes referenced by that test; open the test, identify the
exact types used from package deepple.deepple.datingexam.domain and add
individual import statements for each (e.g., the concrete classes referenced in
the test) to improve clarity and maintainability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 706a9d92-cd13-45d0-9aaa-c8acb3101dcd
📒 Files selected for processing (17)
src/main/java/deepple/deepple/datingexam/adapter/webapi/DatingExamApi.javasrc/main/java/deepple/deepple/datingexam/application/DatingExamModifyService.javasrc/main/java/deepple/deepple/datingexam/application/DatingExamQueryService.javasrc/main/java/deepple/deepple/datingexam/application/dto/DominantPersonalityTypeResponse.javasrc/main/java/deepple/deepple/datingexam/application/provided/DatingExamFinder.javasrc/main/java/deepple/deepple/datingexam/application/required/DatingExamAnswerRepository.javasrc/main/java/deepple/deepple/datingexam/application/required/DatingExamSubmitResultRepository.javasrc/main/java/deepple/deepple/datingexam/domain/AnswerPersonalityType.javasrc/main/java/deepple/deepple/datingexam/domain/DatingExamAnswer.javasrc/main/java/deepple/deepple/datingexam/domain/DatingExamSubmitResult.javasrc/main/resources/db/migration/V11__reset_dating_exams_and_add_personality_type.sqlsrc/main/resources/db/migration/V12__create_dating_exam_submit_result.sqlsrc/test/java/deepple/deepple/datingexam/adapter/database/DatingExamQueryRepositoryImplTest.javasrc/test/java/deepple/deepple/datingexam/application/DatingExamModifyServiceTest.javasrc/test/java/deepple/deepple/datingexam/application/DatingExamSubmitRequestValidatorTest.javasrc/test/java/deepple/deepple/datingexam/domain/DatingExamAnswerTest.javasrc/test/java/deepple/deepple/datingexam/domain/DatingExamSubmitResultTest.java
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/main/java/deepple/deepple/datingexam/application/DatingExamModifyService.java (1)
81-84:⚠️ Potential issue | 🟠 Majorrow 부재 케이스에서는 현재 lock 전략만으로 insert race를 막지 못합니다.
Line 81-84에서
findByMemberIdForUpdate가 empty면 lock 대상 row가 없어 동시 요청이 모두create+save로 진입할 수 있습니다. memberId unique constraint로 한 요청이 실패(보통 500)할 수 있으니upsert또는 duplicate key retry 경로를 추가해야 합니다.Diff 제안 (duplicate key retry 경로)
+import org.springframework.dao.DataIntegrityViolationException; ... - DatingExamSubmitResult result = datingExamSubmitResultRepository.findByMemberIdForUpdate(memberId) - .orElseGet(() -> DatingExamSubmitResult.create(memberId)); - result.addCounts(counts); - datingExamSubmitResultRepository.save(result); + DatingExamSubmitResult result = datingExamSubmitResultRepository.findByMemberIdForUpdate(memberId) + .orElse(null); + + if (result == null) { + try { + result = datingExamSubmitResultRepository.save(DatingExamSubmitResult.create(memberId)); + } catch (DataIntegrityViolationException e) { + result = datingExamSubmitResultRepository.findByMemberIdForUpdate(memberId) + .orElseThrow(() -> new IllegalStateException("submit result를 재조회하지 못했습니다.", e)); + } + } + + result.addCounts(counts); + datingExamSubmitResultRepository.save(result);#!/bin/bash set -euo pipefail # 1) memberId unique constraint 존재 확인 fd "DatingExamSubmitResult.java" -x sed -n '1,120p' {} # 2) updateSubmitResult의 동시성 처리 경로 확인 fd "DatingExamModifyService.java" -x sed -n '68,125p' {} # 3) duplicate key retry/예외 처리 부재 여부 확인 rg -n "findByMemberIdForUpdate|orElseGet\\(|DataIntegrityViolationException|Duplicate|upsert|merge|on conflict" \ src/main/java/deepple/deepple/datingexamAs per coding guidelines "Performance and scalability: consider high traffic scenarios, database query optimization, proper indexing, caching strategies, connection pool management, and async processing where appropriate".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/deepple/deepple/datingexam/application/DatingExamModifyService.java` around lines 81 - 84, The current logic using datingExamSubmitResultRepository.findByMemberIdForUpdate(...) plus DatingExamSubmitResult.create(memberId) can race when the row is absent; implement a duplicate-key retry or upsert path around the create/save to avoid unique constraint failures: after or instead of orElseGet(...), attempt an update-first or perform the insert inside a try/catch that catches the DB duplicate-key/DataIntegrityViolationException, and on that exception re-query with findByMemberIdForUpdate() and apply result.addCounts(counts) then save; reference the methods findByMemberIdForUpdate, DatingExamSubmitResult.create, result.addCounts, and datingExamSubmitResultRepository.save when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/deepple/deepple/datingexam/application/DatingExamModifyService.java`:
- Around line 73-79: The code in DatingExamModifyService using
datingExamAnswerRepository.findAllByIdIn(answerIds) should validate that all
requested answerIds were returned; compare the returned answers' IDs (from
DatingExamAnswer::getId) with the input answerIds set, compute the missing IDs
(set difference), and if any are missing throw a clear exception (e.g.,
IllegalArgumentException or a domain NotFoundException) rather than proceeding
to compute counts, so dominant personality calculation isn't corrupted.
In `@src/main/resources/db/migration/V12__add_personality_type_column.sql`:
- Around line 1-2: The migration adds a NOT NULL column personality_type to
table dating_exam_answer which will fail if existing rows exist; modify
V12__add_personality_type_column.sql to either add the column with a safe
DEFAULT (e.g., ALTER TABLE dating_exam_answer ADD COLUMN personality_type
VARCHAR(50) NOT NULL DEFAULT '<default>') and then, if desired, backfill/update
and drop the DEFAULT, or delete/clear existing dating_exam_answer rows before
adding the NOT NULL column within the same migration; locate the ALTER TABLE
statement in V12__add_personality_type_column.sql and implement one of these
safe approaches so the migration does not fail on existing data.
---
Duplicate comments:
In
`@src/main/java/deepple/deepple/datingexam/application/DatingExamModifyService.java`:
- Around line 81-84: The current logic using
datingExamSubmitResultRepository.findByMemberIdForUpdate(...) plus
DatingExamSubmitResult.create(memberId) can race when the row is absent;
implement a duplicate-key retry or upsert path around the create/save to avoid
unique constraint failures: after or instead of orElseGet(...), attempt an
update-first or perform the insert inside a try/catch that catches the DB
duplicate-key/DataIntegrityViolationException, and on that exception re-query
with findByMemberIdForUpdate() and apply result.addCounts(counts) then save;
reference the methods findByMemberIdForUpdate, DatingExamSubmitResult.create,
result.addCounts, and datingExamSubmitResultRepository.save when making this
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ceafde3e-eb40-45a0-b922-c3d431d12886
📒 Files selected for processing (6)
src/main/java/deepple/deepple/datingexam/application/DatingExamModifyService.javasrc/main/java/deepple/deepple/datingexam/application/required/DatingExamSubmitResultRepository.javasrc/main/resources/db/migration/V11__reset_dating_exams.sqlsrc/main/resources/db/migration/V12__add_personality_type_column.sqlsrc/main/resources/db/migration/V13__insert_dating_exam_data.sqlsrc/main/resources/db/migration/V14__create_dating_exam_submit_result.sql
✅ Files skipped from review due to trivial changes (1)
- src/main/resources/db/migration/V14__create_dating_exam_submit_result.sql
Comment on lines
+73
to
+79
| List<DatingExamAnswer> answers = datingExamAnswerRepository.findAllByIdIn(answerIds); | ||
|
|
||
| Map<AnswerPersonalityType, Integer> counts = answers.stream() | ||
| .collect(Collectors.groupingBy( | ||
| DatingExamAnswer::getPersonalityType, | ||
| Collectors.summingInt(e -> 1) | ||
| )); |
There was a problem hiding this comment.
조회 누락 answerId를 검증하지 않아 personality count가 왜곡될 수 있습니다.
Line 73-79은 findAllByIdIn 결과 개수 검증이 없어, 요청 ID 일부가 누락되면 under-count가 그대로 저장됩니다. dominant personality type 계산이 틀어질 수 있으니 요청 ID 집합과 조회 결과를 비교해 mismatch 시 예외 처리하세요.
Diff 제안
List<DatingExamAnswer> answers = datingExamAnswerRepository.findAllByIdIn(answerIds);
+ long requestedUniqueCount = answerIds.stream().distinct().count();
+ if (answers.size() != requestedUniqueCount) {
+ throw new IllegalArgumentException("유효하지 않은 답변 ID가 포함되어 있습니다.");
+ }
Map<AnswerPersonalityType, Integer> counts = answers.stream()
.collect(Collectors.groupingBy(
DatingExamAnswer::getPersonalityType,
Collectors.summingInt(e -> 1)
));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/deepple/deepple/datingexam/application/DatingExamModifyService.java`
around lines 73 - 79, The code in DatingExamModifyService using
datingExamAnswerRepository.findAllByIdIn(answerIds) should validate that all
requested answerIds were returned; compare the returned answers' IDs (from
DatingExamAnswer::getId) with the input answerIds set, compute the missing IDs
(set difference), and if any are missing throw a clear exception (e.g.,
IllegalArgumentException or a domain NotFoundException) rather than proceeding
to compute counts, so dominant personality calculation isn't corrupted.
Contributor
Author
There was a problem hiding this comment.
DatingExamSubmitRequestValidator 로 이미 검증하고 있습니다.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and display DatingExamSubmitRequestValidator
fd 'DatingExamSubmitRequestValidator.java' --exec cat {}
Length of output: 3849
@hainho, 확인했습니다. DatingExamSubmitRequestValidator.validateAnswer()에서 각 answerId가 해당 questionId의 유효한 답변 집합(validAnswers.contains(answerId))에 속하는지 이미 검증하고 있어, findAllByIdIn 호출 전에 유효하지 않은 ID는 모두 차단됩니다. 제 지적이 잘못되었습니다.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/test/java/deepple/deepple/datingexam/application/DatingExamModifyServiceTest.java`:
- Around line 96-97: 테스트가 save(any(DatingExamSubmitResult.class))처럼 상호작용만 검증해 집계
로직(성격별 count 누적 및 우세 성격 선택)이 깨져도 놓치기 때문에, DatingExamModifyServiceTest에서
datingExamSubmitResultRepository.save 호출을
ArgumentCaptor<DatingExamSubmitResult>로 캡처한 뒤 캡처한 객체의 필드(성격별 count 맵,
dominantPersonality 등)를 직접 assert 하도록 변경하세요; 구체적으로
datingExamSubmitResultRepository.save를 호출하는 테스트 케이스들(현재 save 검증이 있는 곳들)에서
ArgumentCaptor를 생성하고 verify(...).save(captor.capture())를 사용해 저장 대상 객체를 얻은 다음 예상
누적값과 우세 성격을 assertEquals/assertThat로 검사하고 동일한 패턴으로 다른 관련 테스트(현재 save(any(...))로만
검사된 케이스들)에도 동일한 보강을 적용하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1dff6ce2-8054-4cd0-ae88-2003288fc557
📒 Files selected for processing (1)
src/test/java/deepple/deepple/datingexam/application/DatingExamModifyServiceTest.java
| DatingExamAnswer mockAnswer = mock(DatingExamAnswer.class); | ||
| when(mockAnswer.getPersonalityType()).thenReturn(AnswerPersonalityType.DECISIVE_INDEPENDENT); | ||
| when(datingExamAnswerRepository.findAllByIdIn(List.of(answerId))).thenReturn(List.of(mockAnswer)); | ||
| when(datingExamSubmitResultRepository.findByMemberIdForUpdate(memberId)).thenReturn(Optional.empty()); |
There was a problem hiding this comment.
Pessimistic lock 경로 호출을 assert하지 않아 동시성 보호 의도가 테스트에 고정되지 않습니다.
Line 77, Line 159에서 findByMemberIdForUpdate(memberId)를 stub만 설정하고 호출 검증이 없습니다. 서비스가 non-lock 경로로 회귀해도 테스트가 통과할 수 있으니 Then 블록에서 명시적으로 verify해 주세요.
검증 추가 예시
verify(datingExamSubmitRepository).save(mockSubmit);
+ verify(datingExamSubmitResultRepository).findByMemberIdForUpdate(memberId);
verify(datingExamSubmitResultRepository).save(any(DatingExamSubmitResult.class));Also applies to: 159-159
Comment on lines
+96
to
97
| verify(datingExamSubmitResultRepository).save(any(DatingExamSubmitResult.class)); | ||
| } |
There was a problem hiding this comment.
DatingExamSubmitResult 검증이 interaction 수준에 머물러 핵심 집계 로직 회귀를 놓칠 수 있습니다.
Line 96, Line 184에서 save(any(...)) 호출만 확인하면 personality count 누적/갱신과 dominant personality selection이 깨져도 테스트가 통과합니다. save 대상 객체의 집계 결과를 직접 assert하도록 보강해 주세요.
Based on learnings: **/*Test.java의 service test는 mocks/test slice를 사용하더라도 service behavior를 검증해야 합니다.
Also applies to: 184-185
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/deepple/deepple/datingexam/application/DatingExamModifyServiceTest.java`
around lines 96 - 97, 테스트가 save(any(DatingExamSubmitResult.class))처럼 상호작용만 검증해
집계 로직(성격별 count 누적 및 우세 성격 선택)이 깨져도 놓치기 때문에, DatingExamModifyServiceTest에서
datingExamSubmitResultRepository.save 호출을
ArgumentCaptor<DatingExamSubmitResult>로 캡처한 뒤 캡처한 객체의 필드(성격별 count 맵,
dominantPersonality 등)를 직접 assert 하도록 변경하세요; 구체적으로
datingExamSubmitResultRepository.save를 호출하는 테스트 케이스들(현재 save 검증이 있는 곳들)에서
ArgumentCaptor를 생성하고 verify(...).save(captor.capture())를 사용해 저장 대상 객체를 얻은 다음 예상
누적값과 우세 성격을 assertEquals/assertThat로 검사하고 동일한 패턴으로 다른 관련 테스트(현재 save(any(...))로만
검사된 케이스들)에도 동일한 보강을 적용하세요.
stemmmm
approved these changes
Mar 4, 2026
| public interface DatingExamSubmitResultRepository extends Repository<DatingExamSubmitResult, Long> { | ||
| Optional<DatingExamSubmitResult> findByMemberId(Long memberId); | ||
|
|
||
| @Lock(LockModeType.PESSIMISTIC_WRITE) |
Contributor
Author
There was a problem hiding this comment.
서로 다른 과목 답변 제출이 동시에 처리되면 동시성 문제 발생 가능해요
앱이라서 실제 과목 답변 제출 사이에 텀이 있겠지만, 네트워크 지연이나 등의 이유로 발생이 가능하긴해서
방지하는게 안전할듯 하네요
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/java/deepple/deepple/datingexam/application/dto/DominantPersonalityTypeResponse.java (1)
8-9:personalityType를String대신 enum으로 유지하세요Line 9/17에서 enum을 String으로 평탄화하면 compile-time validation이 사라집니다.
AnswerPersonalityType를 그대로 DTO field로 두는 편이 API contract와 domain 모델 정합성에 더 안전합니다.♻️ Proposed refactor
public record DominantPersonalityTypeResponse( `@Schema`(implementation = AnswerPersonalityType.class) - String personalityType, + AnswerPersonalityType personalityType, int decisiveIndependentCount, int growingRunningMateCount, int devotedRomanticCount, int realisticShelterCount ) { public static DominantPersonalityTypeResponse from(DatingExamSubmitResult result) { return new DominantPersonalityTypeResponse( - result.getDominantPersonalityType().name(), + result.getDominantPersonalityType(), result.getDecisiveIndependentCount(), result.getGrowingRunningMateCount(), result.getDevotedRomanticCount(), result.getRealisticShelterCount() ); } }Also applies to: 17-17
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/deepple/deepple/datingexam/application/dto/DominantPersonalityTypeResponse.java` around lines 8 - 9, The DTO currently flattens the enum into a String (field personalityType) which loses compile-time validation; change the field type from String to the enum AnswerPersonalityType in DominantPersonalityTypeResponse (and any other similar DTO field noted) so the class uses AnswerPersonalityType directly, update the corresponding getter/setter/constructor signatures and imports to use AnswerPersonalityType, and keep or adjust the `@Schema`(implementation = AnswerPersonalityType.class) annotation to reflect the enum type for correct API contract and serialization.src/main/java/deepple/deepple/datingexam/application/DatingExamQueryService.java (1)
43-48: Query method에는@Transactional(readOnly = true)를 명시하세요Line 44 메서드는 read-only 조회이므로 method-level로
readOnly = true를 주는 편이 transaction 비용 측면에서 유리합니다.♻️ Proposed refactor
`@Override` + `@Transactional`(readOnly = true) public DominantPersonalityTypeResponse findDominantPersonalityType(Long memberId) { DatingExamSubmitResult result = datingExamSubmitResultRepository.findByMemberId(memberId) .orElseThrow(() -> new IllegalStateException("연애고사 제출 결과가 없습니다. memberId: " + memberId)); return DominantPersonalityTypeResponse.from(result); }As per coding guidelines, "Performance and scalability: consider high traffic scenarios, database query optimization, proper indexing, caching strategies, connection pool management, and async processing where appropriate".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/deepple/deepple/datingexam/application/DatingExamQueryService.java` around lines 43 - 48, The findDominantPersonalityType method in DatingExamQueryService is a read-only DB query; annotate this method with `@Transactional`(readOnly = true) to reduce transaction overhead and mark intent. Add the annotation directly on the findDominantPersonalityType method (or at class-level if you prefer all query methods to be read-only) and ensure you import org.springframework.transaction.annotation.Transactional; no other logic changes required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/deepple/deepple/datingexam/application/DatingExamQueryService.java`:
- Around line 45-47: Replace the IllegalStateException in DatingExamQueryService
when calling datingExamSubmitResultRepository.findByMemberId(memberId) with the
project’s domain/application "not found" exception (e.g., a NotFoundException or
ResourceNotFoundException that carries an ErrorCode) so the API layer can map it
to the intended HTTP status; update the thrown exception instance and message
accordingly and ensure DominantPersonalityTypeResponse.from(result) is still
used after the lookup.
---
Nitpick comments:
In
`@src/main/java/deepple/deepple/datingexam/application/DatingExamQueryService.java`:
- Around line 43-48: The findDominantPersonalityType method in
DatingExamQueryService is a read-only DB query; annotate this method with
`@Transactional`(readOnly = true) to reduce transaction overhead and mark intent.
Add the annotation directly on the findDominantPersonalityType method (or at
class-level if you prefer all query methods to be read-only) and ensure you
import org.springframework.transaction.annotation.Transactional; no other logic
changes required.
In
`@src/main/java/deepple/deepple/datingexam/application/dto/DominantPersonalityTypeResponse.java`:
- Around line 8-9: The DTO currently flattens the enum into a String (field
personalityType) which loses compile-time validation; change the field type from
String to the enum AnswerPersonalityType in DominantPersonalityTypeResponse (and
any other similar DTO field noted) so the class uses AnswerPersonalityType
directly, update the corresponding getter/setter/constructor signatures and
imports to use AnswerPersonalityType, and keep or adjust the
`@Schema`(implementation = AnswerPersonalityType.class) annotation to reflect the
enum type for correct API contract and serialization.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7cb4ee70-6cba-461f-81f4-c5e8e080ae27
📒 Files selected for processing (2)
src/main/java/deepple/deepple/datingexam/application/DatingExamQueryService.javasrc/main/java/deepple/deepple/datingexam/application/dto/DominantPersonalityTypeResponse.java
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
2 participants
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.
Summary by CodeRabbit
신규 기능
도메인 / 영속성
데이터베이스 마이그레이션
테스트
참고 자료
노트