Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 42 commits into from
Mar 8, 2023
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
d370b54
or filters interface
danieljbruce Jan 13, 2023
3291df8
implementation of new filter
danieljbruce Jan 16, 2023
69390d4
Remove duplicate code
danieljbruce Jan 16, 2023
c90e932
Parse with type guard
danieljbruce Jan 16, 2023
c2b29fc
Remove the newFilter variable
danieljbruce Jan 16, 2023
69f1cbf
Add filter and enable chaining
danieljbruce Jan 16, 2023
3f720c0
test add filter
danieljbruce Jan 16, 2023
20f616b
Add another unit test
danieljbruce Jan 16, 2023
5dea24a
Add system-test stubs
danieljbruce Jan 17, 2023
bb931e7
Add system-tests for the OR filter
danieljbruce Jan 18, 2023
44be8a9
Move things around
danieljbruce Jan 19, 2023
0055a09
Merge branch 'main' into or-filters
danieljbruce Jan 19, 2023
b6bb3b3
Added a unit test
danieljbruce Jan 19, 2023
83668a1
Merge branch 'or-filters' of https://github.com/danieljbruce/nodejs-d…
danieljbruce Jan 19, 2023
c4fe304
Add a unit test for OR filters
danieljbruce Jan 19, 2023
c397608
Just use filter method
danieljbruce Jan 25, 2023
37400a6
new warning test
danieljbruce Jan 26, 2023
8ab6b80
Revert "new warning test"
danieljbruce Jan 26, 2023
db02a50
Now removes deprecation warning properly
danieljbruce Jan 27, 2023
bc15f32
Add a test for new warning
danieljbruce Jan 27, 2023
e84582f
Add setAncestor
danieljbruce Feb 7, 2023
86841d6
Basic unit tests for setAncestor
danieljbruce Feb 7, 2023
fe50d56
change expected result for OR query
danieljbruce Feb 8, 2023
1159b37
Fix a test by not requiring the done callback
danieljbruce Feb 16, 2023
49ffcf9
Merge branch 'main' into or-filters
danieljbruce Feb 17, 2023
3aa7a35
Merge branch 'main' of https://github.com/googleapis/nodejs-datastore…
danieljbruce Feb 24, 2023
a7a8d27
Revert "Fix a test by not requiring the done callback"
danieljbruce Feb 27, 2023
1c77b6c
Revert "Basic unit tests for setAncestor"
danieljbruce Feb 27, 2023
5f38179
Revert "Add setAncestor"
danieljbruce Feb 27, 2023
059d4dc
Separate filters and new filters internally
danieljbruce Feb 27, 2023
ec752e8
Move AND/OR into their own separate function
danieljbruce Mar 1, 2023
a5c41ab
Eliminate unused imports
danieljbruce Mar 1, 2023
1ddeea6
Revert "Add a test for new warning"
danieljbruce Mar 1, 2023
8e96bf5
Revert "Now removes deprecation warning properly"
danieljbruce Mar 1, 2023
5883096
Modify test cases to capture nuances in data
danieljbruce Mar 7, 2023
0a68ed1
Added comments to code that was refactored
danieljbruce Mar 7, 2023
35bae06
rename NewFilter to entity filter
danieljbruce Mar 7, 2023
262894a
Switch around last and first
danieljbruce Mar 7, 2023
30c3832
Change the comment to reflect the new name
danieljbruce Mar 8, 2023
fa2235d
rename and move constant map up
danieljbruce Mar 8, 2023
4e7f9d5
Rename newFilters to entityFilters
danieljbruce Mar 8, 2023
c72fcc9
Merge branch 'main' into or-filters
danieljbruce Mar 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 15 additions & 40 deletions src/entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, upper case is usually reserved for variable constants const FOO = 9; BAR = 99, to make it clear that this is an exported method, I might go with:

and/or, or applyAnd, applyOr.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choosing the former


// eslint-disable-next-line @typescript-eslint/no-namespace
export namespace entity {
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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__') {
kolea2 marked this conversation as resolved.
Show resolved Hide resolved
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 newFilters = query.entityFilters;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit - rename variable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was done

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;
Expand Down
159 changes: 159 additions & 0 deletions src/filter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
// 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';

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 OP_TO_OPERATOR = new Map([
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I would probably hoist this to the top of the file, rather than declaring in toProto, at scale this will speed things up since you're allocating less Maps.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good plan.

['=', '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,
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;
}
43 changes: 27 additions & 16 deletions src/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
| '='
Expand Down Expand Up @@ -76,6 +76,7 @@ class Query {
namespace?: string | null;
kinds: string[];
filters: Filter[];
entityFilters: EntityFilter[];
orders: Order[];
groupByVal: Array<{}>;
selectVal: Array<{}>;
Expand Down Expand Up @@ -123,6 +124,11 @@ class Query {
* @type {array}
*/
this.filters = [];
/**
* @name Query#entityFilters
* @type {array}
*/
this.entityFilters = [];
/**
* @name Query#orders
* @type {array}
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good use of TypeScript's is functionality 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

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;
}

Expand Down
6 changes: 6 additions & 0 deletions system-test/data/index.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ indexes:
- name: family
- name: appearances

- kind: Character
ancestor: no
properties:
- name: family
- name: appearances

- kind: Character
ancestor: yes
properties:
Expand Down
Loading