-
Notifications
You must be signed in to change notification settings - Fork 2
전반적인 도메인 로직 내에서 validation 체크 및 NPE 안정성 보강 #250
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Caution Review failedThe pull request is closed. WalkthroughThis PR applies validations, null-safety, and query refinements across application, exam, event, auth, inquiry, refund, caching, and rate-limiting subsystems; it also adds new endpoints/DTOs, new caching infrastructure, and several repository/query signature updates without changing most public method contracts. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ApplicationController
participant ApplicationService
participant ApplicationValidator
User->>ApplicationController: POST /apply(request)
ApplicationController->>ApplicationService: apply(request)
ApplicationService->>ApplicationValidator: agreedToTerms(request)
ApplicationValidator-->>ApplicationService: (throws if not agreed)
ApplicationService->>ApplicationValidator: requestNoDuplicateExams(examIds)
ApplicationValidator-->>ApplicationService: (throws if duplicate/none)
ApplicationService->>ApplicationService: handleApplication(request)
ApplicationService-->>ApplicationController: 200/created
sequenceDiagram
participant Admin
participant AdminDashboardService
participant ExamApplicationRepo
participant RefundFailureLogRepo
participant UserRepo
Admin->>AdminDashboardService: getAll()
AdminDashboardService->>ExamApplicationRepo: countAll()
AdminDashboardService->>RefundFailureLogRepo: count()
AdminDashboardService->>UserRepo: countByUserRoleNot(ROLE_ADMIN)
AdminDashboardService-->>Admin: DashBoardResponse.of(appCounts, refundAbortedCounts, userCounts)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 primarily focuses on enhancing the application's validation mechanisms and addressing several incomplete or minor issues. I've introduced more robust validation checks across various services and DTOs, particularly for application submissions and exam registrations. Additionally, I've refined the admin dashboard's data reporting, improved data consistency by ensuring queries filter out deleted records, and performed general code cleanup and refactoring to improve maintainability.
Highlights
- Enhanced Validation: New validation rules were added for application requests, including checks for agreeing to terms, preventing duplicate exam applications, and ensuring exam dates are correctly set after the deadline.
- Dashboard Metrics Refinement: The admin dashboard now provides more detailed metrics, specifically separating refund counts into 'done' and 'aborted' statuses and accurately counting users by excluding administrative roles.
- Data Consistency Improvements: Several JPA queries were updated to explicitly filter records by
p.deleted = false, ensuring that only active payment and application records are considered, improving data integrity. - Code Clean-up and Refactoring: Unnecessary logging statements and commented-out code were removed, and a helper method was extracted in
ExamApplicationServiceto streamline the codebase. - DTO and Entity Adjustments: Field names in recommendation-related projections and DTOs were corrected, and null checks were added for optional fields in various DTOs to prevent potential
NullPointerExceptions.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces several validation checks and code refactorings, which generally improve the quality and robustness of the codebase. I've identified a few areas for improvement, including a missing validation for exam existence after a refactoring, a potential minor information leak due to an insufficient authorization check, a bug where a request parameter is ignored in favor of the current date, and an incorrect usage of the @Valid annotation. My review includes specific suggestions to address these points.
src/main/java/life/mosu/mosuserver/application/application/vaildator/ApplicationValidator.java
Show resolved
Hide resolved
src/main/java/life/mosu/mosuserver/presentation/event/dto/DurationRequest.java
Show resolved
Hide resolved
| public ExamApplicationInfoResponse getApplication(Long userId, Long examApplicationId, | ||
| Long applicationId) { | ||
| validateUser(userId, examApplicationId); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validateUser call was removed. While the query for findExamApplicationInfoById now includes a userId check for examApplicationId, there is no validation that the user has access to the provided applicationId. A user could potentially access information (like lunchCount) from another user's application by providing a valid examApplicationId they own, but an applicationId belonging to someone else. Please add a check to verify that the applicationId belongs to the userId.
| applicationJpaRepository.findById(applicationId) | |
| .filter(app -> app.getUserId().equals(userId)) | |
| .orElseThrow(() -> new CustomRuntimeException(ErrorCode.USER_NOT_ACCESS_FORBIDDEN)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/gemini 그럼 어떻게 해결하는 것을 추천해
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 이전에 제안했던 코드 스니펫이 applicationId가 userId에 속하는지 확인하는 권한 부여 검사를 추가하는 권장 해결책입니다. 이 코드를 getApplication 메서드 시작 부분에 추가하여 사용자가 액세스하려는 applicationId에 대한 권한이 있는지 확인해야 합니다. 이렇게 하면 다른 사용자의 애플리케이션 정보에 대한 무단 액세스를 방지할 수 있습니다.
| applicationJpaRepository.findById(applicationId) | |
| .filter(app -> app.getUserId().equals(userId)) | |
| .orElseThrow(() -> new CustomRuntimeException(ErrorCode.USER_NOT_ACCESS_FORBIDDEN)); |
| @GetMapping | ||
| public ResponseEntity<ApiResponseWrapper<List<ExamResponse>>> getByArea( | ||
| @RequestParam String areaName | ||
| @Valid @RequestParam String areaName |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The @Valid annotation on a @RequestParam of type String has no effect here. For validation to be triggered on a simple type parameter, you would need to add constraint annotations directly to the parameter (e.g., @NotBlank @RequestParam String areaName) and have a @Validated annotation on the controller class. As it is, @Valid is not doing anything. Please remove it.
| @Valid @RequestParam String areaName | |
| @RequestParam String areaName |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/main/java/life/mosu/mosuserver/presentation/event/dto/EventRequest.java (2)
27-30: Remove commented code.Instead of leaving commented code in the codebase, consider removing it entirely to improve code cleanliness and maintainability.
-// public List<FileRequest> optionalAttachment() { -// FileRequest parsedAttachment = this.attachment; -// return parsedAttachment == null ? List.of() : List.of(parsedAttachment); -// }
36-39: LGTM: Good null safety improvements with a minor fix needed.The null checks before accessing nested properties are excellent defensive programming practices that prevent
NullPointerException.However, there's a small inconsistency on lines 37-38. You're calling
attachment()method instead of accessing theattachmentfield directly:- .fileName(attachment != null ? attachment().fileName() : null) - .s3Key(attachment != null ? attachment().s3Key() : null) + .fileName(attachment != null ? attachment.fileName() : null) + .s3Key(attachment != null ? attachment.s3Key() : null)src/main/java/life/mosu/mosuserver/presentation/exam/ExamController.java (1)
49-49: Consider if validation is needed for simple String parameter.While
@Validis applied to theareaNameparameter, it's typically used for complex objects with validation constraints. For a simpleStringparameter, consider using specific validation annotations like@NotBlankinstead.Apply this change for more appropriate validation:
-@Valid @RequestParam String areaName +@NotBlank @RequestParam String areaNamesrc/main/java/life/mosu/mosuserver/application/application/vaildator/ApplicationValidator.java (1)
18-18: Consider removing unused @slf4j annotation.The
@Slf4jannotation and import are present but no logging statements are used in the class. Consider removing them if logging is not needed, or add appropriate log statements for validation failures.-import lombok.extern.slf4j.Slf4j;-@Slf4jAlso applies to: 21-21
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
src/main/java/life/mosu/mosuserver/application/admin/AdminDashboardService.java(2 hunks)src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java(1 hunks)src/main/java/life/mosu/mosuserver/application/application/processor/GetApplicationsStepProcessor.java(0 hunks)src/main/java/life/mosu/mosuserver/application/application/vaildator/ApplicationValidator.java(3 hunks)src/main/java/life/mosu/mosuserver/application/event/EventService.java(1 hunks)src/main/java/life/mosu/mosuserver/application/exam/ExamService.java(2 hunks)src/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java(3 hunks)src/main/java/life/mosu/mosuserver/domain/admin/projection/RecommendationDetailsProjection.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/application/repository/ApplicationJpaRepository.java(0 hunks)src/main/java/life/mosu/mosuserver/domain/exam/entity/ExamJpaEntity.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/repository/ExamApplicationJpaRepository.java(7 hunks)src/main/java/life/mosu/mosuserver/domain/examapplication/repository/ExamSubjectJpaRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/profile/entity/ProfileJpaEntity.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/refund/repository/RefundJpaRepository.java(2 hunks)src/main/java/life/mosu/mosuserver/domain/user/repository/UserJpaRepository.java(2 hunks)src/main/java/life/mosu/mosuserver/global/exception/ErrorCode.java(3 hunks)src/main/java/life/mosu/mosuserver/infra/persistence/jpa/ExamApplicationBulkRepository.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/AdminBannerController.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/dto/BannerRequest.java(0 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/dto/DashBoardResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/dto/RecommendationExcelDto.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/AgreementRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/application/dto/ApplicationRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/event/dto/DurationRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/event/dto/EventRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/event/dto/EventResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/exam/ExamController.java(4 hunks)src/main/java/life/mosu/mosuserver/presentation/exam/dto/ExamRequest.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/profile/dto/EditProfileRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/recommendation/dto/RecommendationRequest.java(1 hunks)
💤 Files with no reviewable changes (3)
- src/main/java/life/mosu/mosuserver/application/application/processor/GetApplicationsStepProcessor.java
- src/main/java/life/mosu/mosuserver/domain/application/repository/ApplicationJpaRepository.java
- src/main/java/life/mosu/mosuserver/presentation/admin/dto/BannerRequest.java
🔇 Additional comments (49)
src/main/java/life/mosu/mosuserver/presentation/admin/AdminBannerController.java (2)
3-3: Good addition of Jakarta validation import.The import is correctly added to support the
@Validannotation usage.
32-32: Excellent addition of input validation.Adding
@Validannotation enables automatic validation of theBannerRequestobject based on its validation constraints. This is a good security practice that ensures invalid data is rejected before reaching the service layer.src/main/java/life/mosu/mosuserver/domain/user/repository/UserJpaRepository.java (2)
5-5: LGTM!The UserRole import is correctly added to support the new repository method.
19-19: LGTM!The repository method follows Spring Data JPA naming conventions and will correctly generate a query to count users excluding the specified role.
src/main/java/life/mosu/mosuserver/domain/admin/projection/RecommendationDetailsProjection.java (1)
13-14: Rename Verified: No Remaining References to Old Fields
The search forrecommendeeBankandrecommendeeAccountNumberacross the Java codebase returned no hits, confirming that all references have been updated torecommenderBankandrecommenderAccountNumber. No further changes are needed—approving the rename.src/main/java/life/mosu/mosuserver/global/exception/ErrorCode.java (3)
53-53: LGTM!The new error code
NOT_AGREED_TO_TERMSappropriately uses BAD_REQUEST status and provides a clear Korean message for terms agreement validation.
129-130: LGTM!The new exam-related error codes are well-defined:
EXAM_NOT_APPLIED: Ensures at least one exam is selectedEXAM_DATE_AFTER_DEADLINE: Validates exam date logicBoth use appropriate BAD_REQUEST status and clear Korean messages.
168-169: Good cleanup!Removing the trailing comma from the last enum constant improves code style consistency.
src/main/java/life/mosu/mosuserver/presentation/admin/dto/RecommendationExcelDto.java (2)
34-40: Consistent semantic correction!The field renaming and annotation updates correctly align with the projection layer changes. Both Swagger descriptions and Excel column headers are properly updated from recommendee to recommender bank details.
51-52: LGTM!The factory method correctly references the renamed projection fields, maintaining consistency across layers.
src/main/java/life/mosu/mosuserver/presentation/recommendation/dto/RecommendationRequest.java (1)
9-9: Enhanced validation looks good!Adding
@NotBlankto complement the existing@PhoneNumberPatternstrengthens input validation by ensuring the phone number is not null, empty, or whitespace-only. This follows validation best practices.src/main/java/life/mosu/mosuserver/application/event/EventService.java (2)
29-29: LGTM: Code simplificationThe direct save operation without storing the result is appropriate since the method returns void and the saved entity reference wasn't being used.
29-29: LGTM! Good refactoring.Removing the unnecessary variable assignment makes the code more concise without changing functionality.
src/main/java/life/mosu/mosuserver/presentation/application/dto/AgreementRequest.java (2)
16-18: LGTM: Clean validation methodThe
validateAgreement()method correctly validates that both required agreements are accepted. The implementation is concise and follows the expected boolean logic.
16-18: LGTM! Good validation encapsulation.The
validateAgreement()method properly encapsulates the validation logic within the DTO, ensuring both required agreements are true. This follows good practices for keeping validation logic close to the data structure.src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java (2)
50-51: LGTM: Improved validation placementMoving the validation logic to the public
applymethod is a good architectural improvement. Validating user agreement to terms and checking for duplicate exams earlier in the process prevents unnecessary processing when validation fails.
50-51: LGTM! Improved validation structure.Moving validation logic to the public
applymethod is a good architectural improvement:
- All validation now occurs upfront before any processing
- Clear separation between validation (public method) and business logic (private method)
- The new
agreedToTermsvalidation ensures users have accepted required agreementssrc/main/java/life/mosu/mosuserver/domain/refund/repository/RefundJpaRepository.java (4)
7-7: LGTM: Required import additionThe
RefundStatusimport is necessary for the new repository method.
58-58: LGTM: Clean repository method additionThe
countByRefundStatusmethod follows Spring Data JPA naming conventions and will automatically generate the appropriate query for counting refunds by their status. This supports the enhanced dashboard metrics functionality.
7-7: LGTM! Proper import addition.Adding the
RefundStatusimport is necessary for the new method parameter.
58-58: LGTM! Well-designed repository method.The
countByRefundStatusmethod follows Spring Data JPA naming conventions and provides clean separation for counting refunds by status. This supports the dashboard functionality improvements.src/main/java/life/mosu/mosuserver/presentation/profile/dto/EditProfileRequest.java (1)
12-12: Documentation simplification - consider impact on API consumers.The removal of enum details and example values from the Swagger description reduces API documentation clarity. While this aligns with the broader simplification pattern, ensure that API consumers have adequate documentation elsewhere to understand valid
Educationenum values.src/main/java/life/mosu/mosuserver/domain/profile/entity/ProfileJpaEntity.java (1)
92-92: Excellent null safety improvement.The addition of the null check prevents potential
NullPointerExceptionwhenrequest.schoolInfo()is null. This defensive programming practice enhances robustness without changing the method's behavior for valid inputs.src/main/java/life/mosu/mosuserver/infra/persistence/jpa/ExamApplicationBulkRepository.java (2)
56-56: Ensure parameter indexing remains correct.The addition of the
deletedparameter shifts all subsequent parameter indices. Verify that the parameter setting at position 8 aligns correctly with the SQL statement structure.
32-33: Verify deleted flag binding in bulk inserts.The bulk insert SQL now includes the
deletedcolumn but always binds it tofalse, which will override any pre-set deletion state on the entity and may not align with your soft-delete requirements.Please confirm the following:
- The
ExamApplicationJpaEntityclass actually declares adeletedboolean field with a corresponding getter (e.g.,isDeleted()orgetDeleted()).- New bulk inserts should indeed always default to
deleted = false; otherwise update the binding to use the entity’s state.Suggested binding update:
- statement.setBoolean(7, false); + statement.setBoolean(7, e.isDeleted()); // or e.getDeleted()src/main/java/life/mosu/mosuserver/application/exam/ExamService.java (2)
26-26: Good validation placement using fail-fast principle.Calling validation at the start of the
registermethod follows the fail-fast principle, preventing unnecessary processing when validation fails.
77-81: LocalDateTime comparison is correct; confirm timezone strategy
- In ExamRequest,
deadlineTimeis aLocalDateTimeandexamDate().atStartOfDay()also returns aLocalDateTime, so the types are compatible andisBefore(...)will compile and run as expected.- Neither type carries zone information—this comparison occurs in the JVM’s default time zone. Ensure this aligns with your overall date/time policy.
- If you require timezone-aware checks (e.g. comparing against a client’s timezone or UTC), consider switching to
ZonedDateTimeor normalizing both values to a specificZoneIdbefore performing the validation.Location:
- src/main/java/life/mosu/mosuserver/application/exam/ExamService.java (validateExamDate method, lines 77–81)
src/main/java/life/mosu/mosuserver/domain/examapplication/repository/ExamSubjectJpaRepository.java (1)
32-32: Soft-delete filter added to payment join queryThis change ensures we only operate on non-deleted payment records when deleting exam subjects.
Please verify the following before merging:
- The underlying payment table in your database schema has a boolean deleted column.
- Your Payment JPA entity (e.g. PaymentJpaEntity) includes a mapped deleted field.
- There’s an index on deleted (or a composite index covering it) to maintain query performance.
src/main/java/life/mosu/mosuserver/domain/exam/entity/ExamJpaEntity.java (1)
82-84: LGTM: Enhanced lunch validation logic.The expanded condition to check both
lunchNameandlunchPricefor null values is more comprehensive and logical. If either the name or price is missing, the lunch should indeed be considered unavailable.src/main/java/life/mosu/mosuserver/presentation/event/dto/EventResponse.java (1)
37-39: LGTM: Excellent null safety improvements.The null checks for
getDuration()andgetEventLink()properly preventNullPointerExceptionand handle optional fields gracefully. This defensive programming practice improves the robustness of the response generation.src/main/java/life/mosu/mosuserver/presentation/admin/dto/DashBoardResponse.java (1)
3-22: LGTM: Enhanced dashboard metrics granularity.Splitting the refund counts into
refundDoneCountsandrefundAbortedCountsprovides more detailed and useful metrics for the admin dashboard. The factory method is properly updated to accommodate the new structure.src/main/java/life/mosu/mosuserver/presentation/exam/dto/ExamRequest.java (2)
3-4: LGTM! Proper validation annotations added.The validation annotations are correctly applied:
@NotBlankfor string fields that require content@NotNullfor required object/primitive fieldsThis ensures proper input validation at the controller level.
Also applies to: 12-16
30-31: LGTM! Null safety improvement.The null checks for the
lunchfield preventNullPointerExceptionwhen accessingname()andprice()methods on a potentially null object.src/main/java/life/mosu/mosuserver/application/admin/AdminDashboardService.java (1)
22-32: LGTM! Enhanced dashboard metrics with better granularity.The changes provide more accurate dashboard metrics by:
- Using
countAll()instead ofcount()for exam applications (likely filters for valid applications only)- Separating refund counts by status for better insights
- Excluding admin users from user counts for more meaningful business metrics
The method calls align with the repository enhancements mentioned in the AI summary.
src/main/java/life/mosu/mosuserver/application/examapplication/ExamApplicationService.java (2)
98-98: LGTM! Code refactoring improves readability.Extracting the S3 URL logic into a separate method improves code organization and readability.
180-188: LGTM! Well-implemented helper method.The extracted
getExamTicketImgUrlmethod:
- Properly handles null S3 keys
- Has clear single responsibility
- Improves code reusability and maintainability
src/main/java/life/mosu/mosuserver/domain/examapplication/repository/ExamApplicationJpaRepository.java (5)
47-48: LGTM! Enhanced query filtering for data integrity.Adding
ea.userId = :userIdfilter andp.deleted = falsecondition ensures:
- Users can only access their own exam applications
- Deleted payments are excluded from results
This improves both security and data consistency.
50-52: LGTM! Proper parameter annotation.Adding explicit
@Paramannotations improves code clarity and ensures proper parameter binding in JPQL queries.
73-73: LGTM! Consistent filtering of deleted payments.Adding
p.deleted = falsecondition across all payment-related queries ensures consistent data integrity by excluding soft-deleted payment records.Also applies to: 112-112, 129-129, 146-146, 158-158
173-186: LGTM! New validation method with proper filtering.The
existsByUserIdAndExamIdsmethod properly:
- Filters by payment status ('DONE')
- Excludes deleted payments
- Checks for specific user and exam combinations
This supports duplicate application validation as mentioned in the AI summary.
189-196: LGTM! Count method with appropriate filtering.The
countAllmethod correctly counts only exam applications with:
- Completed payments (DONE status)
- Non-deleted payment records
This aligns with the dashboard service requirements for accurate metrics.
src/main/java/life/mosu/mosuserver/presentation/exam/ExamController.java (2)
33-33: LGTM! Proper request validation enabled.Adding
@Validannotation enables automatic validation of theExamRequestfields according to the validation constraints defined in the DTO (e.g.,@NotBlank,@NotNull).
71-71: LGTM! More descriptive endpoint path.Changing from
/{examId}to/{examId}/closemakes the API more self-documenting and follows RESTful conventions for action-specific endpoints.src/main/java/life/mosu/mosuserver/application/application/vaildator/ApplicationValidator.java (5)
27-27: LGTM! Repository dependency correctly updated.The change from
ApplicationJpaRepositorytoExamApplicationJpaRepositoryaligns with the migration of theexistsByUserIdAndExamIdsmethod mentioned in the summary.
41-43: LGTM! Empty exam list validation is appropriate.The addition of empty set validation prevents applications with no exams and uses the correct error code.
54-56: LGTM! Using Set for database query is efficient.Converting to Set before the database query is good practice as it automatically deduplicates exam IDs and potentially improves query performance.
68-73: Excellent refactor to functional style!The stream-based approach with
findFirst().ifPresent()is more concise and readable than the previous boolean flag approach. It also fails fast when an invalid lunch selection is detected.
78-79: LGTM! Repository migration completed correctly.The method now uses
examApplicationJpaRepositorywhich includes enhanced filtering for deleted payments as mentioned in the summary, improving the accuracy of duplicate application detection.src/main/java/life/mosu/mosuserver/presentation/event/dto/DurationRequest.java (1)
19-19: Clarify handling of the user-provided startDateThe
toDurationJpaVO()implementation in
src/main/java/life/mosu/mosuserver/presentation/event/dto/DurationRequest.java
always usesLocalDate.now()and never the record’sstartDatefield. This means anystartDatesent by the client is ignored.Please confirm the intended business logic:
- If events must always start “now,” remove the
startDatefield fromDurationRequest(and its JSON schema) and document that startDate is system-assigned.- Otherwise, use the user-supplied
startDatein the call tonew DurationJpaVO(startDate, endDate)instead ofLocalDate.now().
| public void agreedToTerms(ApplicationRequest request) { | ||
| if (!request.agreement().validateAgreement()) { | ||
| throw new CustomRuntimeException(ErrorCode.NOT_AGREED_TO_TERMS); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add null safety check for the agreement field.
The method logic is correct, but consider adding a null check for the agreement field to prevent potential NullPointerException.
public void agreedToTerms(ApplicationRequest request) {
+ if (request.agreement() == null) {
+ throw new CustomRuntimeException(ErrorCode.NOT_AGREED_TO_TERMS);
+ }
if (!request.agreement().validateAgreement()) {
throw new CustomRuntimeException(ErrorCode.NOT_AGREED_TO_TERMS);
}
}📝 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 void agreedToTerms(ApplicationRequest request) { | |
| if (!request.agreement().validateAgreement()) { | |
| throw new CustomRuntimeException(ErrorCode.NOT_AGREED_TO_TERMS); | |
| } | |
| } | |
| public void agreedToTerms(ApplicationRequest request) { | |
| if (request.agreement() == null) { | |
| throw new CustomRuntimeException(ErrorCode.NOT_AGREED_TO_TERMS); | |
| } | |
| if (!request.agreement().validateAgreement()) { | |
| throw new CustomRuntimeException(ErrorCode.NOT_AGREED_TO_TERMS); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/application/vaildator/ApplicationValidator.java
around lines 30 to 34, add a null check for the agreement field in the
agreedToTerms method before calling validateAgreement() to prevent
NullPointerException. Modify the condition to first verify that
request.agreement() is not null, and if it is null or validateAgreement()
returns false, then throw the CustomRuntimeException with
ErrorCode.NOT_AGREED_TO_TERMS.
src/main/java/life/mosu/mosuserver/presentation/application/dto/ApplicationRequest.java
Show resolved
Hide resolved
|
|
||
| public DurationJpaVO toDurationJpaVO() { | ||
| return new DurationJpaVO(startDate, endDate); | ||
| return new DurationJpaVO(LocalDate.now(), endDate); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: User-provided startDate is ignored
The method now uses LocalDate.now() instead of the user-provided startDate field, which breaks the expected behavior. Users submitting a specific start date will have their input ignored, and all events will start "today" regardless of the intended start date.
Apply this diff to use the provided startDate:
- return new DurationJpaVO(LocalDate.now(), endDate);
+ return new DurationJpaVO(startDate, endDate);If null safety was the concern, add validation instead:
+ public DurationJpaVO toDurationJpaVO() {
+ if (startDate == null) {
+ throw new IllegalArgumentException("Start date cannot be null");
+ }
+ return new DurationJpaVO(startDate, endDate);
+ }📝 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.
| return new DurationJpaVO(LocalDate.now(), endDate); | |
| public DurationJpaVO toDurationJpaVO() { | |
| - return new DurationJpaVO(LocalDate.now(), endDate); | |
| + return new DurationJpaVO(startDate, endDate); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/presentation/event/dto/DurationRequest.java
at line 19, the method incorrectly uses LocalDate.now() instead of the
user-provided startDate field, ignoring user input. Fix this by replacing
LocalDate.now() with the startDate field when creating the DurationJpaVO
instance. If null safety is a concern, add validation to ensure startDate is not
null before usage rather than defaulting to the current date.
polyglot-k
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WhiteList 를 처리하려고 하다보니, 보이는게 있어서 당장에 급한건 아니라서, 일어나서 한번 확인해보시면 좋을 것 같아요!
코드 자체에서 NPE 처리를 보강한 것은 좋은 것 같습니다. 해당 부분에 대해서 WhiteList 처리하고 리뷰 재요청 해주세요 ~
src/main/java/life/mosu/mosuserver/application/admin/AdminDashboardService.java
Outdated
Show resolved
Hide resolved
src/main/java/life/mosu/mosuserver/domain/exam/entity/ExamJpaEntity.java
Show resolved
Hide resolved
src/main/java/life/mosu/mosuserver/presentation/admin/dto/DashBoardResponse.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
♻️ Duplicate comments (1)
src/main/java/life/mosu/mosuserver/application/admin/AdminDashboardService.java (1)
26-31: I/O 절감: 집계 쿼리 1회로 병합 + 캐싱 고려
- 현재 DB round-trip이 3회입니다. 과거 피드백과 동일하게, 하나의 쿼리/프로젝션으로 묶으면 대시보드 렌더링 시 I/O를 크게 줄일 수 있습니다. (예: 서브쿼리 3개를 SELECT 한 번에 반환하는 Native/JPQL + Projection)
- 관리 대시보드 특성상 초단위 최신성이 필수 아니라면, 단기 캐싱(@Cacheable, TTL 30~60s)로 부하를 추가로 낮출 수 있습니다.
- 변수명은 단수형 Count가 더 자연스럽습니다(Counts → Count). 선택 사항이지만 가독성 개선에 도움 됩니다.
예시(아키텍처 제안):
- AdminDashboardRepository.getCounts() → Projection { long applicationCount, long refundAbortedCount, long userCount }
- Service는 Projection → DashBoardResponse.of(...) 매핑
🧹 Nitpick comments (2)
src/main/java/life/mosu/mosuserver/global/filter/Whitelist.java (2)
45-47: EXACT exam endpoints will not match trailing slash variantsWith EXACT, /api/v1/exam/ won’t match. If trailing slashes can appear, either normalize (see earlier comment) or switch to segment-safe START_WITH on the exact segment.
- If normalization is not added, consider:
- Keep EXACT and enforce no trailing slash at the gateway/router, or
- Use segment-safe prefix matching for these endpoints.
82-85: Minor naming nit: START_WITH → STARTS_WITH (or PREFIX) and add brief JavadocImproves readability and avoids “off-by-one” feel in naming.
Example:
- public enum MatchType { - EXACT, - START_WITH - } + public enum MatchType { + /** Full path must match (trailing slash ignored). */ + EXACT, + /** Request path must equal prefix or start with prefix followed by '/'. */ + STARTS_WITH + }Note: update usages accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/life/mosu/mosuserver/application/admin/AdminDashboardService.java(2 hunks)src/main/java/life/mosu/mosuserver/domain/refund/repository/RefundJpaRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/global/filter/Whitelist.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/dto/DashBoardResponse.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/life/mosu/mosuserver/domain/refund/repository/RefundJpaRepository.java
- src/main/java/life/mosu/mosuserver/presentation/admin/dto/DashBoardResponse.java
🔇 Additional comments (1)
src/main/java/life/mosu/mosuserver/application/admin/AdminDashboardService.java (1)
17-17: 리포지토리 교체 주입 LGTMRefundFailureLogJpaRepository 주입 전환은 의도(“중단 건 집계”)와 일치합니다. 이 클래스 내에서 더 이상 RefundJpaRepository가 필요 없다면 불필요한 의존성은 제거됐는지 전체 컴파일 단에서 확인만 부탁드립니다.
src/main/java/life/mosu/mosuserver/application/admin/AdminDashboardService.java
Show resolved
Hide resolved
| Long applicationCounts = examApplicationJpaRepository.countAll(); | ||
| Long refundAbortedCounts = refundFailureLogJpaRepository.count(); | ||
| Long userCounts = userJpaRepository.countByUserRoleNot(UserRole.ROLE_ADMIN); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
count 계열은 primitive 사용 권장 + 시그니처/의도 재확인
- Spring Data의 count()는 primitive long 반환이 일반적입니다. 지역 변수도 primitive 사용을 권장합니다(불필요한 박싱/널 가능성 제거).
- examApplicationJpaRepository.countAll()은 기본 count()와 의미가 다른가요? 소프트 삭제 제외/필터링 등의 의도가 있다면 주석/메서드명으로 드러나게 해 주세요. 단순 전체 건수라면 기본 count() 사용이 명확합니다.
- ROLE 필터: Not(≠ ADMIN)는 향후 새로운 Role 추가 시 예기치 않은 포함 이슈가 생길 수 있습니다. “포함해야 할 일반 사용자 Role의 명시적 포함(In) 조건”으로 전환하는 방안도 검토해 주세요(예: ROLE_USER, ROLE_STUDENT 등 도메인 합의 필요).
적용 가능한 최소 변경(박싱 제거):
- Long applicationCounts = examApplicationJpaRepository.countAll();
- Long refundAbortedCounts = refundFailureLogJpaRepository.count();
- Long userCounts = userJpaRepository.countByUserRoleNot(UserRole.ROLE_ADMIN);
+ long applicationCounts = examApplicationJpaRepository.countAll();
+ long refundAbortedCounts = refundFailureLogJpaRepository.count();
+ long userCounts = userJpaRepository.countByUserRoleNot(UserRole.ROLE_ADMIN);읽기 전용 트랜잭션 부여(스냅샷 일관성 및 미세한 성능 이점):
// add import:
import org.springframework.transaction.annotation.Transactional;
// annotate method:
@Transactional(readOnly = true)
public DashBoardResponse getAll() { ... }🏁 Script executed:
#!/bin/bash
# Locate the repository interface for examApplicationJpaRepository
repo_file=$(fd --full-path "ExamApplicationJpaRepository.java")
echo "Found file: $repo_file"
# Show the countAll method signature
rg -n "countAll\s*\(" "$repo_file"
# Also confirm the return types for other count methods
rg -n "countByUserRoleNot\s*\(" "$repo_file"Length of output: 694
🏁 Script executed:
#!/bin/bash
# Locate UserJpaRepository and show the signature of countByUserRoleNot
user_repo=$(fd --full-path "UserJpaRepository.java")
echo "UserJpaRepository found at: $user_repo"
rg -n "countByUserRoleNot" "$user_repo"
# Locate RefundFailureLogJpaRepository and show count signature
refund_repo=$(fd --full-path "RefundFailureLogJpaRepository.java")
echo "RefundFailureLogJpaRepository found at: $refund_repo"
rg -n "count" "$refund_repo" | head -n 10
# Check AdminDashboardService for existing @Transactional on getAll method
service_file=$(fd --full-path "AdminDashboardService.java")
echo "Service file: $service_file"
rg -n "@Transactional" "$service_file"
rg -n "public DashBoardResponse getAll" -n "$service_file"Length of output: 1690
🏁 Script executed:
#!/bin/bash
# Show context around countAll in ExamApplicationJpaRepository to verify if it's custom or just alias
rg -n -A3 -B3 'countAll' src/main/java/life/mosu/mosuserver/domain/examapplication/repository/ExamApplicationJpaRepository.javaLength of output: 264
Use primitive long, clarify custom countAll, and add read-only transaction
- The custom
countAll()inExamApplicationJpaRepositoryapplies filters (paymentStatus = 'DONE'&deleted = false), so its name should reflect that intent (e.g.countDoneNonDeleted()or add Javadoc) rather than implying an unfiltered “all.” - Switch to primitive
longinAdminDashboardService.getAll()to avoid unnecessary boxing and nullability. - Consider replacing
countByUserRoleNot(UserRole.ROLE_ADMIN)with an explicit inclusion query (e.g.countByUserRoleIn(...)) to prevent accidental inclusion of future roles. - Mark the service method as a read-only transaction for consistency snapshot and slight performance gain.
Locations:
src/main/java/life/mosu/mosuserver/application/admin/AdminDashboardService.java(lines 22–24 and method signature)src/main/java/life/mosu/mosuserver/domain/examapplication/repository/ExamApplicationJpaRepository.java(line 196)
Suggested changes:
--- a/src/main/java/life/mosu/mosuserver/application/admin/AdminDashboardService.java
+++ b/src/main/java/life/mosu/mosuserver/application/admin/AdminDashboardService.java
@@ -20,6 +20,7 @@
import org.springframework.transaction.annotation.Transactional;
+@Transactional(readOnly = true)
public class AdminDashboardService {
public DashBoardResponse getAll() {
- Long applicationCounts = examApplicationJpaRepository.countAll();
- Long refundAbortedCounts = refundFailureLogJpaRepository.count();
- Long userCounts = userJpaRepository.countByUserRoleNot(UserRole.ROLE_ADMIN);
+ long applicationCounts = examApplicationJpaRepository.countAll();
+ long refundAbortedCounts = refundFailureLogJpaRepository.count();
+ long userCounts = userJpaRepository.countByUserRoleNot(UserRole.ROLE_ADMIN);
// ...
}
}And in the repository:
// src/.../ExamApplicationJpaRepository.java
/**
* Counts exam applications with paymentStatus = DONE and not deleted.
*/
@Query("""
SELECT COUNT(p) FROM ExamApplication p
WHERE p.paymentStatus = 'DONE'
AND p.deleted = false
""")
long countDoneNonDeleted();🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/admin/AdminDashboardService.java
around lines 22 to 24 and the method signature, change the return types from
Long to primitive long to avoid unnecessary boxing and nullability. Rename the
custom repository method countAll() in
src/main/java/life/mosu/mosuserver/domain/examapplication/repository/ExamApplicationJpaRepository.java
at line 196 to countDoneNonDeleted() and update its Javadoc to clarify it counts
only exam applications with paymentStatus = DONE and deleted = false. Replace
countByUserRoleNot(UserRole.ROLE_ADMIN) with countByUserRoleIn(...) specifying
all non-admin roles explicitly to avoid accidental inclusion of future roles.
Finally, annotate the service method with a read-only transaction annotation to
ensure consistency and improve performance.
src/main/java/life/mosu/mosuserver/global/filter/Whitelist.java
Outdated
Show resolved
Hide resolved
| USER_FIND_PASSWORD("/api/v1/user/me/find-password", WhitelistMethod.POST), | ||
| USER_FIND_PASSWORD("/api/v1/user/me/find-password", WhitelistMethod.POST, MatchType.START_WITH), | ||
|
|
||
| APPLICATION_GUEST("/api/v1/applications/guest", WhitelistMethod.ALL, MatchType.START_WITH); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
APPLICATION_GUEST is too permissive (ALL + START_WITH)
This whitelists every subpath and method under /api/v1/applications/guest, potentially exposing mutating endpoints without auth.
- Narrow scope to EXACT and READ methods, and explicitly whitelist only the POST paths you truly need.
- APPLICATION_GUEST("/api/v1/applications/guest", WhitelistMethod.ALL, MatchType.START_WITH);
+ APPLICATION_GUEST("/api/v1/applications/guest", WhitelistMethod.GET, MatchType.EXACT);- If specific POST endpoints are needed, add explicit entries (e.g., APPLICATION_GUEST_SUBMIT with POST).
Quick check of controller mappings:
🏁 Script executed:
#!/bin/bash
# List mappings under /api/v1/applications/guest*
rg -n -A2 -B2 '@(RequestMapping|GetMapping|PostMapping|PutMapping|DeleteMapping)\(' \
| rg '/api/v1/applications/guest'Length of output: 117
🏁 Script executed:
#!/bin/bash
# Search for any occurrences of the guest application path in the codebase
rg -n "/api/v1/applications/guest"
# Look specifically for class- or method-level Spring mappings referencing guest
rg -n "@RequestMapping.*guest"
rg -n "@GetMapping.*guest"
rg -n "@PostMapping.*guest"Length of output: 406
Restrict APPLICATION_GUEST to POST+EXACT match
The only controller mapping under /api/v1/applications/guest is a single @PostMapping("/guest"). Whitelisting ALL methods and every sub‐path overexposes mutating endpoints. Please update to:
- APPLICATION_GUEST("/api/v1/applications/guest", WhitelistMethod.ALL, MatchType.START_WITH);
+ APPLICATION_GUEST("/api/v1/applications/guest", WhitelistMethod.POST, MatchType.EXACT);• If you later add read or other guest endpoints, whitelist each new path/method explicitly.
• See ApplicationController.java:33 for the existing POST /guest.
🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/global/filter/Whitelist.java at line 55,
the APPLICATION_GUEST whitelist entry currently allows all HTTP methods and any
path starting with /api/v1/applications/guest, which is too permissive. Change
the WhitelistMethod from ALL to POST and the MatchType from START_WITH to EXACT
to restrict access strictly to POST requests on the exact
/api/v1/applications/guest path. This limits exposure to only the intended
mutating endpoint.
| return Arrays.stream(values()) | ||
| .filter(url -> requestUri.startsWith(url.path)) | ||
| .filter(url -> { | ||
| if (url.matchType == MatchType.EXACT) { | ||
| return requestUri.equals(url.path); | ||
| } else { | ||
| return requestUri.startsWith(url.path); | ||
| } | ||
| }) | ||
| .filter(url -> url.method == WhitelistMethod.ALL || url.method.name() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Harden path matching: segment-safe prefix + trailing-slash handling
Current START_WITH uses raw startsWith, which can match unintended paths (e.g., /api/v1/faq2). EXACT also fails on trailing slash variants (/exam/). Use segment-aware prefix and normalize trailing slashes.
Apply:
- return Arrays.stream(values())
- .filter(url -> {
- if (url.matchType == MatchType.EXACT) {
- return requestUri.equals(url.path);
- } else {
- return requestUri.startsWith(url.path);
- }
- })
+ return Arrays.stream(values())
+ .filter(url -> {
+ if (url.matchType == MatchType.EXACT) {
+ return equalsIgnoreTrailingSlash(requestUri, url.path);
+ } else {
+ return startsWithPathSegment(requestUri, url.path);
+ }
+ })Add helpers (outside the selected range):
private static boolean equalsIgnoreTrailingSlash(String a, String b) {
return trimTrailingSlash(a).equals(trimTrailingSlash(b));
}
private static boolean startsWithPathSegment(String requestUri, String prefix) {
String ru = trimTrailingSlash(requestUri);
String px = trimTrailingSlash(prefix);
return ru.equals(px) || ru.startsWith(px + "/");
}
private static String trimTrailingSlash(String s) {
if (s == null) return "";
int end = s.length();
while (end > 1 && s.charAt(end - 1) == '/') end--;
return s.substring(0, end);
}🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/global/filter/Whitelist.java around lines
69 to 77, the current path matching logic uses raw startsWith and exact equals,
which can incorrectly match unintended paths or fail on trailing slash variants.
To fix this, replace the raw string comparisons with segment-aware prefix
matching and trailing slash normalization by adding helper methods outside the
current code range: equalsIgnoreTrailingSlash, startsWithPathSegment, and
trimTrailingSlash. Then update the filter logic to use these helpers for more
accurate and robust path matching.
| return Arrays.stream(values()) | ||
| .filter(url -> requestUri.startsWith(url.path)) | ||
| .filter(url -> { | ||
| if (url.matchType == MatchType.EXACT) { | ||
| return requestUri.equals(url.path); | ||
| } else { | ||
| return requestUri.startsWith(url.path); | ||
| } | ||
| }) | ||
| .filter(url -> url.method == WhitelistMethod.ALL || url.method.name() | ||
| .equalsIgnoreCase(requestMethod)) | ||
| .findFirst(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Prefer most-specific match (EXACT > longer prefixes) to avoid overbroad earlier entries
Enumeration order currently decides. Sort matches so EXACT wins, then longer path first.
- return Arrays.stream(values())
+ return Arrays.stream(values())
+ .sorted(java.util.Comparator
+ .comparing((Whitelist w) -> w.matchType == MatchType.EXACT ? 0 : 1)
+ .thenComparing((Whitelist w) -> -w.path.length()))
.filter(url -> {
...
})
.filter(url -> url.method == WhitelistMethod.ALL || url.method.name()
.equalsIgnoreCase(requestMethod))
.findFirst();Add import if preferred:
import java.util.Comparator;🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/global/filter/Whitelist.java around lines
69 to 79, the current matching logic relies on enumeration order, which can
cause less specific prefix matches to be chosen over exact matches or longer
prefixes. To fix this, modify the stream to sort the filtered URLs so that exact
matches come first, then among prefix matches, longer paths come before shorter
ones. Use a Comparator that prioritizes MatchType.EXACT over others and sorts by
descending path length before calling findFirst. Add the import for
java.util.Comparator if not already present.
| .filter(url -> url.method == WhitelistMethod.ALL || url.method.name() | ||
| .equalsIgnoreCase(requestMethod)) | ||
| .findFirst(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
❓ Verification inconclusive
Allow HEAD when GET is whitelisted
HEAD is commonly used by proxies/monitors for GET endpoints. Consider allowing it when GET is whitelisted.
- .filter(url -> url.method == WhitelistMethod.ALL || url.method.name()
- .equalsIgnoreCase(requestMethod))
+ .filter(url -> url.method == WhitelistMethod.ALL
+ || url.method.name().equalsIgnoreCase(requestMethod)
+ || (url.method == WhitelistMethod.GET && "HEAD".equalsIgnoreCase(requestMethod)))Please verify your infra actually needs HEAD on GET endpoints (health, assets).
Allow HEAD on GET-whitelisted endpoints
To support common proxy/monitor HEAD requests on GET routes, update the whitelist filter in Whitelist.java:
- File:
src/main/java/life/mosu/mosuserver/global/filter/Whitelist.java(lines 77–79)
.filter(url -> url.method == WhitelistMethod.ALL
- || url.method.name().equalsIgnoreCase(requestMethod))
+ || url.method.name().equalsIgnoreCase(requestMethod)
+ || (url.method == WhitelistMethod.GET && "HEAD".equalsIgnoreCase(requestMethod)))
.findFirst();Please confirm that your infrastructure (health checks, asset requests, etc.) actually requires HEAD support for GET endpoints.
📝 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.
| .filter(url -> url.method == WhitelistMethod.ALL || url.method.name() | |
| .equalsIgnoreCase(requestMethod)) | |
| .findFirst(); | |
| .filter(url -> url.method == WhitelistMethod.ALL | |
| || url.method.name().equalsIgnoreCase(requestMethod) | |
| || (url.method == WhitelistMethod.GET && "HEAD".equalsIgnoreCase(requestMethod))) | |
| .findFirst(); |
🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/global/filter/Whitelist.java around lines
77 to 79, update the filter logic to allow HTTP HEAD requests on endpoints
whitelisted for GET. Modify the condition to also accept HEAD method when the
whitelist method is GET, so that HEAD requests are permitted on GET routes.
Confirm that your infrastructure needs this support before applying the change.
…d filter payments by DONE status
…n PaymentJpaRepository
…ateVirtualAccountProcessor
…ner creation validation
…proving duplicate exam handling
…urationJpaVO method
…ements in ErrorCode
…st and EventResponse
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Other Changes