Skip to content

Conversation

@kwon204
Copy link
Contributor

@kwon204 kwon204 commented Jun 9, 2025

#️⃣ 연관된 이슈>

  • 캘린더 연동 후 구글 서버로부터 가져온 일정들이 DB에 저장할 때 userService.getCurrentUser()를 시도하여 에러가 발생함.
  • 클라이언트 요청에 의해 일정 생성 로직과 동기화를 위해 일정 생성하는 로직을 분리하여 해결

수정 사항 (25.07.13)

  • 캘린더 연동 요청 후 캘린더 정보 저장까지 비동기에서 동기로 처리하도록 수정
  • TransactionalEventListener의 BEFORE_COMMIT 옵션에서 문제가 있어 코드를 수정하였습니다.
    • 이벤트를 발행하는 메소드가 커밋되기 전에 실행되고 이벤트 리스너 쪽의 코드와 함께 커밋되는 것은 맞으나, 구글 토큰을 저장하기 전에 토큰을 사용하려고 시도하는 경우가 발생할 수 있기 때문에 문제가 됩니다.
    • 따라서 BEFORE_COMMIT을 사용할 수 없습니다.
  • 그리고 서치를 하던 중, 트랜잭션을 물고 외부 API를 호출하게 되면 결과를 기다리는 동안 트랜잭션을 물고 있어 자원을 낭비하는 코드 구조라고 합니다. 그래서 기존의 linkGoogleCalendar에 적용했던 트랜잭션을 제거하는 것이 필요하였습니다.
  • 그리고 프론트에서 필요한 것은 캘린더의 정보만 필요하기 때문에 캘린더의 일정을 모두 가져오는 syncWithCalendar는 동기로 처리할 필요가 없다고 생각하였습니다. 그래서 syncWithCalendar는 비동기로 처리하는 게 적절하다고 생각하였습니다.

  • 위 내용을 종합해서 Facade 패턴을 사용해서 구현하였습니다.
  • 기존의 코드에서 조금만 변경해서 이벤트를 연속해서 발행하는 것도 생각해봤는데, 너무 복잡해지는 문제가 있었습니다.
  • CalendarFacade의 linkGoogleCalendar는 트랜잭션을 적용하지 않고 googleCalendarService를 직접 호출해서 getPrimaryCalendar는 순차적으로 진행합니다. 그리고 비동기 처리가 가능한 syncWithCalendar는 이벤트를 발행하여 기존과 동일하게 진행됩니다.

📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)

🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

Summary by CodeRabbit

Summary by CodeRabbit

  • Refactor

    • Improved internal handling of personal event creation for better maintainability and consistency.
    • Introduced a new calendar management service to streamline Google Calendar linking and retrieval.
    • Updated calendar and user services to support enhanced token management and calendar operations.
    • Adjusted event handling for Google Calendar integration to optimize timing and subscription behavior.
  • Tests

    • Updated tests to align with the improved personal event creation logic and response format.

- 캘린더 연동 후 구글 서버로부터 가져온 일정들이 DB에 저장할 때 userService.getCurrentUser()를 시도하여 에러가 발생함.
- 클라이언트 요청에 의해 일정 생성 로직과 동기화를 위해 일정 생성하는 로직을 분리하여 해결
@kwon204 kwon204 self-assigned this Jun 9, 2025
@kwon204 kwon204 requested a review from efdao as a code owner June 9, 2025 06:41
@kwon204 kwon204 added the 🛠️ BE Backend label Jun 9, 2025
@coderabbitai
Copy link

coderabbitai bot commented Jun 9, 2025

Walkthrough

The PersonalEventService.createWithRequest method was refactored to return a PersonalEventResponse DTO instead of a PersonalEvent entity, with central event creation logic extracted to a new private method. Corresponding updates were made in the controller and related tests to align with the new return type. Additionally, the GoogleEventHandler.link method's event listener annotation was changed to execute before transaction commit instead of asynchronously after commit. The calendar-related logic was refactored by introducing a new CalendarFacade service to handle Google OAuth linking and calendar retrieval, while CalendarService was simplified by removing OAuth and user context dependencies. A new updateTokens method was added to UserService to update both access and refresh tokens conditionally.

