Skip to content

Commit

Permalink
fix: Change tests to use new filter and use gax warn for warning just…
Browse files Browse the repository at this point in the history
… once (#1185)

* trim operator

In the legacy version of filter we trim the operator. We should do this here too.

* In test cases, replace various filter calls

Always pass entity filters into the filter function in tests to reflect new behavior.

* accept string too

A string is needed as an input parameter because we might trim it to get an operator.

* Use .filter the old way

Revert changes on the .filter method

* Change docs to use new filter

The new property filter should be used and this should be reflected in the doc comments.

* Use new Property filter

In entity.ts, use the new property filter constructor.

* Revert changes to operator

The changes cause compiler errors. Do not trim the operator for now.

* Move test up and add change

Add change to issue warning just once. Use google-gax so that the warning is issued just once.

* Revert "Change docs to use new filter"

This reverts commit c290481.

* run linter

* Add a test to ensure just one warning

A test to make sure the warning is emitted just once might provide proper coverage.
  • Loading branch information
danieljbruce committed Oct 23, 2023
1 parent 52adb5e commit 532711b
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 15 deletions.
4 changes: 3 additions & 1 deletion src/query.ts
Expand Up @@ -22,6 +22,7 @@ import {EntityFilter, isFilter, AllowedFilterValueType} from './filter';
import {Transaction} from './transaction';
import {CallOptions} from 'google-gax';
import {RunQueryStreamOptions} from '../src/request';
import * as gaxInstance from 'google-gax';

export type Operator =
| '='
Expand Down Expand Up @@ -223,7 +224,8 @@ class Query {
value?: AllowedFilterValueType<T>
): Query {
if (arguments.length > 1) {
process.emitWarning(
gaxInstance.warn(
'filter',
'Providing Filter objects like Composite Filter or Property Filter is recommended when using .filter'
);
}
Expand Down
8 changes: 6 additions & 2 deletions test/entity.ts
Expand Up @@ -1929,7 +1929,7 @@ describe('entity', () => {

const query = ds
.createQuery('Kind1')
.filter('name', 'John')
.filter(new PropertyFilter('name', '=', 'John'))
.start('start')
.end('end')
.groupBy(['name'])
Expand Down Expand Up @@ -1989,7 +1989,11 @@ describe('entity', () => {

const query = ds
.createQuery('Kind1')
.filter('__key__', 'IN', [new entity.Key({path: ['Kind1', 'key1']})]);
.filter(
new PropertyFilter('__key__', 'IN', [
new entity.Key({path: ['Kind1', 'key1']}),
])
);

assert.deepStrictEqual(
testEntity.queryToQueryProto(query),
Expand Down
35 changes: 23 additions & 12 deletions test/query.ts
Expand Up @@ -206,6 +206,29 @@ describe('Query', () => {
});

describe('filter', () => {
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 like Composite Filter or Property Filter is recommended when using .filter'
);
process.removeListener('warning', onWarning);
done();
};
process.on('warning', onWarning);
new Query(['kind1']).filter('name', 'Stephen');
});
it('should not issue a warning again when a Filter instance is not provided', done => {
const onWarning = () => {
assert.fail();
};
process.on('warning', onWarning);
new Query(['kind1']).filter('name', 'Stephen');
setImmediate(() => {
process.removeListener('warning', onWarning);
done();
});
});
it('should support filtering', () => {
const now = new Date();
const query = new Query(['kind1']).filter('date', '<=', now);
Expand Down Expand Up @@ -293,18 +316,6 @@ describe('Query', () => {
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 like Composite Filter or Property Filter 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', () => {
it('should support filter with Filter', () => {
const now = new Date();
Expand Down

0 comments on commit 532711b

Please sign in to comment.