-
Notifications
You must be signed in to change notification settings - Fork 55
Feat - add custom filter parser #3
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
Conversation
|
Testing locally against MariaDB has 🟢 $ docker-compose exec tests vendor/bin/phpunit --debug /usr/src/code/tests/Database/Adapter/MariaDBTest.php
PHPUnit 9.5.4 by Sebastian Bergmann and contributors.
Test 'Utopia\Tests\Adapter\MariaDBTest::testCreateDelete' started
Test 'Utopia\Tests\Adapter\MariaDBTest::testCreateDelete' ended
Test 'Utopia\Tests\Adapter\MariaDBTest::testCreateDeleteCollection' started
Test 'Utopia\Tests\Adapter\MariaDBTest::testCreateDeleteCollection' ended
Test 'Utopia\Tests\Adapter\MariaDBTest::testCreateDeleteAttribute' started
Test 'Utopia\Tests\Adapter\MariaDBTest::testCreateDeleteAttribute' ended
Test 'Utopia\Tests\Adapter\MariaDBTest::testCreateDeleteIndex' started
Test 'Utopia\Tests\Adapter\MariaDBTest::testCreateDeleteIndex' ended
Test 'Utopia\Tests\Adapter\MariaDBTest::testCreateDocument' started
Test 'Utopia\Tests\Adapter\MariaDBTest::testCreateDocument' ended
Test 'Utopia\Tests\Adapter\MariaDBTest::testGetDocument' started
Test 'Utopia\Tests\Adapter\MariaDBTest::testGetDocument' ended
Test 'Utopia\Tests\Adapter\MariaDBTest::testUpdateDocument' started
Test 'Utopia\Tests\Adapter\MariaDBTest::testUpdateDocument' ended
Test 'Utopia\Tests\Adapter\MariaDBTest::testDeleteDocument' started
Test 'Utopia\Tests\Adapter\MariaDBTest::testDeleteDocument' ended
Test 'Utopia\Tests\Adapter\MariaDBTest::testFindFirst' started
Test 'Utopia\Tests\Adapter\MariaDBTest::testFindFirst' ended
Test 'Utopia\Tests\Adapter\MariaDBTest::testFindLast' started
Test 'Utopia\Tests\Adapter\MariaDBTest::testFindLast' ended
Test 'Utopia\Tests\Adapter\MariaDBTest::testParseQueries' started
Test 'Utopia\Tests\Adapter\MariaDBTest::testParseQueries' ended
Time: 00:03.764, Memory: 6.00 MB
OK (11 tests, 101 assertions) |
|
Thinking about this a bit here are some of my ideas:
|
|
As of e802153 # kodumbeats @ magician in ~/w/kodumbeats/database on git:v0 x [12:28:00]
$ dc exec tests vendor/bin/phpunit tests/Database/Adapter/MariaDBTest.php
PHPUnit 9.5.4 by Sebastian Bergmann and contributors.
.......... 10 / 10 (100%)
Time: 00:00.711, Memory: 6.00 MB
OK (10 tests, 85 assertions)
# kodumbeats @ magician in ~/w/kodumbeats/database on git:v0 x [12:28:17]
$ dc exec tests vendor/bin/phpunit tests/Database/QueryTest.php
PHPUnit 9.5.4 by Sebastian Bergmann and contributors.
.. 2 / 2 (100%)
Time: 00:00.002, Memory: 6.00 MB
OK (2 tests, 10 assertions)
|
src/Database/Query.php
Outdated
| return [$operator, $operand]; | ||
| } | ||
|
|
||
| // /** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this idea! we should probably have a validator class for queries syntax...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How's this for a start? I figure this is a good place to check if the operator is supported too:
179a0d5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a good start, we can have it as a standalone PR
|
We also need to support multiple params for
|
|
As of 631ecfd |
|
Looks good! I would have added more tests to make sure casting works as expected with all types |
src/Database/Query.php
Outdated
| return null; | ||
| } else { | ||
| // strip quotes from queries of type string | ||
| return str_replace(['"',"'"], "", $value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to remove only the first and last chars if they are " or '
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should also handle escaping of \' and \"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Used trim() and stripslashes() for this
New Feature
This PR introduces a new Query class which will hold all related information for custom document filters.
Note - due to
$filterbeing used already, I refer to a custom filter as a$queryuntil we can find something more clear.Test plan
Tests included with PR
Todo
parseExpression()Should we...