Changes

File(s) Change Summary
backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventController.java Simplified createPersonalEvent to directly use the PersonalEventResponse returned by the service method.
backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventService.java Refactored createWithRequest to return PersonalEventResponse; extracted event creation to a private method; updated Google event sync to use new private method.
backend/src/test/java/endolphin/backend/domain/personal_event/PersonalEventControllerTest.java,
backend/src/test/java/endolphin/backend/domain/personal_event/PersonalEventServiceTest.java
Updated tests to expect and assert on PersonalEventResponse instead of PersonalEvent.
backend/src/main/java/endolphin/backend/global/google/event/handler/GoogleEventHandler.java Changed event listener annotations on link methods; swapped async and transactional annotations; updated call from upsertGoogleCalendar to subscribeGoogleCalendar.
backend/src/main/java/endolphin/backend/domain/calendar/CalendarController.java Replaced CalendarService dependency with CalendarFacade; updated method calls accordingly.
backend/src/main/java/endolphin/backend/domain/calendar/CalendarFacade.java Added new CalendarFacade service to handle Google OAuth linking and calendar list retrieval with event publishing.
backend/src/main/java/endolphin/backend/domain/calendar/CalendarService.java Removed Google OAuth linking and user context logic; simplified calendar retrieval; removed related dependencies and methods.
backend/src/main/java/endolphin/backend/domain/user/UserService.java Added updateTokens method to update access and refresh tokens conditionally on a user entity.
backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java Removed @Transactional annotation; deleted upsertGoogleCalendar method; minor formatting changes.
backend/src/test/java/endolphin/backend/global/google/GoogleCalendarServiceTest.java Removed test method for upsertGoogleCalendar_noExistingCalendar.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Controller
    participant Service
    participant Repository

    Client->>Controller: POST /personal-events (with request)
    Controller->>Service: createWithRequest(request)
    Service->>Repository: findDiscussionsForUser()
    Service->>Service: createPersonalEvent(request, user, discussions)
    Service->>Repository: save(personalEvent)
    Service-->>Controller: PersonalEventResponse
    Controller-->>Client: 201 Created + PersonalEventResponse
Loading
sequenceDiagram
    participant Controller
    participant Facade
    participant OAuthService
    participant UserService
    participant CalendarService
    participant EventPublisher

    Controller->>Facade: linkGoogleCalendar(code)
    Facade->>OAuthService: getTokens(code)
    Facade->>OAuthService: getUserInfo(accessToken)
    Facade->>UserService: updateTokens(user, accessToken, refreshToken)
    Facade->>CalendarService: getPrimaryCalendar(user)
    Facade->>CalendarService: attachGoogleCalendar(calendarDto, user)
    Facade->>EventPublisher: publish CalendarLinkEvent(user)
Loading

Possibly related PRs

Suggested reviewers

  • efdao

Poem

🐇 In code I hop, with joyful cheer,

Events now flow, responses clear.
Tokens updated, calendars linked,
Listener timing finely synced.
A rabbit’s dance through service and test,
With refactor done, we’ve passed the quest!
✨🐰✨

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 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.
    • Explain this complex logic.
    • 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 explain this code block.
    • @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 explain its main purpose.
    • @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.

- 캘린더 연동 요청 후 캘린더 정보 저장까지 동기로 처리하도록 수정
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before commit으로 하신 이유가 있을까여?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

잘 몰라서 그냥 질문임당

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

기존에 비동기로 처리할 때는 AFTER_COMMIT이어도 다른 쓰레드로 실행되어 새로운 트랜잭션을 시작하게 됩니다. 하지만 동기로 처리할 때 AFTER_COMMIT인 상태라면 이전의 트랜잭션 내에서 실행하게 되는데, 이 때 commit이 실행된 이후이기 때문에 이후에 실행되는 SQL이 commit되지 않는 문제가 있다고 합니다. 그래서 BEFORE_COMMIT으로 하게 되었습니다.

그런데 더 찾아보니 현재 구조에서 BEFORE_COMMIT을 하면 문제가 생길 거 같아서 수정하고 다시 올리겠습니다

