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 16 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
42 changes: 5 additions & 37 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 {Filter, isFilter, PropertyFilter} from './filter';

// eslint-disable-next-line @typescript-eslint/no-namespace
export namespace entity {
Expand Down Expand Up @@ -1186,18 +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',
'+': 'ASCENDING',
Expand Down Expand Up @@ -1254,32 +1243,11 @@ export namespace entity {

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,
},
};
return isFilter(filter)
? filter
: new PropertyFilter(filter.name, filter.op, filter.val);
});

queryProto.filter = {
compositeFilter: {
filters,
op: 'AND',
},
};
queryProto.filter = Filter.AND(filters).toProto();
}

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

/**
* 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 Filter {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe this should be an interface.

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.
*
*/
// 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 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 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 Filter {
filters: Filter[];
op: string;

/**
* Build a Composite Filter object.
*
* @param {Filter[]} filters
*/
constructor(filters: Filter[], 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 Filter {
return (filter as Filter).toProto !== undefined;
}
42 changes: 26 additions & 16 deletions src/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ import arrify = require('arrify');
import {Key} from 'readline';
import {Datastore} from '.';
import {Entity} from './entity';
import {Filter as NewFilter, isFilter} from './filter';
import {Transaction} from './transaction';
import {CallOptions} from 'google-gax';
import {RunQueryStreamOptions} from '../src/request';
import {AggregateField, AggregateQuery} from './aggregate';
import {string} from 'is';

export type Operator =
| '='
Expand Down Expand Up @@ -75,7 +76,7 @@ class Query {
scope?: Datastore | Transaction;
namespace?: string | null;
kinds: string[];
filters: Filter[];
filters: (Filter | NewFilter)[];
orders: Order[];
groupByVal: Array<{}>;
selectVal: Array<{}>;
Expand Down Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be EntityFilter now? (side note - how is this compiling?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. This needs to change. This is compiling because this is actually just a documentation comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

* @param {string} [operator="="] Operator (=, <, >, <=, >=).
* @param {*} value Value to compare property to.
* @returns {Query}
Expand Down Expand Up @@ -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)) {
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.filters.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