feat/df-1134: Performance improvements for metrics

src/api/forms/repositories/form-metadata-repository.js

@@ -21,20 +21,22 @@ import { METADATA_COLLECTION_NAME, db } from '~/src/mongo.js'
 export const MAX_RESULTS = 100
 
 /**
- * Retrieves the list of documents from the database
+ * Retrieves the list of all form ids from the database
+ * @returns {Promise<string[]>}
  */
-export async function listAll() {
+export async function listAllIds() {
   const coll = /** @type {Collection<Partial<FormMetadataDocument>>} */ (
     db.collection(METADATA_COLLECTION_NAME)
   )
 
-  return coll
+  const documents = await coll
     .find()
     .sort({
       updatedAt: -1
     })
-    .limit(MAX_RESULTS)
+    .project({ _id: 1 })
     .toArray()
+  return documents.map((doc) => doc._id)
 }
 
 /**
@@ -173,17 +175,19 @@ export async function listWithVersions(options) {
 }
 
 /**
- * Retrieves a list of all forms' metadata using a cursor
+ * Retrieves a list of specified forms' metadata using a cursor
+ * @param {string[]} formIds - array of form ids
  * @param {ClientSession} session - MongoDB session for transactions
  */
-export function getMetadataCursorOfAllForms(session) {
-  logger.info('Getting metadata of all forms')
+export function getMetadataCursorOfForms(formIds, session) {
+  logger.info('Getting metadata of forms in list')
 
   const coll = /** @satisfies {Collection<Partial<FormMetadataDocument>>} */ (
     db.collection(METADATA_COLLECTION_NAME)
   )
 
-  return coll.find({}, { session })
+  const formObjectIds = formIds.map((id) => new ObjectId(id))
+  return coll.find({ _id: { $in: formObjectIds } }, { session })
 }
 
 /**

src/api/forms/repositories/form-metadata-repository.test.js

@@ -9,16 +9,15 @@ import {
 import { buildMockCollection } from '~/src/api/forms/__stubs__/mongo.js'
 import { FormAlreadyExistsError } from '~/src/api/forms/errors.js'
 import {
-  MAX_RESULTS,
   addVersionMetadata,
   create,
   get,
   getAndIncrementVersionNumber,
   getBySlug,
-  getMetadataCursorOfAllForms,
+  getMetadataCursorOfForms,
   getVersionMetadata,
   list,
-  listAll,
+  listAllIds,
   listWithVersions,
   remove,
   update,
@@ -161,31 +160,33 @@ describe('form-metadata-repository', () => {
     })
   })
 
-  describe('listAll', () => {
-    it('should retrieve all documents with limit', async () => {
+  describe('listAllIds', () => {
+    it('should retrieve all document ids', async () => {
       const mockDocuments = [metadataBefore, metadataAfter]
       mockCollection.find.mockReturnValue({
         sort: jest.fn().mockReturnThis(),
-        limit: jest.fn().mockReturnThis(),
+        project: jest.fn().mockReturnThis(),
         toArray: jest.fn().mockResolvedValue(mockDocuments)
       })
 
-      const result = await listAll()
+      const result = await listAllIds()
 
       expect(mockCollection.find).toHaveBeenCalledWith()
       expect(mockCollection.find().sort).toHaveBeenCalledWith({ updatedAt: -1 })
-      expect(mockCollection.find().limit).toHaveBeenCalledWith(MAX_RESULTS)
-      expect(result).toEqual(mockDocuments)
+      expect(result).toEqual([
+        new ObjectId('681b184463c68bf6b99e2c62'),
+        new ObjectId('681b184463c68bf6b99e2c62')
+      ])
     })
 
     it('should handle empty results', async () => {
       mockCollection.find.mockReturnValue({
         sort: jest.fn().mockReturnThis(),
-        limit: jest.fn().mockReturnThis(),
+        project: jest.fn().mockReturnThis(),
         toArray: jest.fn().mockResolvedValue([])
       })
 
-      const result = await listAll()
+      const result = await listAllIds()
 
       expect(result).toEqual([])
     })
@@ -994,8 +995,8 @@ describe('form-metadata-repository', () => {
     })
   })
 
-  describe('getMetadataCursorOfAllForms', () => {
-    it('should retrieve metedata array', () => {
+  describe('getMetadataCursorOfForms', () => {
+    it('should retrieve metadata array', () => {
       const metadataList = [
         buildMetadataDocument({
           title: 'Form 1 title',
@@ -1012,7 +1013,14 @@ describe('form-metadata-repository', () => {
       ]
       mockCollection.find.mockReturnValue(metadataList)
 
-      const result = getMetadataCursorOfAllForms(mockSession)
+      const result = getMetadataCursorOfForms(
+        [
+          '681b184463c68bf6b99e2c62',
+          '681b184463c68bf6b99e2c63',
+          '681b184463c68bf6b99e2c64'
+        ],
+        mockSession
+      )
 
       expect(result).toEqual(metadataList)
     })

src/api/forms/service/definition.js

@@ -45,6 +45,15 @@ export async function listForms(options) {
   return { forms, totalItems, filters }
 }
 
+/**
+ * Retrieves a full list of forms
+ * @returns {Promise<string[]>}
+ */
+export async function listAllFormIds() {
+  const ids = await formMetadata.listAllIds()
+  return ids
+}
+
 /**
  * Retrieves the form definition content for a given form ID.
  * @param {string} formId - the ID of the form

src/api/forms/service/definition.test.js

@@ -48,6 +48,7 @@ import {
   createLiveFromDraft,
   deleteDraftFormDefinition,
   getFormDefinition,
+  listAllFormIds,
   listForms,
   makePaymentKeyLive,
   missingPrivacyNotice,
@@ -138,6 +139,14 @@ describe('Forms service', () => {
     })
   })
 
+  describe('listAllFormIds', () => {
+    it('should call the repo method', async () => {
+      jest.mocked(formMetadata.listAllIds).mockResolvedValue(['id1', 'id2'])
+      const res = await listAllFormIds()
+      expect(res).toEqual(['id1', 'id2'])
+    })
+  })
+
   describe('createDraftFromLive', () => {
     beforeEach(() => {
       jest.mocked(formDefinition.createDraftFromLive).mockResolvedValueOnce()

src/api/forms/service/report-overview.js

@@ -13,15 +13,16 @@ import {
 import { StatusCodes } from 'http-status-codes'
 
 import * as formDefinition from '~/src/api/forms/repositories/form-definition-repository.js'
-import { getMetadataCursorOfAllForms } from '~/src/api/forms/repositories/form-metadata-repository.js'
+import { getMetadataCursorOfForms } from '~/src/api/forms/repositories/form-metadata-repository.js'
 import { mapMetadata } from '~/src/api/forms/service/helpers/mapper.js'
 import { logger } from '~/src/helpers/logging/logger.js'
 import { client } from '~/src/mongo.js'
 
 /**
- * Generates a set of overview metrics for each form
+ * Generates a set of overview metrics for the given list of forms
+ * @param {string[]} formIds
  */
-export async function generateReportOverview() {
+export async function generateReportOverview(formIds) {
   logger.info('Generating overview report')
 
   const session = client.startSession()
@@ -33,7 +34,7 @@ export async function generateReportOverview() {
 
   try {
     await session.withTransaction(async () => {
-      const metadataCursor = getMetadataCursorOfAllForms(session)
+      const metadataCursor = getMetadataCursorOfForms(formIds, session)
 
       for await (const metadata of metadataCursor) {
         const strictMetadata = mapMetadata(metadata)
@@ -323,5 +324,5 @@ export function getUniqueAssignedSections(definition) {
 
 /**
  * @import { ClientSession } from 'mongodb'
- * @import { ComponentDef, FormDefinition, FormMetadata } from '@defra/forms-model'
+ * @import { ComponentDef, FormDefinition, FormMetadata, FormOverviewMetric } from '@defra/forms-model'
  */

src/api/forms/service/report-overview.test.js

@@ -19,7 +19,7 @@ import {
 } from '~/src/api/forms/__stubs__/definition.js'
 import { buildMetadataDocument } from '~/src/api/forms/__stubs__/metadata.js'
 import * as formDefinition from '~/src/api/forms/repositories/form-definition-repository.js'
-import { getMetadataCursorOfAllForms } from '~/src/api/forms/repositories/form-metadata-repository.js'
+import { getMetadataCursorOfForms } from '~/src/api/forms/repositories/form-metadata-repository.js'
 import { getExpectedOverviewMetrics } from '~/src/api/forms/service/__stubs__/metrics.js'
 import {
   calcFeatureMetrics,
@@ -116,7 +116,7 @@ describe('report-overview', () => {
       }
 
       jest
-        .mocked(getMetadataCursorOfAllForms)
+        .mocked(getMetadataCursorOfForms)
         // @ts-expect-error - resolves to an async iterator like FindCursor<FormMetadataDocument>
         .mockReturnValueOnce(mockAsyncIterator)
 
@@ -148,13 +148,13 @@ describe('report-overview', () => {
       })
       jest.mocked(client.startSession).mockReturnValue(mockNewSession)
 
-      const metrics = await generateReportOverview()
+      const metrics = await generateReportOverview(['id1', 'id2', 'id3'])
 
       expect(metrics).toEqual(getExpectedOverviewMetrics(new Date()))
     })
 
     it('should handle error and still close session', async () => {
-      jest.mocked(getMetadataCursorOfAllForms).mockImplementationOnce(() => {
+      jest.mocked(getMetadataCursorOfForms).mockImplementationOnce(() => {
         throw new Error('report error')
       })
 
@@ -167,9 +167,9 @@ describe('report-overview', () => {
       })
       jest.mocked(client.startSession).mockReturnValue(mockNewSession)
 
-      await expect(() => generateReportOverview()).rejects.toThrow(
-        'report error'
-      )
+      await expect(() =>
+        generateReportOverview(['id1', 'id2', 'id3'])
+      ).rejects.toThrow('report error')
 
       expect(mockEndSession).toHaveBeenCalled()
     })

src/api/types.js

@@ -29,6 +29,7 @@
  * @typedef {Request<{ Server: { db: Db }, Params: {id: string, nameBefore: string, nameAfter: string } }>} RequestRenameFormSecret
  * @typedef {Request<{ Server: { db: Db }, Params: {id: string, name: string}, Payload: { secretValue: string } }>} RequestSaveFormSecret
  * @typedef {Request<{ Server: { db: Db }, Query: {date: Date} }>} RequestReport
+ * @typedef {Request<{ Server: { db: Db }, Query: {ids: string[]} }>} RequestOverviewReport
  */
 
 /**

src/models/forms.js

@@ -151,3 +151,7 @@ export const sectionAssignmentPayloadSchema = Joi.object()
       .required()
   })
   .required()
+
+export const reportOverviewQuerySchema = Joi.object({
+  ids: Joi.array().single().items(Joi.string()).required()
+})

src/routes/forms.js

@@ -13,6 +13,7 @@ import {
   createLiveFromDraft,
   deleteDraftFormDefinition,
   getFormDefinition,
+  listAllFormIds,
   listForms,
   updateDraftFormDefinition
 } from '~/src/api/forms/service/definition.js'
@@ -36,6 +37,7 @@ import {
   formByIdSchema,
   formBySlugSchema,
   migrateDefinitionParamSchema,
+  reportOverviewQuerySchema,
   updateFormDefinitionSchema
 } from '~/src/models/forms.js'
 
@@ -90,6 +92,18 @@ export default [
       }
     }
   },
+  {
+    method: 'GET',
+    path: '/all-form-ids',
+    async handler() {
+      const ids = await listAllFormIds()
+
+      return ids
+    },
+    options: {
+      auth: false
+    }
+  },
   {
     method: 'POST',
     path: '/forms',
@@ -460,18 +474,25 @@ export default [
   {
     method: 'GET',
     path: '/report/overview',
-    handler() {
-      return generateReportOverview()
+    /**
+     * @param {RequestOverviewReport} request
+     */
+    handler(request) {
+      const { query } = request
+      return generateReportOverview(query.ids)
     },
     options: {
-      auth: false
+      auth: false,
+      validate: {
+        query: reportOverviewQuerySchema
+      }
     }
   }
 ]
 
 /**
  * @import { FormMetadata } from '@defra/forms-model'
  * @import { ServerRoute } from '@hapi/hapi'
- * @import { RequestFormById, RequestFormBySlug, RequestFormDefinition, RequestFormMetadataCreate, RequestFormMetadataUpdateById, RequestListForms, RequestReport, MigrateDraftFormRequest, RequestFormVersionById } from '~/src/api/types.js'
+ * @import { RequestFormById, RequestFormBySlug, RequestFormDefinition, RequestFormMetadataCreate, RequestFormMetadataUpdateById, RequestListForms, RequestOverviewReport, MigrateDraftFormRequest, RequestFormVersionById } from '~/src/api/types.js'
  * @import { ExtendedResponseToolkit } from '~/src/plugins/query-handler/types.js'
  */

src/routes/forms.test.js

@@ -18,6 +18,7 @@ import {
   createLiveFromDraft,
   deleteDraftFormDefinition,
   getFormDefinition,
+  listAllFormIds,
   listForms,
   updateDraftFormDefinition
 } from '~/src/api/forms/service/definition.js'
@@ -43,6 +44,7 @@ jest.mock('~/src/api/forms/service/definition.js', () => ({
   ...jest.requireActual('~/src/api/forms/service/definition.js'),
   getFormDefinition: jest.fn(),
   listForms: jest.fn(),
+  listAllFormIds: jest.fn(),
   updateDraftFormDefinition: jest.fn(),
   createLiveFromDraft: jest.fn(),
   createDraftFromLive: jest.fn(),
@@ -1949,7 +1951,9 @@ describe('Forms route', () => {
         status: 'updated'
       })
     })
+  })
 
+  describe('report-overview', () => {
     test('GET /report-overview returns data', async () => {
       jest.mocked(generateReportOverview).mockResolvedValue({
         draft: [],
@@ -1958,7 +1962,7 @@ describe('Forms route', () => {
 
       const response = await server.inject({
         method: 'GET',
-        url: '/report/overview'
+        url: '/report/overview?ids=form-id-1&ids=form-id-2'
       })
 
       expect(response.statusCode).toEqual(okStatusCode)
@@ -1968,6 +1972,20 @@ describe('Forms route', () => {
       })
     })
   })
+
+  describe('all-form-ids', () => {
+    test('GET /all-form-ids returns data', async () => {
+      jest.mocked(listAllFormIds).mockResolvedValue(['id1', 'id2'])
+
+      const response = await server.inject({
+        method: 'GET',
+        url: '/all-form-ids'
+      })
+
+      expect(response.statusCode).toEqual(okStatusCode)
+      expect(response.result).toEqual(['id1', 'id2'])
+    })
+  })
 })
 
 /**