Unneeded filterQuery redefinition

[vonagam]

Currently you can safely remove filterQuery and nothing will change.

First, it should exit here because filtered.query will be an object. I assume it is a typo and it should have been if (! utils.isPlainObject(query)), it would make sense that way.

But even after this, convertOperators is converting operators, but const key = operators[prop] ? operators[prop] : prop will always be prop itself. defaultOperators returns object where keys and values are equal, there is no mismatch or converting.

Am i missing something?

[dekelev]

@vonagam thanks for this. I've took the updated version of filterQuery method from feathers-sequelize and it looks like it was already fixed there. The fix is published in feathers-objection v2.0.1

[vonagam]

Hm... so you keep this method for convenience, to be comparable with feathers-sequelize in migration paths?

Because there is no need for convertOperators in feathers-objection right now otherwise.

[dekelev]

Right. Feathers Crow release dictates security guidelines that were already implemented in most of the well-known Feathers DB adapters, including feathers-sequelize. Some of the recent updates in feathers-objection were simply synced from feathers-sequelize. All these DB adapters, including feathers-objection, are using the same test-suite from feathers that was released recently to test the new filtering functionalities.

[vonagam]

Ok. Got it.

(Just to be clear: Ran tests. Removed filterQuery method, saw two failings. Fixed by reassignment of whitelist in options super(Object.assign({...}, options, {whitelist})) here. All tests are passing.)

[dekelev]

$like is in the whitelist operators list. it should not be added to the service options.

[vonagam]

Why not? options.whitelist is for additionalOperators in filterQuery. And it is its only usage. And $like is additional operator used in feathers-objection.

[dekelev]

$like is not unique to feathers-objection and is part of the operators that are allowed now by default in feathers-sequelize. It shouldn't be different in feathers-objection.

[vonagam]

In feathers-objection:
const filtered = super.filterQuery(params, { operators: this.whitelist })

In feathers-sequelize:
const filtered = super.filterQuery(params);

So currently they have different behaviours.

In feathers-objection all operators are allowed, and you can enable additional (like $eager, which is really a filter), but you can't disable $like operator for example.

In feathers-sequelize all operators are allowed by default, but if you pass whitelist option and it does not include $like then $like will not be allowed.

[vonagam]

Ok, you can simply remove filterQuery and fix test to match feathers-sequelize's behaviour by adding $like to this whitelist.

Otherwise this test will fall, because $like is used but not whitelisted.

[dekelev]

OK, thanks for that check.

[vonagam]

Sorry, no mistake, you can disable $like in feathers-sequelize and can't in feathers-objection.

[dekelev]

:) ok, releasing a patch

[dekelev]

removing filterQuery method

[vonagam]

It's a breaking change, though, if you want to match sequelize behaviour.

[dekelev]

right, so minor version.

[vonagam]

I mean, that if somebody used { "whitelist": [ "$eager" ] } and used $like in queries. (Like in current tests) And you change behaviour to sequelize's. Then their code will fail (Like tests was). And then they have to add $like towhitelist (Like what you need to fix tests if you match sequelize).

So it is major version change.

[dekelev]

OK. It'll be 3.0.0

[vonagam]

Here is if you remove all unneeded stuff.

[dekelev]

Cool. removed all that too.

[dekelev]

published as 3.0.0