Skip to content

chore: cache projects response #10927

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gastonfournier wants to merge 2 commits into
base: main
Choose a base branch
from
+111 −12
Open

chore: cache projects response #10927

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
886ad57
chore: cache projects response
gastonfournier Nov 5, 2025
a0c5b41
Include cache invalidation and with it increase the TTL
gastonfournier Nov 6, 2025
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
2 changes: 1 addition & 1 deletion src/lib/features/project/project-read-model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export class ProjectReadModel implements IProjectReadModel {
this.db = db;
this.timer = (action) =>
metricsHelper.wrapTimer(eventBus, DB_TIME, {
store: 'project',
store: 'project-read-model',
Copy link
Contributor Author

@gastonfournier gastonfournier Nov 5, 2025

Choose a reason for hiding this comment

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

Just to differentiate this from the project store that also has a similar query

action,
});
this.flagResolver = flagResolver;
Expand Down
59 changes: 50 additions & 9 deletions src/lib/features/project/project-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { subDays } from 'date-fns';
import { subDays, secondsToMilliseconds } from 'date-fns';
import joi from 'joi';
const { ValidationError } = joi;
import createSlug from 'slug';
Expand Down Expand Up @@ -86,6 +86,7 @@ import { canGrantProjectRole } from './can-grant-project-role.js';
import { batchExecute } from '../../util/index.js';
import metricsHelper from '../../util/metrics-helper.js';
import { FUNCTION_TIME } from '../../metric-events.js';
import memoizee from 'memoizee';
import type { ResourceLimitsService } from '../resource-limits/resource-limits-service.js';

type Days = number;
Expand Down Expand Up @@ -156,6 +157,17 @@ export default class ProjectService {

private timer: Function;

private getProjectsForAdminUiCached: ((
query?: IProjectQuery & IProjectsQuery,
userId?: number,
) => Promise<ProjectForUi[]>) &
memoizee.Memoized<
(
query?: IProjectQuery & IProjectsQuery,
userId?: number,
) => Promise<ProjectForUi[]>
>;

constructor(
{
projectStore,
Expand Down Expand Up @@ -221,16 +233,35 @@ export default class ProjectService {
className: 'ProjectService',
functionName,
});
const cacheTtl = secondsToMilliseconds(20);
this.getProjectsForAdminUiCached = memoizee(
(
projectsQuery?: IProjectQuery & IProjectsQuery,
projectsUserId?: number,
) =>
this.projectReadModel.getProjectsForAdminUi(
projectsQuery,
projectsUserId,
),
{
promise: true,
maxAge: cacheTtl,
normalizer: ([projectsQuery, projectsUserId]) =>
this.buildProjectsCacheKey(
projectsQuery as
| (IProjectQuery & IProjectsQuery)
| undefined,
projectsUserId as number | undefined,
),
},
);
}

async getProjects(
query?: IProjectQuery & IProjectsQuery,
userId?: number,
): Promise<ProjectForUi[]> {
const projects = await this.projectReadModel.getProjectsForAdminUi(
query,
userId,
);
const projects = await this.getProjectsForAdminUiCached(query, userId);

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Nov 5, 2025

Choose a reason for hiding this comment

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

P1 Badge Invalidate cached project list after mutations

The memoized getProjectsForAdminUiCached now caches the project list per user for 20 s but the cache is never cleared when the underlying data changes. After a client loads the list once, creating or archiving a project or toggling a favorite within the same process will still return the cached result at the next getProjects() call, so the new project or updated favorite flag will not appear until the TTL expires. This is user-visible for the admin UI and automation that expects immediate consistency (the commit message already hints at Terraform usage). Consider clearing the memoization in the project mutation flows or otherwise invalidating the cache when projects change.

Useful? React with 👍 / 👎.

Copy link
Contributor Author

@gastonfournier gastonfournier Nov 6, 2025

Choose a reason for hiding this comment

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

👍 should be fixed in a0c5b41

if (userId) {
const projectAccess =
Expand All @@ -249,6 +280,20 @@ export default class ProjectService {
return projects;
}

private buildProjectsCacheKey(
Copy link
Member

@sighphyre sighphyre Nov 6, 2025

Choose a reason for hiding this comment

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

Pretty sure this will work but it feels a bit dirty to be using big ass JSON blobs as hash keys, we can't just do this?

private buildProjectsCacheKey(
    query?: IProjectQuery & IProjectsQuery,
    userId?: number,
): string {
    const hash = createHash('sha256');
    if (query?.ids) {
        const sorted = [...query.ids].sort();
        hash.update(sorted.join(','));
    }
    hash.update(String(userId ?? ''));
    hash.update(String(query?.id ?? ''));
    hash.update(String(query?.archived ?? ''));
    return hash.digest('base64url');
}

Copy link
Contributor Author

@gastonfournier gastonfournier Nov 6, 2025

Choose a reason for hiding this comment

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

Yeah, we could just concatenate strings... hsould be better, will do!

Copy link
Contributor Author

@gastonfournier gastonfournier Nov 6, 2025

Choose a reason for hiding this comment

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

But I like your approach

query?: IProjectQuery & IProjectsQuery,
userId?: number,
): string {
const ids = query?.ids === undefined ? null : [...query.ids].sort();
return JSON.stringify({
userId: userId ?? null,
id: query?.id ?? null,
archived:
typeof query?.archived === 'undefined' ? null : query.archived,
ids,
});
}

async addOwnersToProjects(
projects: ProjectForUi[],
): Promise<ProjectForUi[]> {
Expand Down Expand Up @@ -1096,10 +1141,6 @@ export default class ProjectService {
);
}

async getMembers(projectId: string): Promise<number> {
return this.projectStore.getMembersCountByProject(projectId);
}

async getProjectUsers(
projectId: string,
): Promise<Array<Pick<IUser, 'id' | 'email' | 'username'>>> {
Expand Down
Loading