Skip to content

Commit b6541a9

Browse files
abnegateabnegate
authored
Merge pull request #749 from utopia-php/feat-operators
Fix validation
2 parents dce1c4f + 08a86fa commit b6541a9

File tree

9 files changed

+609
-95
lines changed

9 files changed

+609
-95
lines changed

‎.github/workflows/codeql-analysis.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,5 @@ jobs:
1616

1717
- name: Run CodeQL
1818
run: |
19-
docker run --rm -v $PWD:/app composer sh -c \
19+
docker run --rm -v $PWD:/app -w /app phpswoole/swoole:5.1.8-php8.3-alpine sh -c \
2020
"composer install --profile --ignore-platform-reqs && composer check"

‎.github/workflows/linter.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,5 @@ jobs:
1616

1717
- name: Run Linter
1818
run: |
19-
docker run --rm -v $PWD:/app composer sh -c \
19+
docker run --rm -v $PWD:/app -w /app phpswoole/swoole:5.1.8-php8.3-alpine sh -c \
2020
"composer install --profile --ignore-platform-reqs && composer lint"

‎bin/tasks/operators.php‎

Lines changed: 73 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,11 @@
88
*
99
* @example
1010
* docker compose exec tests bin/operators --adapter=mariadb --iterations=1000
11-
* docker compose exec tests bin/operators --adapter=postgres --iterations=1000
12-
* docker compose exec tests bin/operators --adapter=sqlite --iterations=1000
11+
* docker compose exec tests bin/operators --adapter=postgres --iterations=1000 --seed=10000
12+
* docker compose exec tests bin/operators --adapter=sqlite --iterations=1000 --seed=5000
13+
*
14+
* The --seed parameter allows you to pre-populate the collection with a specified
15+
* number of documents to test how operators perform with varying amounts of existing data.
1316
*/
1417

