Skip to content

[fix] #103 같은 유저의 중복 참가 방지 및 참가자 조회 로직 리팩토링#104

Merged
2hyunjinn merged 3 commits intomainfrom
fix/#103
May 5, 2025
Merged

[fix] #103 같은 유저의 중복 참가 방지 및 참가자 조회 로직 리팩토링#104
2hyunjinn merged 3 commits intomainfrom
fix/#103

Conversation

@2hyunjinn
Copy link
Copy Markdown
Member

@2hyunjinn 2hyunjinn commented May 5, 2025

📌 PR 제목

[fix] #103 같은 유저의 중복 참가 방지 및 참가자 조회 로직 리팩토링

📌 PR 내용

  • 같은 유저가 동일한 페스티벌에 여러 번 참가할 수 있는 문제를 해결했습니다.
  • entryFestival에서 중복된 유저·페스티벌 조회 로직을 제거하고 getParticipant()로 일원화했습니다.

🛠 작업 내용

  • 중복 참가자 생성 방지 (PARTICIPANT_ALREADY_EXISTS 예외 처리 추가)
  • entryFestival() 내부 로직 리팩토링
  • 중복 참가 예외 테스트 코드 추가

🔍 관련 이슈

Closes #103

📸 스크린샷 (Optional)

image

📚 레퍼런스 (Optional)

N/A

Summary by CodeRabbit

  • Bug Fixes

    • Improved user validation across the app by ensuring that operations now throw an error if a user is not found, rather than proceeding silently.
  • Tests

    • Updated tests to reflect stricter user validation.
    • Added a new test to confirm that duplicate participant creation is properly blocked with an error.

@2hyunjinn 2hyunjinn added the fix Further information is requested label May 5, 2025
@2hyunjinn 2hyunjinn self-assigned this May 5, 2025
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented May 5, 2025

Walkthrough

The changes systematically update user retrieval logic across multiple service and facade classes by replacing the getUserById method with getUserByIdOrThrow, enforcing strict exception-throwing behavior when a user does not exist. In the participant creation flow, explicit logic is added to prevent duplicate participant creation by throwing a specific exception if a participant already exists for the same user and festival. Related test cases are updated to mock the new method and a new test is introduced to verify the duplicate participant prevention logic.

Changes

Files/Groups Change Summary
src/main/java/org/festimate/team/api/facade/FestivalFacade.java,
.../ParticipantFacade.java,
.../PointFacade.java,
.../MatchingServiceImpl.java,
.../JwtService.java
Replaced all calls to userService.getUserById with userService.getUserByIdOrThrow to enforce strict user existence checks.
src/main/java/org/festimate/team/domain/user/service/UserService.java,
.../UserServiceImpl.java
Renamed getUserById to getUserByIdOrThrow in the interface and implementation; updated internal usages accordingly.
src/main/java/org/festimate/team/api/facade/ParticipantFacade.java Added explicit check and exception for duplicate participant creation; refactored participant retrieval logic.
src/test/java/org/festimate/team/api/facade/FestivalFacadeTest.java,
.../ParticipantFacadeTest.java,
.../PointFacadeTest.java,
.../MatchingServiceImplTest.java
Updated mocks to use getUserByIdOrThrow instead of getUserById; added a test for duplicate participant prevention.
src/test/java/org/festimate/team/api/facade/ParticipantFacadeTest.java Added createParticipant_duplicate_fail test to verify exception on duplicate participant creation.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant ParticipantFacade
    participant UserService
    participant ParticipantService

    Client->>ParticipantFacade: createParticipant(userId, festivalId, ...)
    ParticipantFacade->>UserService: getUserByIdOrThrow(userId)
    UserService-->>ParticipantFacade: User or Exception
    ParticipantFacade->>ParticipantService: findByUserIdAndFestivalId(userId, festivalId)
    alt Participant exists
        ParticipantService-->>ParticipantFacade: Participant
        ParticipantFacade-->>Client: Throw PARTICIPANT_ALREADY_EXISTS Exception
    else Participant does not exist
        ParticipantService-->>ParticipantFacade: null
        ParticipantFacade->>ParticipantService: createParticipant(...)
        ParticipantService-->>ParticipantFacade: Participant
        ParticipantFacade-->>Client: Success
    end
Loading

Assessment against linked issues

