Limit IDs passed to location assocations query

.eslintrc.js

@@ -10,7 +10,7 @@ module.exports = {
     'max-len': [2, 100],
     'no-console': 2,
     'no-multiple-empty-lines': [2, { max: 1, maxBOF: 1, maxEOF: 1 }],
-    'no-unused-vars': [2, { args: 'none' }],
+    'no-unused-vars': [2, { args: 'none', varsIgnorePattern: '^_' }],
     'no-restricted-syntax': ['off', 'ForOfStatement'],
     'no-await-in-loop': ['off'],
   },

src/controllers/locations.js

@@ -15,7 +15,7 @@ import { NotFoundError, ValidationError } from '../utils/errors';
 
 const DEFAULT_MAX_LOCATIONS_RETURNED = 1000;
 
-const isLocationClosed = (occasion, eventRelatedInfos, services) => {
+const isLocationClosed = (occasion, eventRelatedInfos) => {
   if (!occasion) {
     return false;
   }
@@ -111,7 +111,7 @@ async function handleGetInfoResponse(location, locationWithServices, excludeMeta
   const { EventRelatedInfos } = location;
   // FIXME: we should not be hard-coding the COVID19 event here
   // this is logic that needs ot be revisited in this codebase
-  const closed = isLocationClosed('COVID19', EventRelatedInfos, services);
+  const closed = isLocationClosed('COVID19', EventRelatedInfos);
 
   if (excludeMetadata) {
     const [{ lastValidatedDateForLocation }] = await getLastValidatedDateForLocation(location.id);
@@ -169,8 +169,15 @@ export default {
         sortBy,
       } = req.query;
 
+      const capDetailedLocations = (requestedLimit) => {
+        const maxDetailedLocations = 200;
+        return locationFieldsOnly
+          ? requestedLimit
+          : Math.min(requestedLimit, maxDetailedLocations);
+      };
+
       const pageNumber = _pageNumber ? parseInt(_pageNumber, 10) : undefined;
-      const pageSize = _pageNumber ? parseInt(_pageSize, 10) : undefined;
+      const pageSize = _pageNumber ? capDetailedLocations(parseInt(_pageSize, 10)) : undefined;
       const age = _age ? parseInt(_age, 10) : undefined;
       const ageMin = _ageMin ? parseInt(_ageMin, 10) : undefined;
       const ageMax = _ageMax ? parseInt(_ageMax, 10) : undefined;
@@ -232,7 +239,7 @@ export default {
         const taxonomyIds = taxonomyId.split(',');
         filterParameters.taxonomyIds = await models.Taxonomy.getAllIdsWithinTaxonomies(taxonomyIds);
       }
-      const limit = pageSize || maxResults;
+      const limit = pageSize || capDetailedLocations(maxResults);
 
       const offset = pageNumber !== undefined && pageSize !== undefined ?
         pageNumber * pageSize : undefined;
@@ -243,7 +250,7 @@ export default {
       } = await models.Location.search({
         position: (longitude && latitude) ? geometry.createPoint(longitude, latitude) : null,
         radius,
-        minResults,
+        minResults: capDetailedLocations(minResults),
         filterParameters,
         locationFieldsOnly,
         noServices: parseBoolean(noServices),
@@ -252,12 +259,12 @@ export default {
         sortBy,
       });
       const plainLocations = await locations
-        .map(location => location.get({ plain: true }));
+        .map(location => (location.get ? location.get({ plain: true }) : location));
       const paginationCount = Math.ceil(totalNumLocations / pageSize);
 
       const formattedLocations = plainLocations.map((location) => {
-        const { EventRelatedInfos, Services, ...simplifiedLocation } = location;
-        const closed = isLocationClosed(occasion, EventRelatedInfos, Services);
+        const { EventRelatedInfos, Services: _, ...simplifiedLocation } = location;
+        const closed = isLocationClosed(occasion, EventRelatedInfos);
 
         if (locationFieldsOnly) {
           return {

src/models/location.js

@@ -39,7 +39,6 @@ module.exports = (sequelize, DataTypes, Op) => {
   });
 
   const SERVICE_COUNT_COLUMN_ALIAS = 'service_count';
-
   const SERVICE_COUNT_SUBQUERY = [
     sequelize.literal(`(
                 SELECT cast(COUNT(*) as integer)
@@ -49,6 +48,16 @@ module.exports = (sequelize, DataTypes, Op) => {
     SERVICE_COUNT_COLUMN_ALIAS,
   ];
 
+  const EVENTS_WITH_INFO_COLUMN_ALIAS = 'events_with_info';
+  const EVENTS_WITH_INFO_COLUMN_SUBQUERY = [
+    sequelize.literal(`(
+                SELECT JSON_AGG(DISTINCT event_related_info.event)
+                FROM event_related_info
+                WHERE event_related_info.location_id = "Location".id
+            )`),
+    EVENTS_WITH_INFO_COLUMN_ALIAS,
+  ];
+
   Location.associate = (models) => {
     Location.belongsTo(models.Organization, { foreignKey: 'organization_id' });
     Location.belongsToMany(models.Service, {
@@ -297,10 +306,13 @@ module.exports = (sequelize, DataTypes, Op) => {
     return sequelize.and(requiredDocumentCondition, notRequiredDocumentCondition);
   };
 
-  Location.findUniqueLocationIds = async (filterParameters,
+  Location.findUniqueLocationStubs = async (
+    filterParameters,
     additionalConditions,
     originalQueryProps = {},
-    selectedAttributeForOrderBy, noServices) => {
+    selectedAttributeForOrderBy,
+    noServices,
+  ) => {
     const queryProps = { order: originalQueryProps.order };
     // eslint-disable-next-line prefer-destructuring
     const limit = originalQueryProps.limit;
@@ -371,11 +383,12 @@ module.exports = (sequelize, DataTypes, Op) => {
       return Location.findAll({
         ...queryProps,
         where: sequelize.and(..._whereConditions, ...additionalConditions),
-        attributes: [
-          sequelize.fn('DISTINCT', sequelize.col('Location.id')),
-          // For SELECT DISTINCT, ORDER BY expressions must appear in select list.
-          ...(selectedAttributeForOrderBy ? [selectedAttributeForOrderBy] : []),
-        ],
+        attributes: {
+          include: [
+            EVENTS_WITH_INFO_COLUMN_SUBQUERY,
+            ...(selectedAttributeForOrderBy ? [selectedAttributeForOrderBy] : []),
+          ],
+        },
         raw: true,
         // Not like associations and grouping work perfectly out of the box either though...
         // https://github.com/sequelize/sequelize/issues/5481
@@ -390,14 +403,15 @@ module.exports = (sequelize, DataTypes, Op) => {
           sequelize.models.Organization,
           sequelize.models.PhysicalAddress,
           sequelize.models.Phone,
+          sequelize.models.EventRelatedInfo,
           {
             model: sequelize.models.Service,
             required: !noServices,
             include: [
               sequelize.models.Taxonomy,
+              sequelize.models.HolidaySchedule,
               ...(areRequiredDocsSpecified ? [sequelize.models.RequiredDocument] : []),
               ...((openAt && !occasion) ? [sequelize.models.RegularSchedule] : []),
-              ...(occasion ? [sequelize.models.HolidaySchedule] : []),
               ...(servesZipcode ? [sequelize.models.ServiceArea] : []),
               ...(shouldJoinEligibilities ? [{
                 model: sequelize.models.Eligibility,
@@ -489,16 +503,22 @@ module.exports = (sequelize, DataTypes, Op) => {
 
       ].reduce((a, b) => a.concat(b));
 
-      // Remove duplicates
-      locations = Array.from(new Map(searchResults.map(item => [item.id, item])).values());
+      locations = searchResults;
     } else {
       locations = await findAll(whereConditions);
     }
 
-    // apply limit and offset in memory here
-    locations = locations.slice(offset || 0, limit ? (offset || 0) + limit : undefined);
+    const reconstructEvents = ({
+      [EVENTS_WITH_INFO_COLUMN_ALIAS]: events, ...rest
+    }) => ({ ...rest, EventRelatedInfos: (events || []).map(event => ({ event })) });
+
+    // Remove duplicates
+    locations = Array.from(new Map(locations.map(item => [item.id, item])).values())
+      // apply limit and offset in memory here
+      .slice(offset || 0, limit ? (offset || 0) + limit : undefined)
+      .map(reconstructEvents);
 
-    return locations.map(location => location.id);
+    return locations;
   };
 
   Location.search = async ({
@@ -512,7 +532,7 @@ module.exports = (sequelize, DataTypes, Op) => {
     sortBy,
     noServices,
   }) => {
-    let locationIds;
+    let locationStubs;
     let distance;
     let totalNumLocations;
     // order is used to specify the attribute referenced in the ORDER BY
@@ -546,12 +566,12 @@ module.exports = (sequelize, DataTypes, Op) => {
     if (radius && position) {
       const distanceCondition = sequelize.where(distance, { [Op.lte]: radius });
 
-      totalNumLocations = (await Location.findUniqueLocationIds(
+      totalNumLocations = (await Location.findUniqueLocationStubs(
         filterParameters,
         [distanceCondition],
       )).length;
 
-      locationIds = await Location.findUniqueLocationIds(
+      locationStubs = await Location.findUniqueLocationStubs(
         filterParameters,
         [distanceCondition].filter(Boolean), {
           order,
@@ -568,54 +588,53 @@ module.exports = (sequelize, DataTypes, Op) => {
       // However, filtering by window functions requires nested queries, which
       // aren't natively supported by sequelize and would require a raw query.
       // For now, the simplicity and security of sequelize seems worth the slight performance hit.
-      if (minResults && locationIds.length < minResults) {
-        totalNumLocations = (await Location.findUniqueLocationIds(filterParameters, [])).length;
-        locationIds = await Location.findUniqueLocationIds(filterParameters, [], {
+      if (minResults && locationStubs.length < minResults) {
+        totalNumLocations = (await Location.findUniqueLocationStubs(filterParameters, [])).length;
+        locationStubs = await Location.findUniqueLocationStubs(filterParameters, [], {
           order,
           limit: minResults,
           offset,
         }, selectedAttributeForOrderBy, noServices);
       }
     } else {
-      totalNumLocations = (await Location.findUniqueLocationIds(filterParameters, [])).length;
-      locationIds = await Location.findUniqueLocationIds(filterParameters, [], {
+      totalNumLocations = (await Location.findUniqueLocationStubs(filterParameters, [])).length;
+      locationStubs = await Location.findUniqueLocationStubs(filterParameters, [], {
         limit,
         offset,
         order,
       }, selectedAttributeForOrderBy);
     }
 
-    const additionalLocationData = locationFieldsOnly ? [
-      sequelize.models.EventRelatedInfo,
-      {
-        model: sequelize.models.Service,
-        include: [
-          sequelize.models.HolidaySchedule,
-        ],
-      },
-    ] : [
-      sequelize.models.Organization,
-      sequelize.models.EventRelatedInfo,
-      {
-        model: sequelize.models.Service,
-        include: [
-          sequelize.models.Taxonomy,
-          sequelize.models.RequiredDocument,
-          sequelize.models.HolidaySchedule,
-        ],
-      },
-      sequelize.models.Phone,
-      sequelize.models.PhysicalAddress,
-    ];
+    const locationIds = locationStubs.map(location => location.id);
 
-    const locationsWithAssociations = await Location.findAll({
-      attributes: {
-        include: selectedAttributeForOrderBy ? [selectedAttributeForOrderBy] : undefined,
-      },
-      where: { id: { [Op.in]: locationIds } },
-      include: additionalLocationData,
-      order,
-    });
+    let locationsWithAssociations;
+    if (locationFieldsOnly) {
+      locationsWithAssociations = locationStubs;
+    } else {
+      const additionalLocationData = [
+        sequelize.models.Organization,
+        sequelize.models.EventRelatedInfo,
+        {
+          model: sequelize.models.Service,
+          include: [
+            sequelize.models.Taxonomy,
+            sequelize.models.RequiredDocument,
+            sequelize.models.HolidaySchedule,
+          ],
+        },
+        sequelize.models.Phone,
+        sequelize.models.PhysicalAddress,
+      ];
+
+      locationsWithAssociations = await Location.findAll({
+        attributes: {
+          include: selectedAttributeForOrderBy ? [selectedAttributeForOrderBy] : undefined,
+        },
+        where: { id: { [Op.in]: locationIds } },
+        include: additionalLocationData,
+        order,
+      });
+    }
 
     function sortByLocationIds(a, b) {
       return locationIds.indexOf(a.id) - locationIds.indexOf(b.id);