1518
global $cli;
@@ -38,8 +41,9 @@
3841
->desc('Benchmark operator performance vs traditional read-modify-write')
3942
->param('adapter', '', new Text(0), 'Database adapter (mariadb, postgres, sqlite)')
4043
->param('iterations', 1000, new Integer(true), 'Number of iterations per test', true)
44+
->param('seed', 0, new Integer(true), 'Number of documents to pre-seed the collection with', true)
4145
->param('name', 'operator_benchmark_' . uniqid(), new Text(0), 'Name of test database', true)
42-
->action(function (string $adapter, int $iterations, string $name) {
46+
->action(function (string $adapter, int $iterations, int $seed, string $name) {
4347
$namespace = '_ns';
4448
$cache = new Cache(new NoCache());
4549

@@ -48,6 +52,7 @@
4852
Console::info("=============================================================");
4953
Console::info("Adapter: {$adapter}");
5054
Console::info("Iterations: {$iterations}");
55+
Console::info("Seed Documents: {$seed}");
5156
Console::info("Database: {$name}");
5257
Console::info("=============================================================\n");
5358

@@ -110,13 +115,13 @@
110115
->setNamespace($namespace);
111116

112117
// Setup test environment
113-
setupTestEnvironment($database, $name);
118+
setupTestEnvironment($database, $name, $seed);
114119

115120
// Run all benchmarks
116121
$results = runAllBenchmarks($database, $iterations);
117122

118123
// Display results
119-
displayResults($results, $adapter, $iterations);
124+
displayResults($results, $adapter, $iterations, $seed);
120125

121126
// Cleanup
122127
cleanup($database, $name);
@@ -133,7 +138,7 @@
133138
/**
134139
* Setup test environment with collections and sample data
135140
*/
136-
function setupTestEnvironment(Database $database, string $name): void
141+
function setupTestEnvironment(Database $database, string $name, int $seed): void
137142
{
138143
Console::info("Setting up test environment...");
139144

@@ -179,9 +184,69 @@ function setupTestEnvironment(Database $database, string $name): void
179184
$database->createAttribute('operators_test', 'created_at', Database::VAR_DATETIME, 0, false, null, false, false, null, [], ['datetime']);
180185
$database->createAttribute('operators_test', 'updated_at', Database::VAR_DATETIME, 0, false, null, false, false, null, [], ['datetime']);
181186

187+
// Seed documents if requested
188+
if ($seed > 0) {
189+
seedDocuments($database, $seed);
190+
}
191+
182192
Console::success("Test environment setup complete.\n");
183193
}
184194

195+
/**
196+
* Seed the collection with a specified number of documents
197+
*/
198+
function seedDocuments(Database $database, int $count): void
199+
{
200+
Console::info("Seeding {$count} documents...");
201+
202+
$batchSize = 100; // Insert in batches for better performance
203+
$batches = (int) ceil($count / $batchSize);
204+
205+
$seedStart = microtime(true);
206+
207+
for ($batch = 0; $batch < $batches; $batch++) {
208+
$docs = [];
209+
$remaining = min($batchSize, $count - ($batch * $batchSize));
210+
211+
for ($i = 0; $i < $remaining; $i++) {
212+
$docNum = ($batch * $batchSize) + $i;
213+
$docs[] = new Document([
214+
'$id' => 'seed_' . $docNum,
215+
'$permissions' => [
216+
Permission::read(Role::any()),
217+
Permission::update(Role::any()),
218+
],
219+
'counter' => rand(0, 1000),
220+
'score' => round(rand(0, 10000) / 100, 2),
221+
'multiplier' => round(rand(50, 200) / 100, 2),
222+
'divider' => round(rand(5000, 15000) / 100, 2),
223+
'modulo_val' => rand(50, 200),
224+
'power_val' => round(rand(100, 300) / 100, 2),
225+
'name' => 'seed_doc_' . $docNum,
226+
'text' => 'Seed text for document ' . $docNum,
227+
'description' => 'This is seed document ' . $docNum . ' with some foo bar baz content',
228+
'active' => (bool) rand(0, 1),
229+
'tags' => ['seed', 'tag' . ($docNum % 10), 'category' . ($docNum % 5)],
230+
'numbers' => [rand(1, 10), rand(11, 20), rand(21, 30)],
231+
'items' => ['item' . ($docNum % 3), 'item' . ($docNum % 7)],
232+
'created_at' => DateTime::now(),
233+
'updated_at' => DateTime::now(),
234+
]);
235+
}
236+
237+
// Bulk insert documents
238+
$database->createDocuments('operators_test', $docs);
239+
240+
// Show progress
241+
$progress = (($batch + 1) * $batchSize);
242+
$current = min($progress, $count);
243+
Console::log(" Seeded {$current}/{$count} documents...");
244+
}
245+
246+
$seedTime = microtime(true) - $seedStart;
247+
Console::success("Seeding completed in " . number_format($seedTime, 2) . "s\n");
248+
}
249+
185250
/**
186251
* Run all operator benchmarks
187252
*/
@@ -848,13 +913,14 @@ function benchmarkOperatorAcrossOperations(
848913
/**
849914
* Display formatted results table
850915
*/
851-
function displayResults(array $results, string $adapter, int $iterations): void
916+
function displayResults(array $results, string $adapter, int $iterations, int $seed): void
852917
{
853918
Console::info("\n=============================================================");
854919
Console::info(" BENCHMARK RESULTS");
855920
Console::info("=============================================================");
856921
Console::info("Adapter: {$adapter}");
857922
Console::info("Iterations per test: {$iterations}");
923+
Console::info("Seeded documents: {$seed}");
858924
Console::info("=============================================================\n");
859925

860926
// ==================================================================

‎src/Database/Connection.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ class Connection
2121
*/
2222
public static function hasError(\Throwable $e): bool
2323
{
24-
/** @phpstan-ignore-next-line can't find static method */
2524
if (DetectsLostConnections::causedByLostConnection($e)) {
2625
return true;
2726
}

‎src/Database/Database.php‎

Lines changed: 40 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
use Utopia\Database\Validator\Authorization;
2828
use Utopia\Database\Validator\Index as IndexValidator;
2929
use Utopia\Database\Validator\IndexDependency as IndexDependencyValidator;
30-
use Utopia\Database\Validator\Operator as OperatorValidator;
3130
use Utopia\Database\Validator\PartialStructure;
3231
use Utopia\Database\Validator\Permissions;
3332
use Utopia\Database\Validator\Queries\Document as DocumentValidator;
@@ -4952,20 +4951,7 @@ public function updateDocument(string $collection, string $id, Document $documen
49524951
}
49534952
$createdAt = $document->getCreatedAt();
49544953

4955-
// Extract operators from the document before merging
4956-
$documentArray = $document->getArrayCopy();
4957-
$extracted = Operator::extractOperators($documentArray);
4958-
$operators = $extracted['operators'];
4959-
$updates = $extracted['updates'];
4960-
4961-
$operatorValidator = new OperatorValidator($collection, $old);
4962-
foreach ($operators as $attribute => $operator) {
4963-
if (!$operatorValidator->isValid($operator)) {
4964-
throw new StructureException($operatorValidator->getDescription());
4965-
}
4966-
}
4967-
4968-
$document = \array_merge($old->getArrayCopy(), $updates);
4954+
$document = \array_merge($old->getArrayCopy(), $document->getArrayCopy());
49694955
$document['$collection'] = $old->getAttribute('$collection'); // Make sure user doesn't switch collection ID
49704956
$document['$createdAt'] = ($createdAt === null || !$this->preserveDates) ? $old->getCreatedAt() : $createdAt;
49714957

@@ -4989,8 +4975,11 @@ public function updateDocument(string $collection, string $id, Document $documen
49894975
$relationships[$relationship->getAttribute('key')] = $relationship;
49904976
}
49914977

4992-
if (!empty($operators)) {
4993-
$shouldUpdate = true;
4978+
foreach ($document as $key => $value) {
4979+
if (Operator::isOperator($value)) {
4980+
$shouldUpdate = true;
4981+
break;
4982+
}
49944983
}
49954984

49964985
// Compare if the document has any changes
@@ -5110,7 +5099,8 @@ public function updateDocument(string $collection, string $id, Document $documen
51105099
$this->adapter->getIdAttributeType(),
51115100
$this->adapter->getMinDateTime(),
51125101
$this->adapter->getMaxDateTime(),
5113-
$this->adapter->getSupportForAttributes()
5102+
$this->adapter->getSupportForAttributes(),
5103+
$old
51145104
);
51155105
if (!$structureValidator->isValid($document)) { // Make sure updated structure still apply collection rules (if any)
51165106
throw new StructureException($structureValidator->getDescription());
@@ -5120,22 +5110,24 @@ public function updateDocument(string $collection, string $id, Document $documen
51205110
$document = $this->silent(fn () => $this->updateDocumentRelationships($collection, $old, $document));
51215111
}
51225112

5123-
51245113
$document = $this->adapter->castingBefore($collection, $document);
51255114

5126-
// Re-add operators to document for adapter processing
5127-
foreach ($operators as $key => $operator) {
5128-
$document->setAttribute($key, $operator);
5129-
}
5130-
51315115
$this->adapter->updateDocument($collection, $id, $document, $skipPermissionsUpdate);
51325116

51335117
$document = $this->adapter->castingAfter($collection, $document);
51345118

51355119
$this->purgeCachedDocument($collection->getId(), $id);
51365120

51375121
// If operators were used, refetch document to get computed values
5138-
if (!empty($operators)) {
5122+
$hasOperators = false;
5123+
foreach ($document->getArrayCopy() as $value) {
5124+
if (Operator::isOperator($value)) {
5125+
$hasOperators = true;
5126+
break;
5127+
}
5128+
}
5129+
5130+
if ($hasOperators) {
51395131
$refetched = $this->refetchDocuments($collection, [$document]);
51405132
$document = $refetched[0];
51415133
}
@@ -5258,24 +5250,17 @@ public function updateDocuments(
52585250
applyDefaults: false
52595251
);
52605252

5261-
// Separate operators from regular updates for validation
5262-
$extracted = Operator::extractOperators($updates->getArrayCopy());
5263-
$operators = $extracted['operators'];
5264-
$regularUpdates = $extracted['updates'];
5265-
5266-
// Only validate regular updates, not operators
5267-
if (!empty($regularUpdates)) {
5268-
$validator = new PartialStructure(
5269-
$collection,
5270-
$this->adapter->getIdAttributeType(),
5271-
$this->adapter->getMinDateTime(),
5272-
$this->adapter->getMaxDateTime(),
5273-
$this->adapter->getSupportForAttributes()
5274-
);
5253+
$validator = new PartialStructure(
5254+
$collection,
5255+
$this->adapter->getIdAttributeType(),
5256+
$this->adapter->getMinDateTime(),
5257+
$this->adapter->getMaxDateTime(),
5258+
$this->adapter->getSupportForAttributes(),
5259+
null // No old document available in bulk updates
5260+
);
52755261

5276-
if (!$validator->isValid(new Document($regularUpdates))) {
5277-
throw new StructureException($validator->getDescription());
5278-
}
5262+
if (!$validator->isValid($updates)) {
5263+
throw new StructureException($validator->getDescription());
52795264
}
52805265

52815266
$originalLimit = $limit;
@@ -5311,17 +5296,8 @@ public function updateDocuments(
53115296
$currentPermissions = $updates->getPermissions();
53125297
sort($currentPermissions);
53135298

5314-
$this->withTransaction(function () use ($collection, $updates, &$batch, $currentPermissions, $operators) {
5299+
$this->withTransaction(function () use ($collection, $updates, &$batch, $currentPermissions) {
53155300
foreach ($batch as $index => $document) {
5316-
if (!empty($operators)) {
5317-
$operatorValidator = new OperatorValidator($collection, $document);
5318-
foreach ($operators as $attribute => $operator) {
5319-
if (!$operatorValidator->isValid($operator)) {
5320-
throw new StructureException($operatorValidator->getDescription());
5321-
}
5322-
}
5323-
}
5324-
53255301
$skipPermissionsUpdate = true;
53265302

53275303
if ($updates->offsetExists('$permissions')) {
@@ -5369,7 +5345,15 @@ public function updateDocuments(
53695345

53705346
$updates = $this->adapter->castingBefore($collection, $updates);
53715347

5372-
if (!empty($operators)) {
5348+
$hasOperators = false;
5349+
foreach ($updates->getArrayCopy() as $value) {
5350+
if (Operator::isOperator($value)) {
5351+
$hasOperators = true;
5352+
break;
5353+
}
5354+
}
5355+
5356+
if ($hasOperators) {
53735357
$batch = $this->refetchDocuments($collection, $batch);
53745358
}
53755359

@@ -6035,45 +6019,19 @@ public function upsertDocumentsWithIncrease(
60356019
}
60366020
}
60376021

6038-
// Extract operators for validation
6039-
$documentArray = $document->getArrayCopy();
6040-
$extracted = Operator::extractOperators($documentArray);
6041-
$operators = $extracted['operators'];
6042-
$regularUpdates = $extracted['updates'];
6043-
6044-
$operatorValidator = new OperatorValidator($collection, $old->isEmpty() ? null : $old);
6045-
foreach ($operators as $attribute => $operator) {
6046-
if (!$operatorValidator->isValid($operator)) {
6047-
throw new StructureException($operatorValidator->getDescription());
6048-
}
6049-
}
6050-
6051-
// Create a temporary document with only regular updates for encoding and validation
6052-
$tempDocument = new Document($regularUpdates);
6053-
$tempDocument->setAttribute('$id', $document->getId());
6054-
$tempDocument->setAttribute('$collection', $document->getAttribute('$collection'));
6055-
$tempDocument->setAttribute('$createdAt', $document->getAttribute('$createdAt'));
6056-
$tempDocument->setAttribute('$updatedAt', $document->getAttribute('$updatedAt'));
6057-
$tempDocument->setAttribute('$permissions', $document->getAttribute('$permissions'));
6058-
if ($this->adapter->getSharedTables()) {
6059-
$tempDocument->setAttribute('$tenant', $document->getAttribute('$tenant'));
6060-
}
6061-
6062-
$encodedTemp = $this->encode($collection, $tempDocument);
6063-
60646022
$validator = new Structure(
60656023
$collection,
60666024
$this->adapter->getIdAttributeType(),
60676025
$this->adapter->getMinDateTime(),
60686026
$this->adapter->getMaxDateTime(),
6069-
$this->adapter->getSupportForAttributes()
6027+
$this->adapter->getSupportForAttributes(),
6028+
$old->isEmpty() ? null : $old
60706029
);
60716030

6072-
if (!$validator->isValid($encodedTemp)) {
6031+
if (!$validator->isValid($document)) {
60736032
throw new StructureException($validator->getDescription());
60746033
}
60756034

6076-
// Now encode the full document with operators for the adapter
60776035
$document = $this->encode($collection, $document);
60786036

60796037
if (!$old->isEmpty()) {

‎src/Database/Validator/Operator.php‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,12 @@ public function getDescription(): string
6060
public function isValid($value): bool
6161
{
6262
if (!$value instanceof DatabaseOperator) {
63-
$this->message = 'Value must be an instance of Operator';
64-
return false;
63+
try {
64+
$value = DatabaseOperator::parse($value);
65+
} catch (\Throwable $e) {
66+
$this->message = 'Invalid operator: ' . $e->getMessage();
67+
return false;
68+
}
6569
}
6670

6771
$method = $value->getMethod();

0 commit comments

Comments
 (0)