Objective Addressed Explanation
Prevent duplicate participant creation for same user and festival (#103)
Throw PARTICIPANT_ALREADY_EXISTS exception on duplicate participant creation (#103)
Use unified participant retrieval and exception handling in entryFestival (#103)
Ensure non-existent participant queries throw appropriate exceptions (#103)

Poem

In fields of code, the rabbits leap,
No more duplicates in the participant heap!
With strict user checks and exceptions that throw,
The festival runs smoother, as all devs now know.
Tests hop along, ensuring no repeat,
This patch is complete—oh, what a feat!
🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 748e296 and 5c4a00f.

📒 Files selected for processing (11)
  • src/main/java/org/festimate/team/api/facade/FestivalFacade.java (3 hunks)
  • src/main/java/org/festimate/team/api/facade/ParticipantFacade.java (3 hunks)
  • src/main/java/org/festimate/team/api/facade/PointFacade.java (3 hunks)
  • src/main/java/org/festimate/team/domain/matching/service/impl/MatchingServiceImpl.java (1 hunks)
  • src/main/java/org/festimate/team/domain/user/service/UserService.java (1 hunks)
  • src/main/java/org/festimate/team/domain/user/service/impl/UserServiceImpl.java (2 hunks)
  • src/main/java/org/festimate/team/infra/jwt/JwtService.java (2 hunks)
  • src/test/java/org/festimate/team/api/facade/FestivalFacadeTest.java (4 hunks)
  • src/test/java/org/festimate/team/api/facade/ParticipantFacadeTest.java (8 hunks)
  • src/test/java/org/festimate/team/api/facade/PointFacadeTest.java (5 hunks)
  • src/test/java/org/festimate/team/domain/matching/service/impl/MatchingServiceImplTest.java (1 hunks)
🔇 Additional comments (34)
src/test/java/org/festimate/team/domain/matching/service/impl/MatchingServiceImplTest.java (1)

86-86: Test mock updated to reflect the renamed service method.

The test has been correctly updated to use getUserByIdOrThrow instead of getUserById, maintaining consistency with the service interface change that now throws exceptions for non-existent users.

src/main/java/org/festimate/team/infra/jwt/JwtService.java (2)

82-82: Method call updated for consistent exception handling.

The change from getUserById to getUserByIdOrThrow ensures that if a user doesn't exist during token reissue, a clear exception will be thrown rather than potentially causing a NullPointerException later.


103-103: Method call updated for consistent exception handling.

This change improves error handling by using getUserByIdOrThrow which will throw a specific domain exception if the user doesn't exist, making the code more robust and predictable.

src/test/java/org/festimate/team/api/facade/PointFacadeTest.java (1)

64-64: All test mocks updated to use the renamed service method.

The test mocks have been correctly updated to use getUserByIdOrThrow instead of getUserById across all test cases, maintaining consistency with the updated service interface.

Also applies to: 81-81, 102-102, 120-120, 138-138

src/main/java/org/festimate/team/domain/user/service/UserService.java (1)

20-20: Method renamed to better reflect its behavior.

Renaming getUserById to getUserByIdOrThrow is a good practice as it clearly communicates to callers that this method will throw an exception if the user is not found, rather than returning null or an Optional.

This change is part of the broader effort to prevent duplicate participation by ensuring consistent exception handling across the codebase. However, to complete the PR objective, ensure the getParticipant() method is also implemented to check for existing participants and throw a PARTICIPANT_ALREADY_EXISTS exception when appropriate.

src/test/java/org/festimate/team/api/facade/FestivalFacadeTest.java (4)

72-72: Method name change properly reflected in tests

The test has been correctly updated to use getUserByIdOrThrow instead of getUserById, aligning with the API changes in the service implementation.


100-100: Method name change properly reflected in tests

The test has been correctly updated to use getUserByIdOrThrow instead of getUserById, aligning with the API changes in the service implementation.


116-116: Method name change properly reflected in tests

The test has been correctly updated to use getUserByIdOrThrow instead of getUserById, aligning with the API changes in the service implementation.


134-134: Method name change properly reflected in tests

The test has been correctly updated to use getUserByIdOrThrow instead of getUserById, aligning with the API changes in the service implementation.

src/main/java/org/festimate/team/domain/matching/service/impl/MatchingServiceImpl.java (1)

69-69: Method name change for consistent error handling

The change from getUserById to getUserByIdOrThrow correctly implements the stricter user validation approach. This ensures consistent exception handling when a user is not found, which is appropriate for this context since the method is already named getExistingParticipantOrThrow.

src/main/java/org/festimate/team/api/facade/PointFacade.java (3)

36-36: Method name change for consistent error handling

The change from getUserById to getUserByIdOrThrow correctly implements the stricter user validation approach, throwing an exception if the user is not found.


45-45: Method name change for consistent error handling

The change from getUserById to getUserByIdOrThrow correctly implements the stricter user validation approach, throwing an exception if the user is not found.


57-57: Method name change for consistent error handling

The change from getUserById to getUserByIdOrThrow implements a stricter user validation approach in the helper method getExistingParticipantOrThrow, which is consistent with its naming that already suggests it should throw exceptions.

src/main/java/org/festimate/team/api/facade/FestivalFacade.java (5)

31-31: Method name change for consistent error handling

The change from getUserById to getUserByIdOrThrow correctly implements the stricter user validation approach, throwing an exception if the user is not found.


39-39: Method name change for consistent error handling

The change from getUserById to getUserByIdOrThrow correctly implements the stricter user validation approach, throwing an exception if the user is not found.


49-49: Method name change for consistent error handling

The change from getUserById to getUserByIdOrThrow correctly implements the stricter user validation approach, throwing an exception if the user is not found.


58-58: Method name change for consistent error handling

The change from getUserById to getUserByIdOrThrow correctly implements the stricter user validation approach, throwing an exception if the user is not found.


1-72:

Details

✅ Verification successful

Verification of PR objectives implementation

Based on the PR objectives to prevent duplicate participation, I notice these changes support the goal by enforcing stricter user validation, but I don't see the specific implementation of the duplicate participant prevention logic in this file.

If the duplicate participant check implementation is in another file (likely in ParticipantService), please ensure that tests are added to verify this functionality as mentioned in the PR objectives.


🏁 Script executed:

#!/bin/bash
# Search for potential implementation of duplicate participant prevention logic
rg -A 5 "PARTICIPANT_ALREADY_EXISTS|entryFestival|getParticipant" --type java

Length of output: 54124


Duplicate participation prevention is in place and covered by tests

The duplicate check is implemented in ParticipantFacade.createParticipant
(src/main/java/org/festimate/team/api/facade/ParticipantFacade.java):

if (participantService.getParticipant(user, festival) != null) {
    throw new FestimateException(ResponseError.PARTICIPANT_ALREADY_EXISTS);
}

And it’s verified by the createParticipant_fail test in ParticipantFacadeTest
(src/test/java/org/festimate/team/api/facade/ParticipantFacadeTest.java), which asserts that a FestimateException with ResponseError.PARTICIPANT_ALREADY_EXISTS is thrown for duplicate entries.

No further action required.

src/main/java/org/festimate/team/domain/user/service/impl/UserServiceImpl.java (3)

53-56: Method call updated to reflect new naming convention

The method call has been updated to use getUserByIdOrThrow instead of getUserById, which better conveys the exception-throwing behavior when a user is not found.


58-62: Method renamed to clearly indicate exception-throwing behavior

Changing the method name from getUserById to getUserByIdOrThrow is a good practice as it explicitly communicates that this method will throw an exception if the user is not found. This makes the code more readable and sets clear expectations for callers.


71-75: Method call updated to reflect new naming convention

The method call has been properly updated to use the renamed method getUserByIdOrThrow.

src/test/java/org/festimate/team/api/facade/ParticipantFacadeTest.java (9)

4-4: Import added for new test case

The import for ProfileRequest is now needed for the new test case that verifies duplicate participant prevention.


62-63: Test updated to use new method name

Test method has been updated to correctly mock the renamed method getUserByIdOrThrow instead of getUserById.


74-75: Test updated to use new method name

Test method has been updated to correctly mock the renamed method getUserByIdOrThrow.


87-89: Test updated to use new method name

Test method has been updated to correctly mock the renamed method getUserByIdOrThrow.


99-101: Test updated to use new method name

Test method has been updated to correctly mock the renamed method getUserByIdOrThrow.


112-114: Test updated to use new method name

Test method has been updated to correctly mock the renamed method getUserByIdOrThrow.


124-126: Test updated to use new method name

Test method has been updated to correctly mock the renamed method getUserByIdOrThrow.


139-141: Test updated to use new method name

Test method has been updated to correctly mock the renamed method getUserByIdOrThrow.


148-166: New test case for duplicate participant prevention

Excellent addition of a test case that verifies the system correctly prevents duplicate participants by throwing a FestimateException with PARTICIPANT_ALREADY_EXISTS error. The test:

  1. Sets up mocks for user and festival retrieval
  2. Mocks an existing participant for the user-festival combination
  3. Configures the service to throw the expected exception when attempting to create a duplicate participant
  4. Asserts that the facade correctly propagates this exception

This ensures the duplicate prevention feature works as intended.

src/main/java/org/festimate/team/api/facade/ParticipantFacade.java (4)

31-34: Code refactored to eliminate redundant user-festival lookup

The entryFestival method has been improved by using the getParticipant helper method instead of duplicating user and festival retrieval logic. This aligns with the PR objective of consolidating duplicate user-festival lookup code, reducing code duplication and improving maintainability.


37-47: Added validation to prevent duplicate participant creation

This implementation now properly checks if a participant already exists for the given user and festival before creating a new one, throwing a PARTICIPANT_ALREADY_EXISTS exception if needed. This directly addresses the PR objective of preventing duplicate participation of the same user in a single festival.

The method also correctly uses the renamed getUserByIdOrThrow method.


76-84: Method updated to use renamed user retrieval method

The getParticipant helper method has been updated to use getUserByIdOrThrow instead of getUserById, maintaining consistency with the user service API changes.


87-93: Method updated to use renamed user retrieval method

The getParticipantByNickname method has been updated to use getUserByIdOrThrow instead of getUserById, maintaining consistency with the user service API changes.

✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions
Copy link
Copy Markdown

github-actions bot commented May 5, 2025

Test Results

40 tests   40 ✅  1s ⏱️
10 suites   0 💤
10 files     0 ❌

Results for commit 5c4a00f.

@2hyunjinn 2hyunjinn merged commit 5a022be into main May 5, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[hotfix] 같은 유저의 중복 참가 방지 및 참가자 조회 로직 리팩토링

1 participant