Copy link
Collaborator

@efdao efdao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@hamo-o
Copy link
Contributor

hamo-o commented Jun 25, 2025

머지 기원 1일차

kwon204 added 4 commits July 13, 2025 23:44
- linkGoogleCalendar, getCalendarList를 CalendarFacade로 이동
- CalendarService에서 linkGoogleCalendar, getCalendarList를 삭제
- CalendarController는 CalendarFacade 사용
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🔭 Outside diff range comments (1)
backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java (1)

44-44: 트랜잭션 경계 재설정 필요

현재 @TransactionalCalendarFacadeGoogleCalendarService 클래스/메서드 레벨 모두에서 사라져 트랜잭션 경계가 정의되지 않은 상태입니다. 아래 위치를 검토 후, 일관된 트랜잭션 전략에 따라 적절히 @Transactional을 적용해주세요.

  • backend/src/main/java/endolphin/backend/domain/calendar/CalendarFacade.java
    • 클래스 또는 핵심 퍼블릭 메서드에 @Transactional 선언
  • backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java
    • Google Calendar 연동 로직이 수행되는 메서드에 @Transactional 선언

예시:

 public class CalendarFacade {
+   @Transactional
     public void createEvent(User user, EventDto dto) {
         // ...
     }
 }
♻️ Duplicate comments (1)
backend/src/main/java/endolphin/backend/global/google/event/handler/GoogleEventHandler.java (1)

33-33: TransactionalEventListener의 phase 설정이 누락되었습니다.

이전 리뷰 코멘트에서 BEFORE_COMMIT과 AFTER_COMMIT의 차이점과 문제점에 대해 논의했었는데, 현재 코드에는 phase가 명시되어 있지 않습니다. 기본값은 AFTER_COMMIT이므로, 동기 처리 시 트랜잭션이 이미 커밋된 후에 실행되어 SQL이 커밋되지 않는 문제가 발생할 수 있습니다.

명확한 동작을 위해 phase를 명시적으로 설정해주세요:

-    @TransactionalEventListener(classes = {LoginEvent.class})
+    @TransactionalEventListener(classes = {LoginEvent.class}, phase = TransactionPhase.BEFORE_COMMIT)
🧹 Nitpick comments (1)
backend/src/main/java/endolphin/backend/domain/user/UserService.java (1)

47-55: Clarify the conditional token update behavior.

The method includes a guard clause that prevents updating tokens if an access token already exists. Consider either:

  1. Documenting this behavior in a method comment
  2. Renaming to updateTokensIfNotSet to make the conditional nature explicit
  3. Adding a parameter to control whether to force update existing tokens
