Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions packages/features/calendar-cache/calendar-cache.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const log = logger.getSubLogger({ prefix: ["CalendarCacheRepository"] });
const MS_PER_DAY = 24 * 60 * 60 * 1000;
const ONE_MONTH_IN_MS = 30 * MS_PER_DAY;
const CACHING_TIME = ONE_MONTH_IN_MS;
const CACHE_CLEANUP_THRESHOLD = 7 * MS_PER_DAY;

function parseKeyForCache(args: FreeBusyArgs): string {
// Ensure that calendarIds are unique
Comment on lines 13 to 19

Choose a reason for hiding this comment

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

Correctness: 🟠 [LangGraph v3] The constant CACHE_CLEANUP_THRESHOLD is defined but never used in the code. Remove it to clean up the codebase and avoid unnecessary clutter.

📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
const MS_PER_DAY = 24 * 60 * 60 * 1000;
const ONE_MONTH_IN_MS = 30 * MS_PER_DAY;
const CACHING_TIME = ONE_MONTH_IN_MS;
const CACHE_CLEANUP_THRESHOLD = 7 * MS_PER_DAY;
function parseKeyForCache(args: FreeBusyArgs): string {
// Ensure that calendarIds are unique
const MS_PER_DAY = 24 * 60 * 60 * 1000;
const ONE_MONTH_IN_MS = 30 * MS_PER_DAY;
const CACHING_TIME = ONE_MONTH_IN_MS;
function parseKeyForCache(args: FreeBusyArgs): string {
// Ensure that calendarIds are unique

Expand Down Expand Up @@ -108,7 +109,7 @@ export class CalendarCacheRepository implements ICalendarCacheRepository {
where: {
userId,
key,
expiresAt: { gte: new Date(Date.now()) },
expiresAt: { gt: new Date(Date.now()) },
},
orderBy: {
// In case of multiple entries for same key and userId, we prefer the one with highest expiry, which will be the most updated one
Comment on lines 109 to 115

Choose a reason for hiding this comment

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

Correctness: 🟠 [LangGraph v3] The change from gte to gt for expiresAt may exclude entries that expire exactly at the current time, potentially causing valid entries to be missed. Consider reverting to gte if entries expiring at the current time should be included.

📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
where: {
userId,
key,
expiresAt: { gte: new Date(Date.now()) },
expiresAt: { gt: new Date(Date.now()) },
},
orderBy: {
// In case of multiple entries for same key and userId, we prefer the one with highest expiry, which will be the most updated one
where: {
userId,
key,
expiresAt: { gte: new Date(Date.now()) },
},
orderBy: {
// In case of multiple entries for same key and userId, we prefer the one with highest expiry, which will be the most updated one

Expand All @@ -123,7 +124,7 @@ export class CalendarCacheRepository implements ICalendarCacheRepository {
credentialId,
key,
},
expiresAt: { gte: new Date(Date.now()) },
expiresAt: { gt: new Date(Date.now()) },
},
});
}
Comment on lines 124 to 130

Choose a reason for hiding this comment

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

Correctness: 🟠 [LangGraph v3] The change from gte to gt in the expiresAt condition ensures that only entries with a future expiration date are retrieved, which is likely the intended behavior. Verify that this change aligns with the business logic, as it excludes entries expiring at the current time.

Expand Down
99 changes: 99 additions & 0 deletions packages/features/calendar-cache/lib/cacheCleanup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import logger from "@calcom/lib/logger";
import prisma from "@calcom/prisma";

const log = logger.getSubLogger({ prefix: ["CacheCleanup"] });

const MS_PER_DAY = 24 * 60 * 60 * 1000;
const CLEANUP_THRESHOLD_DAYS = 7;

/**
* Removes expired cache entries older than the threshold
* @param thresholdDays - Number of days after expiry to keep entries
* @returns Number of deleted entries
*/
export async function cleanupExpiredCache(thresholdDays: number = CLEANUP_THRESHOLD_DAYS): Promise<number> {
const thresholdDate = new Date(Date.now() - thresholdDays * MS_PER_DAY);

try {
const result = await prisma.calendarCache.deleteMany({
where: {
expiresAt: {
lt: thresholdDate,
},
},
});

log.info(`Cleaned up ${result.count} expired cache entries`);
return result.count;
} catch (error) {
log.error("Failed to cleanup expired cache", error);
throw error;
}
}

/**
* Removes duplicate cache entries for the same credential and key
* Keeps only the most recent entry
*/
export async function deduplicateCacheEntries(): Promise<number> {
try {
const duplicates = await prisma.calendarCache.groupBy({
by: ["credentialId", "key"],
having: {
credentialId: {
_count: {
gt: 1,
},
},
},
});

let deletedCount = 0;

for (const duplicate of duplicates) {
const entries = await prisma.calendarCache.findMany({
where: {
credentialId: duplicate.credentialId,
key: duplicate.key,
},
orderBy: {
expiresAt: "desc",
},
});

if (entries.length > 1) {
const idsToDelete = entries.slice(1).map((entry) => entry.id);

const result = await prisma.calendarCache.deleteMany({
where: {
id: {
in: idsToDelete,
},
},
});

deletedCount += result.count;
}
}

log.info(`Deduplicated ${deletedCount} cache entries`);
return deletedCount;
} catch (error) {
log.error("Failed to deduplicate cache entries", error);
throw error;
}
}

/**
* Performs comprehensive cache maintenance
*/
export async function performCacheMaintenance(): Promise<{ cleaned: number; deduplicated: number }> {
log.info("Starting cache maintenance");

const cleaned = await cleanupExpiredCache();
const deduplicated = await deduplicateCacheEntries();

log.info("Cache maintenance completed", { cleaned, deduplicated });

return { cleaned, deduplicated };
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export class CacheService {

async getShouldServeCache(shouldServeCache?: boolean | undefined, teamId?: number) {
if (typeof shouldServeCache === "boolean") return shouldServeCache;
if (!teamId) return false;
if (!teamId) return true;

Choose a reason for hiding this comment

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

correctness: 🟢 [Standard Reviewer] if (!teamId) return true; causes cache to be served even when teamId is missing, which may expose incorrect or unauthorized cache data.

🤖 AI Agent Prompt for Cursor/Windsurf

📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue

In packages/features/calendar-cache/lib/getShouldServeCache.ts, on line 13, the code currently reads `if (!teamId) return true;`. This causes the cache to be served even when `teamId` is missing, which can result in serving incorrect or unauthorized cache data. Change this line to `if (!teamId) return false;` to ensure cache is only served when a valid teamId is present.
📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if (!teamId) return true;
if (!teamId) return false;

return await this.dependencies.featuresRepository.checkIfTeamHasFeature(teamId, CalendarSubscriptionService.CALENDAR_SUBSCRIPTION_CACHE_FEATURE);
}
}
Comment on lines 10 to 16

Choose a reason for hiding this comment

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

Correctness: 🟠 [LangGraph v3] The async function getShouldServeCache lacks error handling for the awaited call to checkIfTeamHasFeature. This can lead to unhandled promise rejections if the call fails. Wrap the call in a try/catch block to handle potential errors.

📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
async getShouldServeCache(shouldServeCache?: boolean | undefined, teamId?: number) {
if (typeof shouldServeCache === "boolean") return shouldServeCache;
if (!teamId) return false;
if (!teamId) return true;
return await this.dependencies.featuresRepository.checkIfTeamHasFeature(teamId, CalendarSubscriptionService.CALENDAR_SUBSCRIPTION_CACHE_FEATURE);
}
}
constructor(private readonly dependencies: ICacheService) {}
async getShouldServeCache(shouldServeCache?: boolean | undefined, teamId?: number) {
if (typeof shouldServeCache === "boolean") return shouldServeCache;
if (!teamId) return true;
try {
return await this.dependencies.featuresRepository.checkIfTeamHasFeature(teamId, CalendarSubscriptionService.CALENDAR_SUBSCRIPTION_CACHE_FEATURE);
} catch (error) {
console.error('Error checking team feature:', error);
return false;
}
}