[BE-Feat] 개인 일정을 구글 캘린더에 반영하는 기능 구현 #221
Conversation
# Conflicts: # backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventService.java
WalkthroughThe changes disable direct Google Calendar integration in the authentication service while adding and enhancing Google Calendar synchronization for personal events. The modifications update API method signatures, integrate new event records for insert, update, and deletion of personal events, and introduce event handlers for asynchronous calendar operations. Additionally, configuration and DTO records are updated for Google Calendar URL and event formats. Test classes are also updated to align with the new event-driven and synchronization workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant US as UserService
participant GEH as GoogleEventHandler
participant GCS as GoogleCalendarService
U->>US: upsertUser(user)
US->>US: Save user in repository
US->>GEH: Publish LoginEvent(user)
GEH->>GCS: Call upsertGoogleCalendar(user)
sequenceDiagram
participant PEC as PersonalEventController
participant PES as PersonalEventService
participant GE as GoogleEventHandler
participant GCS as GoogleCalendarService
PEC->>PES: deletePersonalEvent(eventId, syncWithGoogleCalendar)
PES-->>PES: Check if sync is enabled and event has Google IDs
PES->>GE: Publish DeletePersonalEvent(event)
GE->>GCS: Call deletePersonalEventFromGoogleCalender(event)
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (15)
backend/src/main/java/endolphin/backend/domain/personal_event/event/UpdatePersonalEvent.java (1)
5-9: Consider adding Javadoc to document the event's purpose and usage.Since this is part of an event-driven architecture for Google Calendar synchronization, adding documentation would help other developers understand:
- The event's role in the system
- When it's published
- Who consumes it
- How it helps avoid circular references
Add documentation like this:
+/** + * Event published when a personal event needs to be updated in Google Calendar. + * <p> + * This event is published by PersonalEventService when a personal event is updated + * and contains a Google Calendar event ID. The event is consumed by GoogleEventHandler + * to perform the actual calendar synchronization. + * <p> + * Part of the event-driven architecture to avoid circular references between + * personal event and Google Calendar synchronization logic. + */ public record UpdatePersonalEvent( + /** + * The personal event that needs to be updated in Google Calendar. + * Must contain a valid Google Calendar event ID. + */ PersonalEvent personalEvent ) { }backend/src/main/java/endolphin/backend/global/google/dto/GoogleEventDateTime.java (1)
7-10: Add documentation for the record fields.Consider adding Javadoc to explain the expected format of
dateTimeandtimeZonefields, as they are crucial for Google Calendar integration.+/** + * Record representing date and time for Google Calendar events. + * @param dateTime ISO-8601 formatted date-time string + * @param timeZone IANA timezone identifier + */ public record GoogleEventDateTime( String dateTime, String timeZone )backend/src/main/java/endolphin/backend/global/google/dto/GoogleEventRequest.java (1)
5-10: Add documentation and consider additional fields.The record could benefit from documentation and might need additional fields commonly used in Google Calendar events.
+/** + * Record representing a Google Calendar event request. + * @param start The event start time + * @param end The event end time + * @param summary The event title/summary + * @param id The unique identifier for the event + */ public record GoogleEventRequest( GoogleEventDateTime start, GoogleEventDateTime end, String summary, String id )Consider adding optional fields like
description,location, andcolorIdwhich are supported by Google Calendar API.backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventService.java (3)
46-46: Consider renaming thePRIMARYconstant for clarity.
While using"primary"here is acceptable, consider a more descriptive constant name likePRIMARY_CALENDAR_IDto indicate its purpose more explicitly.- private final String PRIMARY = "primary"; + private final String PRIMARY_CALENDAR_ID = "primary";Also applies to: 48-48
130-136: Handle empty stringgoogleEventIdscenario.
IfgoogleEventIdis potentially an empty string (rather thannull), consider aligning logic to ensure consistent checks.if (request.syncWithGoogleCalendar()) { if (result.getGoogleEventId() == null || result.getGoogleEventId().isEmpty()) { eventPublisher.publishEvent(new InsertPersonalEvent(List.of(result))); } else { eventPublisher.publishEvent(new UpdatePersonalEvent(result)); } }
285-291: Possible improvement to event ID length.
ThecreateGoogleEventIdmethod is adequate for uniqueness. However, consider truncating the random portion if you prefer shorter IDs. Otherwise, this approach is fine.backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java (4)
82-106: Recursive retry on OAuth failure
TheinsertPersonalEventToGoogleCalendarmethod recursively calls itself after reissuing the access token. This can be effective but be mindful of potential infinite loops if tokens are consistently invalid. Consider a retry limit to prevent unintended recursion.
108-132: Duplicate logic in update flow
TheupdatePersonalEventToGoogleCalendarmethod duplicates logic frominsertPersonalEventToGoogleCalendar(token handling, reissue, re-call). Consider refactoring to a shared helper to reduce repetition.
134-156: Typo in method name: "Calender"
RenamedeletePersonalEventFromGoogleCalendertodeletePersonalEventFromGoogleCalendarfor consistency and clarity.-public void deletePersonalEventFromGoogleCalender(PersonalEvent personalEvent) { +public void deletePersonalEventFromGoogleCalendar(PersonalEvent personalEvent) {
395-396: Remove or clarify commented-out code
ThepersonalEventService.syncWithGoogleEvents(...)call is now commented out, replaced byeventPublisher.publishEvent(...). If there's no plan to restore direct synchronization, consider removing the commented-out code to reduce clutter.backend/src/main/java/endolphin/backend/domain/personal_event/event/InsertPersonalEvent.java (1)
6-10: LGTM! Consider adding Javadoc.The record is well-designed for batch event handling. Consider adding Javadoc to document the purpose and usage of this event record.
Add documentation like:
+/** + * Event record for batch insertion of personal events into Google Calendar. + * Used to decouple the personal event creation from Google Calendar synchronization. + */ public record InsertPersonalEvent( List<PersonalEvent> personalEventList ) {backend/src/main/java/endolphin/backend/global/google/event/GoogleEventChanged.java (1)
7-12: Consider adding null safety annotations.The record handles important synchronization data. Consider adding
@NonNullannotations to prevent null pointer exceptions.+import org.springframework.lang.NonNull; public record GoogleEventChanged( + @NonNull List<GoogleEvent> events, + @NonNull User user, + @NonNull String googleCalendarId ) {backend/src/main/java/endolphin/backend/global/config/GoogleCalendarUrl.java (1)
6-14: Add URL validation and documentation.Consider the following improvements:
- Add validation to ensure URL format correctness
- Document the expected URL formats and endpoints
+import jakarta.validation.constraints.Pattern; +import org.hibernate.validator.constraints.URL; +/** + * Configuration properties for Google Calendar API endpoints. + * All URLs should be absolute URLs with HTTPS protocol. + */ @ConfigurationProperties(prefix = "google.calendar.url") public record GoogleCalendarUrl( + @URL String calendarListUrl, + @URL String subscribeUrl, + @URL String unsubscribeUrl, + @URL String webhookUrl, + @URL String primaryCalendarUrl, + @URL String eventsUrl, + @URL String updateUrl ) {backend/src/main/java/endolphin/backend/domain/personal_event/event/handler/PersonalEventHandler.java (1)
17-23: Consider adding error handling for the sync operation.The event listener implementation is clean and follows Spring best practices. However, consider adding error handling to gracefully handle potential failures during synchronization.
@EventListener(classes = {GoogleEventChanged.class}) public void sync(GoogleEventChanged event) { + try { List<GoogleEvent> events = event.events(); User user = event.user(); String googleCalendarId = event.googleCalendarId(); personalEventService.syncWithGoogleEvents(events, user, googleCalendarId); + } catch (Exception e) { + log.error("Failed to sync Google Calendar events: {}", e.getMessage(), e); + // Consider implementing retry logic or notifying the user + } }backend/src/main/java/endolphin/backend/domain/personal_event/entity/PersonalEvent.java (1)
44-46: Consider using builder pattern for immutability.While the setters are needed for Google Calendar synchronization, consider using the builder pattern to update these fields to maintain immutability. This would make the code more robust by ensuring that the Google Calendar metadata can only be modified through controlled methods.
Apply this diff to replace the setters with a builder-based update method:
- @Setter @Column(name = "calendar_id") private String calendarId; - @Setter @Column(name = "google_event_id") private String googleEventId; + public PersonalEvent updateGoogleMetadata(String calendarId, String googleEventId) { + return PersonalEvent.builder() + .title(this.title) + .startTime(this.startTime) + .endTime(this.endTime) + .user(this.user) + .isAdjustable(this.isAdjustable) + .calendarId(calendarId) + .googleEventId(googleEventId) + .build(); + }Also applies to: 48-50
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
backend/src/main/java/endolphin/backend/domain/auth/AuthService.java(1 hunks)backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventController.java(1 hunks)backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventService.java(9 hunks)backend/src/main/java/endolphin/backend/domain/personal_event/entity/PersonalEvent.java(2 hunks)backend/src/main/java/endolphin/backend/domain/personal_event/event/DeletePersonalEvent.java(1 hunks)backend/src/main/java/endolphin/backend/domain/personal_event/event/InsertPersonalEvent.java(1 hunks)backend/src/main/java/endolphin/backend/domain/personal_event/event/UpdatePersonalEvent.java(1 hunks)backend/src/main/java/endolphin/backend/domain/personal_event/event/handler/PersonalEventHandler.java(1 hunks)backend/src/main/java/endolphin/backend/domain/user/UserService.java(4 hunks)backend/src/main/java/endolphin/backend/domain/user/event/LoginEvent.java(1 hunks)backend/src/main/java/endolphin/backend/global/config/GoogleCalendarUrl.java(1 hunks)backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java(10 hunks)backend/src/main/java/endolphin/backend/global/google/dto/GoogleEventDateTime.java(1 hunks)backend/src/main/java/endolphin/backend/global/google/dto/GoogleEventRequest.java(1 hunks)backend/src/main/java/endolphin/backend/global/google/event/GoogleEventChanged.java(1 hunks)backend/src/main/java/endolphin/backend/global/google/event/handler/GoogleEventHandler.java(1 hunks)backend/src/test/java/endolphin/backend/domain/personal_event/PersonalEventServiceTest.java(6 hunks)backend/src/test/java/endolphin/backend/global/google/GoogleCalendarServiceTest.java(2 hunks)
🔇 Additional comments (18)
backend/src/main/java/endolphin/backend/domain/personal_event/event/UpdatePersonalEvent.java (1)
5-9: LGTM! Clean and focused event record implementation.The record is well-structured and follows the single responsibility principle, serving as a clear data carrier for personal event updates in the event-driven architecture.
backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventService.java (4)
10-11: All new imports look good.
No immediate issues detected regarding the introduction of these event-driven imports.Also applies to: 21-21, 26-26, 31-31, 33-33
76-80: Validate Google Calendar readiness early.
Whenrequest.syncWithGoogleCalendar()is true, the code proceeds to set IDs and publish an insert event. Ensure that upstream code handles a potential lack of proper Google tokens or missing user setups before proceeding with synchronization.
98-114: Ensure correct participant token availability.
This bulk creation assignsgoogleEventIdfor each participant, then publishes insert events. It looks logically solid, but verify that each participant actually has valid credentials when performing synchronization in the downstream handlers. If not, handle missing tokens gracefully.
161-179: Verify deletion path for missing Google IDs.
Conditions for publishingDeletePersonalEventappear correct. If the user setsyncWithGoogleCalendar = truebut no Google event/calendar ID is found, the event is removed locally only. This is logical, but confirm that this is the intended behavior.backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java (5)
5-5: New imports for personal event and event publishing
No immediate concerns with these new dependencies. Looks consistent with the event-driven approach.Also applies to: 15-15, 18-18, 26-26, 29-29
70-71: Removed direct syncing, replaced with event publishing
The oldpersonalEventService.syncWithGoogleEvents(...)call is commented out in favor of publishing theGoogleEventChangedevent. This may reduce coupling but verify that no essential business logic was lost by removing the direct sync call.
76-80: Batch insert events method
Good approach to iterate through the list ofPersonalEventobjects and callinsertPersonalEventToGoogleCalendarindividually. No major issues observed.
274-274: Reissue token approach
The catch block forOAuthExceptionwith a reissued token is consistent with other parts of this class. Looks good.
360-360: Token reissue in subscribe flow
Same token reissue logic applies here. Behavior is consistent.backend/src/main/java/endolphin/backend/domain/user/event/LoginEvent.java (1)
1-9: Record definition appears correct
DefiningLoginEventas a record with aUser userfield seems straightforward and matches typical usage for publishing login-related events.backend/src/main/java/endolphin/backend/domain/personal_event/event/DeletePersonalEvent.java (1)
5-9: LGTM! Well-structured event record.The record is well-designed for event handling with appropriate immutability and clear naming.
backend/src/main/java/endolphin/backend/domain/user/UserService.java (1)
61-62: LGTM! Good event publishing implementation.The event is correctly published after the user is saved, ensuring the event contains the persisted user entity.
backend/src/main/java/endolphin/backend/domain/personal_event/entity/PersonalEvent.java (1)
12-12: LGTM!Added Lombok import for the new
@Setterannotations.backend/src/test/java/endolphin/backend/global/google/GoogleCalendarServiceTest.java (1)
31-31: LGTM!Added
ApplicationEventPublishermock for the new event-driven architecture, which aligns with the PR's objective of handling Google Calendar synchronization through events.Also applies to: 42-46
backend/src/test/java/endolphin/backend/domain/personal_event/PersonalEventServiceTest.java (3)
15-16: LGTM!Added imports for Base64 encoding and UUID generation.
Also applies to: 19-19
45-47: LGTM!Added
ApplicationEventPublishermock for testing event-driven synchronization with Google Calendar.
189-189: LGTM!Updated test methods to include the
syncWithGoogleCalendarparameter, aligning with the service changes.Also applies to: 207-207
| private final static DateTimeFormatter DATE_TIME_FORMATTER = DateTimeFormatter.ISO_DATE_TIME; | ||
| public static GoogleEventDateTime fromLocalDateTime(LocalDateTime dateTime) { | ||
|
|
||
| return new GoogleEventDateTime(dateTime.atOffset(ZoneOffset.ofHours(9)).format(DATE_TIME_FORMATTER), "Asia/Seoul"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve timezone handling and input validation.
The current implementation has several potential issues:
- Hardcoding timezone to "Asia/Seoul" limits international usage
- No validation for null input
- Missing documentation for the factory method
Consider this improved implementation:
- private final static DateTimeFormatter DATE_TIME_FORMATTER = DateTimeFormatter.ISO_DATE_TIME;
+ private static final DateTimeFormatter DATE_TIME_FORMATTER = DateTimeFormatter.ISO_DATE_TIME;
+ private static final ZoneId DEFAULT_ZONE = ZoneId.of("Asia/Seoul");
+
+ /**
+ * Creates a GoogleEventDateTime from a LocalDateTime using the default timezone (Asia/Seoul).
+ * @param dateTime The LocalDateTime to convert (must not be null)
+ * @return A new GoogleEventDateTime instance
+ * @throws NullPointerException if dateTime is null
+ */
public static GoogleEventDateTime fromLocalDateTime(LocalDateTime dateTime) {
+ Objects.requireNonNull(dateTime, "dateTime must not be null");
+ return fromLocalDateTime(dateTime, DEFAULT_ZONE);
+ }
- return new GoogleEventDateTime(dateTime.atOffset(ZoneOffset.ofHours(9)).format(DATE_TIME_FORMATTER), "Asia/Seoul");
+ /**
+ * Creates a GoogleEventDateTime from a LocalDateTime with a specific timezone.
+ * @param dateTime The LocalDateTime to convert (must not be null)
+ * @param zoneId The timezone to use (must not be null)
+ * @return A new GoogleEventDateTime instance
+ * @throws NullPointerException if either parameter is null
+ */
+ public static GoogleEventDateTime fromLocalDateTime(LocalDateTime dateTime, ZoneId zoneId) {
+ Objects.requireNonNull(dateTime, "dateTime must not be null");
+ Objects.requireNonNull(zoneId, "zoneId must not be null");
+ return new GoogleEventDateTime(
+ dateTime.atZone(zoneId).format(DATE_TIME_FORMATTER),
+ zoneId.getId()
+ );
}📝 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.
| private final static DateTimeFormatter DATE_TIME_FORMATTER = DateTimeFormatter.ISO_DATE_TIME; | |
| public static GoogleEventDateTime fromLocalDateTime(LocalDateTime dateTime) { | |
| return new GoogleEventDateTime(dateTime.atOffset(ZoneOffset.ofHours(9)).format(DATE_TIME_FORMATTER), "Asia/Seoul"); | |
| } | |
| private static final DateTimeFormatter DATE_TIME_FORMATTER = DateTimeFormatter.ISO_DATE_TIME; | |
| private static final ZoneId DEFAULT_ZONE = ZoneId.of("Asia/Seoul"); | |
| /** | |
| * Creates a GoogleEventDateTime from a LocalDateTime using the default timezone (Asia/Seoul). | |
| * @param dateTime The LocalDateTime to convert (must not be null) | |
| * @return A new GoogleEventDateTime instance | |
| * @throws NullPointerException if dateTime is null | |
| */ | |
| public static GoogleEventDateTime fromLocalDateTime(LocalDateTime dateTime) { | |
| Objects.requireNonNull(dateTime, "dateTime must not be null"); | |
| return fromLocalDateTime(dateTime, DEFAULT_ZONE); | |
| } | |
| /** | |
| * Creates a GoogleEventDateTime from a LocalDateTime with a specific timezone. | |
| * @param dateTime The LocalDateTime to convert (must not be null) | |
| * @param zoneId The timezone to use (must not be null) | |
| * @return A new GoogleEventDateTime instance | |
| * @throws NullPointerException if either parameter is null | |
| */ | |
| public static GoogleEventDateTime fromLocalDateTime(LocalDateTime dateTime, ZoneId zoneId) { | |
| Objects.requireNonNull(dateTime, "dateTime must not be null"); | |
| Objects.requireNonNull(zoneId, "zoneId must not be null"); | |
| return new GoogleEventDateTime( | |
| dateTime.atZone(zoneId).format(DATE_TIME_FORMATTER), | |
| zoneId.getId() | |
| ); | |
| } |
| public static GoogleEventRequest of(PersonalEvent personalEvent, String id) { | ||
| GoogleEventDateTime start = GoogleEventDateTime.fromLocalDateTime(personalEvent.getStartTime()); | ||
| GoogleEventDateTime end = GoogleEventDateTime.fromLocalDateTime(personalEvent.getEndTime()); | ||
| return new GoogleEventRequest(start, end, personalEvent.getTitle(), id); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and time range check.
The factory method should validate inputs and ensure the event time range is valid.
+ /**
+ * Creates a GoogleEventRequest from a PersonalEvent.
+ * @param personalEvent The source event (must not be null)
+ * @param id The event identifier (must not be null)
+ * @return A new GoogleEventRequest instance
+ * @throws NullPointerException if any parameter is null
+ * @throws IllegalArgumentException if end time is before start time
+ */
public static GoogleEventRequest of(PersonalEvent personalEvent, String id) {
+ Objects.requireNonNull(personalEvent, "personalEvent must not be null");
+ Objects.requireNonNull(id, "id must not be null");
+
+ LocalDateTime startTime = personalEvent.getStartTime();
+ LocalDateTime endTime = personalEvent.getEndTime();
+
+ if (endTime.isBefore(startTime)) {
+ throw new IllegalArgumentException("End time must not be before start time");
+ }
+
- GoogleEventDateTime start = GoogleEventDateTime.fromLocalDateTime(personalEvent.getStartTime());
- GoogleEventDateTime end = GoogleEventDateTime.fromLocalDateTime(personalEvent.getEndTime());
+ GoogleEventDateTime start = GoogleEventDateTime.fromLocalDateTime(startTime);
+ GoogleEventDateTime end = GoogleEventDateTime.fromLocalDateTime(endTime);
return new GoogleEventRequest(start, end, personalEvent.getTitle(), id);
}📝 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 static GoogleEventRequest of(PersonalEvent personalEvent, String id) { | |
| GoogleEventDateTime start = GoogleEventDateTime.fromLocalDateTime(personalEvent.getStartTime()); | |
| GoogleEventDateTime end = GoogleEventDateTime.fromLocalDateTime(personalEvent.getEndTime()); | |
| return new GoogleEventRequest(start, end, personalEvent.getTitle(), id); | |
| } | |
| /** | |
| * Creates a GoogleEventRequest from a PersonalEvent. | |
| * @param personalEvent The source event (must not be null) | |
| * @param id The event identifier (must not be null) | |
| * @return A new GoogleEventRequest instance | |
| * @throws NullPointerException if any parameter is null | |
| * @throws IllegalArgumentException if end time is before start time | |
| */ | |
| public static GoogleEventRequest of(PersonalEvent personalEvent, String id) { | |
| Objects.requireNonNull(personalEvent, "personalEvent must not be null"); | |
| Objects.requireNonNull(id, "id must not be null"); | |
| LocalDateTime startTime = personalEvent.getStartTime(); | |
| LocalDateTime endTime = personalEvent.getEndTime(); | |
| if (endTime.isBefore(startTime)) { | |
| throw new IllegalArgumentException("End time must not be before start time"); | |
| } | |
| GoogleEventDateTime start = GoogleEventDateTime.fromLocalDateTime(startTime); | |
| GoogleEventDateTime end = GoogleEventDateTime.fromLocalDateTime(endTime); | |
| return new GoogleEventRequest(start, end, personalEvent.getTitle(), id); | |
| } |
| @Async | ||
| @TransactionalEventListener(classes = {InsertPersonalEvent.class}) | ||
| public void insert(InsertPersonalEvent event) { | ||
| List<PersonalEvent> personalEventList = | ||
| event.personalEventList(); | ||
| googleCalendarService.insertPersonalEvents(personalEventList); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for async operations and fix typo.
- All async operations should include error handling to prevent silent failures.
- There's a typo in the method name: 'Calender' should be 'Calendar'
@Async
@TransactionalEventListener(classes = {InsertPersonalEvent.class})
public void insert(InsertPersonalEvent event) {
+ try {
List<PersonalEvent> personalEventList =
event.personalEventList();
googleCalendarService.insertPersonalEvents(personalEventList);
+ } catch (Exception e) {
+ log.error("Failed to insert events to Google Calendar: {}", e.getMessage(), e);
+ // Consider implementing retry mechanism
+ }
}
@Async
@TransactionalEventListener(classes = {LoginEvent.class})
public void login(LoginEvent event) {
+ try {
User user = event.user();
googleCalendarService.upsertGoogleCalendar(user);
+ } catch (Exception e) {
+ log.error("Failed to sync calendar during login: {}", e.getMessage(), e);
+ }
}
@Async
@TransactionalEventListener(classes = {UpdatePersonalEvent.class})
public void update(UpdatePersonalEvent event) {
+ try {
googleCalendarService.updatePersonalEventToGoogleCalendar(event.personalEvent());
+ } catch (Exception e) {
+ log.error("Failed to update event in Google Calendar: {}", e.getMessage(), e);
+ }
}
@Async
@TransactionalEventListener(classes = {DeletePersonalEvent.class})
public void delete(DeletePersonalEvent event) {
+ try {
- googleCalendarService.deletePersonalEventFromGoogleCalender(event.personalEvent());
+ googleCalendarService.deletePersonalEventFromGoogleCalendar(event.personalEvent());
+ } catch (Exception e) {
+ log.error("Failed to delete event from Google Calendar: {}", e.getMessage(), e);
+ }
}Also applies to: 29-34, 36-40, 42-46
There was a problem hiding this comment.
@efdao 네, 동의합니다. 다음과 같이 더 구체적인 에러 처리를 추가하면 좋을 것 같습니다:
@Component
@RequiredArgsConstructor
@Slf4j
public class GoogleEventHandler {
private final GoogleCalendarService googleCalendarService;
private final RetryTemplate retryTemplate; // Optional: for retrying failed operations
@Async
@TransactionalEventListener(classes = {InsertPersonalEvent.class})
public void insert(InsertPersonalEvent event) {
try {
List<PersonalEvent> personalEventList = event.personalEventList();
retryTemplate.execute(context -> {
googleCalendarService.insertPersonalEvents(personalEventList);
return null;
});
} catch (GoogleJsonResponseException e) {
log.error("Google API error while inserting events: status={}, message={}",
e.getStatusCode(), e.getMessage(), e);
// Consider notifying the user or admin
} catch (IOException e) {
log.error("Network error while inserting events to Google Calendar", e);
// Consider queuing for retry
} catch (Exception e) {
log.error("Unexpected error while inserting events to Google Calendar", e);
}
}
// Similar error handling for other methods...
}주요 변경사항:
@Slf4j추가로 로깅 활성화- 구체적인 예외 타입별 처리
- 선택적으로
RetryTemplate을 사용한 재시도 메커니즘 - 사용자나 관리자에게 알림을 보내는 옵션 추가
✏️ Learnings added
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.350Z
Learning: Google Calendar 연동 작업에서는 다음과 같은 예외 처리가 필요:
1. GoogleJsonResponseException - Google API 관련 오류
2. IOException - 네트워크 오류
3. 재시도 메커니즘 구현 고려
4. 상세한 로깅과 모니터링
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| User user = userService.upsertUser(userInfo, tokenResponse); | ||
|
|
||
| googleCalendarService.upsertGoogleCalendar(user); | ||
| // googleCalendarService.upsertGoogleCalendar(user); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove commented code.
Instead of keeping commented code, it should be removed as the functionality has been moved to the event-driven implementation.
-// googleCalendarService.upsertGoogleCalendar(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.
| // googleCalendarService.upsertGoogleCalendar(user); |
| public ResponseEntity<Void> deletePersonalEvent( | ||
| @PathVariable("personalEventId") Long personalEventId) { | ||
| personalEventService.deletePersonalEvent(personalEventId); | ||
| @PathVariable("personalEventId") Long personalEventId, | ||
| @RequestParam("syncWithGoogleCalendar") Boolean syncWithGoogleCalendar) { | ||
| personalEventService.deletePersonalEvent(personalEventId, syncWithGoogleCalendar); | ||
| return ResponseEntity.noContent().build(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update API documentation for the new parameter.
The syncWithGoogleCalendar parameter has been added to control Google Calendar synchronization, but the API documentation needs to be updated to reflect this change.
Update the @Operation and @ApiResponses annotations to document the new parameter:
@Operation(summary = "개인 일정 삭제", description = "개인 일정을 삭제합니다.")
+ @Parameter(name = "syncWithGoogleCalendar", description = "구글 캘린더와 동기화 여부", required = true)
@ApiResponses(value = {
@ApiResponse(responseCode = "204", description = "일정 삭제 성공"),
@ApiResponse(responseCode = "400", description = "잘못된 요청 데이터",
content = @Content(schema = @Schema(implementation = ErrorResponse.class))),
@ApiResponse(responseCode = "401", description = "인증 실패",
content = @Content(schema = @Schema(implementation = ErrorResponse.class))),
@ApiResponse(responseCode = "404", description = "해당 일정 없음",
content = @Content(schema = @Schema(implementation = ErrorResponse.class))),
@ApiResponse(responseCode = "500", description = "서버 오류",
content = @Content(schema = @Schema(implementation = ErrorResponse.class)))
})📝 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 ResponseEntity<Void> deletePersonalEvent( | |
| @PathVariable("personalEventId") Long personalEventId) { | |
| personalEventService.deletePersonalEvent(personalEventId); | |
| @PathVariable("personalEventId") Long personalEventId, | |
| @RequestParam("syncWithGoogleCalendar") Boolean syncWithGoogleCalendar) { | |
| personalEventService.deletePersonalEvent(personalEventId, syncWithGoogleCalendar); | |
| return ResponseEntity.noContent().build(); | |
| @Operation(summary = "개인 일정 삭제", description = "개인 일정을 삭제합니다.") | |
| @Parameter(name = "syncWithGoogleCalendar", description = "구글 캘린더와 동기화 여부", required = true) | |
| @ApiResponses(value = { | |
| @ApiResponse(responseCode = "204", description = "일정 삭제 성공"), | |
| @ApiResponse(responseCode = "400", description = "잘못된 요청 데이터", | |
| content = @Content(schema = @Schema(implementation = ErrorResponse.class))), | |
| @ApiResponse(responseCode = "401", description = "인증 실패", | |
| content = @Content(schema = @Schema(implementation = ErrorResponse.class))), | |
| @ApiResponse(responseCode = "404", description = "해당 일정 없음", | |
| content = @Content(schema = @Schema(implementation = ErrorResponse.class))), | |
| @ApiResponse(responseCode = "500", description = "서버 오류", | |
| content = @Content(schema = @Schema(implementation = ErrorResponse.class))) | |
| }) | |
| public ResponseEntity<Void> deletePersonalEvent( | |
| @PathVariable("personalEventId") Long personalEventId, | |
| @RequestParam("syncWithGoogleCalendar") Boolean syncWithGoogleCalendar) { | |
| personalEventService.deletePersonalEvent(personalEventId, syncWithGoogleCalendar); | |
| return ResponseEntity.noContent().build(); | |
| } |
| @Test | ||
| public void test() { | ||
| System.out.println(Base64.getEncoder().encodeToString(UUID.randomUUID().toString().getBytes()).toLowerCase()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unrelated test method.
The test method that prints a Base64 encoded UUID appears to be unrelated to the PR's objective of implementing Google Calendar synchronization.
Remove this test method as it doesn't contribute to testing the functionality:
- @Test
- public void test() {
- System.out.println(Base64.getEncoder().encodeToString(UUID.randomUUID().toString().getBytes()).toLowerCase());
- }📝 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.
| @Test | |
| public void test() { | |
| System.out.println(Base64.getEncoder().encodeToString(UUID.randomUUID().toString().getBytes()).toLowerCase()); | |
| } |
Content-Type 헤더 추가 resouce id를 규칙에 맞게 수정
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (13)
backend/src/main/java/endolphin/backend/global/util/IdGenerator.java (1)
6-8: Consider making the utility class final and adding a private constructor.Since this is a utility class with only static methods, it's recommended to:
- Make the class
finalto prevent inheritance- Add a private constructor to prevent instantiation
-public class IdGenerator { +public final class IdGenerator { private static final String PREFIX = "endolphin"; private static final Base32 base = new Base32(true); + + private IdGenerator() { + throw new AssertionError("Utility class should not be instantiated"); + }backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java (8)
75-79: Consider implementing batch operations for better performance.The current implementation processes events sequentially. For better performance with large batches, consider:
- Implementing parallel processing using CompletableFuture
- Using Google Calendar's batch API
Example implementation:
-public void insertPersonalEvents(List<PersonalEvent> personalEvents) { - for (PersonalEvent personalEvent : personalEvents) { - insertPersonalEventToGoogleCalendar(personalEvent); - } -} +public void insertPersonalEvents(List<PersonalEvent> personalEvents) { + List<CompletableFuture<Void>> futures = personalEvents.stream() + .map(event -> CompletableFuture.runAsync(() -> + insertPersonalEventToGoogleCalendar(event))) + .toList(); + CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])).join(); +}
94-96: Enhance error logging for better debugging.The current error logging only includes the response body. Consider adding more context:
-log.error("Invalid request: {}", response.bodyTo(String.class)); +log.error("Failed to insert event {}: status={}, body={}", + personalEvent.getGoogleEventId(), + response.getStatusCode(), + response.bodyTo(String.class));
142-142: Fix typo in function name.The function name has a typo: "Calender" should be "Calendar".
-public void deletePersonalEventFromGoogleCalender(PersonalEvent personalEvent) { +public void deletePersonalEventFromGoogleCalendar(PersonalEvent personalEvent) {
406-407: Remove commented code.The commented code should be removed as it's been replaced with event publishing.
-// personalEventService.syncWithGoogleEvents(events, user, calendarId); eventPublisher.publishEvent(new GoogleEventChanged(events, user, calendarId));
81-110: Refactor duplicate error handling code.The OAuth token refresh logic is duplicated across multiple methods. Consider extracting this into a reusable wrapper method.
Create a utility method to handle HTTP requests with OAuth token refresh:
private <T> T executeWithTokenRefresh(User user, Function<String, T> operation) { try { return operation.apply(user.getAccessToken()); } catch (OAuthException e) { String accessToken = googleOAuthService.reissueAccessToken(user.getRefreshToken()); userService.updateAccessToken(user, accessToken); return operation.apply(accessToken); } }Then use it in the methods:
public void insertPersonalEventToGoogleCalendar(PersonalEvent personalEvent) { User user = personalEvent.getUser(); executeWithTokenRefresh(user, token -> { String eventUrl = googleCalendarUrl.eventsUrl() .replace("{calendarId}", personalEvent.getCalendarId()); GoogleEventRequest body = GoogleEventRequest.of( personalEvent, personalEvent.getGoogleEventId()); return restClient.post() .uri(eventUrl) .header(HttpHeaders.AUTHORIZATION, "Bearer " + token) .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) .body(body) .exchange((request, response) -> { // ... existing error handling ... return Optional.empty(); }); }); }
142-142: Fix typo in method name.The method name has a spelling error: "Calender" should be "Calendar".
-public void deletePersonalEventFromGoogleCalender(PersonalEvent personalEvent) { +public void deletePersonalEventFromGoogleCalendar(PersonalEvent personalEvent) {
406-407: Remove commented code.The commented code is no longer needed as it's been replaced with event publishing.
-// personalEventService.syncWithGoogleEvents(events, user, calendarId); eventPublisher.publishEvent(new GoogleEventChanged(events, user, calendarId));
406-407: Remove commented code.The commented code should be removed as it's been replaced with event publishing. If needed, the history can be found in version control.
-// personalEventService.syncWithGoogleEvents(events, user, calendarId); eventPublisher.publishEvent(new GoogleEventChanged(events, user, calendarId));backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventService.java (4)
77-81: Add null check for syncWithGoogleCalendar parameter.The code should handle cases where
syncWithGoogleCalendaris null to prevent potentialNullPointerException.- if (request.syncWithGoogleCalendar()) { + if (Boolean.TRUE.equals(request.syncWithGoogleCalendar())) {
99-110: Add validation for participants list.The code should validate the participants list to handle null or empty cases gracefully.
+ if (participants == null || participants.isEmpty()) { + log.warn("No participants provided for personal event creation"); + return; + } List<PersonalEvent> events = participants.stream()
131-137: Improve sync logic readability and null safety.The sync logic could be more maintainable and null-safe.
- if (request.syncWithGoogleCalendar()) { - if (result.getGoogleEventId() == null) { - eventPublisher.publishEvent(new InsertPersonalEvent(List.of(result))); - } else { - eventPublisher.publishEvent(new UpdatePersonalEvent(result)); - } + if (Boolean.TRUE.equals(request.syncWithGoogleCalendar())) { + var event = result.getGoogleEventId() == null + ? new InsertPersonalEvent(List.of(result)) + : new UpdatePersonalEvent(result); + eventPublisher.publishEvent(event); }
173-177: Improve deletion sync logic.The sync logic could be more concise and null-safe.
- if (syncWithGoogleCalendar - && personalEvent.getGoogleEventId() != null - && personalEvent.getCalendarId() != null) { + if (Boolean.TRUE.equals(syncWithGoogleCalendar) + && personalEvent.hasGoogleCalendarInfo()) { eventPublisher.publishEvent(new DeletePersonalEvent(personalEvent)); }Consider adding a helper method to
PersonalEvent:public boolean hasGoogleCalendarInfo() { return getGoogleEventId() != null && getCalendarId() != null; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventService.java(9 hunks)backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java(10 hunks)backend/src/main/java/endolphin/backend/global/util/IdGenerator.java(1 hunks)
👮 Files not reviewed due to content moderation or server errors (2)
- backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventService.java
- backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java
🔇 Additional comments (6)
backend/src/main/java/endolphin/backend/global/util/IdGenerator.java (1)
1-5: LGTM! Clean and well-organized imports.The package structure and imports are minimal and specific, following best practices.
backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventService.java (5)
77-81: LGTM! Google Calendar synchronization is well implemented.The conditional synchronization with Google Calendar is properly handled, including setting the necessary IDs and publishing the event.
99-114: LGTM! Batch creation of Google Calendar events is well implemented.The function efficiently handles multiple participants by:
- Generating unique Google event IDs for each participant
- Publishing a single batch event for all personal events
131-137: LGTM! Smart handling of Google Calendar updates.The function intelligently decides between insert and update operations based on the existence of a Google event ID.
173-177: LGTM! Conditional Google Calendar deletion is well implemented.The function properly checks both Google event ID and calendar ID before attempting deletion, ensuring synchronization only when necessary.
131-137: LGTM!The implementation correctly handles both new and existing Google Calendar events, and the event publishing is properly placed after the save operation.
| public static String generateId(Long id) { | ||
| return String.format("%s%d%s", | ||
| PREFIX, id, | ||
| base.encodeToString(UUID.randomUUID().toString().getBytes()) | ||
| .replace("=","").toLowerCase()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and improve performance.
The ID generation logic can be enhanced with the following improvements:
- Add input validation
- Use UUID bytes directly
- Add documentation
- Use StringBuilder for string concatenation
+ /**
+ * Generates a unique identifier by combining a prefix, an ID, and a UUID.
+ * Format: "endolphin{id}{base32(UUID)}"
+ *
+ * @param id The ID to include in the generated identifier
+ * @return A unique string identifier
+ * @throws IllegalArgumentException if id is null or negative
+ */
public static String generateId(Long id) {
+ if (id == null || id < 0) {
+ throw new IllegalArgumentException("ID must be non-null and non-negative");
+ }
+
+ StringBuilder result = new StringBuilder(50) // Pre-allocate capacity
+ .append(PREFIX)
+ .append(id);
+
+ byte[] uuidBytes = new byte[16];
+ UUID uuid = UUID.randomUUID();
+ long msb = uuid.getMostSignificantBits();
+ long lsb = uuid.getLeastSignificantBits();
+ for (int i = 0; i < 8; i++) {
+ uuidBytes[i] = (byte) (msb >>> (8 * (7 - i)));
+ uuidBytes[8 + i] = (byte) (lsb >>> (8 * (7 - i)));
+ }
+
+ result.append(base.encodeToString(uuidBytes)
+ .replace("=", "")
+ .toLowerCase());
+
+ return result.toString();
- return String.format("%s%d%s",
- PREFIX, id,
- base.encodeToString(UUID.randomUUID().toString().getBytes())
- .replace("=","").toLowerCase());
}📝 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 static String generateId(Long id) { | |
| return String.format("%s%d%s", | |
| PREFIX, id, | |
| base.encodeToString(UUID.randomUUID().toString().getBytes()) | |
| .replace("=","").toLowerCase()); | |
| } | |
| /** | |
| * Generates a unique identifier by combining a prefix, an ID, and a UUID. | |
| * Format: "endolphin{id}{base32(UUID)}" | |
| * | |
| * @param id The ID to include in the generated identifier | |
| * @return A unique string identifier | |
| * @throws IllegalArgumentException if id is null or negative | |
| */ | |
| public static String generateId(Long id) { | |
| if (id == null || id < 0) { | |
| throw new IllegalArgumentException("ID must be non-null and non-negative"); | |
| } | |
| StringBuilder result = new StringBuilder(50) // Pre-allocate capacity | |
| .append(PREFIX) | |
| .append(id); | |
| byte[] uuidBytes = new byte[16]; | |
| UUID uuid = UUID.randomUUID(); | |
| long msb = uuid.getMostSignificantBits(); | |
| long lsb = uuid.getLeastSignificantBits(); | |
| for (int i = 0; i < 8; i++) { | |
| uuidBytes[i] = (byte) (msb >>> (8 * (7 - i))); | |
| uuidBytes[8 + i] = (byte) (lsb >>> (8 * (7 - i))); | |
| } | |
| result.append(base.encodeToString(uuidBytes) | |
| .replace("=", "") | |
| .toLowerCase()); | |
| return result.toString(); | |
| } |
| } catch (OAuthException e) { | ||
| String accessToken = googleOAuthService.reissueAccessToken(user.getRefreshToken()); | ||
| userService.updateAccessToken(user, accessToken); | ||
| updatePersonalEventToGoogleCalendar(personalEvent); |
There was a problem hiding this comment.
Fix incorrect error recovery logic.
The function calls updatePersonalEventToGoogleCalendar instead of deletePersonalEventFromGoogleCalender in the error recovery path.
-updatePersonalEventToGoogleCalendar(personalEvent);
+deletePersonalEventFromGoogleCalender(personalEvent);📝 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.
| updatePersonalEventToGoogleCalendar(personalEvent); | |
| deletePersonalEventFromGoogleCalender(personalEvent); |
Fix incorrect method call in catch block.
The catch block calls updatePersonalEventToGoogleCalendar instead of retrying the delete operation.
- updatePersonalEventToGoogleCalendar(personalEvent);
+ deletePersonalEventFromGoogleCalendar(personalEvent);📝 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.
| updatePersonalEventToGoogleCalendar(personalEvent); | |
| deletePersonalEventFromGoogleCalendar(personalEvent); |
| try { | ||
| restClient.put() | ||
| .uri(eventUrl) | ||
| .header(HttpHeaders.AUTHORIZATION, "Bearer " + token) | ||
| .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) | ||
| .body(body) | ||
| .exchange((request, response) -> { | ||
| if (!response.getStatusCode().is2xxSuccessful()) { | ||
| log.error("Invalid request: {}", response.bodyTo(String.class)); | ||
| } | ||
| if (response.getStatusCode().isSameCodeAs(HttpStatus.UNAUTHORIZED)) { | ||
| throw new OAuthException(ErrorCode.OAUTH_UNAUTHORIZED_ERROR); | ||
| } | ||
| return Optional.empty(); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Add proper error handling for non-OAuth errors.
The function only handles OAuth unauthorized errors but doesn't properly handle other potential errors.
try {
restClient.put()
.uri(eventUrl)
.header(HttpHeaders.AUTHORIZATION, "Bearer " + token)
.header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)
.body(body)
.exchange((request, response) -> {
if (!response.getStatusCode().is2xxSuccessful()) {
log.error("Invalid request: {}", response.bodyTo(String.class));
+ throw new CalendarException(response.getStatusCode(),
+ "Failed to update event: " + response.bodyTo(String.class));
}
if (response.getStatusCode().isSameCodeAs(HttpStatus.UNAUTHORIZED)) {
throw new OAuthException(ErrorCode.OAUTH_UNAUTHORIZED_ERROR);
}
return Optional.empty();
});Committable suggestion skipped: line range outside the PR's diff.
| .map(participant -> { | ||
| String googleEventId = IdGenerator.generateId(participant.getId()); | ||
| return PersonalEvent.builder() | ||
| .title(discussion.getTitle()) | ||
| .startTime(sharedEvent.startDateTime()) | ||
| .endTime(sharedEvent.endDateTime()) | ||
| .user(participant) | ||
| .isAdjustable(false) | ||
| .googleEventId(googleEventId) | ||
| .calendarId(PRIMARY) | ||
| .build(); | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make Google Calendar sync optional for participants.
The function always sets Google Calendar IDs for all participants, which may not be desired behavior. Some participants might not want their events synced with Google Calendar.
Consider adding a parameter to control sync behavior:
public void createPersonalEventsForParticipants(List<User> participants,
Discussion discussion,
- SharedEventDto sharedEvent) {
+ SharedEventDto sharedEvent,
+ boolean syncWithGoogleCalendar) {
List<PersonalEvent> events = participants.stream()
.map(participant -> {
- String googleEventId = IdGenerator.generateId(participant.getId());
+ String googleEventId = syncWithGoogleCalendar ?
+ IdGenerator.generateId(participant.getId()) : null;
return PersonalEvent.builder()
.title(discussion.getTitle())
.startTime(sharedEvent.startDateTime())
.endTime(sharedEvent.endDateTime())
.user(participant)
.isAdjustable(false)
.googleEventId(googleEventId)
- .calendarId(PRIMARY)
+ .calendarId(syncWithGoogleCalendar ? PRIMARY : null)
.build();
})
.toList();📝 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.
| .map(participant -> { | |
| String googleEventId = IdGenerator.generateId(participant.getId()); | |
| return PersonalEvent.builder() | |
| .title(discussion.getTitle()) | |
| .startTime(sharedEvent.startDateTime()) | |
| .endTime(sharedEvent.endDateTime()) | |
| .user(participant) | |
| .isAdjustable(false) | |
| .googleEventId(googleEventId) | |
| .calendarId(PRIMARY) | |
| .build(); | |
| }) | |
| public void createPersonalEventsForParticipants(List<User> participants, | |
| Discussion discussion, | |
| SharedEventDto sharedEvent, | |
| boolean syncWithGoogleCalendar) { | |
| List<PersonalEvent> events = participants.stream() | |
| .map(participant -> { | |
| String googleEventId = syncWithGoogleCalendar | |
| ? IdGenerator.generateId(participant.getId()) : null; | |
| return PersonalEvent.builder() | |
| .title(discussion.getTitle()) | |
| .startTime(sharedEvent.startDateTime()) | |
| .endTime(sharedEvent.endDateTime()) | |
| .user(participant) | |
| .isAdjustable(false) | |
| .googleEventId(googleEventId) | |
| .calendarId(syncWithGoogleCalendar ? PRIMARY : null) | |
| .build(); | |
| }) | |
| .toList(); | |
| } |
| if (syncWithGoogleCalendar | ||
| && personalEvent.getGoogleEventId() != null | ||
| && personalEvent.getCalendarId() != null) { | ||
| eventPublisher.publishEvent(new DeletePersonalEvent(personalEvent)); | ||
| } |
There was a problem hiding this comment.
Move event publishing after successful delete.
The event is published before the personal event is deleted from the database. If the delete operation fails, the event would have already been published, leading to inconsistency.
Apply this diff to fix the ordering:
- if (syncWithGoogleCalendar
- && personalEvent.getGoogleEventId() != null
- && personalEvent.getCalendarId() != null) {
- eventPublisher.publishEvent(new DeletePersonalEvent(personalEvent));
- }
-
personalEventRepository.delete(personalEvent);
+ if (syncWithGoogleCalendar
+ && personalEvent.getGoogleEventId() != null
+ && personalEvent.getCalendarId() != null) {
+ eventPublisher.publishEvent(new DeletePersonalEvent(personalEvent));
+ }📝 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.
| if (syncWithGoogleCalendar | |
| && personalEvent.getGoogleEventId() != null | |
| && personalEvent.getCalendarId() != null) { | |
| eventPublisher.publishEvent(new DeletePersonalEvent(personalEvent)); | |
| } | |
| personalEventRepository.delete(personalEvent); | |
| if (syncWithGoogleCalendar | |
| && personalEvent.getGoogleEventId() != null | |
| && personalEvent.getCalendarId() != null) { | |
| eventPublisher.publishEvent(new DeletePersonalEvent(personalEvent)); | |
| } |
| personalEventRepository.saveAll(events); | ||
|
|
||
| //TODO 구글캘린더에 반영 | ||
| eventPublisher.publishEvent(new InsertPersonalEvent(events)); |
There was a problem hiding this comment.
Move event publishing after successful save.
The event is published before confirming that all events were saved successfully.
Apply this diff to fix the ordering:
- personalEventRepository.saveAll(events);
- eventPublisher.publishEvent(new InsertPersonalEvent(events));
+ List<PersonalEvent> savedEvents = personalEventRepository.saveAll(events);
+ if (syncWithGoogleCalendar) {
+ eventPublisher.publishEvent(new InsertPersonalEvent(savedEvents));
+ }📝 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.
| personalEventRepository.saveAll(events); | |
| //TODO 구글캘린더에 반영 | |
| eventPublisher.publishEvent(new InsertPersonalEvent(events)); | |
| List<PersonalEvent> savedEvents = personalEventRepository.saveAll(events); | |
| if (syncWithGoogleCalendar) { | |
| eventPublisher.publishEvent(new InsertPersonalEvent(savedEvents)); | |
| } |
| if (request.syncWithGoogleCalendar()) { | ||
| personalEvent.setGoogleEventId(IdGenerator.generateId(user.getId())); | ||
| personalEvent.setCalendarId(PRIMARY); | ||
| eventPublisher.publishEvent(new InsertPersonalEvent(List.of(personalEvent))); | ||
| } |
There was a problem hiding this comment.
Move event publishing after successful save.
The event is published before the personal event is saved to the database. If the save operation fails, the event would have already been published, leading to inconsistency.
Apply this diff to fix the ordering:
- if (request.syncWithGoogleCalendar()) {
- personalEvent.setGoogleEventId(IdGenerator.generateId(user.getId()));
- personalEvent.setCalendarId(PRIMARY);
- eventPublisher.publishEvent(new InsertPersonalEvent(List.of(personalEvent)));
- }
PersonalEvent result = personalEventRepository.save(personalEvent);
+ if (request.syncWithGoogleCalendar()) {
+ result.setGoogleEventId(IdGenerator.generateId(user.getId()));
+ result.setCalendarId(PRIMARY);
+ eventPublisher.publishEvent(new InsertPersonalEvent(List.of(result)));
+ }📝 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.
| if (request.syncWithGoogleCalendar()) { | |
| personalEvent.setGoogleEventId(IdGenerator.generateId(user.getId())); | |
| personalEvent.setCalendarId(PRIMARY); | |
| eventPublisher.publishEvent(new InsertPersonalEvent(List.of(personalEvent))); | |
| } | |
| PersonalEvent result = personalEventRepository.save(personalEvent); | |
| if (request.syncWithGoogleCalendar()) { | |
| result.setGoogleEventId(IdGenerator.generateId(user.getId())); | |
| result.setCalendarId(PRIMARY); | |
| eventPublisher.publishEvent(new InsertPersonalEvent(List.of(result))); | |
| } |
| public void updatePersonalEventToGoogleCalendar(PersonalEvent personalEvent) { | ||
| String eventUrl = String.format(googleCalendarUrl.updateUrl(), | ||
| personalEvent.getCalendarId(), personalEvent.getGoogleEventId()); | ||
| User user = personalEvent.getUser(); | ||
| String token = user.getAccessToken(); | ||
|
|
||
| GoogleEventRequest body = GoogleEventRequest.of(personalEvent, personalEvent.getGoogleEventId()); | ||
| try { | ||
| restClient.put() | ||
| .uri(eventUrl) | ||
| .header(HttpHeaders.AUTHORIZATION, "Bearer " + token) | ||
| .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) | ||
| .body(body) | ||
| .exchange((request, response) -> { | ||
| if (!response.getStatusCode().is2xxSuccessful()) { | ||
| log.error("Invalid request: {}", response.bodyTo(String.class)); | ||
| } | ||
| if (response.getStatusCode().isSameCodeAs(HttpStatus.UNAUTHORIZED)) { | ||
| throw new OAuthException(ErrorCode.OAUTH_UNAUTHORIZED_ERROR); | ||
| } | ||
| return Optional.empty(); | ||
| }); | ||
|
|
||
| } catch (OAuthException e) { | ||
| String accessToken = googleOAuthService.reissueAccessToken(user.getRefreshToken()); | ||
| userService.updateAccessToken(user, accessToken); | ||
| updatePersonalEventToGoogleCalendar(personalEvent); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reduce code duplication with insert method.
The update method shares significant code with the insert method. Consider extracting common logic to a helper method.
+ private <T> T executeGoogleCalendarOperation(
+ PersonalEvent personalEvent,
+ String operationUrl,
+ Function<RestClient.RequestBodySpec, RestClient.ResponseSpec> operation
+ ) {
+ int retries = 0;
+ while (true) {
+ try {
+ User user = personalEvent.getUser();
+ return operation.apply(
+ restClient.post()
+ .uri(operationUrl)
+ .header(HttpHeaders.AUTHORIZATION, "Bearer " + user.getAccessToken())
+ .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)
+ ).body();
+ } catch (OAuthException e) {
+ // ... retry logic from previous comment
+ }
+ }
+ }
public void updatePersonalEventToGoogleCalendar(PersonalEvent personalEvent) {
String eventUrl = String.format(googleCalendarUrl.updateUrl(),
personalEvent.getCalendarId(), personalEvent.getGoogleEventId());
GoogleEventRequest body = GoogleEventRequest.of(
personalEvent, personalEvent.getGoogleEventId());
- try {
- // ... existing code ...
- } catch (OAuthException e) {
- // ... existing code ...
- }
+ executeGoogleCalendarOperation(
+ personalEvent,
+ eventUrl,
+ request -> request.body(body)
+ );
}Committable suggestion skipped: line range outside the PR's diff.
| public void insertPersonalEventToGoogleCalendar(PersonalEvent personalEvent) { | ||
| String eventUrl = googleCalendarUrl.eventsUrl().replace("{calendarId}", personalEvent.getCalendarId()); | ||
| User user = personalEvent.getUser(); | ||
|
|
||
| GoogleEventRequest body = GoogleEventRequest.of(personalEvent, personalEvent.getGoogleEventId()); | ||
|
|
||
| try { | ||
| restClient.post() | ||
| .uri(eventUrl) | ||
| .header(HttpHeaders.AUTHORIZATION, "Bearer " + user.getAccessToken()) | ||
| .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) | ||
| .body(body) | ||
| .exchange((request, response) -> { | ||
| if (!response.getStatusCode().is2xxSuccessful()) { | ||
| log.error("Invalid request: {}", response.bodyTo(String.class)); | ||
| } | ||
| if (response.getStatusCode().isSameCodeAs(HttpStatus.UNAUTHORIZED)) { | ||
| throw new OAuthException(ErrorCode.OAUTH_UNAUTHORIZED_ERROR); | ||
| } | ||
| return Optional.empty(); | ||
| }); | ||
|
|
||
| } catch (OAuthException e) { | ||
| String accessToken = googleOAuthService.reissueAccessToken(user.getRefreshToken()); | ||
| userService.updateAccessToken(user, accessToken); | ||
| insertPersonalEventToGoogleCalendar(personalEvent); | ||
| } catch (Exception e) { | ||
| throw new CalendarException(HttpStatus.BAD_REQUEST, e.getMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling and add retry mechanism.
The method could benefit from a more robust error handling approach and a retry mechanism for transient failures.
+ private static final int MAX_RETRIES = 3;
+ private static final long RETRY_DELAY_MS = 1000;
+
public void insertPersonalEventToGoogleCalendar(PersonalEvent personalEvent) {
+ int retries = 0;
+ while (true) {
+ try {
String eventUrl = googleCalendarUrl.eventsUrl()
.replace("{calendarId}", personalEvent.getCalendarId());
User user = personalEvent.getUser();
GoogleEventRequest body = GoogleEventRequest.of(
personalEvent, personalEvent.getGoogleEventId());
- try {
restClient.post()
// ... existing code ...
+ break; // Success, exit the retry loop
} catch (OAuthException e) {
String accessToken = googleOAuthService
.reissueAccessToken(user.getRefreshToken());
userService.updateAccessToken(user, accessToken);
- insertPersonalEventToGoogleCalendar(personalEvent);
+ if (++retries >= MAX_RETRIES) {
+ throw new CalendarException(
+ HttpStatus.UNAUTHORIZED, "Failed to refresh token after " + MAX_RETRIES + " attempts");
+ }
+ Thread.sleep(RETRY_DELAY_MS);
} catch (Exception e) {
+ if (++retries >= MAX_RETRIES) {
throw new CalendarException(
- HttpStatus.BAD_REQUEST, e.getMessage());
+ HttpStatus.BAD_REQUEST, "Failed after " + MAX_RETRIES + " attempts: " + e.getMessage());
+ }
+ Thread.sleep(RETRY_DELAY_MS);
}
+ }
}Committable suggestion skipped: line range outside the PR's diff.
efdao
left a comment
efdao
left a comment
There was a problem hiding this comment.
p2; calendarId 처리가 이루어지지 않은 것같습니다
| @Async | ||
| @TransactionalEventListener(classes = {InsertPersonalEvent.class}) | ||
| public void insert(InsertPersonalEvent event) { | ||
| List<PersonalEvent> personalEventList = | ||
| event.personalEventList(); | ||
| googleCalendarService.insertPersonalEvents(personalEventList); | ||
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java (2)
76-80: Consider using batch operations for multiple events.The current implementation makes individual API calls for each event. For better performance, consider using Google Calendar's batch operations API.
public void insertPersonalEvents(List<PersonalEvent> personalEvents) { - for (PersonalEvent personalEvent : personalEvents) { - insertPersonalEventToGoogleCalendar(personalEvent); - } + // TODO: Implement batch operations using Google Calendar API's batchInsert + // https://developers.google.com/calendar/api/guides/batch }
70-71: Consider adding event metadata.The
GoogleEventChangedevent could benefit from additional metadata to help handlers make better decisions.-eventPublisher.publishEvent(new GoogleEventChanged(events, user, calendarId)); +eventPublisher.publishEvent(new GoogleEventChanged( + events, + user, + calendarId, + LocalDateTime.now(), + "webhook_notification" // or "initial_sync" +));Also applies to: 416-416
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
backend/src/main/java/endolphin/backend/domain/auth/AuthService.java(0 hunks)backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventService.java(10 hunks)backend/src/main/java/endolphin/backend/domain/personal_event/entity/PersonalEvent.java(3 hunks)backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java(10 hunks)backend/src/main/java/endolphin/backend/global/google/dto/GoogleEventDateTime.java(1 hunks)
💤 Files with no reviewable changes (1)
- backend/src/main/java/endolphin/backend/domain/auth/AuthService.java
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/src/main/java/endolphin/backend/global/google/dto/GoogleEventDateTime.java
- backend/src/main/java/endolphin/backend/domain/personal_event/entity/PersonalEvent.java
- backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventService.java
🔇 Additional comments (1)
backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java (1)
349-349:Details
❓ Verification inconclusive
Verify webhook URL configuration.
The webhook URL and expiration time are critical for the notification system. Ensure they are properly configured and the URL is accessible.
Also applies to: 354-354
🏁 Script executed:
#!/bin/bash # Description: Verify webhook URL configuration and accessibility # Test 1: Check if webhook URL is properly configured rg -l "webhook.*url.*=" --type properties # Test 2: Check if webhook URL is HTTPS rg "webhook.*url.*=.*https://" --type properties # Test 3: Check for potential hardcoded URLs rg "http[s]?://[^/]+" --type javaLength of output: 200
Attention: Confirm Webhook URL Configuration for Google Calendar Service
Please ensure that the webhook URL (referenced by
googleCalendarUrl.webhookUrl()) is correctly set up with the following in mind:
- The URL must be configured in the appropriate properties or configuration files.
- It needs to use HTTPS and be accessible from the expected environment.
- Verify that any expiration time settings are properly defined and aligned with the notification system requirements.
- This check applies to both occurrences at lines 349 and 354 in
backend/src/main/java/endolphin/backend/global/google/GoogleCalendarService.java.Automated searches using regex did not yield conclusive results—likely due to file type constraints—so please perform a manual verification of the configuration files to ensure that the URL and related settings meet these criteria.
| public void updatePersonalEventToGoogleCalendar(PersonalEvent personalEvent) { | ||
| String eventUrl = String.format(googleCalendarUrl.updateUrl(), | ||
| personalEvent.getCalendarId(), personalEvent.getGoogleEventId()); | ||
| User user = personalEvent.getUser(); | ||
| String token = user.getAccessToken(); | ||
|
|
||
| GoogleEventRequest body = GoogleEventRequest.of(personalEvent, | ||
| personalEvent.getGoogleEventId()); | ||
| try { | ||
| restClient.put() | ||
| .uri(eventUrl) | ||
| .header(HttpHeaders.AUTHORIZATION, "Bearer " + token) | ||
| .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) | ||
| .body(body) | ||
| .exchange((request, response) -> { | ||
| if (response.getStatusCode().is2xxSuccessful()) { | ||
| return Optional.empty(); | ||
| } | ||
| log.error("Invalid request: {}", response.bodyTo(String.class)); | ||
| if (response.getStatusCode().isSameCodeAs(HttpStatus.UNAUTHORIZED)) { | ||
| throw new OAuthException(ErrorCode.OAUTH_UNAUTHORIZED_ERROR); | ||
| } | ||
| throw new CalendarException((HttpStatus) response.getStatusCode(), | ||
| response.bodyTo(String.class)); | ||
| }); | ||
|
|
||
| } catch (OAuthException e) { | ||
| String accessToken = googleOAuthService.reissueAccessToken(user.getRefreshToken()); | ||
| userService.updateAccessToken(user, accessToken); | ||
| updatePersonalEventToGoogleCalendar(personalEvent); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reduce code duplication in CRUD operations.
The update and delete methods share significant code with the insert method. Consider extracting common logic to reduce duplication.
+ private <T> T executeGoogleCalendarOperation(
+ PersonalEvent personalEvent,
+ String url,
+ String method,
+ GoogleEventRequest body
+ ) {
+ int retries = 0;
+ long delay = INITIAL_RETRY_DELAY_MS;
+
+ while (true) {
+ try {
+ User user = personalEvent.getUser();
+ RestClient.RequestBodySpec request = switch (method) {
+ case "POST" -> restClient.post();
+ case "PUT" -> restClient.put();
+ case "DELETE" -> restClient.delete();
+ default -> throw new IllegalArgumentException("Unsupported method: " + method);
+ };
+
+ return request
+ .uri(url)
+ .header(HttpHeaders.AUTHORIZATION, "Bearer " + user.getAccessToken())
+ .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)
+ .body(body)
+ .exchange((req, res) -> handleResponse(res));
+ } catch (OAuthException e) {
+ handleOAuthError(personalEvent.getUser(), retries++, delay);
+ delay *= 2;
+ }
+ }
+ }Also applies to: 150-177
| public void insertPersonalEventToGoogleCalendar(PersonalEvent personalEvent) { | ||
| String eventUrl = googleCalendarUrl.eventsUrl() | ||
| .replace("{calendarId}", personalEvent.getCalendarId()); | ||
| User user = personalEvent.getUser(); | ||
|
|
||
| GoogleEventRequest body = GoogleEventRequest.of(personalEvent, | ||
| personalEvent.getGoogleEventId()); | ||
|
|
||
| try { | ||
| restClient.post() | ||
| .uri(eventUrl) | ||
| .header(HttpHeaders.AUTHORIZATION, "Bearer " + user.getAccessToken()) | ||
| .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) | ||
| .body(body) | ||
| .exchange((request, response) -> { | ||
| if (response.getStatusCode().is2xxSuccessful()) { | ||
| return Optional.empty(); | ||
| } | ||
| log.error("Invalid request: {}", response.bodyTo(String.class)); | ||
| if (response.getStatusCode().isSameCodeAs(HttpStatus.UNAUTHORIZED)) { | ||
| throw new OAuthException(ErrorCode.OAUTH_UNAUTHORIZED_ERROR); | ||
| } | ||
| throw new CalendarException((HttpStatus) response.getStatusCode(), | ||
| response.bodyTo(String.class)); | ||
| }); | ||
|
|
||
| } catch (OAuthException e) { | ||
| String accessToken = googleOAuthService.reissueAccessToken(user.getRefreshToken()); | ||
| userService.updateAccessToken(user, accessToken); | ||
| insertPersonalEventToGoogleCalendar(personalEvent); | ||
| } catch (Exception e) { | ||
| throw new CalendarException(HttpStatus.BAD_REQUEST, e.getMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add retry mechanism with exponential backoff.
The current implementation has a simple retry on OAuth errors but could benefit from a more robust retry mechanism for transient failures.
+ private static final int MAX_RETRIES = 3;
+ private static final long INITIAL_RETRY_DELAY_MS = 1000;
+
public void insertPersonalEventToGoogleCalendar(PersonalEvent personalEvent) {
+ int retries = 0;
+ long delay = INITIAL_RETRY_DELAY_MS;
+
+ while (true) {
try {
// ... existing code ...
} catch (OAuthException e) {
String accessToken = googleOAuthService.reissueAccessToken(user.getRefreshToken());
userService.updateAccessToken(user, accessToken);
- insertPersonalEventToGoogleCalendar(personalEvent);
+ if (++retries >= MAX_RETRIES) {
+ throw new CalendarException(HttpStatus.UNAUTHORIZED,
+ "Failed to refresh token after " + MAX_RETRIES + " attempts");
+ }
+ Thread.sleep(delay);
+ delay *= 2; // Exponential backoff
} catch (Exception e) {
+ if (++retries >= MAX_RETRIES) {
throw new CalendarException(HttpStatus.BAD_REQUEST, e.getMessage());
+ }
+ Thread.sleep(delay);
+ delay *= 2; // Exponential backoff
}
+ }
}Committable suggestion skipped: line range outside the PR's diff.
#️⃣ 연관된 이슈>
📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)
구글 캘린더에 개인 일정 변경을 반영하는 기능 구현했습니다.
순환 참조가 생겨서 spring event를 사용해서 해결했습니다.
🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
Summary by CodeRabbit
New Features
Improvements