Skip to content

Add query validator - #5

Merged
eldadfux merged 12 commits into
utopia-php:v0from
kodumbeats:feat-query-validator
Apr 29, 2021
Merged

Add query validator#5
eldadfux merged 12 commits into
utopia-php:v0from
kodumbeats:feat-query-validator

Conversation

@kodumbeats

Copy link
Copy Markdown
Contributor

This PR introduces a query validator which checks a query against a collection schema for validity.

@kodumbeats

Copy link
Copy Markdown
ContributorAuthor

As of ea0c622

$ docker-compose exec tests vendor/bin/phpunit --debug tests/Database/Validator/QueryValidatorTest.php
PHPUnit 9.5.4 by Sebastian Bergmann and contributors.
Test 'Utopia\Tests\Validator\QueryValidatorTest::testQuery' started
Test 'Utopia\Tests\Validator\QueryValidatorTest::testQuery' ended
Test 'Utopia\Tests\Validator\QueryValidatorTest::testInvalidOperator' started
Test 'Utopia\Tests\Validator\QueryValidatorTest::testInvalidOperator' ended
Test 'Utopia\Tests\Validator\QueryValidatorTest::testAttributeNotFound' started
Test 'Utopia\Tests\Validator\QueryValidatorTest::testAttributeNotFound' ended
Test 'Utopia\Tests\Validator\QueryValidatorTest::testAttributeWrongType' started
Test 'Utopia\Tests\Validator\QueryValidatorTest::testAttributeWrongType' ended
Time: 00:00.003, Memory: 6.00 MB
OK (4 tests, 7 assertions)
$ docker-compose exec tests vendor/bin/phpunit --debug 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::testReadPermissionsSuccess' started
Test 'Utopia\Tests\Adapter\MariaDBTest::testReadPermissionsSuccess' ended
Test 'Utopia\Tests\Adapter\MariaDBTest::testReadPermissionsFailure' started
Test 'Utopia\Tests\Adapter\MariaDBTest::testReadPermissionsFailure' ended
Test 'Utopia\Tests\Adapter\MariaDBTest::testWritePermissionsSuccess' started
Test 'Utopia\Tests\Adapter\MariaDBTest::testWritePermissionsSuccess' ended
Test 'Utopia\Tests\Adapter\MariaDBTest::testWritePermissionsFailure' started
Test 'Utopia\Tests\Adapter\MariaDBTest::testWritePermissionsFailure' ended
Test 'Utopia\Tests\Adapter\MariaDBTest::testWritePermissionsUpdateFailure' started
Test 'Utopia\Tests\Adapter\MariaDBTest::testWritePermissionsUpdateFailure' ended
Test 'Utopia\Tests\Adapter\MariaDBTest::testExceptionDuplicate' started
Test 'Utopia\Tests\Adapter\MariaDBTest::testExceptionDuplicate' ended
Time: 00:00.752, Memory: 6.00 MB
OK (16 tests, 94 assertions)

Comment threadtests/Database/Validator/QueryValidatorTest.php Outdated
Comment threadtests/Database/Validator/QueryValidatorTest.php Outdated
Comment threadtests/Database/Validator/QueryValidatorTest.php Outdated
Comment threadsrc/Database/Database.php
@kodumbeats
kodumbeats marked this pull request as ready for review April 29, 2021 16:56
@eldadfux
eldadfux merged commit 1d8df79 into utopia-php:v0Apr 29, 2021
premtsd-code added a commit that referenced this pull request Apr 13, 2026
#1 Drop $enable flag on skipDuplicates() scope guard
The $enable param made every non-skipDuplicates createDocuments call pay
for a closure allocation + extra function call. Branch at the call site
instead so the cost only applies when the flag is actually set.
- Adapter::skipDuplicates(callable, bool) → skipDuplicates(callable)
- Database::skipDuplicates(callable, bool) → skipDuplicates(callable)
- Database::createDocuments, Mirror::createDocuments, Pool::delegate,
Pool::withTransaction now branch inline.
#2 Drop fetchExistingByIds helper, inline find()
The helper's per-tenant grouping defended a hypothetical multi-tenant
batching scenario that no caller exercises (relationships are intra-
tenant, callers always batch per tenant). Existing patterns in the same
file (refetchDocuments, relationship loading) just call find() directly.
Match that idiom and drop ~70 lines.
#4 Mirror: only capture inserted docs in skipDuplicates mode
The captureOnNext accumulator paid the cost (closure + per-doc array
push) on every createDocuments call, including the common non-skip path.
Branch at the entry of Mirror::createDocuments so the capture only
happens when skipDuplicates is set; the non-skip path passes through
to source/destination unchanged.
#5 Move getInsertKeyword/Suffix/PermissionsSuffix to getters cluster
Were sitting next to createDocuments(); moved to the getSupport*
cluster around line 1030 where other adapter-capability shims live.
Not addressed:
- #2 partial: the existing patterns (refetchDocuments etc.) don't handle
tenant-per-document multi-tenant batches either, so this is consistent.
- #3 (drop the pre-filter): rejected. createDocumentRelationships runs
in the encoding loop BEFORE the adapter's INSERT IGNORE no-ops the
parent, so dropping the pre-filter would deterministically duplicate
child rows on every CSV re-import of a collection with relationships
(not a race window — every call). The relationships test verifies
this. Reverting would require reintroducing the deferred-relationships
scaffolding we just removed, and the adapter still couldn't tell us
which parents were actually inserted (SQL INSERT IGNORE has no per-row
reporting). Pre-filter stays.
premtsd-code added a commit that referenced this pull request Apr 15, 2026
Follow-up to Jake's #5 review comment — the fix was only applied to
SQL.php. Propagate the same grouping to the other adapters and fix one
cosmetic byproduct from the Mirror refactor.
- Postgres.php: move getInsertKeyword/Suffix/PermissionsSuffix from
between createDocuments() and getUpsertStatement() down to the
getSupportFor* cluster (matches SQL.php:1036 layout).
- SQLite.php: move getInsertKeyword override from the top of the class
to after the getSupportFor* cluster.
- Adapter.php: skipDuplicates() method was inserted between protected
property declarations, breaking the property cluster. Move the method
down next to withTransaction() — the other callable-taking scope
guard. $skipDuplicates property stays with its peers.
- Mirror.php: revert the incidental fn () => collapse on the non-skip
destination createDocuments call back to the original two-line form,
minimizing unrelated diff noise.
Pure relocation / style. No behavior change.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@kodumbeats@eldadfux