feat: introduce EntityFilter class with support for and/or filters (#1061)

* or filters interface

* implementation of new filter

* Remove duplicate code

* Parse with type guard

* Remove the newFilter variable

* Add filter and enable chaining

* test add filter

* Add another unit test

* Add system-test stubs

* Add system-tests for the OR filter

* Move things around

* Added a unit test

* Add a unit test for OR filters

* Just use filter method

* new warning test

* Revert "new warning test"

This reverts commit 37400a6.

* Now removes deprecation warning properly

* Add a test for new warning

* Add setAncestor

Adds a new setAncestor method for ensuring only one ancestor is set for the query at a time. This will avoid errors that result because of setting multiple ancestors. Also deprecate hasAncestor because it will lead to warnings like this. Add parser logic to use the value provided in setAncestor for query sent to backend.

* Basic unit tests for setAncestor

Added tests for the query proto, one to make sure the query structure is right when setting ancestor once and one to make sure the query structure is right when setting ancestor twice. Also added a unit test to make sure that ancestor is set the right way internally when using setAncestor.

* change expected result for OR query

This code change adjusts the expected result for running an OR query. Old result used to correspond with AND, but now corresponds to OR.

* Fix a test by not requiring the done callback

A test is timing out because we are waiting for done to be called. This fix does not require done to be called.

* Revert "Fix a test by not requiring the done callback"

This reverts commit 1159b37.

* Revert "Basic unit tests for setAncestor"

This reverts commit 86841d6.

* Revert "Add setAncestor"

This reverts commit e84582f.

* Separate filters and new filters internally

This commit is done to avoid a breaking typescript change which could have the potential to affect some users who read `filters` on a query as its type had been changed to be more flexible, but is now back to what it was.

* Move AND/OR into their own separate function

AND and OR should not be static functions of the filter class because then the user has to type Filter.AND instead of AND for example.

* Eliminate unused imports

Artifacts of having imports laying around and moving functionality between files

* Revert "Add a test for new warning"

This reverts commit bc15f32.

* Revert "Now removes deprecation warning properly"

This reverts commit db02a50.

* Modify test cases to capture nuances in data

We add additional asserts to the data in order to capture the nuances of the composite operator. For example, for the OR test we make sure the filter doesn’t always require both conditions to be true.

* Added comments to code that was refactored

Code for building the `filter` property of the query proto was pulled into the `Filter` object. Comments indicate how legacy functionality was maintained and which lines of code perform which task.

* rename NewFilter to entity filter

rename new filter to entity filter to eliminate the need for an internal rename that causes confusion

* Switch around last and first

These test cases have mistakes in their names. We should change them to reflect the position of `hasAncestor`.

* Change the comment to reflect the new name

The name of the base class is now EntityFilter. Adjust this comment so that it correctly matches the parameter type.

* rename and move constant map up

rename composite filter functions so that they don’t look like constants and move a map up so that it doesn’t have to be initialized every time.

* Rename newFilters to entityFilters

Author: danieljbruce

src/entity.ts

@@ -21,6 +21,7 @@ import {PathType} from '.';
 import {protobuf as Protobuf} from 'google-gax';
 import * as path from 'path';
 import {google} from '../protos/protos';
+import {and, PropertyFilter} from './filter';
 
 // eslint-disable-next-line @typescript-eslint/no-namespace
 export namespace entity {
@@ -1183,18 +1184,6 @@ export namespace entity {
    * ```
    */
   export function queryToQueryProto(query: Query): QueryProto {
-    const OP_TO_OPERATOR = {
-      '=': 'EQUAL',
-      '>': 'GREATER_THAN',
-      '>=': 'GREATER_THAN_OR_EQUAL',
-      '<': 'LESS_THAN',
-      '<=': 'LESS_THAN_OR_EQUAL',
-      HAS_ANCESTOR: 'HAS_ANCESTOR',
-      '!=': 'NOT_EQUAL',
-      IN: 'IN',
-      NOT_IN: 'NOT_IN',
-    };
-
     const SIGN_TO_ORDER = {
       '-': 'DESCENDING',
       '+': 'ASCENDING',
@@ -1249,34 +1238,20 @@ export namespace entity {
       queryProto.startCursor = query.startVal;
     }
 
-    if (query.filters.length > 0) {
-      const filters = query.filters.map(filter => {
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        let value: any = {};
-
-        if (filter.name === '__key__') {
-          value.keyValue = entity.keyToKeyProto(filter.val);
-        } else {
-          value = entity.encodeValue(filter.val, filter.name);
-        }
-
-        return {
-          propertyFilter: {
-            property: {
-              name: filter.name,
-            },
-            op: OP_TO_OPERATOR[filter.op],
-            value,
-          },
-        };
-      });
-
-      queryProto.filter = {
-        compositeFilter: {
-          filters,
-          op: 'AND',
-        },
-      };
+    // Check to see if there is at least one type of legacy filter or new filter.
+    if (query.filters.length > 0 || query.entityFilters.length > 0) {
+      // Convert all legacy filters into new property filter objects
+      const filters = query.filters.map(
+        filter => new PropertyFilter(filter.name, filter.op, filter.val)
+      );
+      const entityFilters = query.entityFilters;
+      const allFilters = entityFilters.concat(filters);
+      /*
+        To be consistent with prior implementation, apply an AND composite filter
+        to the collection of Filter objects. Then, set the filter property as before
+        to the output of the toProto method.
+       */
+      queryProto.filter = and(allFilters).toProto();
     }
 
     return queryProto;

src/filter.ts

@@ -0,0 +1,160 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import {Operator, Filter as IFilter} from './query';
+import {entity} from './entity';
+
+const OP_TO_OPERATOR = new Map([
+  ['=', 'EQUAL'],
+  ['>', 'GREATER_THAN'],
+  ['>=', 'GREATER_THAN_OR_EQUAL'],
+  ['<', 'LESS_THAN'],
+  ['<=', 'LESS_THAN_OR_EQUAL'],
+  ['HAS_ANCESTOR', 'HAS_ANCESTOR'],
+  ['!=', 'NOT_EQUAL'],
+  ['IN', 'IN'],
+  ['NOT_IN', 'NOT_IN'],
+]);
+
+enum CompositeOperator {
+  AND = 'AND',
+  OR = 'OR',
+}
+
+export function and(filters: EntityFilter[]): CompositeFilter {
+  return new CompositeFilter(filters, CompositeOperator.AND);
+}
+
+export function or(filters: EntityFilter[]): CompositeFilter {
+  return new CompositeFilter(filters, CompositeOperator.OR);
+}
+
+/**
+ * A Filter is a class that contains data for a filter that can be translated
+ * into a proto when needed.
+ *
+ * @see {@link https://cloud.google.com/datastore/docs/concepts/queries#filters| Filters Reference}
+ *
+ */
+export abstract class EntityFilter {
+  /**
+   * Gets the proto for the filter.
+   *
+   */
+  // eslint-disable-next-line
+  abstract toProto(): any;
+}
+
+/**
+ * A PropertyFilter is a filter that gets applied to a query directly.
+ *
+ * @see {@link https://cloud.google.com/datastore/docs/concepts/queries#property_filters| Property filters Reference}
+ *
+ * @class
+ */
+export class PropertyFilter extends EntityFilter implements IFilter {
+  name: string;
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  val: any;
+  op: Operator;
+
+  /**
+   * Build a Property Filter object.
+   *
+   * @param {string} Property
+   * @param {Operator} operator
+   * @param {any} val
+   */
+  constructor(property: string, operator: Operator, val: any) {
+    super();
+    this.name = property;
+    this.op = operator;
+    this.val = val;
+  }
+
+  private encodedValue(): any {
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    let value: any = {};
+    if (this.name === '__key__') {
+      value.keyValue = entity.keyToKeyProto(this.val);
+    } else {
+      value = entity.encodeValue(this.val, this.name);
+    }
+    return value;
+  }
+
+  /**
+   * Gets the proto for the filter.
+   *
+   */
+  // eslint-disable-next-line
+  toProto(): any {
+    const value = new PropertyFilter(
+      this.name,
+      this.op,
+      this.val
+    ).encodedValue();
+    return {
+      propertyFilter: {
+        property: {
+          name: this.name,
+        },
+        op: OP_TO_OPERATOR.get(this.op),
+        value,
+      },
+    };
+  }
+}
+
+/**
+ * A CompositeFilter is a filter that combines other filters and applies that
+ * combination to a query.
+ *
+ * @see {@link https://cloud.google.com/datastore/docs/concepts/queries#composite_filters| Composite filters Reference}
+ *
+ * @class
+ */
+class CompositeFilter extends EntityFilter {
+  filters: EntityFilter[];
+  op: string;
+
+  /**
+   * Build a Composite Filter object.
+   *
+   * @param {EntityFilter[]} filters
+   */
+  constructor(filters: EntityFilter[], op: CompositeOperator) {
+    super();
+    this.filters = filters;
+    this.op = op;
+  }
+
+  /**
+   * Gets the proto for the filter.
+   *
+   */
+  // eslint-disable-next-line
+  toProto(): any {
+    return {
+      compositeFilter: {
+        filters: this.filters.map(filter => filter.toProto()),
+        op: this.op,
+      },
+    };
+  }
+}
+
+export function isFilter(filter: any): filter is EntityFilter {
+  return (filter as EntityFilter).toProto !== undefined;
+}

src/query.ts

@@ -18,10 +18,10 @@ import arrify = require('arrify');
 import {Key} from 'readline';
 import {Datastore} from '.';
 import {Entity} from './entity';
+import {EntityFilter, isFilter} from './filter';
 import {Transaction} from './transaction';
 import {CallOptions} from 'google-gax';
 import {RunQueryStreamOptions} from '../src/request';
-import {AggregateField, AggregateQuery} from './aggregate';
 
 export type Operator =
   | '='
@@ -76,6 +76,7 @@ class Query {
   namespace?: string | null;
   kinds: string[];
   filters: Filter[];
+  entityFilters: EntityFilter[];
   orders: Order[];
   groupByVal: Array<{}>;
   selectVal: Array<{}>;
@@ -123,6 +124,11 @@ class Query {
      * @type {array}
      */
     this.filters = [];
+    /**
+     * @name Query#entityFilters
+     * @type {array}
+     */
+    this.entityFilters = [];
     /**
      * @name Query#orders
      * @type {array}
@@ -170,7 +176,7 @@ class Query {
    *
    * @see {@link https://cloud.google.com/datastore/docs/concepts/queries#datastore-property-filter-nodejs| Datastore Filters}
    *
-   * @param {string} property The field name.
+   * @param {string | EntityFilter} propertyOrFilter The field name.
    * @param {string} [operator="="] Operator (=, <, >, <=, >=).
    * @param {*} value Value to compare property to.
    * @returns {Query}
@@ -201,24 +207,29 @@ class Query {
    * const keyQuery = query.filter('__key__', key);
    * ```
    */
-  filter(property: string, value: {} | null): Query;
-  filter(property: string, operator: Operator, value: {} | null): Query;
+  filter(propertyOrFilter: string | EntityFilter, value?: {} | null): Query;
+  filter(propertyOrFilter: string, operator: Operator, value: {} | null): Query;
   filter(
-    property: string,
-    operatorOrValue: Operator,
+    propertyOrFilter: string | EntityFilter,
+    operatorOrValue?: Operator,
     value?: {} | null
   ): Query {
-    let operator = operatorOrValue as Operator;
-    if (arguments.length === 2) {
-      value = operatorOrValue as {};
-      operator = '=';
-    }
+    if (isFilter(propertyOrFilter)) {
+      this.entityFilters.push(propertyOrFilter);
+      return this;
+    } else {
+      let operator = operatorOrValue as Operator;
+      if (arguments.length === 2) {
+        value = operatorOrValue as {};
+        operator = '=';
+      }
 
-    this.filters.push({
-      name: property.trim(),
-      op: operator.trim() as Operator,
-      val: value,
-    });
+      this.filters.push({
+        name: (propertyOrFilter as String).trim(),
+        op: operator.trim() as Operator,
+        val: value,
+      });
+    }
     return this;
   }
 

system-test/data/index.yaml

@@ -16,6 +16,12 @@ indexes:
   - name: family
   - name: appearances
 
+- kind: Character
+  ancestor: no
+  properties:
+  - name: family
+  - name: appearances
+
 - kind: Character
   ancestor: yes
   properties: