MOSU-205 feature: 관리자 시험 마감 기능 구현 #222
Conversation
WalkthroughThis change introduces an Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin User
participant Controller as ExamController
participant Service as ExamService
participant Repo as ExamJpaRepository
participant Entity as ExamJpaEntity
Admin->>Controller: PATCH /exams/{examId}
Controller->>Service: close(examId)
Service->>Repo: findById(examId)
Repo-->>Service: ExamJpaEntity
Service->>Entity: close()
Entity-->>Service: (status set to CLOSED)
Service-->>Controller: (void)
Controller-->>Admin: 200 OK, "Exam venue closed"
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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 (
|
![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!
I've implemented a new feature that allows administrators to close exams, preventing further applications. This involved introducing an ExamStatus to track the state of an exam and updating the application validation logic to respect this new status. Additionally, I've refined existing validation checks for exam dates and capacity, ensuring they use the most current exam information and handle the new closed status.
Highlights
- New Exam Status Management: Introduced an ExamStatus enum (OPEN, CLOSED) to manage exam availability, providing a clear state for each exam.
- Exam Entity Enhancements: Added the examStatus field to ExamJpaEntity and implemented utility methods (close() and open()) to programmatically change an exam's status.
- Admin Exam Closing Functionality: Implemented a new admin-only PATCH endpoint (/api/v1/exams/{examId}) that allows administrators to explicitly close specific exams, preventing further applications.
- Application Validation Improvements: Updated the application validation logic to prevent users from applying to exams that are either full, past their deadline, or explicitly marked as CLOSED.
- Date Validation Precision: Refactored exam date validation to use LocalDateTime and the exam's deadlineTime for more precise and accurate checks, moving away from LocalDate.
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 introduces a feature for administrators to manually close an exam for applications. The implementation is solid, adding a new examStatus to the ExamJpaEntity and updating the application validation logic accordingly. The refactoring in ApplicationService to fetch exam entities once and pass them to the validator is a good performance improvement.
Regarding your discussion points:
- It's a good design to keep the
examStatusfor manual admin control separate from the automatic capacity-based logic. The current implementation correctly checks both conditions, and I would recommend keeping it this way to avoid complexity and unexpected behavior (e.g., an admin-closed exam reopening automatically). - For handling exams with past deadlines, implementing a scheduled job (e.g., using Spring's
@Scheduledannotation) is a common and effective approach. This job could run periodically to find exams whosedeadlineTimehas passed and update theirexamStatustoCLOSED.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/main/java/life/mosu/mosuserver/presentation/exam/ExamController.java (1)
74-80: LGTM! Well-structured admin endpoint.The endpoint follows established patterns with proper authorization and consistent response handling. Consider refining the success message for clarity.
Consider updating the message for better clarity:
- ApiResponseWrapper.success(HttpStatus.OK, "시험장 마감 성공")); + ApiResponseWrapper.success(HttpStatus.OK, "시험 마감 성공"));This makes it clearer that the exam itself is being closed rather than just the venue.
src/main/java/life/mosu/mosuserver/application/exam/ExamService.java (1)
30-30: Consider using REQUIRED propagation for read operations.While SUPPORTS propagation works, consider using REQUIRED instead to ensure consistent transaction behavior across the application.
-@Transactional(readOnly = true, propagation = Propagation.SUPPORTS) +@Transactional(readOnly = true)REQUIRED is the default and ensures the method always runs within a transaction context, providing more predictable behavior.
Also applies to: 44-44, 51-51
src/main/java/life/mosu/mosuserver/application/application/vaildator/ApplicationValidator.java (1)
83-106: Consider more specific error handling for cache misses.The method correctly checks both exam status and capacity. However, using the same error code for both missing current applications and missing max capacity could make debugging difficult.
Consider using different error codes or including contextual information:
public void examNotFull(List<ExamJpaEntity> exams) { boolean isFull = exams.stream() .anyMatch(exam -> { if (exam.getExamStatus() == ExamStatus.CLOSED) { return true; } Long currentApplications = examQuotaCacheManager.getCurrentApplications( exam.getId()) .orElseThrow(() -> new CustomRuntimeException( - ErrorCode.EXAM_QUOTA_NOT_FOUND)); + ErrorCode.EXAM_QUOTA_NOT_FOUND, + "Current applications not found for exam: " + exam.getId())); Long maxCapacity = examQuotaCacheManager.getMaxCapacity(exam.getId()) .orElseThrow(() -> new CustomRuntimeException( - ErrorCode.EXAM_QUOTA_NOT_FOUND)); + ErrorCode.EXAM_QUOTA_NOT_FOUND, + "Max capacity not found for exam: " + exam.getId())); return currentApplications >= maxCapacity; });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java(3 hunks)src/main/java/life/mosu/mosuserver/application/application/vaildator/ApplicationValidator.java(2 hunks)src/main/java/life/mosu/mosuserver/application/exam/ExamService.java(4 hunks)src/main/java/life/mosu/mosuserver/domain/exam/entity/ExamJpaEntity.java(2 hunks)src/main/java/life/mosu/mosuserver/domain/exam/entity/ExamStatus.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/exam/ExamController.java(2 hunks)
🔇 Additional comments (11)
src/main/java/life/mosu/mosuserver/domain/exam/entity/ExamStatus.java (1)
3-6: LGTM! Clean enum implementation.The enum is well-structured with descriptive Korean comments that clearly explain the status meanings.
src/main/java/life/mosu/mosuserver/presentation/exam/ExamController.java (1)
14-14: LGTM! Proper import addition.Adding PatchMapping import to support the new endpoint.
src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java (2)
15-16: LGTM! Proper imports for entity-based validation.Adding necessary imports to support the refactored validation approach.
37-37: LGTM! Repository injection for exam fetching.Injecting ExamJpaRepository to enable entity-based validation.
src/main/java/life/mosu/mosuserver/domain/exam/entity/ExamJpaEntity.java (3)
57-59: LGTM! Proper enum field implementation.The examStatus field is correctly implemented with appropriate JPA annotations and a sensible default value.
86-90: LGTM! Well-implemented status transition method.The close() method properly checks current status before updating, preventing unnecessary database updates and maintaining clean state transitions.
92-96: LGTM! Consistent status transition implementation.The open() method follows the same pattern as close() with proper guard clauses.
src/main/java/life/mosu/mosuserver/application/exam/ExamService.java (3)
8-9: LGTM! Proper imports for exception handling and transaction management.Adding necessary imports for the new functionality.
Also applies to: 14-15
24-24: LGTM! Proper transactional annotations for write operations.Adding @transactional to write operations ensures proper transaction management.
Also applies to: 63-63, 68-68
69-73: LGTM! Proper implementation of close operation.The method correctly handles entity retrieval with appropriate error handling and delegates to the domain method for status update.
src/main/java/life/mosu/mosuserver/application/application/vaildator/ApplicationValidator.java (1)
3-3: Import changes look good.The new imports for
LocalDateTimeandExamStatusare appropriate for the updated validation logic.Also applies to: 12-12
| List<ExamJpaEntity> exams = examJpaRepository.findAllById(examIds); | ||
| validator.examNotFull(exams); | ||
| validator.examDateNotPassed(exams); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify exam existence before validation.
The refactoring to entity-based validation is good, but findAllById may return fewer entities than requested IDs if some exams don't exist. Consider validating that all requested exams were found.
🏁 Script executed:
#!/bin/bash
# Check if there's existing validation for exam existence in ApplicationValidator
ast-grep --pattern 'class ApplicationValidator {
$$$
}'Length of output: 59
🏁 Script executed:
#!/bin/bash
# Locate where the validator methods are defined
rg -n "examNotFull" -n src
rg -n "examDateNotPassed" -n src
# Find any Validator classes in the codebase
rg -n "class .*Validator" -n srcLength of output: 798
Add an existence check for all requested exams before running other validators
Currently, findAllById(examIds) will silently drop any IDs that don’t exist, which means you may be validating a subset of exams without noticing the missing ones. You should explicitly verify that the size of the returned list matches the number of requested IDs, and throw a clear exception if not.
• In src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java (around lines 82–84), insert something like:
List<ExamJpaEntity> exams = examJpaRepository.findAllById(examIds);
+ if (exams.size() != examIds.size()) {
+ throw new EntityNotFoundException(
+ "Some exams not found for IDs: " +
+ examIds.stream()
+ .filter(id -> exams.stream().noneMatch(e -> e.getId().equals(id)))
+ .collect(Collectors.toList())
+ );
+ }
validator.examNotFull(exams);
validator.examDateNotPassed(exams);This ensures you fail fast if any requested exam is missing before proceeding with capacity or date checks.
📝 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.
| List<ExamJpaEntity> exams = examJpaRepository.findAllById(examIds); | |
| validator.examNotFull(exams); | |
| validator.examDateNotPassed(exams); | |
| List<ExamJpaEntity> exams = examJpaRepository.findAllById(examIds); | |
| if (exams.size() != examIds.size()) { | |
| throw new EntityNotFoundException( | |
| "Some exams not found for IDs: " + | |
| examIds.stream() | |
| .filter(id -> exams.stream().noneMatch(e -> e.getId().equals(id))) | |
| .collect(Collectors.toList()) | |
| ); | |
| } | |
| validator.examNotFull(exams); | |
| validator.examDateNotPassed(exams); |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/application/ApplicationService.java
around lines 82 to 84, add a check after retrieving exams with findAllById to
verify that the number of exams returned matches the number of requested
examIds. If the sizes differ, throw a clear exception indicating some exams were
not found. This ensures you detect missing exams before running further
validations like examNotFull and examDateNotPassed.
| public void examDateNotPassed(List<ExamJpaEntity> exams) { | ||
| boolean hasPassedExam = exams.stream() | ||
| .anyMatch(exam -> exam.getExamDate().isBefore(LocalDate.now())); | ||
| .anyMatch(exam -> exam.getDeadlineTime().isBefore(LocalDateTime.now())); | ||
|
|
||
| if (hasPassedExam) { | ||
| throw new CustomRuntimeException(ErrorCode.EXAM_DATE_PASSED); | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider timezone handling and deadline boundary behavior.
The deadline comparison has two potential issues:
- Using
LocalDateTime.now()assumes the server's local timezone matches the deadline's timezone. Consider usingZonedDateTimeor ensuring consistent timezone handling. - The
isBefore()check rejects applications at the exact deadline moment. Consider usingisBefore()orisEqual()to allow applications until the deadline expires.
public void examDateNotPassed(List<ExamJpaEntity> exams) {
boolean hasPassedExam = exams.stream()
- .anyMatch(exam -> exam.getDeadlineTime().isBefore(LocalDateTime.now()));
+ .anyMatch(exam -> exam.getDeadlineTime().isBefore(LocalDateTime.now()) ||
+ exam.getDeadlineTime().isEqual(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.
| public void examDateNotPassed(List<ExamJpaEntity> exams) { | |
| boolean hasPassedExam = exams.stream() | |
| .anyMatch(exam -> exam.getExamDate().isBefore(LocalDate.now())); | |
| .anyMatch(exam -> exam.getDeadlineTime().isBefore(LocalDateTime.now())); | |
| if (hasPassedExam) { | |
| throw new CustomRuntimeException(ErrorCode.EXAM_DATE_PASSED); | |
| } | |
| } | |
| public void examDateNotPassed(List<ExamJpaEntity> exams) { | |
| boolean hasPassedExam = exams.stream() | |
| .anyMatch(exam -> exam.getDeadlineTime().isBefore(LocalDateTime.now()) || | |
| exam.getDeadlineTime().isEqual(LocalDateTime.now())); | |
| if (hasPassedExam) { | |
| throw new CustomRuntimeException(ErrorCode.EXAM_DATE_PASSED); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/application/vaildator/ApplicationValidator.java
around lines 74 to 81, the exam deadline comparison uses LocalDateTime.now()
which may cause timezone inconsistencies and excludes the exact deadline moment.
To fix this, update the code to use ZonedDateTime or another timezone-aware time
class consistent with the exam deadlines, and modify the condition to check if
the deadline is before or equal to the current time to allow applications up to
the deadline.
| Long currentApplications = examQuotaCacheManager.getCurrentApplications( | ||
| exam.getId()) | ||
| .orElseThrow(() -> new CustomRuntimeException( | ||
| ErrorCode.EXAM_QUOTA_NOT_FOUND)); | ||
|
|
||
| Long maxCapacity = examQuotaCacheManager.getMaxCapacity(exam.getId()) | ||
| .orElseThrow(() -> new CustomRuntimeException( | ||
| ErrorCode.EXAM_QUOTA_NOT_FOUND)); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify cache synchronization with exam status changes.
Since the validation relies on cached values for application counts and capacities, ensure that the cache is properly invalidated or updated when:
- An admin manually closes/opens an exam
- The exam status changes due to reaching capacity
- Applications are added or removed (payment/refund scenarios mentioned in PR objectives)
🏁 Script executed:
#!/bin/bash
# Description: Check how ExamQuotaCacheManager handles cache updates
# Find cache update/invalidation methods
echo "=== Searching for cache update methods in ExamQuotaCacheManager ==="
rg -A 5 "class ExamQuotaCacheManager"
# Check for cache invalidation on exam status changes
echo "=== Searching for cache invalidation on status changes ==="
rg -A 3 "(invalidate|evict|update|refresh).*cache" --glob "**/*Cache*.java"
# Check ExamService for cache handling when closing exams
echo "=== Checking ExamService close method for cache updates ==="
ast-grep --pattern 'close($$$) {
$$$
}'Length of output: 1294
Cache Invalidation Missing for ExamQuotaCacheManager
I didn’t find any eviction or refresh calls in ExamQuotaCacheManager or related services when an exam’s status or its application count changes. Without explicit cache invalidation:
- Closed exams may still report old quotas
- Capacity changes from new applications or refunds won’t be reflected
- Auto-closure on expiry won’t clear stale entries
To address this, add cache eviction calls at these touchpoints:
• After manually closing/opening an exam
File: src/main/java/life/mosu/mosuserver/application/exam/service/ExamService.java
public void closeExam(Long examId) {
ExamJpaEntity exam = examRepository.findById(examId)…
exam.setStatus(EXAM_STATUS.CLOSED);
examRepository.save(exam);
// Invalidate cached quotas
examQuotaCacheManager.invalidate(examId.toString());
}• When an application is created or canceled (payment/refund)
File: src/main/java/life/mosu/mosuserver/application/application/service/ApplicationService.java
public void createApplication(…) {
…
examQuotaCacheManager.invalidate(examId.toString());
}
public void refundApplication(…) {
…
examQuotaCacheManager.invalidate(examId.toString());
}• If you implement automatic expiry-driven closure, ensure it also calls
examQuotaCacheManager.invalidate(examId.toString());
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/application/vaildator/ApplicationValidator.java
lines 91-99, the cache for exam quotas is read but never invalidated, leading to
stale data issues. To fix this, add cache eviction calls in ExamService.java
after changing an exam's status (e.g., closing/opening an exam), and in
ApplicationService.java after creating or refunding an application. Also ensure
any automatic expiry-driven exam closures call the cache invalidation method.
This will keep the cached quota data consistent with the current state.
1 participant
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타
Summary by CodeRabbit
New Features
Improvements
Bug Fixes