diff --git a/package.json b/package.json index 70bba2d9..d234474a 100644 --- a/package.json +++ b/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": { diff --git a/src/dataLoaders.ts b/src/dataLoaders.ts index f91e68df..c41a5458 100644 --- a/src/dataLoaders.ts +++ b/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, fieldName: string): Promise<(T | null | Error)[]> { + const valuesMap = new Map(); + + 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 { return new DataLoader(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}`) diff --git a/src/index.ts b/src/index.ts index b7173f12..0f847d41 100644 --- a/src/index.ts +++ b/src/index.ts @@ -237,7 +237,6 @@ class HawkAPI { id: userId, accessTokenExpired: isAccessTokenExpired, }, - eventsFactoryCache: new Map(), // accounting, }; } diff --git a/src/resolvers/event.js b/src/resolvers/event.js index 52ec0a06..5b8804da 100644 --- a/src/resolvers/event.js +++ b/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} */ 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} */ 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} */ 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} */ 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, ''); diff --git a/src/resolvers/helpers/eventsFactory.ts b/src/resolvers/helpers/eventsFactory.ts new file mode 100644 index 00000000..6cdc105d --- /dev/null +++ b/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; diff --git a/src/resolvers/project.js b/src/resolvers/project.js index 5d586844..20745d78 100644 --- a/src/resolvers/project.js +++ b/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} */ 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} */ async chartData(project, { days, timezoneOffset }, context) { - const factory = getEventsFactoryForProject(project, context); + const factory = getEventsFactory(context, project._id); return factory.findChartData(days, timezoneOffset); }, diff --git a/src/types/graphql.ts b/src/types/graphql.ts index b7e19024..0c1e09f4 100644 --- a/src/types/graphql.ts +++ b/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; + eventsFactoryCache?: Map; // /** // * SDK for working with CodeX Accounting API diff --git a/test/resolvers/billingNew.test.ts b/test/resolvers/billingNew.test.ts index d8f1a9bf..a42fafa3 100644 --- a/test/resolvers/billingNew.test.ts +++ b/test/resolvers/billingNew.test.ts @@ -71,7 +71,6 @@ function createComposePaymentTestSetup(options: { }; const mockContext: ResolverContextWithUser = { - eventsFactoryCache: new Map(), user: { id: userId, accessTokenExpired: false,