Add deep sort to mongoose, fix sort order on both connectors, cleanup

Signed-off-by: Convly <jean-sebastien.herbaux@epitech.eu>

packages/strapi-connector-bookshelf/lib/buildQuery.js

@@ -1,7 +1,7 @@
 'use strict';
 
 const _ = require('lodash');
-const { each, prop } = require('lodash/fp');
+const { each, prop, reject, isEmpty } = require('lodash/fp');
 const { singular } = require('pluralize');
 const { toQueries, runPopulateQueries } = require('./utils/populate-queries');
 
@@ -18,18 +18,12 @@ const buildQuery = ({ model, filters }) => qb => {
     qb.distinct();
   }
 
-  if (_.has(filters, 'sort')) {
+  const joinsTree = buildJoinsAndFilter(qb, model, filters);
-    qb.orderBy(
-      filters.sort
-        .filter(({ field }) => !field.includes('.'))
-        .map(({ field, order }) => ({
-          column: field,
-          order,
-        }))
-    );
-  }
 
-  buildJoinsAndFilter(qb, model, filters);
+  if (_.has(filters, 'sort')) {
+    const clauses = filters.sort.map(buildSortClauseFromTree(joinsTree));
+    qb.orderBy(reject(isEmpty, clauses));
+  }
 
   if (_.has(filters, 'start')) {
     qb.offset(filters.start);
@@ -47,6 +41,25 @@ const buildQuery = ({ model, filters }) => qb => {
   }
 };
 
