Skip to content

Commit 253c33d

Browse files
volneidevin-ai-integration[bot]keithwillcode
authored
perf: (googlecalendar): batch freebusy calls by delegation credential (calcom#24332)
* perf(googlecalendar): batch freebusy calls by delegation credential - Group selectedCalendars by delegationCredentialId before making API calls - Make one batched freebusy query per delegation credential group - Reduces total API calls while respecting credential boundaries - Maintains existing caching behavior per group - Updated both getAvailability and getAvailabilityWithTimeZones methods - Added groupCalendarsByDelegationCredential helper method - Handles edge case when no calendars provided but fallbackToPrimary is true - Fixed linting issue: replaced hasOwnProperty with 'in' operator Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> * test(googlecalendar): add test for delegation credential batching - Verify calendars are grouped by delegationCredentialId - Ensure exactly 3 API calls made for 3 delegation credential groups - Confirm all busy times from different groups are properly returned - Fix type-safety issues by replacing 'as any' with proper type constraints - Fix ESLint warnings: unused variables and any types in mock functions Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> * fix(googlecalendar): ensure fallback logic works with empty calendar groups - Handle empty calendar groups by ensuring at least one iteration - Add test for chunking groups larger than 50 calendars - Verify all delegation credential batching logic works correctly Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> * fix: fix remaining type errors from merge conflict resolution - Changed getCacheOrFetchAvailability to getFreeBusyData in getAvailabilityWithTimeZones - Removed orphaned merge conflict marker in test file Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> * fix: reset test file to original PR version and update method name - Reset CalendarService.test.ts to original PR version (0e9eb9e) - Updated getCacheOrFetchAvailability to getFreeBusyData to match main branch Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> * fix: reset test file to main branch version The original PR's test file had tests for caching features that have been removed from main. Reset to main's version to fix type errors. Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> * test(googlecalendar): add tests for delegation credential batching logic Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> * Revamp devin's implementation * Remove comsoles.log * fix(tests): update delegation credential batching tests to match reimplementation - Remove tests for private methods that no longer exist (groupCalendarsByDelegationCredential, chunkArray) - Update getAvailability test to verify calendar fetching without expecting multiple API calls per delegation credential - Keep existing tests for fallback to primary calendar and non-google calendar handling Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> * Remove dead code * improve documentation * docs: add README explaining Google Calendar availability batching feature Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> * docs: translate README to English Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> * docs: move README to calendar-batch package with comprehensive documentation - Remove README from googlecalendar lib (wrong location) - Add comprehensive README to packages/features/calendar-batch/ - Document CalendarBatchService and CalendarBatchWrapper - Explain how getCalendar() integrates with batching - Include architecture, data model, and performance considerations Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> * fix: address cubic-dev-ai review comments - Remove misleading comment from getCalendar.ts cache block - Fix typo 'optmization' -> 'optimization' in comment - Add comprehensive tests for CalendarBatchWrapper batching behavior - Test separate calls for calendars without delegationCredentialId - Test batching calendars with same delegationCredentialId - Test chunking into groups of 50 for API limits - Test mixed calendars handling - Test fallbackToPrimary with empty array - Test result flattening from batched calls - Test pass-through methods delegation Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> * fix: add shouldServeCache param to CalendarBatchWrapper and verify batching call count - Fix CalendarBatchWrapper.getAvailability signature to match Calendar interface (add shouldServeCache param) - Update CalendarBatchWrapper tests to pass shouldServeCache parameter - Add integration test in CalendarService.test.ts that verifies CalendarBatchWrapper makes separate API calls for different delegationCredentialIds (call count assertion) - This fixes the getCalendarsEvents test failures caused by signature mismatch Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> * Improve CalendarBatchImplementation * test: add shouldServeCache forwarding and order-independent batching verification tests Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> * test: make CalendarBatchWrapper tests order-independent Refactored tests to avoid relying on Promise.all execution order: - 'should make separate calls for calendars without delegationCredentialId' now uses set comparison instead of toHaveBeenNthCalledWith - 'should batch calendars with the same delegationCredentialId together' now finds calls by delegation credential instead of call order This addresses Sean's review comment about potential flakiness due to Promise.all not guaranteeing execution order of parallel promises. Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> * Remove hardcoded ID * test: add comprehensive tests for resolveCalendarServeStrategy Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * feat: use Promise.allSettled for partial failure handling in CalendarBatchWrapper - Changed Promise.all to Promise.allSettled in getAvailability and getAvailabilityWithTimeZones - Returns partial results when some batches fail instead of failing entirely - Logs warnings for failed batches with error details - Added tests for partial failure scenarios Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * test: add comprehensive tests for getAvailabilityWithTimeZones - Added batching behavior tests (separate calls, batching by delegationCredentialId, chunking) - Added partial failure handling tests (partial results, all fail, no throw) - Added edge case test for underlying calendar not implementing the method - Total: 24 tests now covering both getAvailability and getAvailabilityWithTimeZones Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * fix: add required serviceAccountKey fields to delegatedTo mock objects Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> * fix: add missing client_id and private_key to serviceAccountKey mock Co-Authored-By: keith@cal.com <keithwillcode@gmail.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: keith@cal.com <keithwillcode@gmail.com>
1 parent ed0c939 commit 253c33d

8 files changed

Lines changed: 1699 additions & 87 deletions

File tree

packages/app-store/_utils/__tests__/getCalendar.test.ts

Lines changed: 408 additions & 0 deletions
Large diffs are not rendered by default.

packages/app-store/_utils/getCalendar.ts

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { CalendarBatchService } from "@calcom/features/calendar-batch/lib/CalendarBatchService";
2+
import { CalendarBatchWrapper } from "@calcom/features/calendar-batch/lib/CalendarBatchWrapper";
13
import { CalendarSubscriptionService } from "@calcom/features/calendar-subscription/lib/CalendarSubscriptionService";
24
import { CalendarCacheEventRepository } from "@calcom/features/calendar-subscription/lib/cache/CalendarCacheEventRepository";
35
import { CalendarCacheEventService } from "@calcom/features/calendar-subscription/lib/cache/CalendarCacheEventService";
@@ -42,6 +44,20 @@ export const getCalendar = async (
4244
log.warn(`calendar of type ${calendarType} is not implemented`);
4345
return null;
4446
}
47+
48+
// eslint-disable-next-line
49+
const originalCalendar = new CalendarService(credential as any);
50+
return resolveCalendarServeStrategy(originalCalendar, credential, shouldServeCache);
51+
};
52+
53+
/**
54+
* Resolve best calendar strategy for current calendar and credential
55+
*/
56+
const resolveCalendarServeStrategy = async (
57+
originalCalendar: Calendar,
58+
credential: CredentialForCalendarService,
59+
shouldServeCache?: boolean
60+
): Promise<Calendar> => {
4561
// if shouldServeCache is not supplied, determine on the fly.
4662
if (typeof shouldServeCache === "undefined") {
4763
const featuresRepository = new FeaturesRepository(prisma);
@@ -58,20 +74,26 @@ export const getCalendar = async (
5874
);
5975
shouldServeCache = isCalendarSubscriptionCacheEnabled && isCalendarSubscriptionCacheEnabledForUser;
6076
}
61-
if (CalendarCacheEventService.isCalendarTypeSupported(calendarType) && shouldServeCache) {
62-
log.info(`Calendar Cache is enabled, using CalendarCacheService for credential ${credential.id}`);
63-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
64-
const originalCalendar = new CalendarService(credential as any);
65-
if (originalCalendar) {
66-
// return cacheable calendar
67-
const calendarCacheEventRepository = new CalendarCacheEventRepository(prisma);
68-
return new CalendarCacheWrapper({
69-
originalCalendar,
70-
calendarCacheEventRepository,
71-
});
72-
}
77+
if (CalendarCacheEventService.isCalendarTypeSupported(credential.type) && shouldServeCache) {
78+
log.info("Calendar Cache is enabled, using CalendarCacheService for credential", {
79+
credentialId: credential.id,
80+
});
81+
const calendarCacheEventRepository = new CalendarCacheEventRepository(prisma);
82+
return new CalendarCacheWrapper({
83+
originalCalendar: originalCalendar as unknown as Calendar,
84+
calendarCacheEventRepository,
85+
});
86+
} else if (CalendarBatchService.isSupported(credential)) {
87+
// If calendar cache isn't supported, we try calendar batch as the second layer of optimization
88+
log.info("Calendar Batch is supported, using CalendarBatchService for credential", {
89+
credentialId: credential.id,
90+
});
91+
return new CalendarBatchWrapper({ originalCalendar: originalCalendar as unknown as Calendar });
7392
}
7493

75-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
76-
return new CalendarService(credential as any);
94+
// Ended up returning unoptimized original calendar
95+
log.info("Calendar Cache and Batch aren't supported, serving regular calendar for credential", {
96+
credentialId: credential.id,
97+
});
98+
return originalCalendar;
7799
};

packages/app-store/googlecalendar/lib/CalendarService.ts

Lines changed: 14 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,9 @@
22
import type { calendar_v3 } from "@googleapis/calendar";
33
import type { GaxiosResponse } from "googleapis-common";
44
import { RRule } from "rrule";
5-
import { v4 as uuid } from "uuid";
65

76
import { MeetLocationType } from "@calcom/app-store/constants";
87
import { getLocation, getRichDescription } from "@calcom/lib/CalEventParser";
9-
import { uniqueBy } from "@calcom/lib/array";
108
import { ORGANIZER_EMAIL_EXEMPT_DOMAINS } from "@calcom/lib/constants";
119
import logger from "@calcom/lib/logger";
1210
import { safeStringify } from "@calcom/lib/safeStringify";
@@ -34,12 +32,6 @@ interface GoogleCalError extends Error {
3432
code?: number;
3533
}
3634

37-
const MS_PER_DAY = 24 * 60 * 60 * 1000;
38-
const ONE_MONTH_IN_MS = 30 * MS_PER_DAY;
39-
40-
const GOOGLE_WEBHOOK_URL_BASE = process.env.GOOGLE_WEBHOOK_URL || process.env.NEXT_PUBLIC_WEBAPP_URL;
41-
const GOOGLE_WEBHOOK_URL = `${GOOGLE_WEBHOOK_URL_BASE}/api/integrations/googlecalendar/webhook`;
42-
4335
const isGaxiosResponse = (error: unknown): error is GaxiosResponse<calendar_v3.Schema$Event> =>
4436
typeof error === "object" && !!error && Object.prototype.hasOwnProperty.call(error, "config");
4537

@@ -120,50 +112,6 @@ export default class GoogleCalendarService implements Calendar {
120112
return attendees;
121113
};
122114

123-
private async stopWatchingCalendarsInGoogle(
124-
channels: { googleChannelResourceId: string | null; googleChannelId: string | null }[]
125-
) {
126-
const calendar = await this.authedCalendar();
127-
logger.debug(`Unsubscribing from calendars ${channels.map((c) => c.googleChannelId).join(", ")}`);
128-
const uniqueChannels = uniqueBy(channels, ["googleChannelId", "googleChannelResourceId"]);
129-
await Promise.allSettled(
130-
uniqueChannels.map(({ googleChannelResourceId, googleChannelId }) =>
131-
calendar.channels
132-
.stop({
133-
requestBody: {
134-
resourceId: googleChannelResourceId,
135-
id: googleChannelId,
136-
},
137-
})
138-
.catch((err) => {
139-
console.warn(JSON.stringify(err));
140-
})
141-
)
142-
);
143-
}
144-
145-
private async startWatchingCalendarsInGoogle({ calendarId }: { calendarId: string }) {
146-
const calendar = await this.authedCalendar();
147-
logger.debug(`Subscribing to calendar ${calendarId}`, safeStringify({ GOOGLE_WEBHOOK_URL }));
148-
149-
const res = await calendar.events.watch({
150-
// Calendar identifier. To retrieve calendar IDs call the calendarList.list method. If you want to access the primary calendar of the currently logged in user, use the "primary" keyword.
151-
calendarId,
152-
requestBody: {
153-
// A UUID or similar unique string that identifies this channel.
154-
id: uuid(),
155-
type: "web_hook",
156-
address: GOOGLE_WEBHOOK_URL,
157-
token: process.env.GOOGLE_WEBHOOK_TOKEN,
158-
params: {
159-
// The time-to-live in seconds for the notification channel. Default is 604800 seconds.
160-
ttl: `${Math.round(ONE_MONTH_IN_MS / 1000)}`,
161-
},
162-
},
163-
});
164-
return res.data;
165-
}
166-
167115
async createEvent(
168116
calEvent: CalendarServiceEvent,
169117
credentialId: number,
@@ -452,9 +400,7 @@ export default class GoogleCalendarService implements Calendar {
452400
return apiResponse.json;
453401
}
454402

455-
async getFreeBusyResult(
456-
args: FreeBusyArgs,
457-
): Promise<calendar_v3.Schema$FreeBusyResponse> {
403+
async getFreeBusyResult(args: FreeBusyArgs): Promise<calendar_v3.Schema$FreeBusyResponse> {
458404
return await this.fetchAvailability(args);
459405
}
460406

@@ -471,9 +417,7 @@ export default class GoogleCalendarService implements Calendar {
471417
return validCals[0];
472418
}
473419

474-
async getFreeBusyData(
475-
args: FreeBusyArgs,
476-
): Promise<(EventBusyDate & { id: string })[] | null> {
420+
async getFreeBusyData(args: FreeBusyArgs): Promise<(EventBusyDate & { id: string })[] | null> {
477421
const freeBusyResult = await this.getFreeBusyResult(args);
478422
if (!freeBusyResult.calendars) return null;
479423

@@ -604,11 +548,12 @@ export default class GoogleCalendarService implements Calendar {
604548

605549
/**
606550
* Fetches availability data using the cache-or-fetch pattern
551+
*
607552
*/
608553
private async fetchAvailabilityData(
609554
calendarIds: string[],
610555
dateFrom: string,
611-
dateTo: string,
556+
dateTo: string
612557
): Promise<EventBusyDate[]> {
613558
// More efficient date difference calculation using native Date objects
614559
// Use Math.floor to match dayjs diff behavior (truncates, doesn't round up)
@@ -619,13 +564,11 @@ export default class GoogleCalendarService implements Calendar {
619564

620565
// Google API only allows a date range of 90 days for /freebusy
621566
if (diff <= 90) {
622-
const freeBusyData = await this.getFreeBusyData(
623-
{
624-
timeMin: dateFrom,
625-
timeMax: dateTo,
626-
items: calendarIds.map((id) => ({ id })),
627-
}
628-
);
567+
const freeBusyData = await this.getFreeBusyData({
568+
timeMin: dateFrom,
569+
timeMax: dateTo,
570+
items: calendarIds.map((id) => ({ id })),
571+
});
629572

630573
if (!freeBusyData) throw new Error("No response from google calendar");
631574
return freeBusyData.map((freeBusy) => ({ start: freeBusy.start, end: freeBusy.end }));
@@ -647,13 +590,11 @@ export default class GoogleCalendarService implements Calendar {
647590
currentEndTime = originalEndTime;
648591
}
649592

650-
const chunkData = await this.getFreeBusyData(
651-
{
652-
timeMin: new Date(currentStartTime).toISOString(),
653-
timeMax: new Date(currentEndTime).toISOString(),
654-
items: calendarIds.map((id) => ({ id })),
655-
}
656-
);
593+
const chunkData = await this.getFreeBusyData({
594+
timeMin: new Date(currentStartTime).toISOString(),
595+
timeMax: new Date(currentEndTime).toISOString(),
596+
items: calendarIds.map((id) => ({ id })),
597+
});
657598

658599
if (chunkData) {
659600
busyData.push(...chunkData.map((freeBusy) => ({ start: freeBusy.start, end: freeBusy.end })));

0 commit comments

Comments
 (0)