Merge pull request #567 from codex-team/feat/opt-dupl

feat(perf): rm duplicated ids from batched queries

Author: slaveeks

package.json

@@ -1,6 +1,6 @@
 {
   "name": "hawk.api",
-  "version": "1.2.10",
+  "version": "1.2.11",
   "main": "index.ts",
   "license": "BUSL-1.1",
   "scripts": {

src/dataLoaders.ts

@@ -82,11 +82,17 @@ export default class DataLoaders {
   private async batchByField<
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     T extends { [key: string]: any },
-    FieldType extends object | string
+    FieldType extends ObjectId | string
   >(collectionName: string, values: ReadonlyArray<FieldType>, fieldName: string): Promise<(T | null | Error)[]> {
+    const valuesMap = new Map<string, FieldType>();
+
+    for (const value of values) {
+      valuesMap.set(value.toString(), value);
+    }
+
     const queryResult = await this.dbConnection.collection(collectionName)
       .find({
-        [fieldName]: { $in: values },
+        [fieldName]: { $in: Array.from(valuesMap.values()) },
       })
       .toArray();
 
@@ -115,7 +121,10 @@ export function createProjectEventsByIdLoader(
   projectId: string
 ): DataLoader<string, EventDbScheme | null> {
   return new DataLoader<string, EventDbScheme | null>(async (ids) => {
-    const objectIds = ids.map((id) => new ObjectId(id));
+    /**
+     * Deduplicate only for the DB query; keep original ids array for mapping
+     */
+    const objectIds = [ ...new Set(ids) ].map(id => new ObjectId(id));
 
     const docs = await eventsDb
       .collection(`events:${projectId}`)

src/index.ts

@@ -237,7 +237,6 @@ class HawkAPI {
         id: userId,
         accessTokenExpired: isAccessTokenExpired,
       },
-      eventsFactoryCache: new Map(),
       // accounting,
     };
   }

src/resolvers/event.js

@@ -1,25 +1,6 @@
-const EventsFactory = require('../models/eventsFactory');
+const getEventsFactory = require('./helpers/eventsFactory').default;
 const sendPersonalNotification = require('../utils/personalNotifications').default;
 
-/**
- * Returns a per-request, per-project EventsFactory instance
- * Uses context.eventsFactoryCache to memoize by projectId
- *
- * @param {ResolverContextBase} context - resolver context
- * @param {string} projectId - project id to get EventsFactory instance for
- * @returns {EventsFactory} - EventsFactory instance bound to a specific project object
- */
-function getEventsFactoryForProjectId(context, projectId) {
-  const cache = context.eventsFactoryCache || (context.eventsFactoryCache = new Map());
-  const cacheKey = projectId.toString();
-
-  if (!cache.has(cacheKey)) {
-    cache.set(cacheKey, new EventsFactory(projectId));
-  }
-
-  return cache.get(cacheKey);
-}
-
 /**
  * See all types and fields here {@see ../typeDefs/event.graphql}
  */
@@ -48,7 +29,7 @@ module.exports = {
      * @return {RepetitionsPortion}
      */
     async repetitionsPortion({ projectId, originalEventId }, { limit, cursor }, context) {
-      const factory = getEventsFactoryForProjectId(context, projectId);
+      const factory = getEventsFactory(context, projectId);
 
       return factory.getEventRepetitions(originalEventId, limit, cursor);
     },
@@ -103,7 +84,7 @@ module.exports = {
      * @returns {Promise<ProjectChartItem[]>}
      */
     async chartData({ projectId, groupHash }, { days, timezoneOffset }, context) {
-      const factory = getEventsFactoryForProjectId(context, projectId);
+      const factory = getEventsFactory(context, projectId);
 
       return factory.findChartData(days, timezoneOffset, groupHash);
     },
@@ -116,7 +97,7 @@ module.exports = {
      * @returns {Promise<Release>}
      */
     async release({ projectId, id: eventId }, _args, context) {
-      const factory = getEventsFactoryForProjectId(context, projectId);
+      const factory = getEventsFactory(context, projectId);
       const release = await factory.getEventRelease(eventId);
 
       return release;
@@ -133,7 +114,7 @@ module.exports = {
      * @return {Promise<boolean>}
      */
     async visitEvent(_obj, { projectId, eventId }, { user, ...context }) {
-      const factory = getEventsFactoryForProjectId(context, projectId);
+      const factory = getEventsFactory(context, projectId);
 
       const { result } = await factory.visitEvent(eventId, user.id);
 
@@ -150,7 +131,7 @@ module.exports = {
      * @return {Promise<boolean>}
      */
     async toggleEventMark(_obj, { project, eventId, mark }, context) {
-      const factory = getEventsFactoryForProjectId(context, project);
+      const factory = getEventsFactory(context, project);
 
       const { result } = await factory.toggleEventMark(eventId, mark);
 
@@ -175,7 +156,7 @@ module.exports = {
      */
     async updateAssignee(_obj, { input }, { factories, user, ...context }) {
       const { projectId, eventId, assignee } = input;
-      const factory = getEventsFactoryForProjectId(context, projectId);
+      const factory = getEventsFactory(context, projectId);
 
       const userExists = await factories.usersFactory.findById(assignee);
 
@@ -226,7 +207,7 @@ module.exports = {
      */
     async removeAssignee(_obj, { input }, context) {
       const { projectId, eventId } = input;
-      const factory = getEventsFactoryForProjectId(context, projectId);
+      const factory = getEventsFactory(context, projectId);
 
       const { result } = await factory.updateAssignee(eventId, '');
 

src/resolvers/helpers/eventsFactory.ts

@@ -0,0 +1,32 @@
+import { ResolverContextBase } from '../../types/graphql';
+
+// eslint-disable-next-line @typescript-eslint/no-var-requires
+const EventsFactory = require('../../models/eventsFactory');
+
+/**
+ * Returns a per-request, per-project EventsFactory instance
+ * Uses context.eventsFactoryCache to memoize by projectId
+ *
+ * @param {ResolverContextBase} context - resolver context
+ * @param {string} projectId - project id to get EventsFactory instance for
+ * @returns {EventsFactory} - EventsFactory instance bound to a specific project object
+ */
+export function getEventsFactory(context: ResolverContextBase, projectId: string) {
+  if (!context.eventsFactoryCache) {
+    context.eventsFactoryCache = new Map();
+  }
+
+  const cache = context.eventsFactoryCache;
+
+  if (cache) {
+    if (!cache.has(projectId)) {
+      cache.set(projectId, new EventsFactory(projectId));
+    }
+
+    return cache.get(projectId);
+  }
+
+  return new EventsFactory(projectId);
+}
+
+export default getEventsFactory;

src/resolvers/project.js

@@ -5,6 +5,7 @@ const { ApolloError, UserInputError } = require('apollo-server-express');
 const Validator = require('../utils/validator');
 const UserInProject = require('../models/userInProject');
 const EventsFactory = require('../models/eventsFactory');
+const getEventsFactory = require('./helpers/eventsFactory').default;
 const ProjectToWorkspace = require('../models/projectToWorkspace');
 const { dateFromObjectId } = require('../utils/dates');
 const ProjectModel = require('../models/project').default;
@@ -14,30 +15,6 @@ const REPETITIONS_GROUP_HASH_INDEX_NAME = 'groupHash_hashed';
 const REPETITIONS_USER_ID_INDEX_NAME = 'userId';
 const MAX_SEARCH_QUERY_LENGTH = 50;
 
-/**
- * Returns a singleton EventsFactory instance bound to a specific project object.
- * Uses request-scoped cache to share across nested resolvers.
- *
- * @param {ProjectDBScheme|Object} project - project instance to make a instance of EventsFactory
- * @param {ResolverContextBase} context - resolver context
- * @returns {EventsFactory} - EventsFactory instance bound to a specific project object
- */
-function getEventsFactoryForProject(project, context) {
-  const cache = context && context.eventsFactoryCache;
-  const key = project._id.toString();
-
-  if (cache) {
-    if (!cache.has(key)) {
-      cache.set(key, new EventsFactory(project._id));
-    }
-
-    return cache.get(key);
-  }
-
-  // Fallback (shouldn't happen in normal resolver flow): return a fresh instance
-  return new EventsFactory(project._id);
-}
-
 /**
  * See all types and fields here {@see ../typeDefs/project.graphql}
  */
@@ -386,7 +363,7 @@ module.exports = {
      * @returns {EventRepetitionSchema}
      */
     async event(project, { eventId: repetitionId, originalEventId }, context) {
-      const factory = getEventsFactoryForProject(project, context);
+      const factory = getEventsFactory(context, project._id);
       const repetition = await factory.getEventRepetition(repetitionId, originalEventId);
 
       if (!repetition) {
@@ -408,7 +385,7 @@ module.exports = {
      * @returns {Event[]}
      */
     async events(project, { limit, skip }, context) {
-      const factory = getEventsFactoryForProject(project, context);
+      const factory = getEventsFactory(context, project._id);
 
       return factory.find({}, limit, skip);
     },
@@ -423,7 +400,7 @@ module.exports = {
      * @return {Promise<number>}
      */
     async unreadCount(project, data, { user, ...context }) {
-      const eventsFactory = getEventsFactoryForProject(project, context);
+      const eventsFactory = getEventsFactory(context, project._id);
       const userInProject = new UserInProject(user.id, project._id);
       const lastVisit = await userInProject.getLastVisit();
 
@@ -449,7 +426,7 @@ module.exports = {
         }
       }
 
-      const factory = getEventsFactoryForProject(project, context);
+      const factory = getEventsFactory(context, project._id);
 
       const dailyEventsPortion = await factory.findDailyEventsPortion(limit, nextCursor, sort, filters, search);
 
@@ -466,7 +443,7 @@ module.exports = {
      * @return {Promise<ProjectChartItem[]>}
      */
     async chartData(project, { days, timezoneOffset }, context) {
-      const factory = getEventsFactoryForProject(project, context);
+      const factory = getEventsFactory(context, project._id);
 
       return factory.findChartData(days, timezoneOffset);
     },

src/types/graphql.ts

@@ -22,9 +22,10 @@ export interface ResolverContextBase {
 
   /**
    * Request-scoped cache for EventsFactory instances keyed by projectId
+   * Set by getEventsFactory helper in event & project resolvers
    */
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  eventsFactoryCache: Map<string, any>;
+  eventsFactoryCache?: Map<string, any>;
 
   // /**
   //  * SDK for working with CodeX Accounting API

test/resolvers/billingNew.test.ts

@@ -71,7 +71,6 @@ function createComposePaymentTestSetup(options: {
   };
 
   const mockContext: ResolverContextWithUser = {
-    eventsFactoryCache: new Map(),
     user: { 
       id: userId,
       accessTokenExpired: false,