✨[CCI-54] 게스트 로그인 API #55
Conversation
WalkthroughThis update introduces guest user functionality, including guest login, token handling, and scheduled cleanup of expired guest accounts. The Interview entity's host reference is refactored from a primitive ID to a Member association, with related adjustments across converters and services. Additional cascade and orphan removal options are added to several entity relationships. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OauthController
participant OauthService
participant MemberRepository
participant JwtProvider
Client->>OauthController: POST /auth/guest
OauthController->>OauthService: loginAsGuest()
OauthService->>MemberRepository: save(guestMember)
OauthService->>JwtProvider: generateAccessToken(guestId, true)
JwtProvider-->>OauthService: accessToken
OauthService-->>OauthController: GuestLoginResponse
OauthController-->>Client: ApiResponse<GuestLoginResponse>
sequenceDiagram
participant GuestAccountCleaner
participant MemberRepository
loop Every hour
GuestAccountCleaner->>MemberRepository: deleteByIsGuestTrueAndCreatedAtBefore(threshold)
end
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
src/main/java/cloudcomputinginha/demo/config/auth/JwtProvider.java (1)
23-34: Extract claim key to a constant & reuseSecretKeyHard-coding the
"isGuest"string everywhere risks typos and makes refactors harder.
Likewise,Keys.hmacShaKeyFor(secretKey.getBytes())is computed on every call; building theSecretKeyonce and re-using it avoids repeated Base64 decoding and object creation.+private static final String CLAIM_IS_GUEST = "isGuest"; +private SecretKey key; + +@PostConstruct +void init() { + key = Keys.hmacShaKeyFor(secretKey.getBytes()); +} ... - .claim("isGuest", isGuest) + .claim(CLAIM_IS_GUEST, isGuest) - .signWith(key, SignatureAlgorithm.HS256) + .signWith(this.key, SignatureAlgorithm.HS256)
🧹 Nitpick comments (12)
src/main/java/cloudcomputinginha/demo/domain/Coverletter.java (1)
8-10: Unused import list can be trimmed
java.util.ArrayListis needed, butjava.util.Listis already imported by default whenListis referenced later; leaving both is fine but redundant imports might accumulate.
Minor nit – consider organising imports.src/main/java/cloudcomputinginha/demo/DemoApplication.java (1)
7-12: Optional: customise scheduler thread-pool
@EnableSchedulingactivates the default single-threaded scheduler.
If multiple tasks (e.g., guest clean-up + other future jobs) will run, consider defining aTaskSchedulerbean with a thread pool to avoid blocking jobs from delaying each other.src/main/java/cloudcomputinginha/demo/service/interview/InterviewCommandServiceImpl.java (1)
139-141: Permission check can be reduced to one DB hitCalling
interview.getHost().getId()issues an extra lazy-load query in most JPA setups.
A lighter alternative is to have the repository expose:boolean existsByIdAndHost_Id(Long interviewId, Long hostId);and use it for permission validation before fetching the whole entity. Improves performance on hot paths.
src/main/java/cloudcomputinginha/demo/service/interviewOption/InterviewOptionCommandServiceImpl.java (1)
31-33: Apply the same optimisation as inInterviewCommandServiceImplAs above, consider replacing the lazy load of
hostwith a repository existence check to avoid an extra select just for the permission guard.src/main/java/cloudcomputinginha/demo/config/auth/controller/OauthController.java (1)
85-89: Consider surfacing basic observability & rate-limits for the new guest endpoint
/auth/guestwill probably be hit far more frequently than social-login because there is no prior credential. You may want to:
- Add a simple
log.info()insideguestLogin()to track usage (helps when you need to distinguish bot abuse from normal traffic).- Discuss – with the security config – whether per-IP throttling or a plain
@RateLimiter(e.g. Bucket4j) is needed.The implementation itself looks fine – no blocking issues.
src/main/java/cloudcomputinginha/demo/repository/MemberRepository.java (1)
5-11: Return the deleted-row count for better feedback
deleteByIsGuestTrueAndCreatedAtBeforecurrently returnsvoid; if you change the return type tolong, Spring Data will automatically return the number of rows removed.
That allows the caller (the scheduled cleaner) to log how many guest accounts were purged, which is useful for monitoring and alerting.-void deleteByIsGuestTrueAndCreatedAtBefore(LocalDateTime threshold); +long deleteByIsGuestTrueAndCreatedAtBefore(LocalDateTime threshold);src/main/java/cloudcomputinginha/demo/config/auth/GuestAccountCleaner.java (1)
15-20: Expose scheduling constants & log purge results
Small tweaks improve readability and observability:+private static final long ONE_HOUR_MILLIS = 60 * 60 * 1000L; -@Scheduled(fixedDelay = 3600000) +@Scheduled(fixedDelay = ONE_HOUR_MILLIS) -memberRepository.deleteByIsGuestTrueAndCreatedAtBefore(threshold); +long deleted = memberRepository.deleteByIsGuestTrueAndCreatedAtBefore(threshold); +log.info("Guest cleanup removed {} accounts older than {}", deleted, threshold);(Requires the repository method to return
long– see previous comment.)src/main/java/cloudcomputinginha/demo/config/auth/dto/GuestLoginResponse.java (1)
8-16: Align DTO Lombok style with the rest of the codebaseOther DTOs use
@NoArgsConstructor(access = AccessLevel.PROTECTED)to prevent unwanted instantiation while keeping frameworks happy.
Consider applying the same modifier for consistency:-@NoArgsConstructor +@NoArgsConstructor(access = AccessLevel.PROTECTED)Minor, but it keeps the public API surface minimal.
src/main/java/cloudcomputinginha/demo/converter/InterviewConverter.java (1)
133-144: Repeatedequalslogic – extract helperThe same
mi.getMember().getId().equals(interview.getHost().getId())predicate appears twice.
Extract to a local boolean or method to avoid duplication and improve readability.+boolean isHost = mi.getMember().getId().equals(interview.getHost().getId()); ... - .filter(mi -> mi.getMember().getId().equals(interview.getHost().getId())) + .filter(mi -> isHost) ... - .isHost(mi.getMember().getId().equals(interview.getHost().getId())) + .isHost(isHost)src/main/java/cloudcomputinginha/demo/domain/Interview.java (2)
47-48: Remove deadhostIdcommentLeaving the legacy
hostIdline commented invites confusion during future refactors.
Recommend deleting it entirely.
68-74: Eager removal on large collections could hit N+1
cascade = REMOVE, orphanRemoval = trueonmemberInterviewstriggers individual delete for each child.
If interviews can have many participants, consider batching or using@OnDeleteto let the DB handle cascading deletes.src/main/java/cloudcomputinginha/demo/domain/Member.java (1)
38-42: Consider explicitly marking nullable columns and documenting rationale
socialProviderandproviderIdare now nullable to accommodate guest accounts.
That is perfectly valid, but it is worth adding:@Column(length = 10, nullable = true) // explain: nullable for guest accountsso the intent is crystal-clear to future readers. JPA defaults to
nullable = true, yet being explicit avoids accidental regressions when someone later adds a global rule to flag “implicit nullable”.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/main/java/cloudcomputinginha/demo/DemoApplication.java(1 hunks)src/main/java/cloudcomputinginha/demo/config/auth/GuestAccountCleaner.java(1 hunks)src/main/java/cloudcomputinginha/demo/config/auth/JwtProvider.java(2 hunks)src/main/java/cloudcomputinginha/demo/config/auth/SecurityConfig.java(1 hunks)src/main/java/cloudcomputinginha/demo/config/auth/controller/OauthController.java(2 hunks)src/main/java/cloudcomputinginha/demo/config/auth/dto/GuestLoginResponse.java(1 hunks)src/main/java/cloudcomputinginha/demo/config/auth/service/OauthService.java(4 hunks)src/main/java/cloudcomputinginha/demo/converter/InterviewConverter.java(3 hunks)src/main/java/cloudcomputinginha/demo/domain/Coverletter.java(2 hunks)src/main/java/cloudcomputinginha/demo/domain/Interview.java(3 hunks)src/main/java/cloudcomputinginha/demo/domain/InterviewOption.java(1 hunks)src/main/java/cloudcomputinginha/demo/domain/Member.java(3 hunks)src/main/java/cloudcomputinginha/demo/repository/MemberRepository.java(1 hunks)src/main/java/cloudcomputinginha/demo/service/interview/InterviewCommandService.java(1 hunks)src/main/java/cloudcomputinginha/demo/service/interview/InterviewCommandServiceImpl.java(1 hunks)src/main/java/cloudcomputinginha/demo/service/interviewOption/InterviewOptionCommandServiceImpl.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/cloudcomputinginha/demo/config/auth/dto/GuestLoginResponse.java (1)
src/main/java/cloudcomputinginha/demo/web/dto/InterviewResponseDTO.java (1)
Builder(12-19)
🔇 Additional comments (7)
src/main/java/cloudcomputinginha/demo/config/auth/SecurityConfig.java (1)
73-75: Confirm public exposure of the guest-login endpoint
/auth/guestis now publicly accessible. Double-check that:
- The controller mapping path exactly matches this string (case-sensitive).
- No additional sub-paths (e.g.
/auth/guest/**) are required; otherwise you may need a wildcard to avoid 403s on nested routes.Nothing blocking, just a verification point.
src/main/java/cloudcomputinginha/demo/domain/InterviewOption.java (1)
43-44: Cascade removal is now handled on the owning side – looks good
Removingcascade = CascadeType.ALLhere prevents accidental propagation fromInterviewOptiontoInterview.
Given thatInterviewnow owns the relationship withcascade = REMOVE&orphanRemoval = true, lifecycle rules remain intact.src/main/java/cloudcomputinginha/demo/service/interview/InterviewCommandService.java (1)
12-13: Redundantpublicremoved – correct for interface methods
Java interface methods are implicitly public; dropping the keyword keeps the code concise and idiomatic.src/main/java/cloudcomputinginha/demo/config/auth/JwtProvider.java (1)
70-83: Race-condition on refresh-token overwrite
member.setRefreshToken(newRefreshToken)happens after we already validated the saved token.
If two parallel re-issue requests pass validation at the same time, the second update will silently overwrite the first one and both clients will hold valid token pairs.Mitigate by synchronising on the member row (optimistic lock/version column) or performing an atomic update via
UPDATE … WHERE refresh_token = :oldToken.
Alternatively, return HTTP 409 on concurrent refresh attempts.src/main/java/cloudcomputinginha/demo/config/auth/service/OauthService.java (1)
56-57: 👍 Explicitly passingfalseclarifies intentCalling
generateAccessToken(member.getId(), false)makes the non-guest assumption explicit—good change.src/main/java/cloudcomputinginha/demo/converter/InterviewConverter.java (1)
48-51: Builder now setshost– verify missing mandatory fields
Interview.builder().host(member)is added, but the builder still setscurrentParticipants = 1by default.
Make sure caller incrementscurrentParticipantsonly afterMemberInterviewis persisted, otherwise host may be counted twice.src/main/java/cloudcomputinginha/demo/domain/Member.java (1)
47-58: Cascade-REMOVE everywhere may be too aggressiveEvery one-to-many collection now uses
cascade = CascadeType.REMOVE, orphanRemoval = true.
While this is required for scheduled guest purges, it also means that any deletion of a normal member will hard-delete all related entities.
- Do we really intend to wipe résumés / interviews when a regular user deletes their account?
- In case GDPR “right to be forgotten” is the goal, that’s fine – but then we probably also need to cascade to other entities (comments, likes, etc.) for consistency.
If the behaviour should be limited to guest cleanup only, consider performing manual deletes in the cleaner rather than globally cascading.
| @OneToMany(mappedBy = "coverletter", cascade = CascadeType.ALL, orphanRemoval = true) | ||
| private List<Qna> qnas = new ArrayList<>(); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure non-null qnas when using Lombok builder
When the entity is created through Lombok’s @Builder, the qnas field will be null unless explicitly set, because new ArrayList<>() is only executed on field initialisation, not in the builder.
Add @Builder.Default to guarantee an empty list and prevent NPEs when business logic calls coverletter.getQnas().
- @OneToMany(mappedBy = "coverletter", cascade = CascadeType.ALL, orphanRemoval = true)
- private List<Qna> qnas = new ArrayList<>();
+ @OneToMany(mappedBy = "coverletter", cascade = CascadeType.ALL, orphanRemoval = true)
+ @Builder.Default
+ private List<Qna> qnas = new ArrayList<>();📝 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.
| @OneToMany(mappedBy = "coverletter", cascade = CascadeType.ALL, orphanRemoval = true) | |
| private List<Qna> qnas = new ArrayList<>(); | |
| @OneToMany(mappedBy = "coverletter", cascade = CascadeType.ALL, orphanRemoval = true) | |
| @Builder.Default | |
| private List<Qna> qnas = new ArrayList<>(); |
🤖 Prompt for AI Agents
In src/main/java/cloudcomputinginha/demo/domain/Coverletter.java around lines 26
to 28, the qnas list is initialized with new ArrayList<>() but when using
Lombok's @Builder, this initialization is bypassed causing qnas to be null. To
fix this, add the @Builder.Default annotation to the qnas field to ensure it is
initialized to an empty list by default when the builder is used, preventing
potential NullPointerExceptions.
| // 게스트 사용자 | ||
| public GuestLoginResponse loginAsGuest() { | ||
| String name = "게스트" + new Random().nextInt(10000); | ||
| Member guest = Member.createGuest(name); | ||
| memberRepository.save(guest); | ||
|
|
||
| String accessToken = jwtProvider.generateAccessToken(guest.getId(), true); | ||
|
|
||
| return new GuestLoginResponse(accessToken, guest.getId(), guest.getName()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use SecureRandom + transactional save for guest creation
Randomisn’t cryptographically strong.SecureRandomprevents predictable guest names.- No transaction → if token generation fails after persisting, the orphan guest remains. Annotate with
@Transactional(or handle rollback manually). - Capture the returned entity from
savefor clarity.
-String name = "게스트" + new Random().nextInt(10_000);
-Member guest = Member.createGuest(name);
-memberRepository.save(guest);
+String name = "게스트" + new SecureRandom().nextInt(10_000);
+Member guest = memberRepository.save(Member.createGuest(name));📝 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 GuestLoginResponse loginAsGuest() { | |
| String name = "게스트" + new Random().nextInt(10000); | |
| Member guest = Member.createGuest(name); | |
| memberRepository.save(guest); | |
| String accessToken = jwtProvider.generateAccessToken(guest.getId(), true); | |
| return new GuestLoginResponse(accessToken, guest.getId(), guest.getName()); | |
| } | |
| // 게스트 사용자 | |
| public GuestLoginResponse loginAsGuest() { | |
| String name = "게스트" + new SecureRandom().nextInt(10_000); | |
| Member guest = memberRepository.save(Member.createGuest(name)); | |
| String accessToken = jwtProvider.generateAccessToken(guest.getId(), true); | |
| return new GuestLoginResponse(accessToken, guest.getId(), guest.getName()); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/cloudcomputinginha/demo/config/auth/service/OauthService.java
around lines 78 to 87, replace the use of java.util.Random with
java.security.SecureRandom to generate the guest name for stronger randomness.
Annotate the loginAsGuest method with @Transactional to ensure the guest
creation and token generation occur atomically, preventing orphan guest records
if token generation fails. Also, capture and use the entity returned from
memberRepository.save for clarity and consistency.
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "host_id", nullable = false) | ||
| private Member host; | ||
|
|
There was a problem hiding this comment.
Missing @OnDelete may leave orphan FK rows in DB
@ManyToOne with cascade = REMOVE on the parent side is fine, but deleting a Member will currently fail due to FK constraint on Interview.host_id.
Add @OnDelete(action = OnDeleteAction.CASCADE) (Hibernate) or handle manually in service layer.
🤖 Prompt for AI Agents
In src/main/java/cloudcomputinginha/demo/domain/Interview.java around lines 53
to 56, the @ManyToOne relationship to Member as host lacks the @OnDelete
annotation, causing foreign key constraint failures when deleting a Member. To
fix this, add @OnDelete(action = OnDeleteAction.CASCADE) to the host field to
ensure that deleting a Member cascades and removes related Interview entries,
preventing orphaned foreign key rows.
| private boolean isGuest; | ||
|
|
||
| public static Member createGuest(String randomName) { | ||
| Member member = new Member(); | ||
| member.email = "guest_" + UUID.randomUUID().toString().substring(0, 5)+ "@guest.com"; | ||
| member.name = randomName; | ||
| member.isGuest = true; | ||
| return member; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add column annotation & reduce collision risk in guest email generation
-
Persistence:
@Column(nullable = false) private boolean isGuest;
Even though a primitive
booleanis non-null at JVM level, some dialects still generate a nullable column ifnullableisn’t set. -
Collision risk: 5 random chars give 1.0M combinations → not huge at scale.
Switching to 8–10 chars is safer and still keeps the email under the 30-char limit:
-member.email = "guest_" + UUID.randomUUID().toString().substring(0, 5) + "@guest.com";
+member.email = "guest_" + UUID.randomUUID().toString().substring(0, 10) + "@guest.com";🤖 Prompt for AI Agents
In src/main/java/cloudcomputinginha/demo/domain/Member.java around lines 78 to
86, add the @Column(nullable = false) annotation to the isGuest field to ensure
the database column is non-nullable. Also, increase the substring length in the
UUID used for generating the guest email from 5 to between 8 and 10 characters
to reduce collision risk while keeping the email length reasonable.
✨ 작업 개요
게스트 사용자 로그인 API입니다.
✅ 작업 내용
📌 참고 사항(선택)
💬리뷰 요구사항(선택)
🔗 관련 이슈
Summary by CodeRabbit
New Features
Enhancements
Security
Improvements