MOSU-121 refator: 신청 관련 기능 리팩토링 (구현 중, 닫지 말아주세요) #124
Conversation
WalkthroughThis update introduces a major refactoring and extension of the payment, exam application, and refund domains. It restructures payment event handling to use transactional event publishing and rollback listeners, adds new DTOs and projections, modularizes application logic, and introduces new endpoints and services for refunds. Numerous classes and interfaces are added, removed, or updated for improved modularity and clarity. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ApplicationController
participant ApplicationService
participant ExamApplicationBulkRepository
participant ExamApplicationJpaRepository
participant ExamSubjectJpaRepository
participant PaymentService
participant PaymentEventTxService
participant PaymentTxEventListener
User->>ApplicationController: POST /applications (apply)
ApplicationController->>ApplicationService: apply(userId, request)
ApplicationService->>ExamApplicationBulkRepository: saveAllExamApplicationsWithSubjects(...)
ExamApplicationBulkRepository-->>ApplicationService: List<examApplicationIds>
ApplicationService-->>ApplicationController: CreateApplicationResponse
User->>PaymentService: POST /payments (confirm)
PaymentService->>PaymentEventTxService: publishSuccessEvent(...)
PaymentEventTxService->>PaymentTxEventListener: (on commit/rollback)
sequenceDiagram
actor User
participant RefundController
participant RefundService
participant RefundEventTxService
participant RefundTxEventListener
User->>RefundController: POST /refunds/{paymentKey}
RefundController->>RefundService: doProcess(paymentKey, request)
RefundService->>RefundEventTxService: publishSuccessEvent(...)
RefundEventTxService->>RefundTxEventListener: (on commit/rollback)
Estimated code review effort4 (~90 minutes) Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @chominju02, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request is a significant refactoring effort focused on the application-related features within the system. The primary goal is to streamline the management and presentation of exam applications, including how lunch selections are handled. It introduces more robust data structures, enhances validation to prevent invalid selections, and updates the API surface to provide clearer access to exam and application information. Several comments indicate that this is a work-in-progress, with some areas still requiring further implementation or refinement.
Highlights
- Application Data Model Refactoring: The internal representation of exam applications has been refactored, specifically changing the
lunchId(Long) to aisLunchChecked(boolean) inExamApplicationJpaEntityand related DTOs (RegisterExamApplicationEvent,ExamApplicationRequest). This simplifies the logic for lunch selection. - New Response DTO for Exam Applications: A new DTO,
ExamApplicationResponse, has been introduced to provide a more comprehensive and consistent structure for displaying exam application details. This replaces theExamWithSubjectsDTO, which has been commented out/removed. - Enhanced Application Validation: New validation logic has been added to
ApplicationServiceto prevent users from selecting lunch for exams that do not offer a lunch option, improving data integrity and user experience. - API Endpoint Updates and Additions: The base path for application-related endpoints has been changed from
/applicationto/applications. Additionally, new endpoints have been added:/exams/allto retrieve all available exams, and/exam-applications/{id}to fetch detailed information for a specific exam application. - Payment Entity Field Renaming: The
applicationSchoolIdfield inPaymentJpaEntityhas been renamed toexamApplicationIdfor improved clarity and consistency across the payment domain.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request refactors the application-related features. The changes include replacing lunchId with a boolean isLunchChecked, renaming DTOs for clarity, and introducing new endpoints for fetching exam and application details. The refactoring improves code clarity and consistency. I've identified a few areas for improvement: a potential NullPointerException in a repository method that can be solved by using Optional, a validation logic flaw that can be fixed by using a Set, and the use of magic strings which should be replaced by constants for better maintainability. Overall, this is a good step forward in refactoring the application logic.
src/main/java/life/mosu/mosuserver/domain/payment/PaymentRepository.java
Show resolved
Hide resolved
| examApplication.isLunchChecked() ? exam.getLunchName() | ||
| : "신청 안 함" |
There was a problem hiding this comment.
The string "신청 안 함" is a "magic string". Using hardcoded strings like this can lead to maintenance issues, as they can be scattered throughout the codebase, and changing them requires finding all occurrences. Consider defining this as a constant to improve maintainability and readability.
| List<Long> examIds = requests.stream() | ||
| .map(ExamApplicationRequest::examId).toList(); |
There was a problem hiding this comment.
The current logic for validating exam existence might be flawed if the input requests list contains duplicate examIds. requests.stream().map(ExamApplicationRequest::examId).toList() will create a list with duplicates, so examIds.size() could be larger than the number of unique exams. examJpaRepository.findAllById(examIds) will return only unique entities. This will cause the check existingExams.size() != examIds.size() to fail incorrectly. To fix this, work with a Set of exam IDs to ensure uniqueness.
| List<Long> examIds = requests.stream() | |
| .map(ExamApplicationRequest::examId).toList(); | |
| Set<Long> examIds = requests.stream() | |
| .map(ExamApplicationRequest::examId).collect(Collectors.toSet()); |
| exam.getSchoolName(), | ||
| AddressResponse.from(exam.getAddress()), | ||
| subjects, | ||
| examApplication.isLunchChecked() ? exam.getLunchName() : "신청 안 함" |
There was a problem hiding this comment.
The string "신청 안 함" is a "magic string". Using hardcoded strings like this can lead to maintenance issues. It's better to define this as a constant to improve maintainability and readability. This seems to be used in ApplicationService as well, so a shared constant would be ideal.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (2)
src/main/java/life/mosu/mosuserver/presentation/application/dto/ApplicationRequest.java (1)
33-35:subjectsshould be marked@NotNull(NPE risk invalidatedSubjects())
validatedSubjects()callssubjects.stream()without a null-check.
Either add@NotNulltosubjectsor guard against null before streaming.- Set<String> subjects + @NotNull + Set<String> subjectssrc/main/java/life/mosu/mosuserver/presentation/application/dto/ExamWithSubjects.java (1)
1-32: Remove commented-out code instead of keeping it.The entire
ExamWithSubjectsclass is commented out since it's being replaced byExamApplicationResponse. Commented-out code should be removed entirely rather than kept in the codebase, as version control already maintains the history and commented code creates unnecessary clutter.-//package life.mosu.mosuserver.presentation.application.dto; -// -//import io.swagger.v3.oas.annotations.media.Schema; -//import java.time.LocalDate; -//import java.util.Set; -// -//@Schema(description = "시험 정보와 과목 정보") -//public record ExamWithSubjects( -// @Schema(description = "시험 신청 ID", example = "1") -// Long examId, -// -// @Schema(description = "지역", example = "대치") -// String area, -// -// @Schema(description = "시험 날짜", example = "2025-08-10") -// LocalDate examDate, -// -// @Schema(description = "학교명", example = "대치중학교") -// String schoolName, -// -// @Schema(description = "점심 메뉴", example = "신청 안 함") -// String lunch, -// -// @Schema(description = "수험 번호", example = "20250001") -// String examNumber, -// -// @Schema(description = "응시 과목 목록", example = "[\"생활과 윤리\", \"정치와 법\"]") -// Set<String> subjects -//) { -// -//}
🧹 Nitpick comments (11)
src/main/java/life/mosu/mosuserver/presentation/application/dto/ApplicationRequest.java (3)
25-29: Restore & update lost JavaDoc to reflect the new boolean lunch flagThe previous JavaDoc that clarified the structure of each
ExamApplicationRequest(formerly containingexam_idandlunch_id) was removed, but nothing replaced it.
Given that the DTO structure has just changed (nowisLunchCheckedvslunchId), the absence of updated documentation will confuse API consumers and future maintainers.- @Schema(description = "시험 신청 Id 목록", required = true) + /** + * 시험 신청 정보 목록 + * • examId : 신청할 시험 ID + * • isLunchChecked : 점심 선택 여부 (true = 선택, false = 미선택) + */ + @Schema(description = "시험 신청 정보 목록", required = true)
25-29: Plural-name the collection for clarity (examApplications)
List<ExamApplicationRequest> examApplicationis a collection but is named in singular form, which conflicts with the project’s own conventions (subjects,schoolIds, etc.) and hurts readability.- List<ExamApplicationRequest> examApplication, + List<ExamApplicationRequest> examApplications,Be sure to rename the corresponding record component and adjust callers.
58-66: Minor: collapse double-stream by usingEnumSet
validatedSubjects()can avoid an intermediateCollectors.toSet()and produce anEnumSetfor cheaper look-ups:- return subjects.stream() - .map(Subject::valueOf) - .collect(Collectors.toSet()); + return subjects.stream() + .map(Subject::valueOf) + .collect(Collectors.toCollection(() -> EnumSet.noneOf(Subject.class)));src/main/java/life/mosu/mosuserver/presentation/application/ApplicationController.java (1)
28-28: Address the TODO comment about response modification.The comment indicates that the response structure needs modification. Since this is marked as "구현 중" (in progress), ensure this TODO is tracked and completed before the PR is finalized.
Would you like me to help identify what specific response modifications might be needed based on the broader refactoring context?
src/main/java/life/mosu/mosuserver/presentation/exam/dto/LunchRequest.java (1)
3-6: Consider adding validation and improving price field type.The DTO looks clean but could benefit from improvements:
- Add validation annotations for required fields
- Consider using
BigDecimalinstead ofIntegerfor price to handle currency properly- Add JavaDoc for clarity
+import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Positive; +import java.math.BigDecimal; + +/** + * Request DTO for lunch information containing name and price + */ public record LunchRequest( + @NotBlank(message = "Lunch name is required") String name, + @NotNull(message = "Price is required") + @Positive(message = "Price must be positive") - Integer price + BigDecimal price ) {src/main/java/life/mosu/mosuserver/application/payment/PaymentFailureHandler.java (1)
25-27: Consider updating variable names for consistency.While the method call was updated correctly, the variable names still reference the old naming convention. Consider updating for consistency:
- List<Long> missingAppSchoolIds = event.applicationSchoolIds().stream() - .filter(appSchoolId -> !existingAppIds.contains(appSchoolId)) + List<Long> missingExamApplicationIds = event.applicationSchoolIds().stream() + .filter(examApplicationId -> !existingAppIds.contains(examApplicationId)) .toList();Also, verify if
event.applicationSchoolIds()should be renamed toevent.examApplicationIds()for consistency.src/main/java/life/mosu/mosuserver/application/exam/ExamService.java (1)
43-46: Consider pagination and performance implications.The method implementation is correct and follows the existing service patterns. However, for production use, consider these enhancements:
- Pagination support for large datasets:
public Page<ExamResponse> getExams(Pageable pageable) { Page<ExamJpaEntity> exams = examJpaRepository.findAll(pageable); return exams.map(ExamResponse::from); }
- Caching if exam data doesn't change frequently:
@Cacheable("exams") public List<ExamResponse> getExams() { List<ExamJpaEntity> exams = examJpaRepository.findAll(); return ExamResponse.fromList(exams); }
- More descriptive method name like
getAllExams()to distinguish from filtered methods.src/main/java/life/mosu/mosuserver/presentation/exam/dto/LunchInfo.java (1)
1-12: LGTM: Clean record implementation.The
LunchInforecord is well-implemented with a proper static factory method. However, consider whether having multiple similar lunch DTOs (LunchInfo,LunchRequest,LunchResponse) is necessary or if they could be consolidated to reduce duplication.src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java (3)
47-47: Address the TODO comment for duplicate application validation.The comment indicates this validation logic needs modification. Consider implementing more comprehensive duplicate checking that validates exam scheduling conflicts and existing exam-lunch combinations.
Would you like me to help design a more robust duplicate application validation strategy?
94-94: Address the TODO comment for application retrieval.The comment indicates this method needs modification. Consider what specific improvements are needed for the application retrieval logic.
Would you like me to help identify potential improvements for the application retrieval logic?
150-171: Optimize validation performance with single database query.The current implementation makes two separate database queries. Consider combining exam existence and lunch availability checks into a single query for better performance.
private void validateExamIdsAndLunchSelection(List<ExamApplicationRequest> requests) { List<Long> examIds = requests.stream() .map(ExamApplicationRequest::examId).toList(); - List<ExamJpaEntity> existingExams = examJpaRepository.findAllById(examIds); - if (existingExams.size() != examIds.size()) { - throw new CustomRuntimeException(ErrorCode.EXAM_NOT_FOUND); - } - - Set<Long> examsWithoutLunch = existingExams.stream() - .filter(exam -> exam.getLunchName() == null) - .map(ExamJpaEntity::getId) - .collect(Collectors.toSet()); + Map<Long, Boolean> examLunchAvailability = examJpaRepository.findExamLunchAvailabilityByIds(examIds); + + if (examLunchAvailability.size() != examIds.size()) { + throw new CustomRuntimeException(ErrorCode.EXAM_NOT_FOUND); + } boolean invalidLunchSelection = requests.stream() - .anyMatch(req -> examsWithoutLunch.contains(req.examId()) && req.isLunchChecked()); + .anyMatch(req -> !examLunchAvailability.get(req.examId()) && req.isLunchChecked()); if (invalidLunchSelection) { throw new CustomRuntimeException(ErrorCode.LUNCH_SELECTION_INVALID); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java(7 hunks)src/main/java/life/mosu/mosuserver/application/exam/ExamService.java(1 hunks)src/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java(5 hunks)src/main/java/life/mosu/mosuserver/application/examapplication/dto/RegisterExamApplicationEvent.java(2 hunks)src/main/java/life/mosu/mosuserver/application/payment/PaymentFailureHandler.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/exam/ExamJpaEntity.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationJpaEntity.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/payment/PaymentJpaEntity.java(4 hunks)src/main/java/life/mosu/mosuserver/domain/payment/PaymentRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/global/exception/ErrorCode.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/application/ApplicationController.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/ApplicationRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/ApplicationResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/ExamApplicationRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/ExamWithSubjects.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/exam/ExamController.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/exam/dto/ExamRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/exam/dto/ExamResponse.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/exam/dto/LunchInfo.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/exam/dto/LunchRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/examapplication/ExamApplicationController.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/examapplication/dto/ExamApplicationResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/lunch/dto/LunchResponse.java(1 hunks)
🔇 Additional comments (26)
src/main/java/life/mosu/mosuserver/presentation/application/ApplicationController.java (1)
22-22: Good improvement for REST API consistency.Changing the request mapping from "/application" to "/applications" follows REST naming conventions better by using plural resource names.
src/main/java/life/mosu/mosuserver/domain/payment/PaymentRepository.java (1)
13-13: Consider adding database index for performance.The new method looks correct and follows Spring Data JPA naming conventions. However, similar to the existing TODO comment on line 8, this method might also benefit from a database index on the
examApplicationIdcolumn to avoid potential full table scans.Consider adding an index on
exam_application_idcolumn in the payment table to optimize this query:CREATE INDEX idx_payment_exam_application_id ON payment(exam_application_id);src/main/java/life/mosu/mosuserver/application/payment/PaymentFailureHandler.java (1)
22-22: Method call update aligns with domain refactoring.The change from
getApplicationSchoolId()togetExamApplicationId()is correct and consistent with the domain entity field renaming.src/main/java/life/mosu/mosuserver/presentation/exam/dto/ExamRequest.java (2)
16-16: LGTM: DTO type change aligns with refactoring.The change from
ExamLunchRequesttoLunchRequestis consistent with the broader refactoring to simplify lunch data handling.
28-29: No action needed:lunchNameandlunchPriceare in the correct order
Verified that the Lombok-generated constructor onExamJpaEntitydeclares parameters ending with… String lunchName, Integer lunchPrice ) { … }which matches the builder usage in
ExamRequest.src/main/java/life/mosu/mosuserver/presentation/exam/ExamController.java (1)
36-41: Endpoint/exams/allapproved after confirmingExamService.getExams()existsVerified that the
getExams()method is implemented insrc/main/java/life/mosu/mosuserver/application/exam/ExamService.java(line 43). The new controller endpoint follows existing patterns and can be merged.src/main/java/life/mosu/mosuserver/global/exception/ErrorCode.java (1)
104-104: LGTM! Well-structured error code addition.The new
LUNCH_SELECTION_INVALIDerror code is properly structured with appropriate HTTP status, clear Korean message, and logical positioning with other lunch-related errors.src/main/java/life/mosu/mosuserver/application/examapplication/dto/RegisterExamApplicationEvent.java (3)
32-35: LGTM! Consistent field type change.The
TargetExamrecord has been properly updated to useboolean isLunchCheckedinstead ofLong lunchId, which aligns with the overall refactoring goal.
17-17: Refactoring Verified: isLunchChecked Mapping Approved
- Confirmed
ExamApplicationRequestrecord declaresboolean isLunchChecked.- Confirmed
ExamApplicationJpaEntity.create(Long applicationId, Long examId, boolean isLunchChecked)signature matches the new boolean field.All dependent method signatures align with the refactoring. No further changes required.
27-27: No changes needed:createsignature matchesThe
ExamApplicationJpaEntity.createmethod is defined as:public static ExamApplicationJpaEntity create( Long applicationId, Long examId, boolean isLunchChecked ) { … }which exactly matches the call in
src/main/java/life/mosu/mosuserver/application/examapplication/dto/RegisterExamApplicationEvent.java.src/main/java/life/mosu/mosuserver/presentation/examapplication/ExamApplicationController.java (2)
6-6: LGTM! Proper import addition.The import for
ExamApplicationResponseis correctly added to support the new endpoint.
27-34: Verified service method implementation
ThegetApplication(Long examApplicationId)method is present insrc/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java(line 93). No further action required—this endpoint is fully supported.src/main/java/life/mosu/mosuserver/domain/exam/ExamJpaEntity.java (1)
55-73: No direct usages of ExamJpaEntity constructor found – change is safeA search across all
.javafiles revealed no directnew ExamJpaEntity(...)calls outside its own declaration. With the existing@Builderin place, callers are unaffected by the parameter reorder.• Scanned
src/**/*.javafornew ExamJpaEntity(...)– no hits found
• All instantiations should go through the builder, which is unchangedNo further action required.
src/main/java/life/mosu/mosuserver/presentation/application/dto/ApplicationResponse.java (2)
5-5: LGTM! Clean type migration from ExamWithSubjects to ExamApplicationResponse.The import addition and field type change are consistent with the broader refactoring effort to replace ExamWithSubjects with ExamApplicationResponse.
Also applies to: 14-14
21-21: LGTM! Factory method signature updated consistently.The static factory method parameter type correctly matches the updated field type, maintaining type safety.
src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationJpaEntity.java (1)
37-37: LGTM! Constructor and factory method consistently updated.The constructor and static factory method have been properly updated to use the new
boolean isLunchCheckedparameter, maintaining consistency with the field type change.Also applies to: 40-40, 43-44, 48-48
src/main/java/life/mosu/mosuserver/presentation/exam/dto/ExamResponse.java (1)
8-8: LGTM! Good refactoring to simplify area representation and add structured lunch information.The import addition, area field simplification from
Areaobject toString, and addition of theLunchResponse lunchfield improve the response structure and provide better lunch information encapsulation.Also applies to: 14-14, 18-18
src/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java (2)
4-5: LGTM! Appropriate imports and dependency injection added.The import additions for collection operations, repository, and response DTOs are necessary for the new functionality. The PaymentRepository dependency injection follows the established pattern.
Also applies to: 17-17, 24-25, 41-41
92-125: LGTM —getLunchName()Validated
- Confirmed
ExamJpaEntityis annotated with Lombok’s@Getter, sogetLunchName()is generated and available.- The
getApplicationmethod is well-structured with proper error handling and data aggregation.- The payment retrieval remains marked with a TODO for future implementation.
src/main/java/life/mosu/mosuserver/presentation/examapplication/dto/ExamApplicationResponse.java (1)
1-39: LGTM! Well-designed DTO record with appropriate field types and structure.The
ExamApplicationResponserecord is well-structured with:
- Comprehensive fields covering all exam application details
- Good use of
Set<String>for subjects to prevent duplicates- Structured
AddressResponsefor address data- Static factory method following Java best practices
- Payment fields prepared for future implementation
The field types and naming conventions are appropriate and consistent.
src/main/java/life/mosu/mosuserver/domain/payment/PaymentJpaEntity.java (1)
29-30: LGTM: Field renaming improves semantic clarity.The renaming from
applicationSchoolIdtoexamApplicationIdbetter reflects the domain model and improves code readability.src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java (5)
24-25: LGTM: Import additions support the refactoring.The new imports for
AddressResponseandExamApplicationResponsealign with the domain model changes.
64-66: LGTM: Validation placement is appropriate.The validation is correctly placed after persisting the application but before creating exam applications, ensuring data integrity while maintaining transaction consistency.
113-143: LGTM: Response construction updated correctly.The migration from
ExamWithSubjectstoExamApplicationResponseis well-implemented with proper null handling for removed fields and correct lunch selection representation.
165-166: LGTM: Lunch selection validation logic is correct.The validation properly checks if lunch is selected for exams that don't offer lunch, preventing invalid combinations.
133-134: Clarify or Populate Payment Fields in ExamApplicationResponseThe two
nullarguments inExamApplicationResponse.of( examApplication.getId(), null, // paymentKey null, // paymentStatus … )correspond to
paymentKeyandpaymentStatus—notexamNumberorarea. If you intend to return payment details here, load them (e.g., viapaymentRepository) and pass their values instead ofnull. Otherwise, consider removing these fields fromExamApplicationResponseif they’re not needed.Likely an incorrect or invalid review comment.
| @Column(name = "lunch_checked") | ||
| private boolean isLunchChecked; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Approve the field type change, but verify schema migration and dependent code.
The change from Long lunchId to boolean isLunchChecked simplifies the lunch selection logic. The explicit column mapping to "lunch_checked" is clear and appropriate.
However, this is a breaking change that affects the database schema and all dependent code.
Verify that:
- Database migration script exists for the schema change
- All code referencing the old
lunchIdfield has been updated - Data migration strategy is in place if there's existing data
🏁 Script executed:
#!/bin/bash
# Search for any remaining references to lunchId in the codebase
rg -i "lunchid" --type javaLength of output: 657
Update generated QueryDSL class and remove stale lunchId references
The refactoring from Long lunchId to boolean isLunchChecked is breaking and requires updates beyond the JPA entity:
• src/main/generated/life/mosu/mosuserver/domain/examapplication/QExamApplicationJpaEntity.java
– Regenerate QueryDSL classes so that the NumberPath<Long> lunchId field is replaced by a BooleanPath for isLunchChecked (typically named lunchChecked).
• src/main/java/life/mosu/mosuserver/presentation/application/dto/ApplicationRequest.java
– Remove or update the commented-out methods and variables referencing validatedDuplicatedSchoolLunchIds, seenSchoolLunchIds, and any logic tied to schoolId lists.
• Database migration
– Ensure there’s a migration script dropping the old lunch_id column, adding the lunch_checked boolean column, and migrating existing data if needed.
• Data migration strategy
– Confirm how existing lunch_id values map to the new boolean flag (e.g., non-null → true).
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationJpaEntity.java
at lines 30-31, after changing the field from Long lunchId to boolean
isLunchChecked, regenerate the QueryDSL classes in
src/main/generated/life/mosu/mosuserver/domain/examapplication/QExamApplicationJpaEntity.java
to replace the old NumberPath<Long> lunchId with a BooleanPath for
isLunchChecked. Also, update or remove any references to lunchId and related
school lunch ID lists in
src/main/java/life/mosu/mosuserver/presentation/application/dto/ApplicationRequest.java.
Additionally, create a database migration script to drop the lunch_id column,
add the lunch_checked boolean column, and migrate existing data by mapping
non-null lunch_id values to true for isLunchChecked.
| @Builder(access = AccessLevel.PRIVATE) | ||
| private PaymentJpaEntity( | ||
| Long applicationSchoolId, | ||
| Long examApplicationId, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify database migration for column name consistency.
While the field renaming is consistent throughout the class, ensure that the database column name (exam_application_id) aligns with this change and that proper migration scripts are in place.
Also applies to: 58-58, 67-67, 75-75, 85-85, 92-92
🏁 Script executed:
#!/bin/bash
# Description: Check if database migration scripts exist for the column rename
# Expected: Migration files that rename the column from application_school_id to exam_application_id
fd -e sql -e java | xargs rg -l "application_school_id|exam_application_id" | head -10Length of output: 387
Add missing database migration for column rename
No existing SQL/YAML/XML migration scripts reference renaming application_school_id → exam_application_id. Please introduce a proper migration (SQL or Java-based) under your migrations directory (e.g., src/main/resources/db/migration) to:
- Rename the column in the database (e.g.
ALTER TABLE payment RENAME COLUMN application_school_id TO exam_application_id;) - Update any associated indexes or foreign-key constraints
Affected lines in PaymentJpaEntity.java:
- Line 51
- Line 58
- Line 67
- Line 75
- Line 85
- Line 92
🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/domain/payment/PaymentJpaEntity.java
around line 51 and related lines, the column name was changed from
application_school_id to exam_application_id but no database migration script
was added. Create a new migration script in src/main/resources/db/migration that
renames the column in the payment table using SQL (e.g., ALTER TABLE payment
RENAME COLUMN application_school_id TO exam_application_id) and update any
related indexes or foreign key constraints accordingly to reflect this rename.
| public record ExamApplicationRequest( | ||
| Long examId, | ||
| Long lunchId | ||
| boolean isLunchChecked | ||
| ) { |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Verify lunch validation logic is implemented.
The change from lunchId to isLunchChecked boolean simplifies the structure but loses information about specific lunch options. Ensure that:
- Validation prevents lunch selection for exams without lunch offerings
- The application layer properly handles the boolean flag
- This change is consistently applied across all related entities and DTOs
🏁 Script executed:
#!/bin/bash
# Description: Verify lunch validation logic exists in ApplicationService
# Expected: Should find validation that prevents invalid lunch selections
rg -A 5 -B 5 "LUNCH_SELECTION_INVALID\|isLunchChecked" --type javaLength of output: 68
Implement lunch-selection validation in the service layer
I didn’t find any existing logic that guards against invalid lunch selections after replacing lunchId with isLunchChecked. Please add validation to ensure lunch can only be selected for exams that actually offer it:
- In the service handling
ExamApplicationRequest(e.g.ExamApplicationService#createApplication):
– Ifrequest.isLunchChecked()is true, verify the correspondingExamhas a lunch offering; otherwise throw a validation exception. - Ensure your mappers and other application‐layer components propagate the
isLunchCheckedflag correctly. - Add or update unit and integration tests to cover both valid and invalid lunch‐selection scenarios.
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/presentation/application/dto/ExamApplicationRequest.java
around lines 3 to 6, the lunch selection flag is present but lacks validation.
In the service layer method handling ExamApplicationRequest, such as
ExamApplicationService#createApplication, add logic to check if isLunchChecked
is true and verify that the corresponding Exam entity actually offers lunch; if
not, throw a validation exception. Also, ensure that any mappers or
application-layer components correctly propagate the isLunchChecked flag.
Finally, add or update unit and integration tests to cover cases where lunch
selection is valid and invalid.
|
|
||
| public static ExamResponse from(ExamJpaEntity exam) { | ||
| AddressResponse address = AddressResponse.from(exam.getAddress()); | ||
| LunchResponse lunch = LunchResponse.of(exam.getLunchName(), exam.getLunchPrice()); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify that ExamJpaEntity supports the new method calls.
The factory method implementation looks correct, but please verify that ExamJpaEntity has the getLunchName() and getLunchPrice() methods that are being called on line 23.
Also applies to: 28-28, 32-32
🏁 Script executed:
#!/bin/bash
# Search for getLunchName and getLunchPrice methods in ExamJpaEntity
rg "getLunchName|getLunchPrice" --type javaLength of output: 1318
Implement missing getters on ExamJpaEntity
The calls to exam.getLunchName() and exam.getLunchPrice() in ExamResponse.of(...) will not compile because ExamJpaEntity doesn’t declare these methods. Please add them to your JPA entity (e.g. src/main/java/life/mosu/mosuserver/domain/exam/ExamJpaEntity.java):
// in ExamJpaEntity.java
public String getLunchName() {
return this.lunchName;
}
public BigDecimal getLunchPrice() {
return this.lunchPrice;
}Once these getters are in place, the factory method in ExamResponse will resolve correctly.
🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/domain/exam/ExamJpaEntity.java, add the
missing getter methods getLunchName() and getLunchPrice() that return the
lunchName and lunchPrice fields respectively. This will allow the calls to
exam.getLunchName() and exam.getLunchPrice() in
src/main/java/life/mosu/mosuserver/presentation/exam/dto/ExamResponse.java line
23 to compile correctly.
| package life.mosu.mosuserver.presentation.lunch.dto; | ||
|
|
||
| public record LunchResponse( | ||
| String name, | ||
| Integer price | ||
| ) { | ||
|
|
||
| public static LunchResponse of(String name, Integer price) { | ||
| return new LunchResponse(name, price); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider consolidating with LunchInfo.
LunchResponse is identical to LunchInfo in structure and functionality. This duplication suggests potential over-engineering. Consider:
- Using a single
LunchorLunchDatarecord for all contexts - If separation is needed, clearly document the different use cases
- Evaluate whether the distinction between request/response/info DTOs is necessary for such simple data
-public record LunchResponse(
- String name,
- Integer price
-) {
-
- public static LunchResponse of(String name, Integer price) {
- return new LunchResponse(name, price);
- }
-}
+// Consider using LunchInfo instead, or create a base Lunch record
+// that both LunchResponse and LunchInfo extendCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/presentation/lunch/dto/LunchResponse.java
lines 1 to 11, LunchResponse duplicates the structure and functionality of
LunchInfo. To fix this, consolidate both into a single record such as Lunch or
LunchData to avoid redundancy. If you must keep them separate, add clear
documentation explaining their distinct purposes. Evaluate if separate DTOs are
necessary given the simplicity of the data.
Co-authored-by: polyglot-k <kang4030@knu.ac.kr>
…property injection
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
src/main/java/life/mosu/mosuserver/presentation/application/ApplicationControllerDocs.java (1)
25-36: Update the API response code to match the new behavior.The method signature now returns a ResponseEntity which typically uses 201 Created for resource creation, but the @apiresponse annotation still indicates "200".
@Operation(summary = "신청 등록", description = "사용자가 신청을 진행합니다.") @ApiResponses(value = { - @ApiResponse(responseCode = "200", description = "신청 성공") + @ApiResponse(responseCode = "201", description = "신청 성공", + headers = @io.swagger.v3.oas.annotations.headers.Header( + name = "Location", + description = "생성된 신청 리소스의 URI", + schema = @Schema(type = "string"))) })
♻️ Duplicate comments (3)
src/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java (1)
95-129: Address TODO comment and fix critical issues.
- The TODO comment indicates missing tests - please add unit tests for this method.
- Magic string issue (duplicate from previous review).
- The discount calculation appears incorrect.
The discount calculation logic seems inverted. Typically, discounts reduce the payment amount:
- Integer paymentAmount = - examApplicationInfo.paymentAmount().getTotalAmount() + discountAmount; + Integer paymentAmount = + examApplicationInfo.paymentAmount().getTotalAmount() - discountAmount;Would you like me to help generate unit tests for this method or open an issue to track the testing task?
src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java (2)
132-138: Fix potential validation issue with duplicate exam IDs.The validation logic might fail incorrectly if the input contains duplicate exam IDs. Use a Set to ensure uniqueness before validation.
- List<Long> examIds = requests.stream() - .map(ExamApplicationRequest::examId).toList(); + Set<Long> examIds = requests.stream() + .map(ExamApplicationRequest::examId) + .collect(Collectors.toSet()); List<ExamJpaEntity> existingExams = examJpaRepository.findAllById(examIds); if (existingExams.size() != examIds.size()) { throw new CustomRuntimeException(ErrorCode.EXAM_NOT_FOUND); }
239-239: Extract magic string to a constant.The string "신청 안 함" should be defined as a constant to improve maintainability. Consider creating a shared constant since this is used in multiple places.
+ private static final String NO_LUNCH_SELECTED = "신청 안 함"; + // In the method: - examApplication.getIsLunchChecked() ? exam.getLunchName() : "신청 안 함" + examApplication.getIsLunchChecked() ? exam.getLunchName() : NO_LUNCH_SELECTED
🧹 Nitpick comments (5)
src/main/java/life/mosu/mosuserver/presentation/admin/dto/ExamTicketResponse.java (1)
46-61: Consider reducing code duplication by reusing the existing factory method.The new factory method serves its purpose well, but it duplicates the constructor call logic.
public static ExamTicketResponse ofWithoutExamTicket( String userName, LocalDate birth, String examNumber, List<Subject> subjects, String schoolName) { - return new ExamTicketResponse( - null, - userName, - birth, - examNumber, - subjects, - schoolName - ); + return of(null, userName, birth, examNumber, subjects, schoolName); }src/main/java/life/mosu/mosuserver/infra/respository/ExamApplicationBulkRepository.java (1)
75-83: Consider memory optimization for large batch operations.For large numbers of entities and subjects, the subjectParams list could consume significant memory.
Consider processing in chunks if dealing with very large datasets:
// Process in batches of 1000 to manage memory int batchSize = 1000; for (int start = 0; start < subjectParams.size(); start += batchSize) { int end = Math.min(start + batchSize, subjectParams.size()); List<Object[]> batch = subjectParams.subList(start, end); jdbcTemplate.batchUpdate(SQL_INSERT_EXAM_SUBJECT, batch); }src/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java (2)
55-71: Remove unnecessary empty lines for better readability.The deletion logic is correct, but there are unnecessary empty lines that affect readability.
@Transactional(propagation = Propagation.REQUIRES_NEW) public void deleteExamApplication(Long examApplicationId) { ExamApplicationJpaEntity examApplication = examApplicationJpaRepository.findById( examApplicationId) .orElseThrow( () -> new CustomRuntimeException(ErrorCode.EXAM_APPLICATION_NOT_FOUND)); Long applicationId = examApplication.getApplicationId(); - examApplicationJpaRepository.deleteById(examApplicationId); - if (!examApplicationJpaRepository.existsByApplicationId(applicationId)) { applicationJpaRepository.deleteById(applicationId); } - examSubjectJpaRepository.deleteByExamApplicationId(examApplicationId); - }
74-93: LGTM! Efficient use of projection for data retrieval.The refactoring to use projection improves performance by fetching only required fields. Consider removing the empty line at the end.
String examTicketImgUrl = s3Service.getPreSignedUrl(examTicketInfo.s3Key()); return ExamTicketResponse.of(examTicketImgUrl, examTicketInfo.userName(), examTicketInfo.birth(), examTicketInfo.examNumber(), subjects, examTicketInfo.schoolName()); - }src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java (1)
107-107: Address the TODO comment for testing.Please add unit tests for the
getApplicationsmethod as indicated by the TODO comment.Would you like me to help generate unit tests for this method or create an issue to track this task?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java(3 hunks)src/main/java/life/mosu/mosuserver/application/exam/ExamQuotaCacheManager.java(1 hunks)src/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java(2 hunks)src/main/java/life/mosu/mosuserver/application/examapplication/dto/RegisterExamApplicationEvent.java(2 hunks)src/main/java/life/mosu/mosuserver/domain/application/ExamTicketImageJpaRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationInfoProjection.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationJpaEntity.java(2 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationJpaRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/ExamSubjectJpaRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/ExamTicketInfoProjection.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/respository/ExamApplicationBulkRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/dto/ExamTicketResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/application/ApplicationController.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/application/ApplicationControllerDocs.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/ApplicationResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/CreateApplicationResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/ExamApplicationRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/ExamApplicationResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/exam/ExamController.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/examapplication/ExamApplicationController.java(3 hunks)src/main/java/life/mosu/mosuserver/presentation/examapplication/dto/ExamApplicationInfoResponse.java(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- src/main/java/life/mosu/mosuserver/application/exam/ExamQuotaCacheManager.java
- src/main/java/life/mosu/mosuserver/domain/examapplication/ExamTicketInfoProjection.java
- src/main/java/life/mosu/mosuserver/presentation/application/dto/CreateApplicationResponse.java
- src/main/java/life/mosu/mosuserver/presentation/application/dto/ExamApplicationResponse.java
- src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationInfoProjection.java
- src/main/java/life/mosu/mosuserver/presentation/examapplication/dto/ExamApplicationInfoResponse.java
🚧 Files skipped from review as they are similar to previous changes (7)
- src/main/java/life/mosu/mosuserver/presentation/application/dto/ExamApplicationRequest.java
- src/main/java/life/mosu/mosuserver/application/examapplication/dto/RegisterExamApplicationEvent.java
- src/main/java/life/mosu/mosuserver/presentation/examapplication/ExamApplicationController.java
- src/main/java/life/mosu/mosuserver/presentation/exam/ExamController.java
- src/main/java/life/mosu/mosuserver/presentation/application/ApplicationController.java
- src/main/java/life/mosu/mosuserver/presentation/application/dto/ApplicationResponse.java
- src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationJpaEntity.java
🔇 Additional comments (9)
src/main/java/life/mosu/mosuserver/domain/examapplication/ExamSubjectJpaRepository.java (1)
12-12: LGTM! Efficient bulk fetching method added.The new method follows Spring Data JPA naming conventions and enables efficient bulk retrieval of exam subjects for multiple exam applications.
src/main/java/life/mosu/mosuserver/domain/application/ExamTicketImageJpaRepository.java (1)
3-3: Good improvement to null safety!Wrapping the return type in Optional is a best practice that makes the API more explicit about potential absence of data and helps prevent NullPointerExceptions.
Also applies to: 9-9
src/main/java/life/mosu/mosuserver/infra/respository/ExamApplicationBulkRepository.java (2)
29-34: Column mapping forlunch_checkedis correct – ignore this comment
The JPA entity explicitly mapsisLunchCheckedtolunch_checked(@Column(name = "lunch_checked")), so the SQL constant aligns with the mapping. No changes required.Likely an incorrect or invalid review comment.
81-81: VerifySubjectconversion and database mapping
The call toString.valueOf(subj)invokessubj.toString(), which by default returns the enum’sname()(e.g.KOREAN). Since yourSubjectenum carries a separatesubjectNamefield (e.g."국어") via Lombok’s@Getter, please confirm which value yourexam_subject.subjectcolumn should store:
- If the column is meant to hold the enum constant (e.g.
KOREAN), this code is correct.- If it should hold the display name (e.g.
"국어"), switch tosubj.getSubjectName()or overridetoString()on the enum.src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationJpaRepository.java (4)
14-31: LGTM! Well-structured JPQL query with projection.The query efficiently fetches only the required fields using constructor-based projection, which is a performance best practice.
34-50: LGTM! Appropriate use of LEFT JOINs for optional relationships.The query correctly uses LEFT JOINs to handle cases where exam ticket images or related data might not exist.
52-52: LGTM! Standard existence check method.Follows Spring Data JPA naming conventions for efficient existence checking.
54-54: LGTM! Efficient batch fetching method.Good use of the IN clause for fetching multiple exam applications in a single query.
src/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java (1)
41-46: LGTM! Good separation of concerns.The refactoring to return entities instead of saving them directly allows for bulk operations, improving performance.
| Set<Long> examIdSet = new HashSet<>(examIds); | ||
| if (examIds.size() != examIdSet.size()) { | ||
| throw new RuntimeException("같은 시험을 신청할 수 없습니다."); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use consistent exception handling.
Good fix for the duplicate exam ID validation issue. However, use CustomRuntimeException for consistency with the rest of the codebase.
- if (examIds.size() != examIdSet.size()) {
- throw new RuntimeException("같은 시험을 신청할 수 없습니다.");
- }
+ if (examIds.size() != examIdSet.size()) {
+ throw new CustomRuntimeException(ErrorCode.DUPLICATE_EXAM_APPLICATION);
+ }Note: You'll need to add the corresponding error code to the ErrorCode enum.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Set<Long> examIdSet = new HashSet<>(examIds); | |
| if (examIds.size() != examIdSet.size()) { | |
| throw new RuntimeException("같은 시험을 신청할 수 없습니다."); | |
| } | |
| Set<Long> examIdSet = new HashSet<>(examIds); | |
| if (examIds.size() != examIdSet.size()) { | |
| throw new CustomRuntimeException(ErrorCode.DUPLICATE_EXAM_APPLICATION); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java
around lines 65 to 68, replace the generic RuntimeException thrown for duplicate
exam IDs with CustomRuntimeException to maintain consistency with the codebase.
Also, add a new error code representing this specific validation error to the
ErrorCode enum and use it when constructing the CustomRuntimeException.
| public List<Long> saveAllExamApplicationsWithSubjects( | ||
| List<ExamApplicationJpaEntity> entities, Set<Subject> subjects) { | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation to prevent potential issues.
The method should validate inputs before processing.
public List<Long> saveAllExamApplicationsWithSubjects(
List<ExamApplicationJpaEntity> entities, Set<Subject> subjects) {
+
+ if (entities == null || entities.isEmpty()) {
+ throw new IllegalArgumentException("Entities list cannot be null or empty");
+ }
+ if (subjects == null || subjects.isEmpty()) {
+ throw new IllegalArgumentException("Subjects set cannot be null or empty");
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public List<Long> saveAllExamApplicationsWithSubjects( | |
| List<ExamApplicationJpaEntity> entities, Set<Subject> subjects) { | |
| public List<Long> saveAllExamApplicationsWithSubjects( | |
| List<ExamApplicationJpaEntity> entities, Set<Subject> subjects) { | |
| if (entities == null || entities.isEmpty()) { | |
| throw new IllegalArgumentException("Entities list cannot be null or empty"); | |
| } | |
| if (subjects == null || subjects.isEmpty()) { | |
| throw new IllegalArgumentException("Subjects set cannot be null or empty"); | |
| } | |
| // ... existing implementation ... | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/infra/respository/ExamApplicationBulkRepository.java
around lines 42 to 44, the method saveAllExamApplicationsWithSubjects lacks
input validation. Add checks to ensure that the entities list and subjects set
are not null and not empty before proceeding with processing. If either input is
invalid, handle it appropriately, such as throwing an IllegalArgumentException
or returning early, to prevent potential runtime errors.
| ps.setTimestamp(1, Timestamp.valueOf(LocalDateTime.now())); | ||
| ps.setTimestamp(2, Timestamp.valueOf(LocalDateTime.now())); |
There was a problem hiding this comment.
Use entity timestamps instead of hardcoding new timestamps.
The code creates new timestamps instead of using the entity's created/updated timestamps if they exist.
- ps.setTimestamp(1, Timestamp.valueOf(LocalDateTime.now()));
- ps.setTimestamp(2, Timestamp.valueOf(LocalDateTime.now()));
+ ps.setTimestamp(1, Timestamp.valueOf(e.getCreatedAt() != null ? e.getCreatedAt() : LocalDateTime.now()));
+ ps.setTimestamp(2, Timestamp.valueOf(e.getUpdatedAt() != null ? e.getUpdatedAt() : LocalDateTime.now()));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ps.setTimestamp(1, Timestamp.valueOf(LocalDateTime.now())); | |
| ps.setTimestamp(2, Timestamp.valueOf(LocalDateTime.now())); | |
| ps.setTimestamp(1, Timestamp.valueOf( | |
| e.getCreatedAt() != null | |
| ? e.getCreatedAt() | |
| : LocalDateTime.now() | |
| )); | |
| ps.setTimestamp(2, Timestamp.valueOf( | |
| e.getUpdatedAt() != null | |
| ? e.getUpdatedAt() | |
| : LocalDateTime.now() | |
| )); |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/infra/respository/ExamApplicationBulkRepository.java
around lines 49 to 50, replace the hardcoded new timestamps with the entity's
existing created and updated timestamp values. Retrieve these timestamps from
the entity object and use them in the ps.setTimestamp calls instead of
LocalDateTime.now(), ensuring the correct timestamps are persisted.
| } catch (SQLException e) { | ||
| log.info("SQL Exception : {}", e); | ||
| throw new CustomRuntimeException(ErrorCode.EXAM_APPLICATION_MULTI_INSERT_ERROR); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error logging to include more context.
The current log message doesn't provide enough information for debugging.
- log.info("SQL Exception : {}", e);
+ log.error("Failed to insert exam applications. Error: {}", e.getMessage(), e);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (SQLException e) { | |
| log.info("SQL Exception : {}", e); | |
| throw new CustomRuntimeException(ErrorCode.EXAM_APPLICATION_MULTI_INSERT_ERROR); | |
| } catch (SQLException e) { | |
| - log.info("SQL Exception : {}", e); | |
| + log.error("Failed to insert exam applications. Error: {}", e.getMessage(), e); | |
| throw new CustomRuntimeException(ErrorCode.EXAM_APPLICATION_MULTI_INSERT_ERROR); |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/infra/respository/ExamApplicationBulkRepository.java
around lines 68 to 70, the SQLException catch block logs the exception with a
generic message that lacks sufficient context. Enhance the log message by
including more detailed information such as the operation being performed or
relevant identifiers, and ensure the exception stack trace or message is fully
logged to aid debugging.
…er and related interfaces
…d failure management - event driven transaction - when error, rollback
…countAmount method refactor: recreate ExamNumberGenerationService to implement NumberGenerator interface
…ityDiscountCalculator
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
src/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java (1)
124-124: Magic string "신청 안 함" should be a constantsrc/main/java/life/mosu/mosuserver/application/application/ApplicationService.java (1)
239-239: Magic string "신청 안 함" should be a constant
🧹 Nitpick comments (3)
src/main/java/life/mosu/mosuserver/infra/respository/ExamApplicationBulkRepository.java (2)
48-56: Consider generating timestamps once for consistencyCreating timestamps inside the loop could result in slightly different creation times for entities in the same batch. Consider generating the timestamp once before the loop to ensure all entities in the batch have identical creation times.
+LocalDateTime now = LocalDateTime.now(); +Timestamp timestamp = Timestamp.valueOf(now); for (ExamApplicationJpaEntity e : entities) { - ps.setTimestamp(1, Timestamp.valueOf(LocalDateTime.now())); - ps.setTimestamp(2, Timestamp.valueOf(LocalDateTime.now())); + ps.setTimestamp(1, timestamp); + ps.setTimestamp(2, timestamp); ps.setLong(3, e.getApplicationId()); ps.setLong(4, e.getExamId()); ps.setBoolean(5, e.getIsLunchChecked()); ps.setString(6, e.getExamNumber()); ps.addBatch(); }
69-69: Use error level for SQL exceptionsSQL exceptions should be logged at error level, not info level, as they indicate actual problems that need attention.
-log.info("SQL Exception : {}", e); +log.error("SQL Exception during bulk exam application insert", e);src/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java (1)
95-95: Address the TODO commentThe TODO indicates that the
getApplicationmethod needs testing. Please ensure tests are added before merging.Would you like me to help generate unit tests for this method or create an issue to track this task?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
pinpoint-agent/profiles/.gitignore(0 hunks)src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java(3 hunks)src/main/java/life/mosu/mosuserver/application/exam/ExamQuotaCacheManager.java(1 hunks)src/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java(2 hunks)src/main/java/life/mosu/mosuserver/application/examapplication/dto/RegisterExamApplicationEvent.java(2 hunks)src/main/java/life/mosu/mosuserver/domain/application/ExamTicketImageJpaRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationInfoProjection.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationJpaEntity.java(2 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationJpaRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/ExamSubjectJpaRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/ExamTicketInfoProjection.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/property/S3Properties.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/respository/ExamApplicationBulkRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/storage/application/S3Service.java(4 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/dto/ExamTicketResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/application/ApplicationController.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/application/ApplicationControllerDocs.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/ApplicationResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/CreateApplicationResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/ExamApplicationRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/ExamApplicationResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/exam/ExamController.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/examapplication/ExamApplicationController.java(3 hunks)src/main/java/life/mosu/mosuserver/presentation/examapplication/dto/ExamApplicationInfoResponse.java(1 hunks)
💤 Files with no reviewable changes (1)
- pinpoint-agent/profiles/.gitignore
✅ Files skipped from review due to trivial changes (7)
- src/main/java/life/mosu/mosuserver/application/exam/ExamQuotaCacheManager.java
- src/main/java/life/mosu/mosuserver/domain/examapplication/ExamTicketInfoProjection.java
- src/main/java/life/mosu/mosuserver/infra/storage/application/S3Service.java
- src/main/java/life/mosu/mosuserver/presentation/application/dto/CreateApplicationResponse.java
- src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationInfoProjection.java
- src/main/java/life/mosu/mosuserver/presentation/examapplication/dto/ExamApplicationInfoResponse.java
- src/main/java/life/mosu/mosuserver/presentation/application/dto/ExamApplicationResponse.java
🚧 Files skipped from review as they are similar to previous changes (7)
- src/main/java/life/mosu/mosuserver/presentation/application/dto/ExamApplicationRequest.java
- src/main/java/life/mosu/mosuserver/application/examapplication/dto/RegisterExamApplicationEvent.java
- src/main/java/life/mosu/mosuserver/presentation/exam/ExamController.java
- src/main/java/life/mosu/mosuserver/presentation/application/dto/ApplicationResponse.java
- src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationJpaEntity.java
- src/main/java/life/mosu/mosuserver/presentation/application/ApplicationController.java
- src/main/java/life/mosu/mosuserver/presentation/examapplication/ExamApplicationController.java
🔇 Additional comments (10)
src/main/java/life/mosu/mosuserver/infra/property/S3Properties.java (1)
15-22: Good refactoring to centralize S3 configuration.The change to centralize S3 properties in a dedicated configuration class is a good practice. This improves maintainability and ensures consistent configuration access throughout the application.
src/main/java/life/mosu/mosuserver/domain/examapplication/ExamSubjectJpaRepository.java (1)
12-12: Excellent addition for bulk data retrieval optimization.The new
findByExamApplicationIdInmethod enables efficient bulk retrieval of exam subjects, which should improve performance when fetching data for multiple exam applications simultaneously. This aligns well with the refactoring goals mentioned in the PR.src/main/java/life/mosu/mosuserver/presentation/admin/dto/ExamTicketResponse.java (1)
46-61: Good addition of factory method for handling optional exam ticket images.The
ofWithoutExamTicketfactory method provides clear semantics for creating responses when no exam ticket image is available. This improves code readability and handles the optional nature of exam ticket images gracefully.src/main/java/life/mosu/mosuserver/domain/application/ExamTicketImageJpaRepository.java (1)
3-3: Excellent improvement using Optional for safer data access.Changing the return type to
Optional<ExamTicketImageJpaEntity>is a best practice that explicitly handles the possibility that no exam ticket image exists for a given application ID. This prevents potential NullPointerExceptions and makes the API more expressive.Also applies to: 9-9
src/main/java/life/mosu/mosuserver/presentation/application/ApplicationControllerDocs.java (1)
16-16: Good refactoring to improve REST API design.The changes improve the API design by:
- Using a specific
CreateApplicationResponseDTO for creation operations- Adding
UriComponentsBuilderto enable proper Location header construction for created resourcesThese changes align with REST best practices.
Also applies to: 20-20, 29-36
src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationJpaRepository.java (2)
34-50: LGTM! Well-structured query with appropriate LEFT JOINsThe query correctly uses LEFT JOINs for optional relationships and efficiently retrieves all necessary data in a single query using projections.
14-31: Verify payment optionality before switching to LEFT JOINI wasn’t able to find references in the codebase that show whether an
ExamApplicationJpaEntitycan exist without a correspondingPaymentJpaEntity. Before changing theINNER JOINonPaymentJpaEntityto aLEFT JOIN, please confirm:
- Are payments always created at the same time as exam applications, or can an application exist in a “pending payment” state?
- Is the relationship between
ExamApplicationJpaEntityandPaymentJpaEntitymarked as nullable/optional in the JPA mappings?If payments can be missing or delayed, switch to:
- JOIN PaymentJpaEntity p on p.examApplicationId = ea.id + LEFT JOIN PaymentJpaEntity p on p.examApplicationId = ea.idsrc/main/java/life/mosu/mosuserver/application/application/ApplicationService.java (3)
64-68: Good fix for duplicate exam ID validation!The implementation now correctly uses a Set to check for duplicate exam IDs, addressing the issue raised in the previous review.
130-151: Well-structured validation methodThe lunch selection validation is properly implemented with clear logic and appropriate error handling.
91-93: Subject handling is as intendedThe
subjectspassed tosaveAllExamApplicationsWithSubjectscome fromApplicationRequest.validatedSubjects()—a single, application-level set. SinceExamApplicationRequestdoes not include per-exam subjects, applying the same subjects to everyExamApplicationJpaEntityis the correct behavior.
No changes required.Likely an incorrect or invalid review comment.
src/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java
Show resolved
Hide resolved
src/main/java/life/mosu/mosuserver/infra/respository/ExamApplicationBulkRepository.java
Show resolved
Hide resolved
src/main/java/life/mosu/mosuserver/presentation/application/ApplicationControllerDocs.java
Show resolved
Hide resolved
…server into refactor/mosu-121
…o refactor/mosu-121
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/life/mosu/mosuserver/application/payment/PaymentService.java (3)
40-45: Clean implementation with good separation of concerns.The prepare method is much more readable and follows the delegation principle effectively. Consider adding null validation for the request parameter to prevent NullPointerException.
public PaymentPrepareResponse prepare(PreparePaymentRequest request) { + if (request == null || request.applicationId() == null) { + throw new CustomRuntimeException(ErrorCode.INVALID_REQUEST); + } List<ExamApplicationJpaEntity> applications = getValidApplications(request.applicationId());
47-74: Solid transaction handling with proper event publishing.The confirm method demonstrates good practices:
- Proper
@Transactionalannotation- Exception-driven validation approach
- Success/failure event publishing
- Transaction rollback on exceptions
However, clarify the lunch amount calculation logic on lines 51 and 57. The method calculates
lunchAmountseparately but then adds it torequest.amount()for verification. This seems to suggest thatrequest.amount()doesn't include lunch costs, but this should be clearly documented or validated.Consider adding a comment to clarify the lunch amount calculation:
int lunchAmount = amountCalculator.calculateLunchAmount(applications); +// Verify total amount including lunch costs not included in the original request amountCalculator.verifyAmount(applications.size(), request.amount() + lunchAmount);
76-83: Good validation approach with clear error handling.The method follows the fail-fast principle effectively. However, consider the naming inconsistency:
findByApplicationId()takes a single ID but returns aList<ExamApplicationJpaEntity>. This suggests either:
- One application can have multiple exam applications (which makes sense)
- The method name could be more descriptive like
findExamApplicationsByApplicationId()The empty list validation is appropriate and prevents downstream processing errors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
logs/app.logis excluded by!**/*.log
📒 Files selected for processing (57)
src/main/java/life/mosu/mosuserver/MosuServerApplication.java(1 hunks)src/main/java/life/mosu/mosuserver/application/exam/ExamService.java(1 hunks)src/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java(2 hunks)src/main/java/life/mosu/mosuserver/application/payment/OrderIdGenerator.java(0 hunks)src/main/java/life/mosu/mosuserver/application/payment/PaymentEvent.java(0 hunks)src/main/java/life/mosu/mosuserver/application/payment/PaymentEventListener.java(0 hunks)src/main/java/life/mosu/mosuserver/application/payment/PaymentEventTxService.java(1 hunks)src/main/java/life/mosu/mosuserver/application/payment/PaymentFailureHandler.java(0 hunks)src/main/java/life/mosu/mosuserver/application/payment/PaymentService.java(2 hunks)src/main/java/life/mosu/mosuserver/application/payment/processor/PaymentProcessor.java(1 hunks)src/main/java/life/mosu/mosuserver/application/payment/processor/TossPaymentProcessor.java(1 hunks)src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentContext.java(1 hunks)src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentTxEvent.java(1 hunks)src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentTxEventFactory.java(1 hunks)src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentTxEventListener.java(1 hunks)src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentTxFailureHandler.java(1 hunks)src/main/java/life/mosu/mosuserver/application/refund/RefundEventTxService.java(1 hunks)src/main/java/life/mosu/mosuserver/application/refund/RefundService.java(1 hunks)src/main/java/life/mosu/mosuserver/application/refund/processor/RefundProcessor.java(1 hunks)src/main/java/life/mosu/mosuserver/application/refund/processor/TossRefundProcessor.java(1 hunks)src/main/java/life/mosu/mosuserver/application/refund/tx/RefundContext.java(1 hunks)src/main/java/life/mosu/mosuserver/application/refund/tx/RefundTxEvent.java(1 hunks)src/main/java/life/mosu/mosuserver/application/refund/tx/RefundTxEventFactory.java(1 hunks)src/main/java/life/mosu/mosuserver/application/refund/tx/RefundTxEventListener.java(1 hunks)src/main/java/life/mosu/mosuserver/application/refund/tx/RefundTxFailureHandler.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/admin/ApplicationQueryRepositoryImpl.java(3 hunks)src/main/java/life/mosu/mosuserver/domain/discount/FixedQuantityDiscountCalculator.java(2 hunks)src/main/java/life/mosu/mosuserver/domain/exam/ExamJpaRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationJpaEntity.java(3 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/ExamNumberGenerationService.java(0 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/service/ExamNumberGenerationService.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/payment/PaymentJpaRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/payment/service/PaymentAmountCalculator.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/payment/service/PaymentMapper.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/payment/service/PaymentOrderIdGenerator.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/refund/RefundStatus.java(1 hunks)src/main/java/life/mosu/mosuserver/global/NumberGenerator.java(1 hunks)src/main/java/life/mosu/mosuserver/global/config/SecurityConfig.java(1 hunks)src/main/java/life/mosu/mosuserver/global/config/WebMvcConfig.java(1 hunks)src/main/java/life/mosu/mosuserver/global/exception/ErrorCode.java(2 hunks)src/main/java/life/mosu/mosuserver/global/initializer/DatabaseInitializer.java(3 hunks)src/main/java/life/mosu/mosuserver/global/tx/DefaultTxEventPublisher.java(1 hunks)src/main/java/life/mosu/mosuserver/global/tx/TxEvent.java(1 hunks)src/main/java/life/mosu/mosuserver/global/tx/TxEventFactory.java(1 hunks)src/main/java/life/mosu/mosuserver/global/tx/TxEventPublisher.java(1 hunks)src/main/java/life/mosu/mosuserver/global/tx/TxFailureHandler.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/payment/dto/ConfirmTossPaymentResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/respository/BulkRepository.java(0 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/dto/ApplicationListResponse.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/dto/ExamTicketResponse.java(4 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/ExamApplicationRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/payment/PaymentWidgetController.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/payment/dto/PaymentRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/refund/RefundController.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/refund/dto/RefundRequest.java(1 hunks)src/main/resources/application.yml(1 hunks)src/test/java/life/mosu/mosuserver/discount/FixedQuantityDiscountCalculatorTest.java(1 hunks)
🧬 Code Graph Analysis (1)
src/main/java/life/mosu/mosuserver/application/payment/PaymentService.java (1)
src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentTxEventListener.java (1)
Transactional(12-28)
💤 Files with no reviewable changes (6)
- src/main/java/life/mosu/mosuserver/application/payment/OrderIdGenerator.java
- src/main/java/life/mosu/mosuserver/application/payment/PaymentFailureHandler.java
- src/main/java/life/mosu/mosuserver/domain/examapplication/ExamNumberGenerationService.java
- src/main/java/life/mosu/mosuserver/infra/respository/BulkRepository.java
- src/main/java/life/mosu/mosuserver/application/payment/PaymentEvent.java
- src/main/java/life/mosu/mosuserver/application/payment/PaymentEventListener.java
✅ Files skipped from review due to trivial changes (14)
- src/main/resources/application.yml
- src/main/java/life/mosu/mosuserver/presentation/admin/dto/ApplicationListResponse.java
- src/main/java/life/mosu/mosuserver/application/refund/processor/RefundProcessor.java
- src/main/java/life/mosu/mosuserver/application/refund/tx/RefundTxFailureHandler.java
- src/main/java/life/mosu/mosuserver/global/tx/TxEventFactory.java
- src/main/java/life/mosu/mosuserver/presentation/refund/RefundController.java
- src/main/java/life/mosu/mosuserver/application/refund/tx/RefundTxEventFactory.java
- src/main/java/life/mosu/mosuserver/global/initializer/DatabaseInitializer.java
- src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentTxEvent.java
- src/main/java/life/mosu/mosuserver/application/refund/tx/RefundTxEventListener.java
- src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentTxEventFactory.java
- src/main/java/life/mosu/mosuserver/application/refund/tx/RefundTxEvent.java
- src/main/java/life/mosu/mosuserver/domain/payment/service/PaymentMapper.java
- src/main/java/life/mosu/mosuserver/domain/payment/service/PaymentAmountCalculator.java
🚧 Files skipped from review as they are similar to previous changes (36)
- src/main/java/life/mosu/mosuserver/global/config/SecurityConfig.java
- src/main/java/life/mosu/mosuserver/global/NumberGenerator.java
- src/main/java/life/mosu/mosuserver/presentation/payment/PaymentWidgetController.java
- src/main/java/life/mosu/mosuserver/global/config/WebMvcConfig.java
- src/main/java/life/mosu/mosuserver/domain/exam/ExamJpaRepository.java
- src/test/java/life/mosu/mosuserver/discount/FixedQuantityDiscountCalculatorTest.java
- src/main/java/life/mosu/mosuserver/global/tx/TxEventPublisher.java
- src/main/java/life/mosu/mosuserver/presentation/refund/dto/RefundRequest.java
- src/main/java/life/mosu/mosuserver/application/payment/processor/PaymentProcessor.java
- src/main/java/life/mosu/mosuserver/domain/refund/RefundStatus.java
- src/main/java/life/mosu/mosuserver/presentation/payment/dto/PaymentRequest.java
- src/main/java/life/mosu/mosuserver/MosuServerApplication.java
- src/main/java/life/mosu/mosuserver/application/refund/processor/TossRefundProcessor.java
- src/main/java/life/mosu/mosuserver/application/exam/ExamService.java
- src/main/java/life/mosu/mosuserver/presentation/application/dto/ExamApplicationRequest.java
- src/main/java/life/mosu/mosuserver/infra/payment/dto/ConfirmTossPaymentResponse.java
- src/main/java/life/mosu/mosuserver/global/tx/TxEvent.java
- src/main/java/life/mosu/mosuserver/domain/payment/service/PaymentOrderIdGenerator.java
- src/main/java/life/mosu/mosuserver/presentation/admin/dto/ExamTicketResponse.java
- src/main/java/life/mosu/mosuserver/application/payment/processor/TossPaymentProcessor.java
- src/main/java/life/mosu/mosuserver/global/tx/DefaultTxEventPublisher.java
- src/main/java/life/mosu/mosuserver/application/refund/RefundService.java
- src/main/java/life/mosu/mosuserver/application/refund/tx/RefundContext.java
- src/main/java/life/mosu/mosuserver/domain/admin/ApplicationQueryRepositoryImpl.java
- src/main/java/life/mosu/mosuserver/global/tx/TxFailureHandler.java
- src/main/java/life/mosu/mosuserver/application/payment/PaymentEventTxService.java
- src/main/java/life/mosu/mosuserver/domain/payment/PaymentJpaRepository.java
- src/main/java/life/mosu/mosuserver/domain/discount/FixedQuantityDiscountCalculator.java
- src/main/java/life/mosu/mosuserver/application/refund/RefundEventTxService.java
- src/main/java/life/mosu/mosuserver/domain/examapplication/ExamApplicationJpaEntity.java
- src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentContext.java
- src/main/java/life/mosu/mosuserver/global/exception/ErrorCode.java
- src/main/java/life/mosu/mosuserver/domain/examapplication/service/ExamNumberGenerationService.java
- src/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java
- src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentTxFailureHandler.java
- src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentTxEventListener.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/life/mosu/mosuserver/application/payment/PaymentService.java (1)
src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentTxEventListener.java (1)
Transactional(12-28)
🔇 Additional comments (2)
src/main/java/life/mosu/mosuserver/application/payment/PaymentService.java (2)
4-38: Excellent modularization and dependency injection structure.The refactoring introduces well-separated components following the Single Responsibility Principle. Each injected dependency has a clear, focused purpose:
TossPaymentProcessor: Handles payment processing logicPaymentAmountCalculator: Centralizes amount calculationsPaymentMapper: Handles entity mappingPaymentOrderIdGenerator: Generates unique order IDsPaymentEventTxService: Manages transactional event publishingThe use of
finalfields ensures immutability and proper dependency injection.
85-89: Effective duplicate prevention mechanism.The duplicate order ID validation is essential for payment integrity. The method is clean and uses appropriate exception handling.
2 participants
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Refactor
Chores