fix: reduce project overview query count to 2. (#1356)

* fix: reduce project overview query count to 2.

Previously we've been doing N+1 queries for projects, this now changes to doing one query for projects with feature counts, and then one query for membercounts for all projects and merging that with the first query.

package.json

@@ -104,7 +104,7 @@
     "nodemailer": "^6.5.0",
     "owasp-password-strength-test": "^1.3.0",
     "parse-database-url": "^0.3.0",
-    "pg": "^8.7.1",
+    "pg": "^8.7.3",
     "pg-connection-string": "^2.5.0",
     "pkginfo": "^0.4.1",
     "prom-client": "^14.0.0",

src/lib/db/index.ts

@@ -51,7 +51,7 @@ export const createStores = (
         contextFieldStore: new ContextFieldStore(db, getLogger),
         settingStore: new SettingStore(db, getLogger),
         userStore: new UserStore(db, getLogger),
-        projectStore: new ProjectStore(db, getLogger),
+        projectStore: new ProjectStore(db, eventBus, getLogger),
         tagStore: new TagStore(db, eventBus, getLogger),
         tagTypeStore: new TagTypeStore(db, eventBus, getLogger),
         addonStore: new AddonStore(db, eventBus, getLogger),

src/lib/db/project-store.ts

@@ -2,14 +2,17 @@ import { Knex } from 'knex';
 import { Logger, LogProvider } from '../logger';
 
 import NotFoundError from '../error/notfound-error';
-import { IProject } from '../types/model';
+import { IProject, IProjectWithCount } from '../types/model';
 import {
     IProjectHealthUpdate,
     IProjectInsert,
     IProjectQuery,
     IProjectStore,
 } from '../types/stores/project-store';
 import { DEFAULT_ENV } from '../util/constants';
+import metricsHelper from '../util/metrics-helper';
+import { DB_TIME } from '../metric-events';
+import EventEmitter from 'events';
 
 const COLUMNS = [
     'id',
@@ -26,9 +29,16 @@ class ProjectStore implements IProjectStore {
 
     private logger: Logger;
 
-    constructor(db: Knex, getLogger: LogProvider) {
+    private timer: Function;
+
+    constructor(db: Knex, eventBus: EventEmitter, getLogger: LogProvider) {
         this.db = db;
         this.logger = getLogger('project-store.ts');
+        this.timer = (action) =>
+            metricsHelper.wrapTimer(eventBus, DB_TIME, {
+                store: 'project',
+                action,
+            });
     }
 
     // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
@@ -44,13 +54,61 @@ class ProjectStore implements IProjectStore {
 
     async exists(id: string): Promise<boolean> {
         const result = await this.db.raw(
-            `SELECT EXISTS (SELECT 1 FROM ${TABLE} WHERE id = ?) AS present`,
+            `SELECT EXISTS(SELECT 1 FROM ${TABLE} WHERE id = ?) AS present`,
             [id],
         );
         const { present } = result.rows[0];
         return present;
     }
 
+    async getProjectsWithCounts(
+        query?: IProjectQuery,
+    ): Promise<IProjectWithCount[]> {
+        const projectTimer = this.timer('getProjectsWithCount');
+        let projects = this.db(TABLE)
+            .select(
+                this.db.raw(
+                    'projects.id, projects.name, projects.description, projects.health, projects.updated_at, count(features.name) AS number_of_features',
+                ),
+            )
+            .leftJoin('features', 'features.project', 'projects.id')
+            .groupBy('projects.id');
+        if (query) {
+            projects = projects.where(query);
+        }
+        const projectAndFeatureCount = await projects;
+
+        // @ts-ignore
+        const projectsWithFeatureCount = projectAndFeatureCount.map(
+            this.mapProjectWithCountRow,
+        );
+        projectTimer();
+        const memberTimer = this.timer('getMemberCount');
+        const memberCount = await this.db.raw(
+            `SELECT count(role_id) as member_count, project FROM role_user GROUP BY project`,
+        );
+        memberTimer();
+        const memberMap = new Map<string, number>(
+            memberCount.rows.map((c) => [c.project, Number(c.member_count)]),
+        );
+        return projectsWithFeatureCount.map((r) => {
+            return { ...r, memberCount: memberMap.get(r.id) };
+        });
+    }
+
+    // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
+    mapProjectWithCountRow(row): IProjectWithCount {
+        return {
+            name: row.name,
+            id: row.id,
+            description: row.description,
+            health: row.health,
+            featureCount: row.number_of_features,
+            memberCount: row.number_of_users || 0,
+            updatedAt: row.updated_at,
+        };
+    }
+
     async getAll(query: IProjectQuery = {}): Promise<IProject[]> {
         const rows = await this.db
             .select(COLUMNS)
@@ -71,7 +129,7 @@ class ProjectStore implements IProjectStore {
 
     async hasProject(id: string): Promise<boolean> {
         const result = await this.db.raw(
-            `SELECT EXISTS (SELECT 1 FROM ${TABLE} WHERE id = ?) AS present`,
+            `SELECT EXISTS(SELECT 1 FROM ${TABLE} WHERE id = ?) AS present`,
             [id],
         );
         const { present } = result.rows[0];
@@ -208,5 +266,5 @@ class ProjectStore implements IProjectStore {
         };
     }
 }
+
 export default ProjectStore;
-module.exports = ProjectStore;

src/lib/services/project-service.ts

@@ -100,24 +100,7 @@ export default class ProjectService {
     }
 
     async getProjects(query?: IProjectQuery): Promise<IProjectWithCount[]> {
-        const projects = await this.store.getAll(query);
-        const projectsWithCount = await Promise.all(
-            projects.map(async (p) => {
-                let featureCount = 0;
-                let memberCount = 0;
-                try {
-                    featureCount =
-                        await this.featureToggleService.getFeatureCountForProject(
-                            p.id,
-                        );
-                    memberCount = await this.getMembers(p.id);
-                } catch (e) {
-                    this.logger.warn('Error fetching project counts', e);
-                }
-                return { ...p, featureCount, memberCount };
-            }),
-        );
-        return projectsWithCount;
+        return this.store.getProjectsWithCounts(query);
     }
 
     async getProject(id: string): Promise<IProject> {

src/lib/types/stores/project-store.ts

@@ -1,4 +1,4 @@
-import { IProject } from '../model';
+import { IProject, IProjectWithCount } from '../model';
 import { Store } from './store';
 
 export interface IProjectInsert {
@@ -32,6 +32,7 @@ export interface IProjectStore extends Store<IProject, string> {
     deleteEnvironmentForProject(id: string, environment: string): Promise<void>;
     getEnvironmentsForProject(id: string): Promise<string[]>;
     getMembers(projectId: string): Promise<number>;
+    getProjectsWithCounts(query?: IProjectQuery): Promise<IProjectWithCount[]>;
     count(): Promise<number>;
     getAll(query?: IProjectQuery): Promise<IProject[]>;
 }

src/test/e2e/api/admin/project/features.e2e.test.ts

@@ -2039,7 +2039,7 @@ test('Can update impression data with PUT', async () => {
 });
 
 test('Can create toggle with impression data on different project', async () => {
-    db.stores.projectStore.create({
+    await db.stores.projectStore.create({
         id: 'impression-data',
         name: 'ImpressionData',
         description: '',

src/test/e2e/api/client/feature.e2e.test.ts

@@ -282,7 +282,7 @@ test('returns a feature toggles impression data for a different project', async
         description: '',
     };
 
-    db.stores.projectStore.create(project);
+    await db.stores.projectStore.create(project);
 
     const toggle = {
         name: 'project-client.impression.data',

src/test/fixtures/fake-project-store.ts

@@ -3,7 +3,7 @@ import {
     IProjectInsert,
     IProjectStore,
 } from '../../lib/types/stores/project-store';
-import { IProject } from '../../lib/types/model';
+import { IProject, IProjectWithCount } from '../../lib/types/model';
 import NotFoundError from '../../lib/error/notfound-error';
 
 export default class FakeProjectStore implements IProjectStore {
@@ -26,6 +26,12 @@ export default class FakeProjectStore implements IProjectStore {
         this.projectEnvironment.set(id, environments);
     }
 
+    async getProjectsWithCounts(): Promise<IProjectWithCount[]> {
+        return this.projects.map((p) => {
+            return { ...p, memberCount: 0, featureCount: 0 };
+        });
+    }
+
     private createInternal(project: IProjectInsert): IProject {
         const newProj: IProject = {
             ...project,

yarn.lock

@@ -5756,6 +5756,11 @@ pg-pool@^3.4.1:
   resolved "https://registry.npmjs.org/pg-pool/-/pg-pool-3.4.1.tgz"
   integrity sha512-TVHxR/gf3MeJRvchgNHxsYsTCHQ+4wm3VIHSS19z8NC0+gioEhq1okDY1sm/TYbfoP6JLFx01s0ShvZ3puP/iQ==
 
+pg-pool@^3.5.1:
+  version "3.5.1"
+  resolved "https://registry.yarnpkg.com/pg-pool/-/pg-pool-3.5.1.tgz#f499ce76f9bf5097488b3b83b19861f28e4ed905"
+  integrity sha512-6iCR0wVrro6OOHFsyavV+i6KYL4lVNyYAB9RD18w66xSzN+d8b66HiwuP30Gp1SH5O9T82fckkzsRjlrhD0ioQ==
+
 pg-protocol@^1.5.0:
   version "1.5.0"
   resolved "https://registry.npmjs.org/pg-protocol/-/pg-protocol-1.5.0.tgz"
@@ -5772,7 +5777,7 @@ pg-types@^2.1.0:
     postgres-date "~1.0.4"
     postgres-interval "^1.1.0"
 
-pg@^8.0.3, pg@^8.7.1:
+pg@^8.0.3:
   version "8.7.1"
   resolved "https://registry.npmjs.org/pg/-/pg-8.7.1.tgz"
   integrity sha512-7bdYcv7V6U3KAtWjpQJJBww0UEsWuh4yQ/EjNf2HeO/NnvKjpvhEIe/A/TleP6wtmSKnUnghs5A9jUoK6iDdkA==
@@ -5785,6 +5790,19 @@ pg@^8.0.3, pg@^8.7.1:
     pg-types "^2.1.0"
     pgpass "1.x"
 
+pg@^8.7.3:
+  version "8.7.3"
+  resolved "https://registry.yarnpkg.com/pg/-/pg-8.7.3.tgz#8a5bdd664ca4fda4db7997ec634c6e5455b27c44"
+  integrity sha512-HPmH4GH4H3AOprDJOazoIcpI49XFsHCe8xlrjHkWiapdbHK+HLtbm/GQzXYAZwmPju/kzKhjaSfMACG+8cgJcw==
+  dependencies:
+    buffer-writer "2.0.0"
+    packet-reader "1.0.0"
+    pg-connection-string "^2.5.0"
+    pg-pool "^3.5.1"
+    pg-protocol "^1.5.0"
+    pg-types "^2.1.0"
+    pgpass "1.x"
+
 pgpass@1.x:
   version "1.0.4"
   resolved "https://registry.npmjs.org/pgpass/-/pgpass-1.0.4.tgz"