+/**
+ * Build a bookshelf sort clause (simple or deep) based on a joins tree
+ * @param tree - The joins tree that contains the aliased associations
+ */
+const buildSortClauseFromTree = tree => ({ field, order }) => {
+  if (!field.includes('.')) {
+    return { column: field, order };
+  }
+
+  const [relation, attribute] = field.split('.');
+  for (const { alias, assoc } of Object.values(tree.joins)) {
+    if (relation === assoc.alias) {
+      return { column: `${alias}.${attribute}`, order };
+    }
+  }
+
+  return {};
+};
+
 /**
  * Add joins and where filters
  * @param {Object} qb - knex query builder
@@ -226,20 +239,7 @@ const buildJoinsAndFilter = (qb, model, filters) => {
    */
   const addFiltersQueriesToJoinTree = tree => {
     _.each(tree.joins, value => {
-      const { alias, model, assoc } = value;
+      const { alias, model } = value;
-
-      // Sort
-      sortClauses
-        // Keep only nested clauses
-        .filter(({ field }) => field.includes('.'))
-        // If the sort is linked to the current join, then add an orderBy
-        .forEach(({ field, order }) => {
-          const [relation, attributeName] = field.split('.');
-
-          if (relation === assoc.alias) {
-            qb.orderBy(`${alias}.${attributeName}`, order);
-          }
-        });
 
       // PublicationState
       runPopulateQueries(
@@ -256,10 +256,13 @@ const buildJoinsAndFilter = (qb, model, filters) => {
   const aliasedWhereClauses = buildWhereClauses(whereClauses, { model });
   aliasedWhereClauses.forEach(w => buildWhereClause({ qb, ...w }));
 
+  // Force needed joins for deep sort clauses
   generateNestedJoinsFromFields(sortClauses.map(prop('field')));
 
   buildJoinsFromTree(qb, tree);
   addFiltersQueriesToJoinTree(tree);
+
+  return tree;
 };
 
 /**

packages/strapi-connector-mongoose/lib/buildQuery.js

@@ -1,7 +1,8 @@
 'use strict';
 
 const _ = require('lodash');
-var semver = require('semver');
+const { gt, isEmpty, reject, set } = require('lodash/fp');
+const semver = require('semver');
 const {
   hasDeepFilters,
   contentTypes: {
@@ -12,6 +13,11 @@ const {
 const utils = require('./utils')();
 const populateQueries = require('./utils/populate-queries');
 
+const sortOrderMapper = {
+  asc: 1,
+  desc: -1,
+};
+
 const combineSearchAndWhere = (search = [], wheres = []) => {
   const criterias = {};
   if (search.length > 0 && wheres.length > 0) {
@@ -88,8 +94,9 @@ const buildQuery = ({
   aggregate = false,
 } = {}) => {
   const search = buildSearchOr(model, searchParam);
+  const { where, sort } = filters;
 
-  if (!hasDeepFilters(filters.where) && aggregate === false) {
+  if (!hasDeepFilters({ where, sort }) && aggregate === false) {
     return buildSimpleQuery({ model, filters, search, populate });
   }
 
@@ -171,7 +178,14 @@ const buildDeepQuery = ({ model, filters, search, populate }) => {
             )
             .populate(populate);
 
-          return applyQueryParams({ model, query, filters });
+          const stringIds = ids.map(id => id.toString());
+          const getIndexInIds = obj => stringIds.indexOf(obj._id.toString());
+
+          return (
+            applyQueryParams({ model, query, filters })
+              // Reorder results using `ids`
+              .then(results => results.sort((a, b) => (gt(...[a, b].map(getIndexInIds)) ? 1 : -1)))
+          );
         })
         .then(...args);
     },
@@ -210,17 +224,6 @@ const buildDeepQuery = ({ model, filters, search, populate }) => {
  * @param {Object} options.filters - Filters object
  */
 const applyQueryParams = ({ model, query, filters }) => {
-  // Apply sort param
-  if (_.has(filters, 'sort')) {
-    const sortFilter = filters.sort.reduce((acc, sort) => {
-      const { field, order } = sort;
-      acc[field] = order === 'asc' ? 1 : -1;
-      return acc;
-    }, {});
-
-    query = query.sort(sortFilter);
-  }
-
   // Apply start param
   if (_.has(filters, 'start')) {
     query = query.skip(filters.start);
@@ -315,9 +318,25 @@ const pathsToTree = paths => paths.reduce((acc, path) => _.merge(acc, _.set({},
  * @param {Object} options.paths - A tree of paths to aggregate e.g { article : { tags : { label: {}}}}
  */
 const buildQueryAggregate = (model, filters, { paths } = {}) => {
-  return Object.keys(paths).reduce((acc, key) => {
+  const aggregate = [];
-    return acc.concat(buildLookup({ model, key, paths: paths[key], filters }));
+
-  }, []);
+  // Build lookups
+  Object.keys(paths).map(key =>
+    aggregate.push(buildLookup({ key, paths: paths[key], model, filters }))
+  );
+
+  // Build the sort operation
+  if (Array.isArray(filters.sort) && !isEmpty(filters.sort)) {
+    aggregate.push({
+      $sort: filters.sort.reduce(
+        (acc, { field, order }) => set([field], sortOrderMapper[order], acc),
+        {}
+      ),
+    });
+  }
+
+  // Remove empty aggregate clauses & return
+  return reject(isEmpty, aggregate);
 };
 
 /**
@@ -657,7 +676,7 @@ const findModelByPath = ({ rootModel, path }) => {
  * Returns a model path from an attribute path and a root model
  * @param {Object} options - Options
  * @param {Object} options.rootModel - Mongoose model
- * @param {string} options.path - Attribute path
+ * @param {string|Object} options.path - Attribute path
  */
 const findModelPath = ({ rootModel, path }) => {
   const parts = (_.isObject(path) ? path.path : path).split('.');

packages/strapi-utils/lib/build-query.js

@@ -11,7 +11,7 @@ const isAttribute = (model, field) =>
 /**
  * Returns the model, attribute name and association from a path of relation
  * @param {Object} options - Options
- * @param {string} options.model - Strapi model
+ * @param {Object} options.model - Strapi model
  * @param {string} options.field - path of relation / attribute
  */
 const getAssociationFromFieldKey = ({ model, field }) => {
@@ -92,16 +92,21 @@ const normalizeFieldName = ({ model, field }) => {
 
 const BOOLEAN_OPERATORS = ['or'];
 
-const hasDeepFilters = (whereClauses = [], { minDepth = 1 } = {}) => {
+const hasDeepFilters = ({ where = [], sort = [], minDepth = 1 } = {}) => {
-  return (
+  // A query uses deep filtering if some of the clauses contains a sort or a match expression on a field of a relation
-    whereClauses.filter(({ field, operator, value }) => {
-      if (BOOLEAN_OPERATORS.includes(operator)) {
-        return value.filter(hasDeepFilters).length > 0;
-      }
 
-      return field.split('.').length > minDepth;
+  // We don't use minDepth here because deep sorting is limited to depth 1
-    }).length > 0
+  const hasDeepSortClauses = sort.some(({ field }) => field.includes('.'));
-  );
+
+  const hasDeepWhereClauses = where.some(({ field, operator, value }) => {
+    if (BOOLEAN_OPERATORS.includes(operator)) {
+      return value.some(clauses => hasDeepFilters({ where: clauses }));
+    }
+
+    return field.split('.').length > minDepth;
+  });
+
+  return hasDeepSortClauses || hasDeepWhereClauses;
 };
 
 const normalizeClauses = (whereClauses, { model }) => {
@@ -142,19 +147,28 @@ const normalizeClauses = (whereClauses, { model }) => {
  * @param {Object} options.rest - In case the database layer requires any other params pass them
  */
 const buildQuery = ({ model, filters = {}, ...rest }) => {
+  const { where, sort } = filters;
+
   // Validate query clauses
-  if (filters.where && Array.isArray(filters.where)) {
+  if ([where, sort].some(Array.isArray)) {
-    if (hasDeepFilters(filters.where, { minDepth: 2 })) {
+    if (hasDeepFilters({ where, sort, minDepth: 2 })) {
       strapi.log.warn(
         'Deep filtering queries should be used carefully (e.g Can cause performance issues).\nWhen possible build custom routes which will in most case be more optimised.'
       );
     }
 
-    // cast where clauses to match the inner types
+    if (sort) {
-    filters.where = normalizeClauses(filters.where, { model });
+      // Check that every field from the sort clauses is valid, throw with error otherwise
+      sort.forEach(({ field }) => getAssociationFromFieldKey({ model, field }));
+    }
+
+    if (where) {
+      // Cast where clauses to match the inner types
+      filters.where = normalizeClauses(where, { model });
+    }
   }
 
-  // call the orm's buildQuery implementation
+  // call the ORM's buildQuery implementation
   return strapi.db.connectors.get(model.orm).buildQuery({ model, filters, ...rest });
 };