perf: Calendar Cache Improvements (#25502)
* wip * Filter only selectedCalendars where ff is enabled * test: fix and add tests for calendar subscription cache feature Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> * test: fix SelectedCalendarRepository tests for new teamIds parameter Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
co-authored by
Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
parent
3bf2a8fa6c
commit
2c8ff89fac
@@ -12,7 +12,7 @@ import type { CalendarSyncService } from "@calcom/features/calendar-subscription
|
||||
import type { FeaturesRepository } from "@calcom/features/flags/features.repository";
|
||||
import logger from "@calcom/lib/logger";
|
||||
import type { ISelectedCalendarRepository } from "@calcom/lib/server/repository/SelectedCalendarRepository.interface";
|
||||
import { SelectedCalendar } from "@calcom/prisma/client";
|
||||
import type { SelectedCalendar } from "@calcom/prisma/client";
|
||||
|
||||
const log = logger.getSubLogger({ prefix: ["CalendarSubscriptionService"] });
|
||||
|
||||
@@ -204,9 +204,14 @@ export class CalendarSubscriptionService {
|
||||
* Subscribe periodically to new calendars
|
||||
*/
|
||||
async checkForNewSubscriptions() {
|
||||
const teamIds = await this.deps.featuresRepository.getTeamsWithFeatureEnabled(
|
||||
CalendarSubscriptionService.CALENDAR_SUBSCRIPTION_CACHE_FEATURE
|
||||
);
|
||||
|
||||
const rows = await this.deps.selectedCalendarRepository.findNextSubscriptionBatch({
|
||||
take: 100,
|
||||
integrations: this.deps.adapterFactory.getProviders(),
|
||||
teamIds,
|
||||
});
|
||||
log.debug("checkForNewSubscriptions", { count: rows.length });
|
||||
await Promise.allSettled(rows.map(({ id }) => this.subscribe(id)));
|
||||
|
||||
+84
@@ -125,6 +125,8 @@ describe("CalendarSubscriptionService", () => {
|
||||
mockFeaturesRepository = {
|
||||
checkIfFeatureIsEnabledGlobally: vi.fn().mockResolvedValue(true),
|
||||
checkIfUserHasFeature: vi.fn().mockResolvedValue(true),
|
||||
checkIfTeamHasFeature: vi.fn().mockResolvedValue(true),
|
||||
getTeamsWithFeatureEnabled: vi.fn().mockResolvedValue([1, 2, 3]),
|
||||
};
|
||||
|
||||
mockCalendarCacheEventService = {
|
||||
@@ -376,12 +378,94 @@ describe("CalendarSubscriptionService", () => {
|
||||
|
||||
await service.checkForNewSubscriptions();
|
||||
|
||||
expect(mockFeaturesRepository.getTeamsWithFeatureEnabled).toHaveBeenCalledWith(
|
||||
"calendar-subscription-cache"
|
||||
);
|
||||
expect(mockSelectedCalendarRepository.findNextSubscriptionBatch).toHaveBeenCalledWith({
|
||||
take: 100,
|
||||
integrations: ["google_calendar", "office365_calendar"],
|
||||
teamIds: [1, 2, 3],
|
||||
});
|
||||
expect(subscribeSpy).toHaveBeenCalledWith(mockSelectedCalendar.id);
|
||||
});
|
||||
|
||||
test("should handle mixed cache scenario where some teams have cache enabled and some do not", async () => {
|
||||
const calendarWithCache = { ...mockSelectedCalendar, id: "calendar-with-cache", userId: 1 };
|
||||
const calendarWithCache2 = { ...mockSelectedCalendar, id: "calendar-with-cache-2", userId: 2 };
|
||||
|
||||
mockFeaturesRepository.getTeamsWithFeatureEnabled.mockResolvedValue([10, 20]);
|
||||
|
||||
mockSelectedCalendarRepository.findNextSubscriptionBatch.mockResolvedValue([
|
||||
calendarWithCache,
|
||||
calendarWithCache2,
|
||||
]);
|
||||
|
||||
const subscribeSpy = vi.spyOn(service, "subscribe").mockResolvedValue(undefined);
|
||||
|
||||
await service.checkForNewSubscriptions();
|
||||
|
||||
expect(mockFeaturesRepository.getTeamsWithFeatureEnabled).toHaveBeenCalledWith(
|
||||
"calendar-subscription-cache"
|
||||
);
|
||||
expect(mockSelectedCalendarRepository.findNextSubscriptionBatch).toHaveBeenCalledWith({
|
||||
take: 100,
|
||||
integrations: ["google_calendar", "office365_calendar"],
|
||||
teamIds: [10, 20],
|
||||
});
|
||||
expect(subscribeSpy).toHaveBeenCalledTimes(2);
|
||||
expect(subscribeSpy).toHaveBeenCalledWith("calendar-with-cache");
|
||||
expect(subscribeSpy).toHaveBeenCalledWith("calendar-with-cache-2");
|
||||
});
|
||||
|
||||
test("should only fetch calendars for teams with feature enabled, not entire organization hierarchy", async () => {
|
||||
const teamId = 100;
|
||||
const parentOrgId = 1;
|
||||
|
||||
mockFeaturesRepository.getTeamsWithFeatureEnabled.mockResolvedValue([teamId]);
|
||||
|
||||
const calendarForTeamMember = { ...mockSelectedCalendar, id: "team-member-calendar", userId: 5 };
|
||||
mockSelectedCalendarRepository.findNextSubscriptionBatch.mockResolvedValue([calendarForTeamMember]);
|
||||
|
||||
const subscribeSpy = vi.spyOn(service, "subscribe").mockResolvedValue(undefined);
|
||||
|
||||
await service.checkForNewSubscriptions();
|
||||
|
||||
expect(mockFeaturesRepository.getTeamsWithFeatureEnabled).toHaveBeenCalledWith(
|
||||
"calendar-subscription-cache"
|
||||
);
|
||||
expect(mockSelectedCalendarRepository.findNextSubscriptionBatch).toHaveBeenCalledWith({
|
||||
take: 100,
|
||||
integrations: ["google_calendar", "office365_calendar"],
|
||||
teamIds: [teamId],
|
||||
});
|
||||
expect(mockSelectedCalendarRepository.findNextSubscriptionBatch).not.toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
teamIds: expect.arrayContaining([parentOrgId]),
|
||||
})
|
||||
);
|
||||
expect(subscribeSpy).toHaveBeenCalledTimes(1);
|
||||
expect(subscribeSpy).toHaveBeenCalledWith("team-member-calendar");
|
||||
});
|
||||
|
||||
test("should not process any calendars when no teams have the feature enabled", async () => {
|
||||
mockFeaturesRepository.getTeamsWithFeatureEnabled.mockResolvedValue([]);
|
||||
|
||||
mockSelectedCalendarRepository.findNextSubscriptionBatch.mockResolvedValue([]);
|
||||
|
||||
const subscribeSpy = vi.spyOn(service, "subscribe").mockResolvedValue(undefined);
|
||||
|
||||
await service.checkForNewSubscriptions();
|
||||
|
||||
expect(mockFeaturesRepository.getTeamsWithFeatureEnabled).toHaveBeenCalledWith(
|
||||
"calendar-subscription-cache"
|
||||
);
|
||||
expect(mockSelectedCalendarRepository.findNextSubscriptionBatch).toHaveBeenCalledWith({
|
||||
take: 100,
|
||||
integrations: ["google_calendar", "office365_calendar"],
|
||||
teamIds: [],
|
||||
});
|
||||
expect(subscribeSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("feature flag methods", () => {
|
||||
|
||||
@@ -4,4 +4,5 @@ export interface IFeaturesRepository {
|
||||
checkIfFeatureIsEnabledGlobally(slug: keyof AppFlags): Promise<boolean>;
|
||||
checkIfUserHasFeature(userId: number, slug: string): Promise<boolean>;
|
||||
checkIfTeamHasFeature(teamId: number, slug: keyof AppFlags): Promise<boolean>;
|
||||
getTeamsWithFeatureEnabled(slug: keyof AppFlags): Promise<number[]>;
|
||||
}
|
||||
|
||||
@@ -351,4 +351,23 @@ export class FeaturesRepository implements IFeaturesRepository {
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
async getTeamsWithFeatureEnabled(slug: keyof AppFlags): Promise<number[]> {
|
||||
try {
|
||||
// If globally disabled, treat as effectively disabled everywhere
|
||||
const isGloballyEnabled = await this.checkIfFeatureIsEnabledGlobally(slug);
|
||||
if (!isGloballyEnabled) return [];
|
||||
|
||||
const rows = await this.prismaClient.teamFeatures.findMany({
|
||||
where: { featureId: slug },
|
||||
select: { teamId: true },
|
||||
orderBy: { teamId: "asc" },
|
||||
});
|
||||
|
||||
return rows.map((r) => r.teamId);
|
||||
} catch (err) {
|
||||
captureException(err);
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -24,9 +24,11 @@ export interface ISelectedCalendarRepository {
|
||||
*/
|
||||
findNextSubscriptionBatch({
|
||||
take,
|
||||
teamIds,
|
||||
integrations,
|
||||
}: {
|
||||
take: number;
|
||||
teamIds: number[];
|
||||
integrations?: string[];
|
||||
}): Promise<SelectedCalendar[]>;
|
||||
|
||||
|
||||
@@ -15,26 +15,24 @@ export class SelectedCalendarRepository implements ISelectedCalendarRepository {
|
||||
return this.prismaClient.selectedCalendar.findFirst({ where: { channelId } });
|
||||
}
|
||||
|
||||
async findNextSubscriptionBatch({ take, integrations }: { take: number; integrations: string[] }) {
|
||||
async findNextSubscriptionBatch({
|
||||
take,
|
||||
teamIds,
|
||||
integrations,
|
||||
}: {
|
||||
take: number;
|
||||
teamIds: number[];
|
||||
integrations: string[];
|
||||
}) {
|
||||
return this.prismaClient.selectedCalendar.findMany({
|
||||
where: {
|
||||
integration: { in: integrations },
|
||||
OR: [{ syncSubscribedAt: null }, { channelExpiration: { lte: new Date() } }],
|
||||
// initially we will run subscription only for teams that have
|
||||
// the feature flags enabled and it should be removed later
|
||||
user: {
|
||||
teams: {
|
||||
some: {
|
||||
team: {
|
||||
features: {
|
||||
some: {
|
||||
OR: [
|
||||
{ featureId: "calendar-subscription-cache" },
|
||||
{ featureId: "calendar-subscription-sync" },
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
teamId: { in: teamIds },
|
||||
accepted: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
@@ -116,6 +116,7 @@ describe("SelectedCalendarRepository", () => {
|
||||
|
||||
const result = await repository.findNextSubscriptionBatch({
|
||||
take: 10,
|
||||
teamIds: [1, 2, 3],
|
||||
integrations: ["google_calendar", "office365_calendar"],
|
||||
});
|
||||
|
||||
@@ -126,16 +127,8 @@ describe("SelectedCalendarRepository", () => {
|
||||
user: {
|
||||
teams: {
|
||||
some: {
|
||||
team: {
|
||||
features: {
|
||||
some: {
|
||||
OR: [
|
||||
{ featureId: "calendar-subscription-cache" },
|
||||
{ featureId: "calendar-subscription-sync" },
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
teamId: { in: [1, 2, 3] },
|
||||
accepted: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -152,6 +145,7 @@ describe("SelectedCalendarRepository", () => {
|
||||
|
||||
const result = await repository.findNextSubscriptionBatch({
|
||||
take: 5,
|
||||
teamIds: [10, 20],
|
||||
integrations: [],
|
||||
});
|
||||
|
||||
@@ -162,16 +156,8 @@ describe("SelectedCalendarRepository", () => {
|
||||
user: {
|
||||
teams: {
|
||||
some: {
|
||||
team: {
|
||||
features: {
|
||||
some: {
|
||||
OR: [
|
||||
{ featureId: "calendar-subscription-cache" },
|
||||
{ featureId: "calendar-subscription-sync" },
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
teamId: { in: [10, 20] },
|
||||
accepted: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -181,6 +167,35 @@ describe("SelectedCalendarRepository", () => {
|
||||
|
||||
expect(result).toEqual(mockCalendars);
|
||||
});
|
||||
|
||||
test("should handle empty teamIds array", async () => {
|
||||
const mockCalendars: SelectedCalendar[] = [];
|
||||
vi.mocked(mockPrismaClient.selectedCalendar.findMany).mockResolvedValue(mockCalendars);
|
||||
|
||||
const result = await repository.findNextSubscriptionBatch({
|
||||
take: 10,
|
||||
teamIds: [],
|
||||
integrations: ["google_calendar"],
|
||||
});
|
||||
|
||||
expect(mockPrismaClient.selectedCalendar.findMany).toHaveBeenCalledWith({
|
||||
where: {
|
||||
integration: { in: ["google_calendar"] },
|
||||
OR: [{ syncSubscribedAt: null }, { channelExpiration: { lte: expect.any(Date) } }],
|
||||
user: {
|
||||
teams: {
|
||||
some: {
|
||||
teamId: { in: [] },
|
||||
accepted: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
take: 10,
|
||||
});
|
||||
|
||||
expect(result).toEqual(mockCalendars);
|
||||
});
|
||||
});
|
||||
|
||||
describe("updateSyncStatus", () => {
|
||||
|
||||
Reference in New Issue
Block a user