Closed kaf-lamed-beyt closed 1 year ago
logicalFilterToJSONObj uses op property to infer type of filter, since we won't have op anymore, you can change the implementation to infer type in a way that as long as first key matches $or or $and it should be a Logical filter.
Similarly the methods (_selectorFilterToString, _selectorFilterToFlatJSONObj) that serialize the selector filter will need to be updated
Awesome! I'll go over these and push a fix, soon.
@adilansari,
I tried making the changes you requested. Kindly take a look. I'm stuck on fixing the test cases with the new spec. I was able to make the simpleSelectorFilter
assertion though.
Ah! Cool!! I'll make these changes, and let you know if I'm still stuck.
In case you missed this, @adilansari
In case you missed this, @adilansari
One way to validate your changes is by running tests npm run test
. At the moment there are compilation errors and types are not being recognized:
$ ➔ npm run test ±[improve-filter]
> @tigrisdata/core@1.0.0 test
> jest --runInBand --coverage --silent --detectOpenHandles
FAIL src/__tests__/tigris.rpc.spec.ts
● Test suite failed to run
src/__tests__/tigris.rpc.spec.ts:9:2 - error TS2305: Module '"../types"' has no exported member 'SelectorFilterOperator'.
9 SelectorFilterOperator,
~~~~~~~~~~~~~~~~~~~~~~
src/__tests__/tigris.rpc.spec.ts:336:5 - error TS2322: Type '{ op: any; fields: { id: number; }; }' is not assignable to type 'Filter<IBook>'.
Object literal may only specify known properties, and 'op' does not exist in type 'Filter<IBook>'.
336 op: SelectorFilterOperator.EQ,
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/types.ts:554:2
554 filter: Filter<T>;
~~~~~~
The expected type comes from property 'filter' which is declared here on type 'UpdateQuery<IBook>'
src/__tests__/tigris.rpc.spec.ts:386:26 - error TS2769: No overload matches this call.
Overload 1 of 4, '(tx: Session): Promise<IBook>', gave the following error.
Argument of type '{ filter: { op: any; fields: { id: number; }; }; }' is not assignable to parameter of type 'Session'.
Object literal may only specify known properties, and 'filter' does not exist in type 'Session'.
Overload 2 of 4, '(query: FindQuery<IBook>): Promise<IBook>', gave the following error.
Type '{ op: any; fields: { id: number; }; }' is not assignable to type 'Filter<IBook>'.
Object literal may only specify known properties, and 'op' does not exist in type 'Filter<IBook>'.
386 const readOnePromise = db1.getCollection<IBook>("books").findOne({
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
387 filter: {
~~~~~~~~~~~~
...
392 },
~~~~~
393 });
~~~~
src/types.ts:511:2
511 filter?: Filter<T>;
~~~~~~
The expected type comes from property 'filter' which is declared here on type 'FindQuery<IBook>'
src/__tests__/tigris.rpc.spec.ts:407:26 - error TS2769: No overload matches this call.
Overload 1 of 4, '(tx: Session): Promise<IBook>', gave the following error.
Argument of type '{ filter: { op: any; fields: { id: number; }; }; }' is not assignable to parameter of type 'Session'.
Object literal may only specify known properties, and 'filter' does not exist in type 'Session'.
Overload 2 of 4, '(query: FindQuery<IBook>): Promise<IBook>', gave the following error.
Type '{ op: any; fields: { id: number; }; }' is not assignable to type 'Filter<IBook>'.
Object literal may only specify known properties, and 'op' does not exist in type 'Filter<IBook>'.
407 const readOnePromise = db1.getCollection<IBook>("books").findOne({
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
408 filter: {
~~~~~~~~~~~~
...
413 },
~~~~~
414 });
~~~~
src/types.ts:511:2
511 filter?: Filter<T>;
~~~~~~
The expected type comes from property 'filter' which is declared here on type 'FindQuery<IBook>'
src/__tests__/tigris.rpc.spec.ts:430:26 - error TS2769: No overload matches this call.
Overload 1 of 4, '(tx: Session): Promise<IBook>', gave the following error.
Argument of type '{ filter: { op: any; fields: { id: number; }; }; }' is not assignable to parameter of type 'Session'.
Object literal may only specify known properties, and 'filter' does not exist in type 'Session'.
Overload 2 of 4, '(query: FindQuery<IBook>): Promise<IBook>', gave the following error.
Type '{ op: any; fields: { id: number; }; }' is not assignable to type 'Filter<IBook>'.
Object literal may only specify known properties, and 'op' does not exist in type 'Filter<IBook>'.
430 const readOnePromise = db1.getCollection<IBook>("books").findOne({
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
431 filter: {
~~~~~~~~~~~~
...
436 },
~~~~~
437 });
~~~~
src/types.ts:511:2
511 filter?: Filter<T>;
~~~~~~
The expected type comes from property 'filter' which is declared here on type 'FindQuery<IBook>'
src/__tests__/tigris.rpc.spec.ts:447:49 - error TS2769: No overload matches this call.
Overload 1 of 4, '(tx: Session): Promise<IBook>', gave the following error.
Argument of type '{ filter: { op: any; selectorFilters: ({ op: any; fields: { id: number; }; } | { op: any; fields: { title: string; }; })[]; }; }' is not assignable to parameter of type 'Session'.
Object literal may only specify known properties, and 'filter' does not exist in type 'Session'.
Overload 2 of 4, '(query: FindQuery<IBook>): Promise<IBook>', gave the following error.
Type '{ op: any; selectorFilters: ({ op: any; fields: { id: number; }; } | { op: any; fields: { title: string; }; })[]; }' is not assignable to type 'Filter<IBook>'.
Object literal may only specify known properties, and 'op' does not exist in type 'Filter<IBook>'.
447 const readOnePromise: Promise<IBook | void> = db1.getCollection<IBook>("books").findOne({
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
448 filter: {
~~~~~~~~~~~~
...
464 },
~~~~~
465 });
~~~~
src/types.ts:511:2
511 filter?: Filter<T>;
~~~~~~
The expected type comes from property 'filter' which is declared here on type 'FindQuery<IBook>'
src/__tests__/tigris.rpc.spec.ts:449:9 - error TS2693: 'LogicalOperator' only refers to a type, but is being used as a value here.
449 op: LogicalOperator.AND,
~~~~~~~~~~~~~~~
src/__tests__/tigris.rpc.spec.ts:481:19 - error TS2769: No overload matches this call.
Overload 1 of 4, '(tx: Session): Cursor<IBook>', gave the following error.
Argument of type '{ filter: { op: any; fields: { author: string; }; }; }' is not assignable to parameter of type 'Session'.
Object literal may only specify known properties, and 'filter' does not exist in type 'Session'.
Overload 2 of 4, '(query: FindQuery<IBook>): Cursor<IBook>', gave the following error.
Type '{ op: any; fields: { author: string; }; }' is not assignable to type 'Filter<IBook>'.
Object literal may only specify known properties, and 'op' does not exist in type 'Filter<IBook>'.
481 const cursor = db.getCollection<IBook>("books").findMany({
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
482 filter: {
~~~~~~~~~~~~~
...
487 },
~~~~~~
488 });
~~~~~
src/types.ts:511:2
511 filter?: Filter<T>;
~~~~~~
The expected type comes from property 'filter' which is declared here on type 'FindQuery<IBook>'
src/__tests__/tigris.rpc.spec.ts:522:19 - error TS2769: No overload matches this call.
Overload 1 of 4, '(tx: Session): Cursor<IBook>', gave the following error.
Argument of type '{ filter: { op: any; fields: { id: number; }; }; }' is not assignable to parameter of type 'Session'.
Object literal may only specify known properties, and 'filter' does not exist in type 'Session'.
Overload 2 of 4, '(query: FindQuery<IBook>): Cursor<IBook>', gave the following error.
Type '{ op: any; fields: { id: number; }; }' is not assignable to type 'Filter<IBook>'.
Object literal may only specify known properties, and 'op' does not exist in type 'Filter<IBook>'.
522 const cursor = db.getCollection<IBook>("books").findMany({
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
523 filter: {
~~~~~~~~~~~~~
...
528 },
~~~~~~
529 });
~~~~~
src/types.ts:511:2
511 filter?: Filter<T>;
~~~~~~
The expected type comes from property 'filter' which is declared here on type 'FindQuery<IBook>'
src/__tests__/tigris.rpc.spec.ts:651:10 - error TS2322: Type '{ op: any; fields: { id: number; }; }' is not assignable to type 'Filter<IBook>'.
Object literal may only specify known properties, and 'op' does not exist in type 'Filter<IBook>'.
651 op: SelectorFilterOperator.EQ,
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/types.ts:511:2
511 filter?: Filter<T>;
~~~~~~
The expected type comes from property 'filter' which is declared here on type 'FindQuery<IBook>'
src/__tests__/tigris.rpc.spec.ts:664:12 - error TS2322: Type '{ op: any; fields: { id: number; }; }' is not assignable to type 'Filter<IBook>'.
Object literal may only specify known properties, and 'op' does not exist in type 'Filter<IBook>'.
664 op: SelectorFilterOperator.EQ,
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/types.ts:554:2
554 filter: Filter<T>;
~~~~~~
The expected type comes from property 'filter' which is declared here on type 'UpdateQuery<IBook>'
src/__tests__/tigris.rpc.spec.ts:680:14 - error TS2322: Type '{ op: any; fields: { id: number; }; }' is not assignable to type 'Filter<IBook>'.
Object literal may only specify known properties, and 'op' does not exist in type 'Filter<IBook>'.
680 op: SelectorFilterOperator.EQ,
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/types.ts:538:2
538 filter: Filter<T>;
~~~~~~
The expected type comes from property 'filter' which is declared here on type 'DeleteQuery<IBook>'
FAIL src/__tests__/tigris.search.spec.ts
● Test suite failed to run
src/collection.ts:22:2 - error TS2305: Module '"./types"' has no exported member 'SelectorFilterOperator'.
22 SelectorFilterOperator,
~~~~~~~~~~~~~~~~~~~~~~
src/collection.ts:529:32 - error TS2322: Type '{ op: any; }' is not assignable to type 'Filter<T>'.
Object literal may only specify known properties, and 'op' does not exist in type 'Filter<T>'.
529 const findAll: Filter<T> = { op: SelectorFilterOperator.NONE };
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
PASS src/__tests__/tigris.schema.spec.ts
FAIL src/__tests__/consumables/cursor.spec.ts
● Test suite failed to run
src/collection.ts:22:2 - error TS2305: Module '"./types"' has no exported member 'SelectorFilterOperator'.
22 SelectorFilterOperator,
~~~~~~~~~~~~~~~~~~~~~~
src/collection.ts:529:32 - error TS2322: Type '{ op: any; }' is not assignable to type 'Filter<T>'.
Object literal may only specify known properties, and 'op' does not exist in type 'Filter<T>'.
529 const findAll: Filter<T> = { op: SelectorFilterOperator.NONE };
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
PASS src/__tests__/search/schema.spec.ts
PASS src/__tests__/search/search.types.spec.ts
FAIL src/__tests__/tigris.utility.spec.ts
● Test suite failed to run
src/__tests__/tigris.utility.spec.ts:12:10 - error TS2305: Module '"../types"' has no exported member 'SelectorFilterOperator'.
12 import { SelectorFilterOperator, SortOrder } from "../types";
~~~~~~~~~~~~~~~~~~~~~~
src/__tests__/tigris.utility.spec.ts:118:15 - error TS2322: Type '{ op: any; fields: { balance: number; }; }' is not assignable to type 'Filter<Student>'.
Object literal may only specify known properties, and 'op' does not exist in type 'Filter<Student>'.
118 filter: { op: SelectorFilterOperator.GT, fields: { balance: 25 } },
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/search/query.ts:21:2
21 filter?: Filter<T>;
~~~~~~
The expected type comes from property 'filter' which is declared here on type 'SearchQuery<Student>'
PASS src/__tests__/tigris.updatefields.spec.ts
PASS src/__tests__/tigris.jsonserde.spec.ts
FAIL src/__tests__/tigris.filters.spec.ts
● Test suite failed to run
src/__tests__/tigris.filters.spec.ts:215:23 - error TS2451: Cannot redeclare block-scoped variable '(Missing)'.
215 balance: {"$lte": 1000},
src/__tests__/tigris.filters.spec.ts:215:28 - error TS2451: Cannot redeclare block-scoped variable '(Missing)'.
215 balance: {"$lte": 1000},
src/__tests__/tigris.filters.spec.ts:120:5 - error TS2322: Type '{ id: bigint; name: string; balance: number; "address.city": string; }' is not assignable to type 'Partial<{ id?: BigInt | { $eq?: BigInt | BigInt[]; $gt?: BigInt | BigInt[]; $gte?: BigInt | BigInt[]; $lt?: BigInt | BigInt[]; $lte?: BigInt | BigInt[]; $none?: BigInt | BigInt[]; }; name?: string | { ...; }; balance?: number | { ...; }; address?: Address | { ...; }; }>'.
Object literal may only specify known properties, and '"address.city"' does not exist in type 'Partial<{ id?: BigInt | { $eq?: BigInt | BigInt[]; $gt?: BigInt | BigInt[]; $gte?: BigInt | BigInt[]; $lt?: BigInt | BigInt[]; $lte?: BigInt | BigInt[]; $none?: BigInt | BigInt[]; }; name?: string | { ...; }; balance?: number | { ...; }; address?: Address | { ...; }; }>'.
120 "address.city": "San Francisco",
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/__tests__/tigris.filters.spec.ts:136:4 - error TS2322: Type '{ "address.zipcode": { $lte: number; }; }' is not assignable to type 'Partial<{ id?: BigInt | { $eq?: BigInt | BigInt[]; $gt?: BigInt | BigInt[]; $gte?: BigInt | BigInt[]; $lt?: BigInt | BigInt[]; $lte?: BigInt | BigInt[]; $none?: BigInt | BigInt[]; }; name?: string | { ...; }; balance?: number | { ...; }; address?: Address | { ...; }; }>'.
Object literal may only specify known properties, and '"address.zipcode"' does not exist in type 'Partial<{ id?: BigInt | { $eq?: BigInt | BigInt[]; $gt?: BigInt | BigInt[]; $gte?: BigInt | BigInt[]; $lt?: BigInt | BigInt[]; $lte?: BigInt | BigInt[]; $none?: BigInt | BigInt[]; }; name?: string | { ...; }; balance?: number | { ...; }; address?: Address | { ...; }; }>'.
136 "address.zipcode": {"$lte": 10}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/__tests__/tigris.filters.spec.ts:204:6 - error TS2322: Type '{ "address.city": string; }' is not assignable to type 'Filter<Student>'.
Object literal may only specify known properties, and '"address.city"' does not exist in type 'Filter<Student>'.
204 "address.city": "Paris"
~~~~~~~~~~~~~~~~~~~~~~~
src/__tests__/tigris.filters.spec.ts:212:20 - error TS2322: Type '{ "address.zipcode": { $gte: number; }; }' is not assignable to type 'Filter<Student>'.
Object literal may only specify known properties, and '"address.zipcode"' does not exist in type 'Filter<Student>'.
212 {"address.zipcode": {"$gte": 2000}}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/__tests__/tigris.filters.spec.ts:221:12 - error TS2304: Cannot find name 'logicalFilter1'.
221 "$or": [logicalFilter1, logicalFilter2],
~~~~~~~~~~~~~~
src/__tests__/tigris.filters.spec.ts:221:28 - error TS2304: Cannot find name 'logicalFilter2'.
221 "$or": [logicalFilter1, logicalFilter2],
~~~~~~~~~~~~~~
src/__tests__/tigris.filters.spec.ts:212:19 - error TS1136: Property assignment expected.
212 {"address.zipcode": {"$gte": 2000}}
~
src/__tests__/tigris.filters.spec.ts:213:5 - error TS1005: ',' expected.
213 },
~
src/__tests__/tigris.filters.spec.ts:215:24 - error TS1003: Identifier expected.
215 balance: {"$lte": 1000},
~~~~
src/__tests__/tigris.filters.spec.ts:215:28 - error TS1005: ':' expected.
215 balance: {"$lte": 1000},
~
src/__tests__/tigris.filters.spec.ts:217:4 - error TS1128: Declaration or statement expected.
217 ],
~
src/__tests__/tigris.filters.spec.ts:217:5 - error TS1128: Declaration or statement expected.
217 ],
~
src/__tests__/tigris.filters.spec.ts:218:4 - error TS1005: ')' expected.
218 };
~
src/__tests__/tigris.filters.spec.ts:256:1 - error TS1128: Declaration or statement expected.
256 });
~
src/__tests__/tigris.filters.spec.ts:256:2 - error TS1128: Declaration or statement expected.
256 });
~
FAIL src/__tests__/index.spec.ts
● Test suite failed to run
src/collection.ts:22:2 - error TS2305: Module '"./types"' has no exported member 'SelectorFilterOperator'.
22 SelectorFilterOperator,
~~~~~~~~~~~~~~~~~~~~~~
src/collection.ts:529:32 - error TS2322: Type '{ op: any; }' is not assignable to type 'Filter<T>'.
Object literal may only specify known properties, and 'op' does not exist in type 'Filter<T>'.
529 const findAll: Filter<T> = { op: SelectorFilterOperator.NONE };
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
PASS src/__tests__/tigris.readfields.spec.ts
PASS src/__tests__/utils/env-loader.spec.ts
--------------------------------|---------|----------|---------|---------|------------------------------
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
--------------------------------|---------|----------|---------|---------|------------------------------
All files | 47.76 | 79.25 | 26.43 | 47.76 |
src | 29.92 | 87.87 | 14.28 | 29.92 |
cache.ts | 0 | 0 | 0 | 0 | 1-211
collection.ts | 0 | 0 | 0 | 0 | 1-773
constants.ts | 100 | 100 | 100 | 100 |
db.ts | 0 | 0 | 0 | 0 | 1-449
error.ts | 69.91 | 100 | 26.31 | 69.91 | ...7,100-101,106-108,111-112
globals.ts | 100 | 100 | 100 | 100 |
index.ts | 0 | 0 | 0 | 0 | 1-12
session.ts | 0 | 0 | 0 | 0 | 1-79
tigris.ts | 0 | 0 | 0 | 0 | 1-399
types.ts | 76.9 | 100 | 0 | 76.9 | ...3,446-447,454-455,458-459
utility.ts | 58.33 | 92.77 | 35.13 | 58.33 | ...8,602-605,609-620,624-670
src/consumables | 51.45 | 0 | 0 | 51.45 |
cursor.ts | 0 | 0 | 0 | 0 | 1-75
iterable-stream.ts | 69.46 | 100 | 0 | 69.46 | ...87,97-107,114-118,127-130
search-iterator.ts | 73.52 | 100 | 0 | 73.52 | ...9,38-40,42-43,57-62,66-67
src/decorators | 94.24 | 77.89 | 100 | 94.24 |
tigris-collection.ts | 100 | 100 | 100 | 100 |
tigris-field.ts | 95.58 | 85.71 | 100 | 95.58 | 95-96,118-119,123-124
tigris-primary-key.ts | 92.68 | 55.55 | 100 | 92.68 | 49-50,62-63,66-67
tigris-search-field.ts | 90.38 | 62.06 | 100 | 90.38 | ...2,68-69,72-73,84-85,89-90
tigris-search-index.ts | 100 | 100 | 100 | 100 |
utils.ts | 100 | 100 | 100 | 100 |
src/schema | 100 | 95.91 | 100 | 100 |
decorated-schema-processor.ts | 100 | 95.91 | 100 | 100 | 84,160
src/search | 72.29 | 61.76 | 37.03 | 72.29 |
index.ts | 100 | 100 | 0 | 100 |
query.ts | 100 | 100 | 100 | 100 |
result.ts | 98.9 | 61.19 | 95.23 | 98.9 | 48-49,93-94
search-index.ts | 37.9 | 100 | 0 | 37.9 | ...9,172-177,220-243,246-247
search.ts | 34.21 | 100 | 0 | 34.21 | ...7-78,81-93,96-109,112-113
types.ts | 69.41 | 100 | 0 | 69.41 | ...9,62-63,66-67,75-77,80-84
src/utils | 82.88 | 54.54 | 55.55 | 82.88 |
env-loader.ts | 83.58 | 37.5 | 100 | 83.58 | 29-30,42-43,55-59,62-63
logger.ts | 81.81 | 100 | 42.85 | 81.81 | 24-25,32-33,36-37,40-41
--------------------------------|---------|----------|---------|---------|------------------------------
Test Suites: 6 failed, 7 passed, 13 total
Tests: 43 passed, 43 total
Snapshots: 0 total
Time: 9.262 s, estimated 10 s
Oh! Great! I'll do that.
I think some of the error are also coming from the other test suites. I can still see SelectorFilterOperator
being used there.
I'll have a thorough look this time around.
Yes give it a shot. If it doesn't work, you can close this PR for now and work on a small task #321 that will give you some hands on experience to get familiar with the codebase.
Yes give it a shot. If it doesn't work, you can close this PR for now and work on a small task #321 that will give you some hands on experience to get familiar with the codebase.
Great! I'll try this, and if it doesn't work out well. I'll work on the issue you recommended.
Thanks for the constant feedback BTW! 🙌🏽
So I ran the tests myself too, and I'll be closing this PR as you've suggested @adilansari
So, I've gone ahead to modify the
filterToString
function. I've also modified the types that are being exported.I removed the
SelectorFilter<T>
type since it won't be used in the updated spec that you shared here 👇🏼 Because, from the spec below, it will only require theSelector
andLogicalFilter
types.But, there are issues that I may still need to check if you'd love me to. One is the test cases. I'll need to modify them to become, something similar to the one mentioned in the issue — originally. Instead of the ones we have currently. For example, this one 👇🏼
The'` filter.
_logicalFilterToJSONObj
method too is affected. I'm getting an error message saying "Property 'op' does not exist on type 'LogicalFilterwhich is a result of the change in the
LogicalFilterand
SelectorI even tried modifying it to use the
_selectorFilterToFlatJSONObj
method. But I kept getting stuck. Perhaps you go through my implementation first and let me know what you think./claim #268
cc: @ovaistariq, @adilansari