Merge branch 'release/4.1.0'

.eslintrc.js

@@ -42,7 +42,8 @@ module.exports = {
     'prefer-destructuring': 'off',
     'prefer-object-spread': 'off',
     'import/extensions': 'off',
-    'import/prefer-default-export': 'off'
+    'import/prefer-default-export': 'off',
+    'max-classes-per-file': 'off'
   },
   env: {
     node: true,

doc/CHANGELOG.md

@@ -1,5 +1,13 @@
 # Changelog
 
+### 4.1.0
+
+- Optimized the sql query so that when only joining onto belongsTo relations, it doesn't create a separate filter query. This significantly improves performance in some specific use cases.
+
+### 4.0.1
+
+- Exported the `getPropertiesFromExpression` function on the main interface
+
 ### 4.0.0
 
 I've made this a major version bump due to the Typescript update, and the removal of Node version 8 support. There are no new features additions though, so no need for migration.
@@ -46,4 +54,4 @@ I've made this a major version bump due to the Typescript update, and the remova
 ### 1.0.0
 
 * Added filtering using nested or/and logical expressions
-* Added object-notation for eager loading and filtering
+* Added object-notation for eager loading and filtering

package.json

@@ -1,6 +1,6 @@
 {
   "name": "objection-filter",
-  "version": "4.0.1",
+  "version": "4.1.0",
   "description": "A filter module for objection.js",
   "main": "./dist/index.js",
   "types": "./dist/index.d.ts",

src/lib/FilterQueryBuilder.ts

@@ -22,7 +22,8 @@ import {
   ModelClass,
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
   Model as ObjModel,
-  OrderByDirection
+  OrderByDirection,
+  Relation
 } from 'objection';
 import * as _ from 'lodash';
 import { debug } from '../config';
@@ -388,6 +389,34 @@ const isRelatedProperty = function (name) {
   return !!sliceRelation(name).relationName;
 };
 
+/**
+ * Test all relations on a set of properties for a particular condition
+ */
+function testAllRelations<M extends BaseModel>(
+  properties: string[],
+  Model: ModelClass<M>,
+  predicate: (relation: Relation) => boolean
+) {
+  let testResult = true;
+  for (const field of properties) {
+    const { relationName } = sliceRelation(field);
+    if (!relationName) continue;
+
+    let rootModel: typeof ObjModel | ModelClass<M> = Model;
+    for (const relatedModelName of relationName.split('.')) {
+      const relation = rootModel.getRelation(relatedModelName);
+      if (!predicate(relation)) {
+        testResult = false;
+        break;
+      }
+
+      rootModel = relation.relatedModelClass;
+    }
+  }
+
+  return testResult;
+}
+
 /**
  * Apply an entire require expression to the query builder
  * e.g. { "prop1": { "$like": "A" }, "prop2": { "$in": [1] } }
@@ -430,22 +459,37 @@ export function applyRequire<M extends BaseModel>(
   const relatedPropertiesSet = propertiesSet.filter(isRelatedProperty);
   if (relatedPropertiesSet.length === 0) {
     applyLogicalExpression(filter, builder, false, getFullyQualifiedName);
-  } else {
-    const filterQuery = Model.query().distinct(...fullIdColumns);
-
-    applyLogicalExpression(filter, filterQuery, false, getFullyQualifiedName);
+    return builder;
+  }
 
-    // If there were related properties, join onto the filter
+  // If only joining belongsTo relationships, create a simpler query
+  const isOnlyJoiningToBelongsTo = testAllRelations(
+    propertiesSet,
+    Model,
+    (relation) => relation instanceof Model.BelongsToOneRelation
+  );
+  if (isOnlyJoiningToBelongsTo) {
+    // If there are only belongsTo relations, then filter on the main query
+    applyLogicalExpression(filter, builder, false, getFullyQualifiedName);
     const joinRelation = createRelationExpression(propertiesSet);
-    filterQuery.joinRelation(joinRelation);
+    builder.joinRelated(joinRelation);
+    return builder;
+  }
 
-    const filterQueryAlias = 'filter_query';
-    builder.innerJoin(filterQuery.as(filterQueryAlias), function () {
-      fullIdColumns.forEach((fullIdColumn, index) => {
-        this.on(fullIdColumn, '=', `${filterQueryAlias}.${idColumns[index]}`);
-      });
+  // If there are a hasMany or manyToMany relations, then create a separate filter query
+  const filterQuery = Model.query().distinct(...fullIdColumns);
+  applyLogicalExpression(filter, filterQuery, false, getFullyQualifiedName);
+
+  // If there were related properties, join onto the filter
+  const joinRelation = createRelationExpression(propertiesSet);
+  filterQuery.joinRelated(joinRelation);
+
+  const filterQueryAlias = 'filter_query';
+  builder.innerJoin(filterQuery.as(filterQueryAlias), function () {
+    fullIdColumns.forEach((fullIdColumn, index) => {
+      this.on(fullIdColumn, '=', `${filterQueryAlias}.${idColumns[index]}`);
     });
-  }
+  });
 
   return builder;
 }

test/complex.test.js

@@ -8,11 +8,12 @@ const { STRING_SORT } = testUtils;
 describe('complex filters', function () {
   _.each(testUtils.testDatabaseConfigs, function (knexConfig) {
     describe(knexConfig.client, function() {
-      let session, Person;
+      let session, Person, Animal;
 
       before(function () {
         session = testUtils.initialize(knexConfig);
         Person = session.models.Person;
+        Animal = session.models.Animal;
       });
 
       before(function () {
@@ -354,6 +355,68 @@ describe('complex filters', function () {
           person.pets[0].name.should.equal('P90');
         });
       });
+
+      describe('optimization', function () {
+        context('given require filter is purely belongsTo', () => {
+          it('should return base model results', async () => {
+            const result = await buildFilter(Animal)
+              .build({
+                require: {
+                  'owner.firstName': 'F00'
+                },
+                eager: 'owner'
+              });
+            result.length.should.equal(10);
+            result.map(animal => animal.owner.firstName.should.equal('F00'));
+          });
+        });
+
+        context('given require filter is not purely belongsTo', () => {
+          it('should return base model results', async () => {
+            const result = await buildFilter(Person)
+              .build({
+                require: {
+                  'pets.name': 'P00'
+                }
+              });
+            result.length.should.equal(1);
+            const person = result[0];
+            person.firstName.should.equal('F00');
+          });
+        });
+
+        context('given eager filter is purely belongsTo', () => {
+          it('should return base model results', async () => {
+            const result = await buildFilter(Animal)
+              .build({
+                eager: {
+                  $where: {
+                    'owner.firstName': 'F00'
+                  },
+                  owner: true
+                }
+              });
+            result.length.should.equal(10);
+            result.map(animal => animal.owner.firstName.should.equal('F00'));
+          });
+        });
+
+        context('given eager filter is not purely belongsTo', () => {
+          it('should return base model results', async () => {
+            const result = await buildFilter(Person)
+              .build({
+                eager: {
+                  $where: {
+                    'pets.name': 'P00'
+                  }
+                }
+              });
+            result.length.should.equal(1);
+            const person = result[0];
+            person.firstName.should.equal('F00');
+          });
+        });
+      })
     });
   });
 });