From d370b541f0cea4e1b4eb5e9c16215f690b8b4790 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Fri, 13 Jan 2023 17:28:24 -0500 Subject: [PATCH 01/37] or filters interface --- src/filter.ts | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 src/filter.ts diff --git a/src/filter.ts b/src/filter.ts new file mode 100644 index 000000000..ef81e1ddc --- /dev/null +++ b/src/filter.ts @@ -0,0 +1,52 @@ +// 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. + +/** + * 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} + * + */ +abstract class Filter { + /** + * 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 + */ +class PropertyFilter { + +} + +/** + * 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 { + +} From 3291df865883b260a712fd6d1ef733431f70e193 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Mon, 16 Jan 2023 11:54:22 -0500 Subject: [PATCH 02/37] implementation of new filter --- src/entity.ts | 41 ++++++--------------- src/filter.ts | 100 ++++++++++++++++++++++++++++++++++++++++++++++++-- src/query.ts | 25 ++++++++++++- 3 files changed, 132 insertions(+), 34 deletions(-) diff --git a/src/entity.ts b/src/entity.ts index 49dddf63d..5fc87e0c9 100644 --- a/src/entity.ts +++ b/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 {PropertyFilter} from './filter'; // eslint-disable-next-line @typescript-eslint/no-namespace export namespace entity { @@ -1186,17 +1187,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', @@ -1252,28 +1242,19 @@ 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 (query.newFilter && query.filters.length > 0) { + throw new Error('setFilter and filter functions cannot both be used on one query.'); + } - if (filter.name === '__key__') { - value.keyValue = entity.keyToKeyProto(filter.val); - } else { - value = entity.encodeValue(filter.val, filter.name); - } + if (query.newFilter) { + queryProto.filter = query.newFilter.toProto(); + } - return { - propertyFilter: { - property: { - name: filter.name, - }, - op: OP_TO_OPERATOR[filter.op], - value, - }, - }; + if (query.filters.length > 0) { + const filters = query.filters.map(filter => { + return (new PropertyFilter(filter.name, filter.op, filter.val)) + .toProto(); }); - queryProto.filter = { compositeFilter: { filters, diff --git a/src/filter.ts b/src/filter.ts index ef81e1ddc..db1d2cb2c 100644 --- a/src/filter.ts +++ b/src/filter.ts @@ -12,6 +12,21 @@ // 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'], +]); + /** * A Filter is a class that contains data for a filter that can be translated * into a proto when needed. @@ -19,7 +34,14 @@ * @see {@link https://cloud.google.com/datastore/docs/concepts/queries#filters| Filters Reference} * */ -abstract class Filter { +export abstract class Filter { + AND(filters: Filter[]): CompositeFilter { + return new CompositeFilter(filters, 'AND'); + } + + OR(filters: Filter[]): CompositeFilter { + return new CompositeFilter(filters, 'OR'); + } /** * Gets the proto for the filter. * @@ -35,8 +57,54 @@ abstract class Filter { * * @class */ -class PropertyFilter { +export class PropertyFilter extends Filter 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 + } + } + } } /** @@ -47,6 +115,32 @@ class PropertyFilter { * * @class */ -class CompositeFilter { +class CompositeFilter extends Filter { + filters: Filter[]; + op: string; + + /** + * Build a Composite Filter object. + * + * @param {Filter[]} filters + */ + constructor(filters: Filter[], op: string) { + 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 + } + } + } } diff --git a/src/query.ts b/src/query.ts index cbc464de7..864be64c0 100644 --- a/src/query.ts +++ b/src/query.ts @@ -18,10 +18,10 @@ import arrify = require('arrify'); import {Key} from 'readline'; import {Datastore} from '.'; import {Entity} from './entity'; +import {Filter as NewFilter} 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[]; + newFilter: NewFilter | undefined; orders: Order[]; groupByVal: Array<{}>; selectVal: Array<{}>; @@ -320,6 +321,28 @@ class Query { return this; } + /** + * Set the filter that this query is going to process. + * + * @see {@link https://cloud.google.com/datastore/docs/concepts/queries#filters| Filters Reference} + * + * @param {NewFilter} filter The filter that will be applied to the query + * + * @example + * ``` + * const {Datastore} = require('@google-cloud/datastore'); + * const datastore = new Datastore(); + * const companyQuery = datastore.createQuery('Company'); + * const filter = new PropertyFilter('state', '=', 'CA'); + * + * // Set the filter. + * const filteredQuery = companyQuery.setFilter(filter); + * ``` + */ + setFilter(filter: NewFilter) { + this.newFilter = filter; + } + /** * Set a starting cursor to a query. * From 69390d4bfd8c5de013806c80e0d9b84fbe31dd6e Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Mon, 16 Jan 2023 13:26:22 -0500 Subject: [PATCH 03/37] Remove duplicate code --- src/entity.ts | 10 ++-------- src/filter.ts | 4 ++-- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/entity.ts b/src/entity.ts index 5fc87e0c9..33f08b84e 100644 --- a/src/entity.ts +++ b/src/entity.ts @@ -21,7 +21,7 @@ import {PathType} from '.'; import {protobuf as Protobuf} from 'google-gax'; import * as path from 'path'; import {google} from '../protos/protos'; -import {PropertyFilter} from './filter'; +import {Filter, PropertyFilter} from './filter'; // eslint-disable-next-line @typescript-eslint/no-namespace export namespace entity { @@ -1253,14 +1253,8 @@ export namespace entity { if (query.filters.length > 0) { const filters = query.filters.map(filter => { return (new PropertyFilter(filter.name, filter.op, filter.val)) - .toProto(); }); - queryProto.filter = { - compositeFilter: { - filters, - op: 'AND', - }, - }; + queryProto.filter = Filter.AND(filters).toProto(); } return queryProto; diff --git a/src/filter.ts b/src/filter.ts index db1d2cb2c..e8744f460 100644 --- a/src/filter.ts +++ b/src/filter.ts @@ -35,11 +35,11 @@ const OP_TO_OPERATOR = new Map([ * */ export abstract class Filter { - AND(filters: Filter[]): CompositeFilter { + static AND(filters: Filter[]): CompositeFilter { return new CompositeFilter(filters, 'AND'); } - OR(filters: Filter[]): CompositeFilter { + static OR(filters: Filter[]): CompositeFilter { return new CompositeFilter(filters, 'OR'); } /** From c90e932b731da37aae9b44d45d149f3a8ce2a597 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Mon, 16 Jan 2023 16:31:30 -0500 Subject: [PATCH 04/37] Parse with type guard --- src/entity.ts | 6 ++++-- src/filter.ts | 4 ++++ test/entity.ts | 22 ++++++++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/entity.ts b/src/entity.ts index 33f08b84e..1b132aefe 100644 --- a/src/entity.ts +++ b/src/entity.ts @@ -21,7 +21,7 @@ import {PathType} from '.'; import {protobuf as Protobuf} from 'google-gax'; import * as path from 'path'; import {google} from '../protos/protos'; -import {Filter, PropertyFilter} from './filter'; +import {Filter, isFilter, PropertyFilter} from './filter'; // eslint-disable-next-line @typescript-eslint/no-namespace export namespace entity { @@ -1252,7 +1252,9 @@ export namespace entity { if (query.filters.length > 0) { const filters = query.filters.map(filter => { - return (new PropertyFilter(filter.name, filter.op, filter.val)) + return isFilter(filter) ? + filter : + new PropertyFilter(filter.name, filter.op, filter.val) }); queryProto.filter = Filter.AND(filters).toProto(); } diff --git a/src/filter.ts b/src/filter.ts index e8744f460..3f613dd9e 100644 --- a/src/filter.ts +++ b/src/filter.ts @@ -144,3 +144,7 @@ class CompositeFilter extends Filter { } } } + +export function isFilter(filter: any): filter is Filter { + return (filter as Filter).toProto !== undefined +} diff --git a/test/entity.ts b/test/entity.ts index e79d6360d..58ee265c3 100644 --- a/test/entity.ts +++ b/test/entity.ts @@ -1891,6 +1891,28 @@ describe('entity', () => { assert.deepStrictEqual(entity.queryToQueryProto(query), queryProto); }); + it('should support the setFilter method', () => { + const ancestorKey = new entity.Key({ + path: ['Kind2', 'somename'], + }); + + const ds = new Datastore({projectId: 'project-id'}); + + const query = ds + .createQuery('Kind1') + .filter('name', 'John') + .start('start') + .end('end') + .groupBy(['name']) + .order('name') + .select('name') + .limit(1) + .offset(1) + .hasAncestor(ancestorKey); + + assert.deepStrictEqual(entity.queryToQueryProto(query), queryProto); + }); + it('should handle buffer start and end values', () => { const ds = new Datastore({projectId: 'project-id'}); const startVal = Buffer.from('start'); From c2b29fc03288fd99aa36723dc452f1b91d414438 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Mon, 16 Jan 2023 16:40:49 -0500 Subject: [PATCH 05/37] Remove the newFilter variable --- src/entity.ts | 8 -------- src/query.ts | 5 ++--- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/entity.ts b/src/entity.ts index 1b132aefe..1499eda05 100644 --- a/src/entity.ts +++ b/src/entity.ts @@ -1242,14 +1242,6 @@ export namespace entity { queryProto.startCursor = query.startVal; } - if (query.newFilter && query.filters.length > 0) { - throw new Error('setFilter and filter functions cannot both be used on one query.'); - } - - if (query.newFilter) { - queryProto.filter = query.newFilter.toProto(); - } - if (query.filters.length > 0) { const filters = query.filters.map(filter => { return isFilter(filter) ? diff --git a/src/query.ts b/src/query.ts index 864be64c0..53e61a119 100644 --- a/src/query.ts +++ b/src/query.ts @@ -75,8 +75,7 @@ class Query { scope?: Datastore | Transaction; namespace?: string | null; kinds: string[]; - filters: Filter[]; - newFilter: NewFilter | undefined; + filters: (Filter | NewFilter)[]; orders: Order[]; groupByVal: Array<{}>; selectVal: Array<{}>; @@ -340,7 +339,7 @@ class Query { * ``` */ setFilter(filter: NewFilter) { - this.newFilter = filter; + this.filters.push(filter); } /** From 69f1cbff97e4f02e349874344d315829bc48b48d Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Mon, 16 Jan 2023 16:53:04 -0500 Subject: [PATCH 06/37] Add filter and enable chaining --- src/query.ts | 7 ++++--- test/entity.ts | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/query.ts b/src/query.ts index 53e61a119..b1e2e0145 100644 --- a/src/query.ts +++ b/src/query.ts @@ -321,7 +321,7 @@ class Query { } /** - * Set the filter that this query is going to process. + * Add a filter that this query is going to process. * * @see {@link https://cloud.google.com/datastore/docs/concepts/queries#filters| Filters Reference} * @@ -335,11 +335,12 @@ class Query { * const filter = new PropertyFilter('state', '=', 'CA'); * * // Set the filter. - * const filteredQuery = companyQuery.setFilter(filter); + * const filteredQuery = companyQuery.addFilter(filter); * ``` */ - setFilter(filter: NewFilter) { + addFilter(filter: NewFilter) { this.filters.push(filter); + return this; } /** diff --git a/test/entity.ts b/test/entity.ts index 58ee265c3..84fb28e27 100644 --- a/test/entity.ts +++ b/test/entity.ts @@ -1891,7 +1891,7 @@ describe('entity', () => { assert.deepStrictEqual(entity.queryToQueryProto(query), queryProto); }); - it('should support the setFilter method', () => { + it('should support the addFilter method', () => { const ancestorKey = new entity.Key({ path: ['Kind2', 'somename'], }); From 3f720c0208956c4146b07a05463af4354e83fd41 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Mon, 16 Jan 2023 17:03:24 -0500 Subject: [PATCH 07/37] test add filter --- test/entity.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/entity.ts b/test/entity.ts index 84fb28e27..5f07bbd4e 100644 --- a/test/entity.ts +++ b/test/entity.ts @@ -19,6 +19,7 @@ import * as sinon from 'sinon'; import {Datastore} from '../src'; import {Entity} from '../src/entity'; import {IntegerTypeCastOptions} from '../src/query'; +import {PropertyFilter} from '../src/filter'; export function outOfBoundsError(opts: { propertyName?: string; @@ -1900,7 +1901,7 @@ describe('entity', () => { const query = ds .createQuery('Kind1') - .filter('name', 'John') + .addFilter(new PropertyFilter('name', '=', 'John')) .start('start') .end('end') .groupBy(['name']) @@ -1909,7 +1910,6 @@ describe('entity', () => { .limit(1) .offset(1) .hasAncestor(ancestorKey); - assert.deepStrictEqual(entity.queryToQueryProto(query), queryProto); }); From 20f616bab749c9988a10be50193d4540ed9e780a Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Mon, 16 Jan 2023 17:10:56 -0500 Subject: [PATCH 08/37] Add another unit test --- test/query.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/query.ts b/test/query.ts index 8e1b7b107..b96620192 100644 --- a/test/query.ts +++ b/test/query.ts @@ -19,6 +19,7 @@ const {Query} = require('../src/query'); // eslint-disable-next-line @typescript-eslint/no-var-requires import {Datastore} from '../src'; import {AggregateField, AggregateQuery} from '../src/aggregate'; +import {PropertyFilter} from '../src/filter'; describe('Query', () => { const SCOPE = {} as Datastore; @@ -165,6 +166,19 @@ describe('Query', () => { }); }); + describe('addFilter', () => { + it('should support addFilter', () => { + const now = new Date(); + const query = new Query(['kind1']) + .addFilter(new PropertyFilter('date', '<=', now)); + const filter = query.filters[0]; + + assert.strictEqual(filter.name, 'date'); + assert.strictEqual(filter.op, '<='); + assert.strictEqual(filter.val, now); + }); + }) + describe('hasAncestor', () => { it('should support ancestor filtering', () => { const query = new Query(['kind1']).hasAncestor(['kind2', 123]); From 5dea24a3cfbe92a6ffcdacac9e97c2ff7543ac92 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Tue, 17 Jan 2023 14:41:26 -0500 Subject: [PATCH 09/37] Add system-test stubs --- src/entity.ts | 7 +++---- src/filter.ts | 20 ++++++++++++-------- system-test/datastore.ts | 11 +++++++++++ test/entity.ts | 20 ++++++++++---------- test/query.ts | 7 ++++--- 5 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/entity.ts b/src/entity.ts index 1499eda05..994629263 100644 --- a/src/entity.ts +++ b/src/entity.ts @@ -1187,7 +1187,6 @@ export namespace entity { * ``` */ export function queryToQueryProto(query: Query): QueryProto { - const SIGN_TO_ORDER = { '-': 'DESCENDING', '+': 'ASCENDING', @@ -1244,9 +1243,9 @@ export namespace entity { if (query.filters.length > 0) { const filters = query.filters.map(filter => { - return isFilter(filter) ? - filter : - new PropertyFilter(filter.name, filter.op, filter.val) + return isFilter(filter) + ? filter + : new PropertyFilter(filter.name, filter.op, filter.val); }); queryProto.filter = Filter.AND(filters).toProto(); } diff --git a/src/filter.ts b/src/filter.ts index 3f613dd9e..7d7972037 100644 --- a/src/filter.ts +++ b/src/filter.ts @@ -94,16 +94,20 @@ export class PropertyFilter extends Filter implements IFilter { */ // eslint-disable-next-line toProto(): any { - const value = (new PropertyFilter(this.name, this.op, this.val)).encodedValue(); + 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 - } - } + value, + }, + }; } } @@ -139,12 +143,12 @@ class CompositeFilter extends Filter { return { compositeFilter: { filters: this.filters.map(filter => filter.toProto()), - op: this.op - } - } + op: this.op, + }, + }; } } export function isFilter(filter: any): filter is Filter { - return (filter as Filter).toProto !== undefined + return (filter as Filter).toProto !== undefined; } diff --git a/system-test/datastore.ts b/system-test/datastore.ts index ae0a473ac..3dd21ed56 100644 --- a/system-test/datastore.ts +++ b/system-test/datastore.ts @@ -798,6 +798,17 @@ describe('Datastore', () => { const [entities] = await datastore.runQuery(q); assert.strictEqual(entities!.length, 6); }); + describe('with the addFilter function', () => { + it('should run a query with one property filter', async () => { + assert.strictEqual(6, 6); + }); + it('should run a query using an AND composite filter', async () => { + assert.strictEqual(6, 6); + }); + it('should run a query using an OR composite filter', async () => { + assert.strictEqual(6, 6); + }); + }); describe('with a count filter', () => { it('should run a count aggregation', async () => { const q = datastore.createQuery('Character'); diff --git a/test/entity.ts b/test/entity.ts index 5f07bbd4e..bdcc01a3e 100644 --- a/test/entity.ts +++ b/test/entity.ts @@ -1900,16 +1900,16 @@ describe('entity', () => { const ds = new Datastore({projectId: 'project-id'}); const query = ds - .createQuery('Kind1') - .addFilter(new PropertyFilter('name', '=', 'John')) - .start('start') - .end('end') - .groupBy(['name']) - .order('name') - .select('name') - .limit(1) - .offset(1) - .hasAncestor(ancestorKey); + .createQuery('Kind1') + .addFilter(new PropertyFilter('name', '=', 'John')) + .start('start') + .end('end') + .groupBy(['name']) + .order('name') + .select('name') + .limit(1) + .offset(1) + .hasAncestor(ancestorKey); assert.deepStrictEqual(entity.queryToQueryProto(query), queryProto); }); diff --git a/test/query.ts b/test/query.ts index b96620192..06054e680 100644 --- a/test/query.ts +++ b/test/query.ts @@ -169,15 +169,16 @@ describe('Query', () => { describe('addFilter', () => { it('should support addFilter', () => { const now = new Date(); - const query = new Query(['kind1']) - .addFilter(new PropertyFilter('date', '<=', now)); + const query = new Query(['kind1']).addFilter( + new PropertyFilter('date', '<=', now) + ); const filter = query.filters[0]; assert.strictEqual(filter.name, 'date'); assert.strictEqual(filter.op, '<='); assert.strictEqual(filter.val, now); }); - }) + }); describe('hasAncestor', () => { it('should support ancestor filtering', () => { From bb931e700915872e2cb86895c2d59cda885397b1 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Wed, 18 Jan 2023 17:04:43 -0500 Subject: [PATCH 10/37] Add system-tests for the OR filter --- system-test/data/index.yaml | 6 ++++ system-test/datastore.ts | 72 +++++++++++++++++++++++++++++++++++-- 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/system-test/data/index.yaml b/system-test/data/index.yaml index 5a2d2b1a8..0900b970f 100644 --- a/system-test/data/index.yaml +++ b/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: diff --git a/system-test/datastore.ts b/system-test/datastore.ts index 3dd21ed56..c4f1bdd18 100644 --- a/system-test/datastore.ts +++ b/system-test/datastore.ts @@ -21,6 +21,7 @@ import {Datastore, Index} from '../src'; import {google} from '../protos/protos'; import {Storage} from '@google-cloud/storage'; import {AggregateField} from '../src/aggregate'; +import {PropertyFilter, Filter} from '../src/filter'; describe('Datastore', () => { const testKinds: string[] = []; @@ -800,13 +801,78 @@ describe('Datastore', () => { }); describe('with the addFilter function', () => { it('should run a query with one property filter', async () => { - assert.strictEqual(6, 6); + const q = datastore + .createQuery('Character') + .addFilter(new PropertyFilter('family', '=', 'Stark')) + .hasAncestor(ancestor); + const [entities] = await datastore.runQuery(q); + assert.strictEqual(entities!.length, 8); + }); + it('should run a query with two property filters', async () => { + const q = datastore + .createQuery('Character') + .addFilter(new PropertyFilter('family', '=', 'Stark')) + .addFilter(new PropertyFilter('appearances', '>=', 20)); + const [entities] = await datastore.runQuery(q); + assert.strictEqual(entities!.length, 6); + }); + it('should run a query using addFilter and filter', async () => { + const q = datastore + .createQuery('Character') + .filter('family', 'Stark') + .addFilter(new PropertyFilter('appearances', '>=', 20)); + const [entities] = await datastore.runQuery(q); + assert.strictEqual(entities!.length, 6); }); it('should run a query using an AND composite filter', async () => { - assert.strictEqual(6, 6); + const q = datastore + .createQuery('Character') + .addFilter( + Filter.AND([ + new PropertyFilter('family', '=', 'Stark'), + new PropertyFilter('appearances', '>=', 20), + ]) + ); + const [entities] = await datastore.runQuery(q); + assert.strictEqual(entities!.length, 6); }); it('should run a query using an OR composite filter', async () => { - assert.strictEqual(6, 6); + const q = datastore + .createQuery('Character') + .addFilter( + Filter.OR([ + new PropertyFilter('family', '=', 'Stark'), + new PropertyFilter('appearances', '>=', 20), + ]) + ); + const [entities] = await datastore.runQuery(q); + assert.strictEqual(entities!.length, 6); + }); + describe('using hasAncestor and addFilter', () => { + const secondAncestor = datastore.key([ + 'Book', + 'GoT', + 'Character', + 'Rickard', + 'Character', + 'Eddard', + ]); + it('should run a query using hasAncestor first', async () => { + const q = datastore + .createQuery('Character') + .addFilter(new PropertyFilter('appearances', '<', 30)) + .hasAncestor(secondAncestor); + const [entities] = await datastore.runQuery(q); + assert.strictEqual(entities!.length, 3); + }); + it('should run a query using hasAncestor last', async () => { + const q = datastore + .createQuery('Character') + .hasAncestor(secondAncestor) + .addFilter(new PropertyFilter('appearances', '<', 30)); + const [entities] = await datastore.runQuery(q); + assert.strictEqual(entities!.length, 3); + }); }); }); describe('with a count filter', () => { From 44be8a9c6ef7e1b142e0b70f98f6a1fe7728904c Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Thu, 19 Jan 2023 10:05:20 -0500 Subject: [PATCH 11/37] Move things around --- src/filter.ts | 23 +++++++++++------------ system-test/datastore.ts | 3 ++- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/filter.ts b/src/filter.ts index 7d7972037..dfbccb050 100644 --- a/src/filter.ts +++ b/src/filter.ts @@ -15,18 +15,6 @@ 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'], -]); - /** * A Filter is a class that contains data for a filter that can be translated * into a proto when needed. @@ -94,6 +82,17 @@ export class PropertyFilter extends Filter implements IFilter { */ // eslint-disable-next-line toProto(): any { + 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'], + ]); const value = new PropertyFilter( this.name, this.op, diff --git a/system-test/datastore.ts b/system-test/datastore.ts index c4f1bdd18..f048c1148 100644 --- a/system-test/datastore.ts +++ b/system-test/datastore.ts @@ -801,9 +801,10 @@ describe('Datastore', () => { }); describe('with the addFilter function', () => { it('should run a query with one property filter', async () => { + const filter = new PropertyFilter('family', '=', 'Stark'); const q = datastore .createQuery('Character') - .addFilter(new PropertyFilter('family', '=', 'Stark')) + .addFilter(filter) .hasAncestor(ancestor); const [entities] = await datastore.runQuery(q); assert.strictEqual(entities!.length, 8); From b6bb3b3758a6f848c6640f5fd50728aff5356487 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Thu, 19 Jan 2023 13:30:23 -0500 Subject: [PATCH 12/37] Added a unit test --- test/entity.ts | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/test/entity.ts b/test/entity.ts index bdcc01a3e..f8802e18c 100644 --- a/test/entity.ts +++ b/test/entity.ts @@ -19,7 +19,7 @@ import * as sinon from 'sinon'; import {Datastore} from '../src'; import {Entity} from '../src/entity'; import {IntegerTypeCastOptions} from '../src/query'; -import {PropertyFilter} from '../src/filter'; +import {PropertyFilter, Filter} from '../src/filter'; export function outOfBoundsError(opts: { propertyName?: string; @@ -1913,6 +1913,34 @@ describe('entity', () => { assert.deepStrictEqual(entity.queryToQueryProto(query), queryProto); }); + it('should support the addFilter method with AND', () => { + const ancestorKey = new entity.Key({ + path: ['Kind2', 'somename'], + }); + + const ds = new Datastore({projectId: 'project-id'}); + + const query = ds + .createQuery('Kind1') + .addFilter( + Filter.AND([ + new PropertyFilter('name', '=', 'John'), + new PropertyFilter('__key__', 'HAS_ANCESTOR', ancestorKey), + ]) + ) + .start('start') + .end('end') + .groupBy(['name']) + .order('name') + .select('name') + .limit(1) + .offset(1); + const testFilters = queryProto.filter; + const computedFilters = + entity.queryToQueryProto(query).filter.compositeFilter.filters[0]; + assert.deepStrictEqual(computedFilters, testFilters); + }); + it('should handle buffer start and end values', () => { const ds = new Datastore({projectId: 'project-id'}); const startVal = Buffer.from('start'); From c4fe30473948483550fe2dac6b29ff6e7f5544f7 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Thu, 19 Jan 2023 13:51:10 -0500 Subject: [PATCH 13/37] Add a unit test for OR filters --- test/query.ts | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/test/query.ts b/test/query.ts index 06054e680..5bb27356b 100644 --- a/test/query.ts +++ b/test/query.ts @@ -19,7 +19,7 @@ const {Query} = require('../src/query'); // eslint-disable-next-line @typescript-eslint/no-var-requires import {Datastore} from '../src'; import {AggregateField, AggregateQuery} from '../src/aggregate'; -import {PropertyFilter} from '../src/filter'; +import {PropertyFilter, Filter} from '../src/filter'; describe('Query', () => { const SCOPE = {} as Datastore; @@ -178,6 +178,26 @@ describe('Query', () => { assert.strictEqual(filter.op, '<='); assert.strictEqual(filter.val, now); }); + it('should support addFilter with OR', () => { + const now = new Date(); + const query = new Query(['kind1']).addFilter( + Filter.OR([ + new PropertyFilter('date', '<=', now), + new PropertyFilter('name', '=', 'Stephen'), + ]) + ); + const filter = query.filters[0]; + assert.strictEqual(filter.op, 'OR'); + // Check filters + const filters = filter.filters; + assert.strictEqual(filters.length, 2); + assert.strictEqual(filters[0].name, 'date'); + assert.strictEqual(filters[0].op, '<='); + assert.strictEqual(filters[0].val, now); + assert.strictEqual(filters[1].name, 'name'); + assert.strictEqual(filters[1].op, '='); + assert.strictEqual(filters[1].val, 'Stephen'); + }); }); describe('hasAncestor', () => { From c39760850937de6f61d933810c6bf808f2a65c28 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Wed, 25 Jan 2023 15:57:03 -0500 Subject: [PATCH 14/37] Just use filter method --- src/filter.ts | 11 +++++-- src/query.ts | 63 ++++++++++++++++------------------------ system-test/datastore.ts | 22 +++++++------- test/entity.ts | 8 ++--- test/query.ts | 10 +++---- 5 files changed, 53 insertions(+), 61 deletions(-) diff --git a/src/filter.ts b/src/filter.ts index dfbccb050..31f954aca 100644 --- a/src/filter.ts +++ b/src/filter.ts @@ -15,6 +15,11 @@ import {Operator, Filter as IFilter} from './query'; import {entity} from './entity'; +enum CompositeOperator { + AND = 'AND', + OR = 'OR', +} + /** * A Filter is a class that contains data for a filter that can be translated * into a proto when needed. @@ -24,11 +29,11 @@ import {entity} from './entity'; */ export abstract class Filter { static AND(filters: Filter[]): CompositeFilter { - return new CompositeFilter(filters, 'AND'); + return new CompositeFilter(filters, CompositeOperator.AND); } static OR(filters: Filter[]): CompositeFilter { - return new CompositeFilter(filters, 'OR'); + return new CompositeFilter(filters, CompositeOperator.OR); } /** * Gets the proto for the filter. @@ -127,7 +132,7 @@ class CompositeFilter extends Filter { * * @param {Filter[]} filters */ - constructor(filters: Filter[], op: string) { + constructor(filters: Filter[], op: CompositeOperator) { super(); this.filters = filters; this.op = op; diff --git a/src/query.ts b/src/query.ts index b1e2e0145..be125935e 100644 --- a/src/query.ts +++ b/src/query.ts @@ -18,10 +18,11 @@ import arrify = require('arrify'); import {Key} from 'readline'; import {Datastore} from '.'; import {Entity} from './entity'; -import {Filter as NewFilter} from './filter'; +import {Filter as NewFilter, isFilter} from './filter'; import {Transaction} from './transaction'; import {CallOptions} from 'google-gax'; import {RunQueryStreamOptions} from '../src/request'; +import {string} from 'is'; export type Operator = | '=' @@ -170,7 +171,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 | NewFilter} propertyOrFilter The field name. * @param {string} [operator="="] Operator (=, <, >, <=, >=). * @param {*} value Value to compare property to. * @returns {Query} @@ -201,20 +202,29 @@ class Query { * const keyQuery = query.filter('__key__', key); * ``` */ - filter(property: string, value: {}): Query; - filter(property: string, operator: Operator, value: {}): Query; - filter(property: string, operatorOrValue: Operator, value?: {}): Query { - let operator = operatorOrValue as Operator; - if (arguments.length === 2) { - value = operatorOrValue as {}; - operator = '='; + filter(propertyOrFilter: string | NewFilter, value?: {}): Query; + filter(propertyOrFilter: string, operator: Operator, value: {}): Query; + filter( + propertyOrFilter: string | NewFilter, + operatorOrValue?: Operator, + value?: {} + ): Query { + if (isFilter(propertyOrFilter)) { + this.filters.push(propertyOrFilter); + return this; + } else { + let operator = operatorOrValue as Operator; + if (arguments.length === 2) { + value = operatorOrValue as {}; + operator = '='; + } + + this.filters.push({ + name: (propertyOrFilter as String).trim(), + op: operator.trim() as Operator, + val: value, + }); } - - this.filters.push({ - name: property.trim(), - op: operator.trim() as Operator, - val: value, - }); return this; } @@ -320,29 +330,6 @@ class Query { return this; } - /** - * Add a filter that this query is going to process. - * - * @see {@link https://cloud.google.com/datastore/docs/concepts/queries#filters| Filters Reference} - * - * @param {NewFilter} filter The filter that will be applied to the query - * - * @example - * ``` - * const {Datastore} = require('@google-cloud/datastore'); - * const datastore = new Datastore(); - * const companyQuery = datastore.createQuery('Company'); - * const filter = new PropertyFilter('state', '=', 'CA'); - * - * // Set the filter. - * const filteredQuery = companyQuery.addFilter(filter); - * ``` - */ - addFilter(filter: NewFilter) { - this.filters.push(filter); - return this; - } - /** * Set a starting cursor to a query. * diff --git a/system-test/datastore.ts b/system-test/datastore.ts index f048c1148..9ad07a6a2 100644 --- a/system-test/datastore.ts +++ b/system-test/datastore.ts @@ -799,12 +799,12 @@ describe('Datastore', () => { const [entities] = await datastore.runQuery(q); assert.strictEqual(entities!.length, 6); }); - describe('with the addFilter function', () => { + describe('with the filter function using the Filter class', () => { it('should run a query with one property filter', async () => { const filter = new PropertyFilter('family', '=', 'Stark'); const q = datastore .createQuery('Character') - .addFilter(filter) + .filter(filter) .hasAncestor(ancestor); const [entities] = await datastore.runQuery(q); assert.strictEqual(entities!.length, 8); @@ -812,23 +812,23 @@ describe('Datastore', () => { it('should run a query with two property filters', async () => { const q = datastore .createQuery('Character') - .addFilter(new PropertyFilter('family', '=', 'Stark')) - .addFilter(new PropertyFilter('appearances', '>=', 20)); + .filter(new PropertyFilter('family', '=', 'Stark')) + .filter(new PropertyFilter('appearances', '>=', 20)); const [entities] = await datastore.runQuery(q); assert.strictEqual(entities!.length, 6); }); - it('should run a query using addFilter and filter', async () => { + it('should run a query using new Filter class with filter', async () => { const q = datastore .createQuery('Character') .filter('family', 'Stark') - .addFilter(new PropertyFilter('appearances', '>=', 20)); + .filter(new PropertyFilter('appearances', '>=', 20)); const [entities] = await datastore.runQuery(q); assert.strictEqual(entities!.length, 6); }); it('should run a query using an AND composite filter', async () => { const q = datastore .createQuery('Character') - .addFilter( + .filter( Filter.AND([ new PropertyFilter('family', '=', 'Stark'), new PropertyFilter('appearances', '>=', 20), @@ -840,7 +840,7 @@ describe('Datastore', () => { it('should run a query using an OR composite filter', async () => { const q = datastore .createQuery('Character') - .addFilter( + .filter( Filter.OR([ new PropertyFilter('family', '=', 'Stark'), new PropertyFilter('appearances', '>=', 20), @@ -849,7 +849,7 @@ describe('Datastore', () => { const [entities] = await datastore.runQuery(q); assert.strictEqual(entities!.length, 6); }); - describe('using hasAncestor and addFilter', () => { + describe('using hasAncestor and Filter class', () => { const secondAncestor = datastore.key([ 'Book', 'GoT', @@ -861,7 +861,7 @@ describe('Datastore', () => { it('should run a query using hasAncestor first', async () => { const q = datastore .createQuery('Character') - .addFilter(new PropertyFilter('appearances', '<', 30)) + .filter(new PropertyFilter('appearances', '<', 30)) .hasAncestor(secondAncestor); const [entities] = await datastore.runQuery(q); assert.strictEqual(entities!.length, 3); @@ -870,7 +870,7 @@ describe('Datastore', () => { const q = datastore .createQuery('Character') .hasAncestor(secondAncestor) - .addFilter(new PropertyFilter('appearances', '<', 30)); + .filter(new PropertyFilter('appearances', '<', 30)); const [entities] = await datastore.runQuery(q); assert.strictEqual(entities!.length, 3); }); diff --git a/test/entity.ts b/test/entity.ts index f8802e18c..d9348a144 100644 --- a/test/entity.ts +++ b/test/entity.ts @@ -1892,7 +1892,7 @@ describe('entity', () => { assert.deepStrictEqual(entity.queryToQueryProto(query), queryProto); }); - it('should support the addFilter method', () => { + it('should support the filter method with Filter objects', () => { const ancestorKey = new entity.Key({ path: ['Kind2', 'somename'], }); @@ -1901,7 +1901,7 @@ describe('entity', () => { const query = ds .createQuery('Kind1') - .addFilter(new PropertyFilter('name', '=', 'John')) + .filter(new PropertyFilter('name', '=', 'John')) .start('start') .end('end') .groupBy(['name']) @@ -1913,7 +1913,7 @@ describe('entity', () => { assert.deepStrictEqual(entity.queryToQueryProto(query), queryProto); }); - it('should support the addFilter method with AND', () => { + it('should support the filter method with AND', () => { const ancestorKey = new entity.Key({ path: ['Kind2', 'somename'], }); @@ -1922,7 +1922,7 @@ describe('entity', () => { const query = ds .createQuery('Kind1') - .addFilter( + .filter( Filter.AND([ new PropertyFilter('name', '=', 'John'), new PropertyFilter('__key__', 'HAS_ANCESTOR', ancestorKey), diff --git a/test/query.ts b/test/query.ts index 5bb27356b..dd803dcbb 100644 --- a/test/query.ts +++ b/test/query.ts @@ -166,10 +166,10 @@ describe('Query', () => { }); }); - describe('addFilter', () => { - it('should support addFilter', () => { + describe('filter with Filter class', () => { + it('should support filter with Filter', () => { const now = new Date(); - const query = new Query(['kind1']).addFilter( + const query = new Query(['kind1']).filter( new PropertyFilter('date', '<=', now) ); const filter = query.filters[0]; @@ -178,9 +178,9 @@ describe('Query', () => { assert.strictEqual(filter.op, '<='); assert.strictEqual(filter.val, now); }); - it('should support addFilter with OR', () => { + it('should support filter with OR', () => { const now = new Date(); - const query = new Query(['kind1']).addFilter( + const query = new Query(['kind1']).filter( Filter.OR([ new PropertyFilter('date', '<=', now), new PropertyFilter('name', '=', 'Stephen'), From 37400a6d3a9d7385a0203db70949f037e4c35de4 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Thu, 26 Jan 2023 17:17:09 -0500 Subject: [PATCH 15/37] new warning test --- src/query.ts | 3 +++ test/query.ts | 11 +++++++++++ 2 files changed, 14 insertions(+) diff --git a/src/query.ts b/src/query.ts index be125935e..17011c4f4 100644 --- a/src/query.ts +++ b/src/query.ts @@ -213,6 +213,9 @@ class Query { this.filters.push(propertyOrFilter); return this; } else { + process.emitWarning( + 'Providing Filter objects is recommended when using .filter' + ); let operator = operatorOrValue as Operator; if (arguments.length === 2) { value = operatorOrValue as {}; diff --git a/test/query.ts b/test/query.ts index dd803dcbb..4d165284a 100644 --- a/test/query.ts +++ b/test/query.ts @@ -164,6 +164,17 @@ describe('Query', () => { assert.strictEqual(filter.op, '='); assert.strictEqual(filter.val, 'Stephen'); }); + + it('should issue a warning when a Filter instance is not provided', done => { + new Query(['kind1']).filter('name', 'Stephen'); + process.on('warning', warning => { + assert.strictEqual( + warning.message, + 'Providing Filter objects is recommended when using .filter' + ); + done(); + }); + }); }); describe('filter with Filter class', () => { From 8ab6b808c8406384904b82f10bbbc6db5616933b Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Thu, 26 Jan 2023 17:17:31 -0500 Subject: [PATCH 16/37] Revert "new warning test" This reverts commit 37400a6d3a9d7385a0203db70949f037e4c35de4. --- src/query.ts | 3 --- test/query.ts | 11 ----------- 2 files changed, 14 deletions(-) diff --git a/src/query.ts b/src/query.ts index 17011c4f4..be125935e 100644 --- a/src/query.ts +++ b/src/query.ts @@ -213,9 +213,6 @@ class Query { this.filters.push(propertyOrFilter); return this; } else { - process.emitWarning( - 'Providing Filter objects is recommended when using .filter' - ); let operator = operatorOrValue as Operator; if (arguments.length === 2) { value = operatorOrValue as {}; diff --git a/test/query.ts b/test/query.ts index 4d165284a..dd803dcbb 100644 --- a/test/query.ts +++ b/test/query.ts @@ -164,17 +164,6 @@ describe('Query', () => { assert.strictEqual(filter.op, '='); assert.strictEqual(filter.val, 'Stephen'); }); - - it('should issue a warning when a Filter instance is not provided', done => { - new Query(['kind1']).filter('name', 'Stephen'); - process.on('warning', warning => { - assert.strictEqual( - warning.message, - 'Providing Filter objects is recommended when using .filter' - ); - done(); - }); - }); }); describe('filter with Filter class', () => { From db02a5089242b25b4bebae4304a4778f0bf6d17f Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Fri, 27 Jan 2023 09:47:29 -0500 Subject: [PATCH 17/37] Now removes deprecation warning properly --- src/query.ts | 3 +++ test/entity.ts | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/query.ts b/src/query.ts index be125935e..17011c4f4 100644 --- a/src/query.ts +++ b/src/query.ts @@ -213,6 +213,9 @@ class Query { this.filters.push(propertyOrFilter); return this; } else { + process.emitWarning( + 'Providing Filter objects is recommended when using .filter' + ); let operator = operatorOrValue as Operator; if (arguments.length === 2) { value = operatorOrValue as {}; diff --git a/test/entity.ts b/test/entity.ts index d9348a144..e967b7667 100644 --- a/test/entity.ts +++ b/test/entity.ts @@ -770,10 +770,12 @@ describe('entity', () => { "' property is outside of bounds of a JavaScript Number.\n" + "Use 'Datastore.int()' to preserve accuracy during the upload."; - process.on('warning', warning => { + const onWarning = (warning: {message: unknown}) => { assert.strictEqual(warning.message, expectedWarning); + process.removeListener('warning', onWarning); done(); - }); + }; + process.on('warning', onWarning); entity.encodeValue(largeIntValue, property); }); From bc15f325d87b4ffe46ae8727293c5cefa2300501 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Fri, 27 Jan 2023 09:52:59 -0500 Subject: [PATCH 18/37] Add a test for new warning --- test/query.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/query.ts b/test/query.ts index dd803dcbb..60f29e177 100644 --- a/test/query.ts +++ b/test/query.ts @@ -164,6 +164,19 @@ describe('Query', () => { assert.strictEqual(filter.op, '='); assert.strictEqual(filter.val, 'Stephen'); }); + + it('should issue a warning when a Filter instance is not provided', done => { + const onWarning = (warning: {message: unknown}) => { + assert.strictEqual( + warning.message, + 'Providing Filter objects is recommended when using .filter' + ); + process.removeListener('warning', onWarning); + done(); + }; + process.on('warning', onWarning); + new Query(['kind1']).filter('name', 'Stephen'); + }); }); describe('filter with Filter class', () => { From e84582faa6041429a4e81d47bd21dbc338a639e7 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Tue, 7 Feb 2023 11:05:06 -0500 Subject: [PATCH 19/37] 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. --- src/entity.ts | 5 +++++ src/query.ts | 25 +++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/src/entity.ts b/src/entity.ts index 994629263..0a54462cb 100644 --- a/src/entity.ts +++ b/src/entity.ts @@ -1247,6 +1247,11 @@ export namespace entity { ? filter : new PropertyFilter(filter.name, filter.op, filter.val); }); + if (typeof query.ancestor !== 'undefined') { + filters.push( + new PropertyFilter('__key__', 'HAS_ANCESTOR', query.ancestor) + ); + } queryProto.filter = Filter.AND(filters).toProto(); } diff --git a/src/query.ts b/src/query.ts index 17011c4f4..042d4fdd8 100644 --- a/src/query.ts +++ b/src/query.ts @@ -73,6 +73,7 @@ export interface Filter { * ``` */ class Query { + ancestor?: Key; scope?: Datastore | Transaction; namespace?: string | null; kinds: string[]; @@ -246,12 +247,36 @@ class Query { * const query = datastore.createQuery('MyKind'); * const ancestoryQuery = query.hasAncestor(datastore.key(['Parent', 123])); * ``` + * + * @deprecated Use setAncestor(). */ hasAncestor(key: Key) { this.filters.push({name: '__key__', op: 'HAS_ANCESTOR', val: key}); return this; } + /** + * Filter a query by ancestors. + * + * @see {@link https://cloud.google.com/datastore/docs/concepts/queries#datastore-ancestor-query-nodejs| Datastore Ancestor Filters} + * + * @param {Key} key Key object to filter by. + * @returns {Query} + * + * @example + * ``` + * const {Datastore} = require('@google-cloud/datastore'); + * const datastore = new Datastore(); + * const query = datastore.createQuery('MyKind'); + * const ancestorQuery = query.setAncestor(datastore.key(['Parent', 123])); + * ``` + * + */ + setAncestor(key: Key) { + this.ancestor = key; + return this; + } + /** * Sort the results by a property name in ascending or descending order. By * default, an ascending sort order will be used. From 86841d6e0a3d26e2df647fa4a42ed849b61b38a6 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Tue, 7 Feb 2023 11:32:20 -0500 Subject: [PATCH 20/37] 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. --- test/entity.ts | 46 ++++++++++++++++++++++++++++++++++++++++++++++ test/query.ts | 8 +++++++- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/test/entity.ts b/test/entity.ts index e967b7667..a8c7a224f 100644 --- a/test/entity.ts +++ b/test/entity.ts @@ -1915,6 +1915,52 @@ describe('entity', () => { assert.deepStrictEqual(entity.queryToQueryProto(query), queryProto); }); + it('should support the filter method with setAncestor', () => { + const ancestorKey = new entity.Key({ + path: ['Kind2', 'somename'], + }); + + const ds = new Datastore({projectId: 'project-id'}); + + const query = ds + .createQuery('Kind1') + .filter(new PropertyFilter('name', '=', 'John')) + .start('start') + .end('end') + .groupBy(['name']) + .order('name') + .select('name') + .limit(1) + .offset(1) + .setAncestor(ancestorKey); + assert.deepStrictEqual(entity.queryToQueryProto(query), queryProto); + }); + + it('should support the filter method using setAncestor twice', () => { + const oldAncestorKey = new entity.Key({ + path: ['Kind1', 'somename1'], + }); + const ancestorKey = new entity.Key({ + path: ['Kind2', 'somename'], + }); + + const ds = new Datastore({projectId: 'project-id'}); + + const query = ds + .createQuery('Kind1') + .filter(new PropertyFilter('name', '=', 'John')) + .start('start') + .end('end') + .groupBy(['name']) + .order('name') + .select('name') + .limit(1) + .offset(1) + .setAncestor(oldAncestorKey) + .setAncestor(ancestorKey); + assert.deepStrictEqual(entity.queryToQueryProto(query), queryProto); + }); + it('should support the filter method with AND', () => { const ancestorKey = new entity.Key({ path: ['Kind2', 'somename'], diff --git a/test/query.ts b/test/query.ts index 60f29e177..af5806630 100644 --- a/test/query.ts +++ b/test/query.ts @@ -178,7 +178,13 @@ describe('Query', () => { new Query(['kind1']).filter('name', 'Stephen'); }); }); - + describe('setAncestor', () => { + it('should set the ancestor for the query to search against', done => { + const key = ['kind2', 123]; + const query = new Query(['kind1']).setAncestor(key); + assert.deepStrictEqual(query.ancestor, key); + }); + }); describe('filter with Filter class', () => { it('should support filter with Filter', () => { const now = new Date(); From fe50d56cbbe30c1a50003f242c3b14c075f15d08 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Wed, 8 Feb 2023 13:04:40 -0500 Subject: [PATCH 21/37] 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. --- system-test/datastore.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system-test/datastore.ts b/system-test/datastore.ts index 9ad07a6a2..c45df6262 100644 --- a/system-test/datastore.ts +++ b/system-test/datastore.ts @@ -847,7 +847,7 @@ describe('Datastore', () => { ]) ); const [entities] = await datastore.runQuery(q); - assert.strictEqual(entities!.length, 6); + assert.strictEqual(entities!.length, 8); }); describe('using hasAncestor and Filter class', () => { const secondAncestor = datastore.key([ From 1159b374f483d107736a765b5b4f1ade80116ce3 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Thu, 16 Feb 2023 15:02:51 -0500 Subject: [PATCH 22/37] 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. --- test/query.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/query.ts b/test/query.ts index af5806630..38630ffc9 100644 --- a/test/query.ts +++ b/test/query.ts @@ -179,7 +179,7 @@ describe('Query', () => { }); }); describe('setAncestor', () => { - it('should set the ancestor for the query to search against', done => { + it('should set the ancestor for the query to search against', () => { const key = ['kind2', 123]; const query = new Query(['kind1']).setAncestor(key); assert.deepStrictEqual(query.ancestor, key); From a7a8d27ea5c7ad9c33d2db79b822f2307a9921e2 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Mon, 27 Feb 2023 15:52:26 -0500 Subject: [PATCH 23/37] Revert "Fix a test by not requiring the done callback" This reverts commit 1159b374f483d107736a765b5b4f1ade80116ce3. --- test/query.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/query.ts b/test/query.ts index 3c19208bb..8896fdb74 100644 --- a/test/query.ts +++ b/test/query.ts @@ -179,7 +179,7 @@ describe('Query', () => { }); }); describe('setAncestor', () => { - it('should set the ancestor for the query to search against', () => { + it('should set the ancestor for the query to search against', done => { const key = ['kind2', 123]; const query = new Query(['kind1']).setAncestor(key); assert.deepStrictEqual(query.ancestor, key); From 1c77b6ca226b634c13baef8f0a6d6b973f908cb5 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Mon, 27 Feb 2023 15:53:10 -0500 Subject: [PATCH 24/37] Revert "Basic unit tests for setAncestor" This reverts commit 86841d6e0a3d26e2df647fa4a42ed849b61b38a6. --- test/entity.ts | 46 ---------------------------------------------- test/query.ts | 8 +------- 2 files changed, 1 insertion(+), 53 deletions(-) diff --git a/test/entity.ts b/test/entity.ts index c753b1ffa..7a4ba7a79 100644 --- a/test/entity.ts +++ b/test/entity.ts @@ -1915,52 +1915,6 @@ describe('entity', () => { assert.deepStrictEqual(entity.queryToQueryProto(query), queryProto); }); - it('should support the filter method with setAncestor', () => { - const ancestorKey = new entity.Key({ - path: ['Kind2', 'somename'], - }); - - const ds = new Datastore({projectId: 'project-id'}); - - const query = ds - .createQuery('Kind1') - .filter(new PropertyFilter('name', '=', 'John')) - .start('start') - .end('end') - .groupBy(['name']) - .order('name') - .select('name') - .limit(1) - .offset(1) - .setAncestor(ancestorKey); - assert.deepStrictEqual(entity.queryToQueryProto(query), queryProto); - }); - - it('should support the filter method using setAncestor twice', () => { - const oldAncestorKey = new entity.Key({ - path: ['Kind1', 'somename1'], - }); - const ancestorKey = new entity.Key({ - path: ['Kind2', 'somename'], - }); - - const ds = new Datastore({projectId: 'project-id'}); - - const query = ds - .createQuery('Kind1') - .filter(new PropertyFilter('name', '=', 'John')) - .start('start') - .end('end') - .groupBy(['name']) - .order('name') - .select('name') - .limit(1) - .offset(1) - .setAncestor(oldAncestorKey) - .setAncestor(ancestorKey); - assert.deepStrictEqual(entity.queryToQueryProto(query), queryProto); - }); - it('should support the filter method with AND', () => { const ancestorKey = new entity.Key({ path: ['Kind2', 'somename'], diff --git a/test/query.ts b/test/query.ts index 8896fdb74..8758d7e68 100644 --- a/test/query.ts +++ b/test/query.ts @@ -178,13 +178,7 @@ describe('Query', () => { new Query(['kind1']).filter('name', 'Stephen'); }); }); - describe('setAncestor', () => { - it('should set the ancestor for the query to search against', done => { - const key = ['kind2', 123]; - const query = new Query(['kind1']).setAncestor(key); - assert.deepStrictEqual(query.ancestor, key); - }); - }); + describe('filter with Filter class', () => { it('should support filter with Filter', () => { const now = new Date(); From 5f38179546b838f43a3700ccea20db79105b4c9b Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Mon, 27 Feb 2023 15:53:32 -0500 Subject: [PATCH 25/37] Revert "Add setAncestor" This reverts commit e84582faa6041429a4e81d47bd21dbc338a639e7. --- src/entity.ts | 5 ----- src/query.ts | 25 ------------------------- 2 files changed, 30 deletions(-) diff --git a/src/entity.ts b/src/entity.ts index 14278b389..050771195 100644 --- a/src/entity.ts +++ b/src/entity.ts @@ -1244,11 +1244,6 @@ export namespace entity { ? filter : new PropertyFilter(filter.name, filter.op, filter.val); }); - if (typeof query.ancestor !== 'undefined') { - filters.push( - new PropertyFilter('__key__', 'HAS_ANCESTOR', query.ancestor) - ); - } queryProto.filter = Filter.AND(filters).toProto(); } diff --git a/src/query.ts b/src/query.ts index 8c48349a7..80ba89726 100644 --- a/src/query.ts +++ b/src/query.ts @@ -73,7 +73,6 @@ export interface Filter { * ``` */ class Query { - ancestor?: Key; scope?: Datastore | Transaction; namespace?: string | null; kinds: string[]; @@ -247,36 +246,12 @@ class Query { * const query = datastore.createQuery('MyKind'); * const ancestoryQuery = query.hasAncestor(datastore.key(['Parent', 123])); * ``` - * - * @deprecated Use setAncestor(). */ hasAncestor(key: Key) { this.filters.push({name: '__key__', op: 'HAS_ANCESTOR', val: key}); return this; } - /** - * Filter a query by ancestors. - * - * @see {@link https://cloud.google.com/datastore/docs/concepts/queries#datastore-ancestor-query-nodejs| Datastore Ancestor Filters} - * - * @param {Key} key Key object to filter by. - * @returns {Query} - * - * @example - * ``` - * const {Datastore} = require('@google-cloud/datastore'); - * const datastore = new Datastore(); - * const query = datastore.createQuery('MyKind'); - * const ancestorQuery = query.setAncestor(datastore.key(['Parent', 123])); - * ``` - * - */ - setAncestor(key: Key) { - this.ancestor = key; - return this; - } - /** * Sort the results by a property name in ascending or descending order. By * default, an ascending sort order will be used. From 059d4dc42a0f55b68484ee20f841eabafb2b4f74 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Mon, 27 Feb 2023 16:54:23 -0500 Subject: [PATCH 26/37] 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. --- src/entity.ts | 13 ++++++------- src/query.ts | 10 ++++++++-- test/query.ts | 4 ++-- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/entity.ts b/src/entity.ts index 050771195..ae705961c 100644 --- a/src/entity.ts +++ b/src/entity.ts @@ -1238,13 +1238,12 @@ export namespace entity { queryProto.startCursor = query.startVal; } - if (query.filters.length > 0) { - const filters = query.filters.map(filter => { - return isFilter(filter) - ? filter - : new PropertyFilter(filter.name, filter.op, filter.val); - }); - queryProto.filter = Filter.AND(filters).toProto(); + if (query.filters.length > 0 || query.newFilters.length > 0) { + const filters = query.filters.map( + filter => new PropertyFilter(filter.name, filter.op, filter.val) + ); + const newFilters = query.newFilters; + queryProto.filter = Filter.AND(newFilters.concat(filters)).toProto(); } return queryProto; diff --git a/src/query.ts b/src/query.ts index 80ba89726..810c45779 100644 --- a/src/query.ts +++ b/src/query.ts @@ -76,7 +76,8 @@ class Query { scope?: Datastore | Transaction; namespace?: string | null; kinds: string[]; - filters: (Filter | NewFilter)[]; + filters: Filter[]; + newFilters: NewFilter[]; orders: Order[]; groupByVal: Array<{}>; selectVal: Array<{}>; @@ -124,6 +125,11 @@ class Query { * @type {array} */ this.filters = []; + /** + * @name Query#newFilters + * @type {array} + */ + this.newFilters = []; /** * @name Query#orders * @type {array} @@ -210,7 +216,7 @@ class Query { value?: {} | null ): Query { if (isFilter(propertyOrFilter)) { - this.filters.push(propertyOrFilter); + this.newFilters.push(propertyOrFilter); return this; } else { process.emitWarning( diff --git a/test/query.ts b/test/query.ts index 8758d7e68..3525d2d01 100644 --- a/test/query.ts +++ b/test/query.ts @@ -185,7 +185,7 @@ describe('Query', () => { const query = new Query(['kind1']).filter( new PropertyFilter('date', '<=', now) ); - const filter = query.filters[0]; + const filter = query.newFilters[0]; assert.strictEqual(filter.name, 'date'); assert.strictEqual(filter.op, '<='); @@ -199,7 +199,7 @@ describe('Query', () => { new PropertyFilter('name', '=', 'Stephen'), ]) ); - const filter = query.filters[0]; + const filter = query.newFilters[0]; assert.strictEqual(filter.op, 'OR'); // Check filters const filters = filter.filters; From ec752e827897ff67068168e45559d3bd1c9c9c9b Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Wed, 1 Mar 2023 09:57:38 -0500 Subject: [PATCH 27/37] 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. --- src/entity.ts | 4 ++-- src/filter.ts | 15 ++++++++------- system-test/datastore.ts | 6 +++--- test/entity.ts | 4 ++-- test/query.ts | 4 ++-- 5 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/entity.ts b/src/entity.ts index ae705961c..46f046966 100644 --- a/src/entity.ts +++ b/src/entity.ts @@ -21,7 +21,7 @@ import {PathType} from '.'; import {protobuf as Protobuf} from 'google-gax'; import * as path from 'path'; import {google} from '../protos/protos'; -import {Filter, isFilter, PropertyFilter} from './filter'; +import {AND, Filter, isFilter, PropertyFilter} from './filter'; // eslint-disable-next-line @typescript-eslint/no-namespace export namespace entity { @@ -1243,7 +1243,7 @@ export namespace entity { filter => new PropertyFilter(filter.name, filter.op, filter.val) ); const newFilters = query.newFilters; - queryProto.filter = Filter.AND(newFilters.concat(filters)).toProto(); + queryProto.filter = AND(newFilters.concat(filters)).toProto(); } return queryProto; diff --git a/src/filter.ts b/src/filter.ts index 31f954aca..7c0092de4 100644 --- a/src/filter.ts +++ b/src/filter.ts @@ -20,6 +20,14 @@ enum CompositeOperator { OR = 'OR', } +export function AND(filters: Filter[]): CompositeFilter { + return new CompositeFilter(filters, CompositeOperator.AND); +} + +export function OR(filters: Filter[]): 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. @@ -28,13 +36,6 @@ enum CompositeOperator { * */ export abstract class Filter { - static AND(filters: Filter[]): CompositeFilter { - return new CompositeFilter(filters, CompositeOperator.AND); - } - - static OR(filters: Filter[]): CompositeFilter { - return new CompositeFilter(filters, CompositeOperator.OR); - } /** * Gets the proto for the filter. * diff --git a/system-test/datastore.ts b/system-test/datastore.ts index 4e8bbc737..86880ff3a 100644 --- a/system-test/datastore.ts +++ b/system-test/datastore.ts @@ -21,7 +21,7 @@ import {Datastore, Index} from '../src'; import {google} from '../protos/protos'; import {Storage} from '@google-cloud/storage'; import {AggregateField} from '../src/aggregate'; -import {PropertyFilter, Filter} from '../src/filter'; +import {PropertyFilter, Filter, AND, OR} from '../src/filter'; describe('Datastore', () => { const testKinds: string[] = []; @@ -842,7 +842,7 @@ describe('Datastore', () => { const q = datastore .createQuery('Character') .filter( - Filter.AND([ + AND([ new PropertyFilter('family', '=', 'Stark'), new PropertyFilter('appearances', '>=', 20), ]) @@ -854,7 +854,7 @@ describe('Datastore', () => { const q = datastore .createQuery('Character') .filter( - Filter.OR([ + OR([ new PropertyFilter('family', '=', 'Stark'), new PropertyFilter('appearances', '>=', 20), ]) diff --git a/test/entity.ts b/test/entity.ts index 7a4ba7a79..2c31d6443 100644 --- a/test/entity.ts +++ b/test/entity.ts @@ -19,7 +19,7 @@ import * as sinon from 'sinon'; import {Datastore} from '../src'; import {Entity} from '../src/entity'; import {IntegerTypeCastOptions} from '../src/query'; -import {PropertyFilter, Filter} from '../src/filter'; +import {PropertyFilter, Filter, AND} from '../src/filter'; export function outOfBoundsError(opts: { propertyName?: string; @@ -1925,7 +1925,7 @@ describe('entity', () => { const query = ds .createQuery('Kind1') .filter( - Filter.AND([ + AND([ new PropertyFilter('name', '=', 'John'), new PropertyFilter('__key__', 'HAS_ANCESTOR', ancestorKey), ]) diff --git a/test/query.ts b/test/query.ts index 3525d2d01..992dbc3d0 100644 --- a/test/query.ts +++ b/test/query.ts @@ -19,7 +19,7 @@ const {Query} = require('../src/query'); // eslint-disable-next-line @typescript-eslint/no-var-requires import {Datastore} from '../src'; import {AggregateField, AggregateQuery} from '../src/aggregate'; -import {PropertyFilter, Filter} from '../src/filter'; +import {PropertyFilter, Filter, OR} from '../src/filter'; describe('Query', () => { const SCOPE = {} as Datastore; @@ -194,7 +194,7 @@ describe('Query', () => { it('should support filter with OR', () => { const now = new Date(); const query = new Query(['kind1']).filter( - Filter.OR([ + OR([ new PropertyFilter('date', '<=', now), new PropertyFilter('name', '=', 'Stephen'), ]) From a5c41ab533c3cf5abf06fce5d62eb251bfaaca36 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Wed, 1 Mar 2023 10:02:44 -0500 Subject: [PATCH 28/37] Eliminate unused imports Artifacts of having imports laying around and moving functionality between files --- src/entity.ts | 2 +- src/query.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/entity.ts b/src/entity.ts index 46f046966..1b8c95ce4 100644 --- a/src/entity.ts +++ b/src/entity.ts @@ -21,7 +21,7 @@ import {PathType} from '.'; import {protobuf as Protobuf} from 'google-gax'; import * as path from 'path'; import {google} from '../protos/protos'; -import {AND, Filter, isFilter, PropertyFilter} from './filter'; +import {AND, PropertyFilter} from './filter'; // eslint-disable-next-line @typescript-eslint/no-namespace export namespace entity { diff --git a/src/query.ts b/src/query.ts index 810c45779..54fa918c0 100644 --- a/src/query.ts +++ b/src/query.ts @@ -22,7 +22,6 @@ import {Filter as NewFilter, isFilter} from './filter'; import {Transaction} from './transaction'; import {CallOptions} from 'google-gax'; import {RunQueryStreamOptions} from '../src/request'; -import {string} from 'is'; export type Operator = | '=' From 1ddeea6a75d01ad3c24e46d85e31eca73235790a Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Wed, 1 Mar 2023 10:09:55 -0500 Subject: [PATCH 29/37] Revert "Add a test for new warning" This reverts commit bc15f325d87b4ffe46ae8727293c5cefa2300501. --- test/query.ts | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/test/query.ts b/test/query.ts index 992dbc3d0..b98f23ee5 100644 --- a/test/query.ts +++ b/test/query.ts @@ -164,19 +164,6 @@ describe('Query', () => { assert.strictEqual(filter.op, '='); assert.strictEqual(filter.val, 'Stephen'); }); - - it('should issue a warning when a Filter instance is not provided', done => { - const onWarning = (warning: {message: unknown}) => { - assert.strictEqual( - warning.message, - 'Providing Filter objects is recommended when using .filter' - ); - process.removeListener('warning', onWarning); - done(); - }; - process.on('warning', onWarning); - new Query(['kind1']).filter('name', 'Stephen'); - }); }); describe('filter with Filter class', () => { From 8e96bf5e5ef11593853ae061fd1858c2e463d6ce Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Wed, 1 Mar 2023 10:11:56 -0500 Subject: [PATCH 30/37] Revert "Now removes deprecation warning properly" This reverts commit db02a5089242b25b4bebae4304a4778f0bf6d17f. --- src/query.ts | 3 --- test/entity.ts | 6 ++---- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/query.ts b/src/query.ts index 54fa918c0..66a9d6466 100644 --- a/src/query.ts +++ b/src/query.ts @@ -218,9 +218,6 @@ class Query { this.newFilters.push(propertyOrFilter); return this; } else { - process.emitWarning( - 'Providing Filter objects is recommended when using .filter' - ); let operator = operatorOrValue as Operator; if (arguments.length === 2) { value = operatorOrValue as {}; diff --git a/test/entity.ts b/test/entity.ts index 2c31d6443..c83c01e1e 100644 --- a/test/entity.ts +++ b/test/entity.ts @@ -770,12 +770,10 @@ describe('entity', () => { "' property is outside of bounds of a JavaScript Number.\n" + "Use 'Datastore.int()' to preserve accuracy during the upload."; - const onWarning = (warning: {message: unknown}) => { + process.on('warning', warning => { assert.strictEqual(warning.message, expectedWarning); - process.removeListener('warning', onWarning); done(); - }; - process.on('warning', onWarning); + }); entity.encodeValue(largeIntValue, property); }); From 58830962c298cc7ae599b630d229dbcfb957999f Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Tue, 7 Mar 2023 10:12:39 -0500 Subject: [PATCH 31/37] Modify test cases to capture nuances in data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- system-test/datastore.ts | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/system-test/datastore.ts b/system-test/datastore.ts index 86880ff3a..e6af95cde 100644 --- a/system-test/datastore.ts +++ b/system-test/datastore.ts @@ -837,6 +837,14 @@ describe('Datastore', () => { .filter(new PropertyFilter('appearances', '>=', 20)); const [entities] = await datastore.runQuery(q); assert.strictEqual(entities!.length, 6); + for (const entity of entities) { + if (Array.isArray(entity.family)) { + assert.strictEqual(entity.family[0], 'Stark'); + } else { + assert.strictEqual(entity.family, 'Stark'); + } + assert(entity.appearances >= 20); + } }); it('should run a query using an AND composite filter', async () => { const q = datastore @@ -849,6 +857,14 @@ describe('Datastore', () => { ); const [entities] = await datastore.runQuery(q); assert.strictEqual(entities!.length, 6); + for (const entity of entities) { + if (Array.isArray(entity.family)) { + assert.strictEqual(entity.family[0], 'Stark'); + } else { + assert.strictEqual(entity.family, 'Stark'); + } + assert(entity.appearances >= 20); + } }); it('should run a query using an OR composite filter', async () => { const q = datastore @@ -861,6 +877,17 @@ describe('Datastore', () => { ); const [entities] = await datastore.runQuery(q); assert.strictEqual(entities!.length, 8); + let atLeastOne = false; + for (const entity of entities) { + const familyHasStark = Array.isArray(entity.family) + ? entity.family[0] === 'Stark' + : entity.family === 'Stark'; + const hasEnoughAppearances = entity.appearances >= 20; + if (familyHasStark && !hasEnoughAppearances) { + atLeastOne = true; + } + } + assert(atLeastOne); }); describe('using hasAncestor and Filter class', () => { const secondAncestor = datastore.key([ From 0a68ed1261980c4040ef159b60e882aa7c52ff8c Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Tue, 7 Mar 2023 10:58:36 -0500 Subject: [PATCH 32/37] 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. --- src/entity.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/entity.ts b/src/entity.ts index 1b8c95ce4..e390028dd 100644 --- a/src/entity.ts +++ b/src/entity.ts @@ -1238,12 +1238,20 @@ export namespace entity { queryProto.startCursor = query.startVal; } + // Check to see if there is at least one type of legacy filter or new filter. if (query.filters.length > 0 || query.newFilters.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 newFilters = query.newFilters; - queryProto.filter = AND(newFilters.concat(filters)).toProto(); + const allFilters = newFilters.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; From 35bae06dd7ae3491fa1b48c00c873c2cc802eedf Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Tue, 7 Mar 2023 17:21:35 -0500 Subject: [PATCH 33/37] rename NewFilter to entity filter rename new filter to entity filter to eliminate the need for an internal rename that causes confusion --- src/entity.ts | 4 ++-- src/filter.ts | 20 ++++++++++---------- src/query.ts | 14 +++++++------- system-test/datastore.ts | 2 +- test/entity.ts | 2 +- test/query.ts | 6 +++--- 6 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/entity.ts b/src/entity.ts index e390028dd..e044feaf4 100644 --- a/src/entity.ts +++ b/src/entity.ts @@ -1239,12 +1239,12 @@ export namespace entity { } // Check to see if there is at least one type of legacy filter or new filter. - if (query.filters.length > 0 || query.newFilters.length > 0) { + 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 newFilters = query.newFilters; + const newFilters = query.entityFilters; const allFilters = newFilters.concat(filters); /* To be consistent with prior implementation, apply an AND composite filter diff --git a/src/filter.ts b/src/filter.ts index 7c0092de4..52a99a447 100644 --- a/src/filter.ts +++ b/src/filter.ts @@ -20,11 +20,11 @@ enum CompositeOperator { OR = 'OR', } -export function AND(filters: Filter[]): CompositeFilter { +export function AND(filters: EntityFilter[]): CompositeFilter { return new CompositeFilter(filters, CompositeOperator.AND); } -export function OR(filters: Filter[]): CompositeFilter { +export function OR(filters: EntityFilter[]): CompositeFilter { return new CompositeFilter(filters, CompositeOperator.OR); } @@ -35,7 +35,7 @@ export function OR(filters: Filter[]): CompositeFilter { * @see {@link https://cloud.google.com/datastore/docs/concepts/queries#filters| Filters Reference} * */ -export abstract class Filter { +export abstract class EntityFilter { /** * Gets the proto for the filter. * @@ -51,7 +51,7 @@ export abstract class Filter { * * @class */ -export class PropertyFilter extends Filter implements IFilter { +export class PropertyFilter extends EntityFilter implements IFilter { name: string; // eslint-disable-next-line @typescript-eslint/no-explicit-any val: any; @@ -124,16 +124,16 @@ export class PropertyFilter extends Filter implements IFilter { * * @class */ -class CompositeFilter extends Filter { - filters: Filter[]; +class CompositeFilter extends EntityFilter { + filters: EntityFilter[]; op: string; /** * Build a Composite Filter object. * - * @param {Filter[]} filters + * @param {EntityFilter[]} filters */ - constructor(filters: Filter[], op: CompositeOperator) { + constructor(filters: EntityFilter[], op: CompositeOperator) { super(); this.filters = filters; this.op = op; @@ -154,6 +154,6 @@ class CompositeFilter extends Filter { } } -export function isFilter(filter: any): filter is Filter { - return (filter as Filter).toProto !== undefined; +export function isFilter(filter: any): filter is EntityFilter { + return (filter as EntityFilter).toProto !== undefined; } diff --git a/src/query.ts b/src/query.ts index 66a9d6466..133ea57c8 100644 --- a/src/query.ts +++ b/src/query.ts @@ -18,7 +18,7 @@ import arrify = require('arrify'); import {Key} from 'readline'; import {Datastore} from '.'; import {Entity} from './entity'; -import {Filter as NewFilter, isFilter} from './filter'; +import {EntityFilter, isFilter} from './filter'; import {Transaction} from './transaction'; import {CallOptions} from 'google-gax'; import {RunQueryStreamOptions} from '../src/request'; @@ -76,7 +76,7 @@ class Query { namespace?: string | null; kinds: string[]; filters: Filter[]; - newFilters: NewFilter[]; + entityFilters: EntityFilter[]; orders: Order[]; groupByVal: Array<{}>; selectVal: Array<{}>; @@ -125,10 +125,10 @@ class Query { */ this.filters = []; /** - * @name Query#newFilters + * @name Query#entityFilters * @type {array} */ - this.newFilters = []; + this.entityFilters = []; /** * @name Query#orders * @type {array} @@ -207,15 +207,15 @@ class Query { * const keyQuery = query.filter('__key__', key); * ``` */ - filter(propertyOrFilter: string | NewFilter, value?: {} | null): Query; + filter(propertyOrFilter: string | EntityFilter, value?: {} | null): Query; filter(propertyOrFilter: string, operator: Operator, value: {} | null): Query; filter( - propertyOrFilter: string | NewFilter, + propertyOrFilter: string | EntityFilter, operatorOrValue?: Operator, value?: {} | null ): Query { if (isFilter(propertyOrFilter)) { - this.newFilters.push(propertyOrFilter); + this.entityFilters.push(propertyOrFilter); return this; } else { let operator = operatorOrValue as Operator; diff --git a/system-test/datastore.ts b/system-test/datastore.ts index e6af95cde..32672b248 100644 --- a/system-test/datastore.ts +++ b/system-test/datastore.ts @@ -21,7 +21,7 @@ import {Datastore, Index} from '../src'; import {google} from '../protos/protos'; import {Storage} from '@google-cloud/storage'; import {AggregateField} from '../src/aggregate'; -import {PropertyFilter, Filter, AND, OR} from '../src/filter'; +import {PropertyFilter, EntityFilter, AND, OR} from '../src/filter'; describe('Datastore', () => { const testKinds: string[] = []; diff --git a/test/entity.ts b/test/entity.ts index c83c01e1e..5e3509523 100644 --- a/test/entity.ts +++ b/test/entity.ts @@ -19,7 +19,7 @@ import * as sinon from 'sinon'; import {Datastore} from '../src'; import {Entity} from '../src/entity'; import {IntegerTypeCastOptions} from '../src/query'; -import {PropertyFilter, Filter, AND} from '../src/filter'; +import {PropertyFilter, EntityFilter, AND} from '../src/filter'; export function outOfBoundsError(opts: { propertyName?: string; diff --git a/test/query.ts b/test/query.ts index b98f23ee5..8c2aa8065 100644 --- a/test/query.ts +++ b/test/query.ts @@ -19,7 +19,7 @@ const {Query} = require('../src/query'); // eslint-disable-next-line @typescript-eslint/no-var-requires import {Datastore} from '../src'; import {AggregateField, AggregateQuery} from '../src/aggregate'; -import {PropertyFilter, Filter, OR} from '../src/filter'; +import {PropertyFilter, EntityFilter, OR} from '../src/filter'; describe('Query', () => { const SCOPE = {} as Datastore; @@ -172,7 +172,7 @@ describe('Query', () => { const query = new Query(['kind1']).filter( new PropertyFilter('date', '<=', now) ); - const filter = query.newFilters[0]; + const filter = query.entityFilters[0]; assert.strictEqual(filter.name, 'date'); assert.strictEqual(filter.op, '<='); @@ -186,7 +186,7 @@ describe('Query', () => { new PropertyFilter('name', '=', 'Stephen'), ]) ); - const filter = query.newFilters[0]; + const filter = query.entityFilters[0]; assert.strictEqual(filter.op, 'OR'); // Check filters const filters = filter.filters; From 262894aaedd1e27eb3f0aecf11eefb739a9cae93 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Tue, 7 Mar 2023 17:26:08 -0500 Subject: [PATCH 34/37] Switch around last and first These test cases have mistakes in their names. We should change them to reflect the position of `hasAncestor`. --- system-test/datastore.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/system-test/datastore.ts b/system-test/datastore.ts index 32672b248..c36e8fb3c 100644 --- a/system-test/datastore.ts +++ b/system-test/datastore.ts @@ -898,7 +898,7 @@ describe('Datastore', () => { 'Character', 'Eddard', ]); - it('should run a query using hasAncestor first', async () => { + it('should run a query using hasAncestor last', async () => { const q = datastore .createQuery('Character') .filter(new PropertyFilter('appearances', '<', 30)) @@ -906,7 +906,7 @@ describe('Datastore', () => { const [entities] = await datastore.runQuery(q); assert.strictEqual(entities!.length, 3); }); - it('should run a query using hasAncestor last', async () => { + it('should run a query using hasAncestor first', async () => { const q = datastore .createQuery('Character') .hasAncestor(secondAncestor) From 30c3832225727ea620c22d4a70f6ff1096e073ef Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Wed, 8 Mar 2023 10:59:10 -0500 Subject: [PATCH 35/37] 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. --- src/query.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/query.ts b/src/query.ts index 133ea57c8..153a39414 100644 --- a/src/query.ts +++ b/src/query.ts @@ -176,7 +176,7 @@ class Query { * * @see {@link https://cloud.google.com/datastore/docs/concepts/queries#datastore-property-filter-nodejs| Datastore Filters} * - * @param {string | NewFilter} propertyOrFilter The field name. + * @param {string | EntityFilter} propertyOrFilter The field name. * @param {string} [operator="="] Operator (=, <, >, <=, >=). * @param {*} value Value to compare property to. * @returns {Query} From fa2235d5ffe01aa3bc26aa70ddcb9e1cc93506a6 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Wed, 8 Mar 2023 13:16:09 -0500 Subject: [PATCH 36/37] rename and move constant map up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/entity.ts | 4 ++-- src/filter.ts | 27 ++++++++++++++------------- system-test/datastore.ts | 6 +++--- test/entity.ts | 4 ++-- test/query.ts | 4 ++-- 5 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/entity.ts b/src/entity.ts index e044feaf4..cb6cd3810 100644 --- a/src/entity.ts +++ b/src/entity.ts @@ -21,7 +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'; +import {and, PropertyFilter} from './filter'; // eslint-disable-next-line @typescript-eslint/no-namespace export namespace entity { @@ -1251,7 +1251,7 @@ export namespace entity { 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(); + queryProto.filter = and(allFilters).toProto(); } return queryProto; diff --git a/src/filter.ts b/src/filter.ts index 52a99a447..5897707e5 100644 --- a/src/filter.ts +++ b/src/filter.ts @@ -15,16 +15,28 @@ 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 { +export function and(filters: EntityFilter[]): CompositeFilter { return new CompositeFilter(filters, CompositeOperator.AND); } -export function OR(filters: EntityFilter[]): CompositeFilter { +export function or(filters: EntityFilter[]): CompositeFilter { return new CompositeFilter(filters, CompositeOperator.OR); } @@ -88,17 +100,6 @@ export class PropertyFilter extends EntityFilter implements IFilter { */ // eslint-disable-next-line toProto(): any { - 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'], - ]); const value = new PropertyFilter( this.name, this.op, diff --git a/system-test/datastore.ts b/system-test/datastore.ts index c36e8fb3c..bf663fc56 100644 --- a/system-test/datastore.ts +++ b/system-test/datastore.ts @@ -21,7 +21,7 @@ import {Datastore, Index} from '../src'; import {google} from '../protos/protos'; import {Storage} from '@google-cloud/storage'; import {AggregateField} from '../src/aggregate'; -import {PropertyFilter, EntityFilter, AND, OR} from '../src/filter'; +import {PropertyFilter, EntityFilter, and, or} from '../src/filter'; describe('Datastore', () => { const testKinds: string[] = []; @@ -850,7 +850,7 @@ describe('Datastore', () => { const q = datastore .createQuery('Character') .filter( - AND([ + and([ new PropertyFilter('family', '=', 'Stark'), new PropertyFilter('appearances', '>=', 20), ]) @@ -870,7 +870,7 @@ describe('Datastore', () => { const q = datastore .createQuery('Character') .filter( - OR([ + or([ new PropertyFilter('family', '=', 'Stark'), new PropertyFilter('appearances', '>=', 20), ]) diff --git a/test/entity.ts b/test/entity.ts index 5e3509523..0557884b6 100644 --- a/test/entity.ts +++ b/test/entity.ts @@ -19,7 +19,7 @@ import * as sinon from 'sinon'; import {Datastore} from '../src'; import {Entity} from '../src/entity'; import {IntegerTypeCastOptions} from '../src/query'; -import {PropertyFilter, EntityFilter, AND} from '../src/filter'; +import {PropertyFilter, EntityFilter, and} from '../src/filter'; export function outOfBoundsError(opts: { propertyName?: string; @@ -1923,7 +1923,7 @@ describe('entity', () => { const query = ds .createQuery('Kind1') .filter( - AND([ + and([ new PropertyFilter('name', '=', 'John'), new PropertyFilter('__key__', 'HAS_ANCESTOR', ancestorKey), ]) diff --git a/test/query.ts b/test/query.ts index 8c2aa8065..2836e596a 100644 --- a/test/query.ts +++ b/test/query.ts @@ -19,7 +19,7 @@ const {Query} = require('../src/query'); // eslint-disable-next-line @typescript-eslint/no-var-requires import {Datastore} from '../src'; import {AggregateField, AggregateQuery} from '../src/aggregate'; -import {PropertyFilter, EntityFilter, OR} from '../src/filter'; +import {PropertyFilter, EntityFilter, or} from '../src/filter'; describe('Query', () => { const SCOPE = {} as Datastore; @@ -181,7 +181,7 @@ describe('Query', () => { it('should support filter with OR', () => { const now = new Date(); const query = new Query(['kind1']).filter( - OR([ + or([ new PropertyFilter('date', '<=', now), new PropertyFilter('name', '=', 'Stephen'), ]) From 4e7f9d58213fcbf76a32d86177290f09891cb034 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Wed, 8 Mar 2023 13:36:40 -0500 Subject: [PATCH 37/37] Rename newFilters to entityFilters --- src/entity.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/entity.ts b/src/entity.ts index cb6cd3810..2023d511d 100644 --- a/src/entity.ts +++ b/src/entity.ts @@ -1244,8 +1244,8 @@ export namespace entity { const filters = query.filters.map( filter => new PropertyFilter(filter.name, filter.op, filter.val) ); - const newFilters = query.entityFilters; - const allFilters = newFilters.concat(filters); + 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