+    /**
+     * Updates user tokens only if access token is not already set.
+     * @return the user with updated tokens, or unchanged user if access token exists
+     */
     public User updateTokens(User user, String accessToken, String refreshToken) {
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 2653c76 and 117e008.

📒 Files selected for processing (7)
  • backend/src/main/java/endolphin/backend/domain/calendar/CalendarController.java (3 hunks)
  • backend/src/main/java/endolphin/backend/domain/calendar/CalendarFacade.java (1 hunks)
  • backend/src/main/java/endolphin/backend/domain/calendar/CalendarService.java (2 hunks)
  • backend/src/main/java/endolphin/backend/domain/user/UserService.java (1 hunks)
  • backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java (1 hunks)
  • backend/src/main/java/endolphin/backend/global/google/event/handler/GoogleEventHandler.java (2 hunks)
  • backend/src/test/java/endolphin/backend/global/google/GoogleCalendarServiceTest.java (0 hunks)
💤 Files with no reviewable changes (1)
  • backend/src/test/java/endolphin/backend/global/google/GoogleCalendarServiceTest.java
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: dioo1461
PR: softeer5th/Team4-enDolphin#53
File: frontend/src/components/Input/MultiInput.tsx:50-59
Timestamp: 2025-02-04T03:10:35.809Z
Learning: User dioo1461 prefers to receive code reviews and responses in Korean language.
Learnt from: hamo-o
PR: softeer5th/Team4-enDolphin#205
File: frontend/src/features/discussion/ui/DiscussionCalendar/CalendarTable.tsx:29-33
Timestamp: 2025-02-19T05:54:56.987Z
Learning: The hardcoded selectedUserIdList in CalendarTable.tsx will be replaced with dynamic user selection when checkbox functionality is implemented in a future PR.
Learnt from: hamo-o
PR: softeer5th/Team4-enDolphin#169
File: frontend/src/features/my-calendar/api/index.ts:9-16
Timestamp: 2025-02-14T11:44:01.511Z
Learning: The API spec for `/api/v1/personal-event` endpoint is planned to be changed, which will affect how datetime parameters are handled. Current implementation uses hardcoded "T00:00:00" time component in the request.
Learnt from: kwon204
PR: softeer5th/Team4-enDolphin#114
File: backend/src/main/java/endolphin/backend/domain/shared_event/SharedEventService.java:25-29
Timestamp: 2025-02-07T07:09:58.456Z
Learning: In the SharedEvent entity class, the builder constructor parameters should match the field names (startDateTime, endDateTime) for consistency.
backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java (1)
Learnt from: efdao
PR: softeer5th/Team4-enDolphin#221
File: backend/src/main/java/endolphin/backend/global/google/event/handler/GoogleEventHandler.java:21-27
Timestamp: 2025-02-19T08:55:59.479Z
Learning: Google Calendar 연동 작업에서는 다음과 같은 예외 처리가 필요:
1. GoogleJsonResponseException - Google API 관련 오류
2. IOException - 네트워크 오류
3. 재시도 메커니즘 구현 고려
4. 상세한 로깅과 모니터링
backend/src/main/java/endolphin/backend/domain/calendar/CalendarService.java (1)
Learnt from: efdao
PR: softeer5th/Team4-enDolphin#221
File: backend/src/main/java/endolphin/backend/global/google/event/handler/GoogleEventHandler.java:21-27
Timestamp: 2025-02-19T08:55:59.479Z
Learning: Google Calendar 연동 작업에서는 다음과 같은 예외 처리가 필요:
1. GoogleJsonResponseException - Google API 관련 오류
2. IOException - 네트워크 오류
3. 재시도 메커니즘 구현 고려
4. 상세한 로깅과 모니터링
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: backend
🔇 Additional comments (8)
backend/src/main/java/endolphin/backend/domain/calendar/CalendarController.java (3)

26-26: LGTM! 좋은 아키텍처 개선입니다.

CalendarService에서 CalendarFacade로의 의존성 변경은 관심사의 분리를 잘 보여주는 좋은 리팩토링입니다. OAuth 관련 로직이 facade 레이어로 이동되었네요.


41-41: 메서드 호출이 새로운 facade와 일관성 있게 업데이트되었습니다.

calendarFacade.linkGoogleCalendarcalendarFacade.getCalendarList 호출이 새로운 아키텍처와 맞게 잘 변경되었습니다.

Also applies to: 52-52


26-26: LGTM! Proper facade pattern implementation.

The dependency injection change from CalendarService to CalendarFacade correctly implements the facade pattern, delegating calendar operations to the new orchestration layer that handles the complex OAuth and user token management flow.

Also applies to: 41-41, 52-52

backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java (1)

140-141: LGTM! 코드 포맷팅이 개선되었습니다.

getPrimaryCalendar 메서드의 변수 할당 포맷팅이 더 읽기 좋게 변경되었습니다.

backend/src/main/java/endolphin/backend/domain/calendar/CalendarFacade.java (1)

47-55: LGTM! 깔끔한 캘린더 목록 조회 구현입니다.

getCalendarList 메서드가 현재 사용자의 캘린더 목록을 잘 조회하고 DTO로 변환하고 있습니다.

backend/src/main/java/endolphin/backend/global/google/event/handler/GoogleEventHandler.java (1)

32-44: PR 목표와 맞지 않는 비동기 처리 및 이벤트 리스너 타입 변경에 대한 설명이 필요합니다.

PR 설명에서는 캘린더 정보 저장 프로세스를 비동기에서 동기 처리로 변경했다고 되어 있지만, 두 link 메서드 모두 여전히 @Async 어노테이션을 유지하고 있습니다. 또한 LoginEventGoogleCalendarLinkEvent의 이벤트 리스너 타입이 서로 바뀐 이유가 명확하지 않습니다.

다음 사항들을 확인해주세요:

  1. 동기 처리로 변경하려면 @Async 어노테이션을 제거해야 하지 않나요?
  2. 이벤트 리스너 타입을 바꾼 특별한 이유가 있나요?
backend/src/main/java/endolphin/backend/domain/calendar/CalendarService.java (2)

97-100: LGTM! Clean repository delegation.

The new findAllByUserId method properly delegates to the repository with appropriate read-only transaction annotation. This aligns with the simplified role of CalendarService after OAuth concerns were moved to CalendarFacade.


29-36: attachGoogleCalendar signature change is safe

I verified that no existing callers rely on a returned Calendar instance:

• In backend/src/main/java/endolphin/backend/domain/calendar/CalendarFacade.java, the call
calendarService.attachGoogleCalendar(calendarDto, user);
isn’t assigned to any variable.

No further changes are required here.

Comment on lines +47 to +55
public User updateTokens(User user, String accessToken, String refreshToken) {
if (user.getAccessToken() != null) {
return user;
}
user.setAccessToken(accessToken);
user.setRefreshToken(refreshToken);

return userRepository.save(user);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

토큰 업데이트 로직 개선이 필요합니다.

새로운 updateTokens 메서드가 추가되었지만, guard clause 로직에 개선이 필요합니다. 현재는 accessToken만 확인하고 있어서 refreshToken이 누락된 경우를 처리하지 못할 수 있습니다.

다음과 같이 개선하는 것을 고려해보세요:

public User updateTokens(User user, String accessToken, String refreshToken) {
-    if (user.getAccessToken() != null) {
+    if (user.getAccessToken() != null && user.getRefreshToken() != null) {
        return user;
    }
    user.setAccessToken(accessToken);
    user.setRefreshToken(refreshToken);

    return userRepository.save(user);
}

또는 더 명확하게 하려면:

public User updateTokens(User user, String accessToken, String refreshToken) {
-    if (user.getAccessToken() != null) {
+    if (user.getAccessToken() != null && user.getRefreshToken() != null) {
        return user;
    }
+    if (accessToken != null) {
        user.setAccessToken(accessToken);
+    }
+    if (refreshToken != null) {
        user.setRefreshToken(refreshToken);
+    }

    return userRepository.save(user);
}
📝 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.

Suggested change
public User updateTokens(User user, String accessToken, String refreshToken) {
if (user.getAccessToken() != null) {
return user;
}
user.setAccessToken(accessToken);
user.setRefreshToken(refreshToken);
return userRepository.save(user);
}
public User updateTokens(User user, String accessToken, String refreshToken) {
- if (user.getAccessToken() != null) {
+ if (user.getAccessToken() != null && user.getRefreshToken() != null) {
return user;
}
user.setAccessToken(accessToken);
user.setRefreshToken(refreshToken);
return userRepository.save(user);
}
🤖 Prompt for AI Agents
In backend/src/main/java/endolphin/backend/domain/user/UserService.java around
lines 47 to 55, the updateTokens method currently only checks if the user's
accessToken is non-null before returning early, which misses cases where the
refreshToken might be missing. To fix this, update the guard clause to check if
both accessToken and refreshToken are non-null and return the user early only if
both tokens are present. Otherwise, proceed to set the new tokens and save the
user.

Comment on lines +30 to +45
public void linkGoogleCalendar(String code) {
if (code == null || code.isBlank()) {
return;
}
GoogleTokens tokens = googleOAuthService.getGoogleTokens(code, false);
GoogleUserInfo userInfo = googleOAuthService.getUserInfo(tokens.accessToken());


User user = userService.getUserByEmail(userInfo.email());
user = userService.updateTokens(user, tokens.accessToken(), tokens.refreshToken());

GoogleCalendarDto calendarDto = googleCalendarService.getPrimaryCalendar(user);
calendarService.attachGoogleCalendar(calendarDto, user);

eventPublisher.publishEvent(new GoogleCalendarLinkEvent(user));
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

좋은 facade 패턴 구현이지만 에러 처리와 로깅 개선이 필요합니다.

linkGoogleCalendar 메서드가 OAuth 플로우를 잘 오케스트레이션하고 있어서 원래 버그를 해결할 것 같습니다. 하지만 몇 가지 개선사항이 있습니다.

다음과 같은 개선을 고려해보세요:

public void linkGoogleCalendar(String code) {
    if (code == null || code.isBlank()) {
+        log.warn("[linkGoogleCalendar] Invalid authorization code provided");
        return;
    }
+    
+    try {
+        log.info("[linkGoogleCalendar] Starting Google Calendar linking process");
        GoogleTokens tokens = googleOAuthService.getGoogleTokens(code, false);
        GoogleUserInfo userInfo = googleOAuthService.getUserInfo(tokens.accessToken());

-
        User user = userService.getUserByEmail(userInfo.email());
        user = userService.updateTokens(user, tokens.accessToken(), tokens.refreshToken());

        GoogleCalendarDto calendarDto = googleCalendarService.getPrimaryCalendar(user);
        calendarService.attachGoogleCalendar(calendarDto, user);

+        log.info("[linkGoogleCalendar] Successfully linked calendar for user: {}", user.getEmail());
        eventPublisher.publishEvent(new GoogleCalendarLinkEvent(user));
+    } catch (Exception e) {
+        log.error("[linkGoogleCalendar] Failed to link Google Calendar: {}", e.getMessage(), e);
+        throw e;
+    }
}

또한 이 메서드가 트랜잭션 내에서 실행되어야 하는지 고려해보세요:

+@Transactional
public void linkGoogleCalendar(String code) {
🤖 Prompt for AI Agents
In backend/src/main/java/endolphin/backend/domain/calendar/CalendarFacade.java
around lines 30 to 45, the linkGoogleCalendar method lacks error handling and
logging. Add try-catch blocks to handle potential exceptions from OAuth token
retrieval, user lookup, and calendar attachment. Log meaningful error messages
when exceptions occur to aid debugging. Also, consider annotating the method
with @Transactional if the operations should be atomic to ensure consistency in
case of failures.

🛠️ Refactor suggestion

Implement comprehensive error handling for Google API operations.

Based on the retrieved learnings and Google Calendar integration best practices, this method needs robust error handling for:

  1. GoogleJsonResponseException - Google API errors
  2. IOException - Network failures
  3. Invalid tokens or user info
  4. Calendar attachment failures

Consider wrapping the Google API calls in try-catch blocks and implementing proper rollback mechanisms.

 public void linkGoogleCalendar(String code) {
     if (code == null || code.isBlank()) {
         return;
     }
-    GoogleTokens tokens = googleOAuthService.getGoogleTokens(code, false);
-    GoogleUserInfo userInfo = googleOAuthService.getUserInfo(tokens.accessToken());
-
-
-    User user = userService.getUserByEmail(userInfo.email());
-    user = userService.updateTokens(user, tokens.accessToken(), tokens.refreshToken());
-
-    GoogleCalendarDto calendarDto = googleCalendarService.getPrimaryCalendar(user);
-    calendarService.attachGoogleCalendar(calendarDto, user);
-
-    eventPublisher.publishEvent(new GoogleCalendarLinkEvent(user));
+    try {
+        GoogleTokens tokens = googleOAuthService.getGoogleTokens(code, false);
+        GoogleUserInfo userInfo = googleOAuthService.getUserInfo(tokens.accessToken());
+        
+        User user = userService.getUserByEmail(userInfo.email());
+        user = userService.updateTokens(user, tokens.accessToken(), tokens.refreshToken());
+        
+        GoogleCalendarDto calendarDto = googleCalendarService.getPrimaryCalendar(user);
+        calendarService.attachGoogleCalendar(calendarDto, user);
+        
+        eventPublisher.publishEvent(new GoogleCalendarLinkEvent(user));
+    } catch (GoogleJsonResponseException e) {
+        log.error("Google API error during calendar linking: {}", e.getMessage());
+        throw new CalendarException(ErrorCode.GC_API_ERROR, e);
+    } catch (IOException e) {
+        log.error("Network error during calendar linking: {}", e.getMessage());
+        throw new CalendarException(ErrorCode.NETWORK_ERROR, e);
+    }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public void linkGoogleCalendar(String code) {
if (code == null || code.isBlank()) {
return;
}
GoogleTokens tokens = googleOAuthService.getGoogleTokens(code, false);
GoogleUserInfo userInfo = googleOAuthService.getUserInfo(tokens.accessToken());
User user = userService.getUserByEmail(userInfo.email());
user = userService.updateTokens(user, tokens.accessToken(), tokens.refreshToken());
GoogleCalendarDto calendarDto = googleCalendarService.getPrimaryCalendar(user);
calendarService.attachGoogleCalendar(calendarDto, user);
eventPublisher.publishEvent(new GoogleCalendarLinkEvent(user));
}
public void linkGoogleCalendar(String code) {
if (code == null || code.isBlank()) {
return;
}
try {
GoogleTokens tokens = googleOAuthService.getGoogleTokens(code, false);
GoogleUserInfo userInfo = googleOAuthService.getUserInfo(tokens.accessToken());
User user = userService.getUserByEmail(userInfo.email());
user = userService.updateTokens(user, tokens.accessToken(), tokens.refreshToken());
GoogleCalendarDto calendarDto = googleCalendarService.getPrimaryCalendar(user);
calendarService.attachGoogleCalendar(calendarDto, user);
eventPublisher.publishEvent(new GoogleCalendarLinkEvent(user));
} catch (GoogleJsonResponseException e) {
log.error("Google API error during calendar linking: {}", e.getMessage());
throw new CalendarException(ErrorCode.GC_API_ERROR, e);
} catch (IOException e) {
log.error("Network error during calendar linking: {}", e.getMessage());
throw new CalendarException(ErrorCode.NETWORK_ERROR, e);
}
}
🤖 Prompt for AI Agents
In backend/src/main/java/endolphin/backend/domain/calendar/CalendarFacade.java
around lines 30 to 45, the method linkGoogleCalendar lacks error handling for
Google API calls and related operations. Wrap the calls to
googleOAuthService.getGoogleTokens, googleOAuthService.getUserInfo, userService
methods, googleCalendarService.getPrimaryCalendar, and
calendarService.attachGoogleCalendar in try-catch blocks to handle
GoogleJsonResponseException, IOException, and other potential exceptions. Add
checks for invalid tokens or user info and handle failures gracefully. Implement
rollback or compensation logic if calendar attachment fails after tokens are
updated. Ensure exceptions are logged or propagated appropriately.

public void link(LoginEvent event) {
User user = event.user();
googleCalendarService.upsertGoogleCalendar(user);
googleCalendarService.subscribeGoogleCalendar(user);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

GoogleCalendarService의 메서드 변경 확인 필요

upsertGoogleCalendar에서 subscribeGoogleCalendar로 메서드가 변경되었습니다. 새로운 메서드의 구현과 기능을 확인해주세요.


🏁 Script executed:

#!/bin/bash
# Description: subscribeGoogleCalendar 메서드의 구현을 확인합니다.

# GoogleCalendarService에서 새로운 메서드 검색
ast-grep --pattern 'subscribeGoogleCalendar($_) {
  $$$
}'

# 이전 upsertGoogleCalendar 메서드가 제거되었는지 확인
rg -A 5 "upsertGoogleCalendar"

Length of output: 1572


Missing subscribeGoogleCalendar implementation in GoogleCalendarService

The subscribeGoogleCalendar(user) call was added on line 36, but there’s no corresponding method in GoogleCalendarService. Meanwhile, tests still refer to the old upsertGoogleCalendar, which has been removed from the main code.

Please add or locate the subscribeGoogleCalendar(User user) method in:

  • backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java

And update the tests in:

  • backend/src/test/java/endolphin/backend/global/google/GoogleCalendarServiceTest.java

so that they no longer reference upsertGoogleCalendar and validate the new subscribeGoogleCalendar behavior.

🤖 Prompt for AI Agents
In
backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java
around line 36, add the missing subscribeGoogleCalendar(User user) method to
implement the functionality called in GoogleEventHandler.java. Then, in
backend/src/test/java/endolphin/backend/global/google/GoogleCalendarServiceTest.java,
remove references to the old upsertGoogleCalendar method and replace them with
tests that validate the new subscribeGoogleCalendar method's behavior.

Comment on lines +140 to +141
GoogleCalendarDto result =
googleCalendarApi.getPrimaryCalendar(eventsUrl, user.getAccessToken());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify transaction management after @transactional removal.

The formatting change is fine, but the removal of @Transactional from this service (mentioned in the summary) needs verification. This service still calls calendarService methods that perform database operations. Ensure that:

  1. Transactions are properly managed at the CalendarFacade level
  2. Database operations in methods like syncWithCalendar() are still atomic
  3. Rollback behavior is preserved for failed Google API calls

🏁 Script executed:

#!/bin/bash
# Verify that CalendarFacade or calling methods have proper transaction management
ast-grep --pattern $'@Transactional
class CalendarFacade {
  $$$
}'

# Check if calendarService calls are within transactional contexts
ast-grep --pattern $'calendarService.$_($$$)'

Length of output: 3564


🏁 Script executed:

#!/bin/bash
# Check for @Transactional annotation on CalendarService
rg '@Transactional' -n backend/src/main/java/endolphin/backend/domain/calendar/CalendarService.java || echo "No @Transactional in CalendarService"

# Check for @Service annotation on CalendarService
rg '@Service' -n backend/src/main/java/endolphin/backend/domain/calendar/CalendarService.java || echo "No @Service in CalendarService"

Length of output: 418


Reintroduce @Transactional on GoogleCalendarService to restore atomicity

The removal of @Transactional from this service means each calendarService call runs in its own transaction (CalendarService is annotated at class‐level), but multi‐step flows in GoogleCalendarService (e.g. syncWithCalendar) are no longer wrapped in a single transaction. If you need all DB updates and the Google API call to roll back together, re-add @Transactional at the class or method level.

Locations to update:

  • backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java (class declaration or specific methods like syncWithCalendar)

Suggested diff:

@@ backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java
- public class GoogleCalendarService {
+ @Transactional
+ public class GoogleCalendarService {
📝 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.

Suggested change
GoogleCalendarDto result =
googleCalendarApi.getPrimaryCalendar(eventsUrl, user.getAccessToken());
@Transactional
public class GoogleCalendarService {
// … rest of class …
}
🤖 Prompt for AI Agents
In
backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java
around lines 140-141, the @Transactional annotation was removed, causing
multi-step flows like syncWithCalendar to no longer run within a single
transaction. To fix this, re-add the @Transactional annotation either at the
class level or on specific methods such as syncWithCalendar to ensure all
database updates and Google API calls are executed atomically and can roll back
together on failure.

Comment on lines +19 to +21
@Service
@RequiredArgsConstructor
public class CalendarFacade {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add transaction management to the facade.

The facade orchestrates multiple service calls that modify data but lacks @Transactional annotation. If any step in linkGoogleCalendar fails (e.g., after token update but before calendar attachment), it could leave the system in an inconsistent state.

+import org.springframework.transaction.annotation.Transactional;
+
 @Service
+@Transactional
 @RequiredArgsConstructor
 public class CalendarFacade {
🤖 Prompt for AI Agents
In backend/src/main/java/endolphin/backend/domain/calendar/CalendarFacade.java
around lines 19 to 21, the CalendarFacade class is missing the @Transactional
annotation. Add @Transactional to the class declaration to ensure that all
service calls within methods like linkGoogleCalendar are executed within a
transaction, preventing partial updates and maintaining data consistency if any
step fails.

@kwon204 kwon204 requested a review from efdao July 13, 2025 15:07
@kwon204 kwon204 merged commit 86005cc into dev Jul 27, 2025
4 checks passed
@kwon204 kwon204 deleted the bugfix/be/sync-personal-event branch July 27, 2025 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🛠️ BE Backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants