Skip to content

Fix validation #749

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.

Sign up for GitHub

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

Merged
abnegate merged 4 commits into from
Oct 30, 2025
Merged

Fix validation #749

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c31f611
Fix validation
abnegate Oct 30, 2025
78854fb
Use swoole images
abnegate Oct 30, 2025
6f6544c
Set workdir
abnegate Oct 30, 2025
08a86fa
Fix stan ignore
abnegate Oct 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 73 additions & 7 deletions bin/tasks/operators.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@
*
* @example
* docker compose exec tests bin/operators --adapter=mariadb --iterations=1000
* docker compose exec tests bin/operators --adapter=postgres --iterations=1000
* docker compose exec tests bin/operators --adapter=sqlite --iterations=1000
* docker compose exec tests bin/operators --adapter=postgres --iterations=1000 --seed=10000
* docker compose exec tests bin/operators --adapter=sqlite --iterations=1000 --seed=5000
*
* The --seed parameter allows you to pre-populate the collection with a specified
* number of documents to test how operators perform with varying amounts of existing data.
*/

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

Expand All @@ -48,6 +52,7 @@
Console::info("=============================================================");
Console::info("Adapter: {$adapter}");
Console::info("Iterations: {$iterations}");
Console::info("Seed Documents: {$seed}");
Console::info("Database: {$name}");
Console::info("=============================================================\n");

Expand Down Expand Up @@ -110,13 +115,13 @@
->setNamespace($namespace);

// Setup test environment
setupTestEnvironment($database, $name);
setupTestEnvironment($database, $name, $seed);

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

// Display results
displayResults($results, $adapter, $iterations);
displayResults($results, $adapter, $iterations, $seed);

// Cleanup
cleanup($database, $name);
Expand All @@ -133,7 +138,7 @@
/**
* Setup test environment with collections and sample data
*/
function setupTestEnvironment(Database $database, string $name): void
function setupTestEnvironment(Database $database, string $name, int $seed): void
{
Console::info("Setting up test environment...");

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

// Seed documents if requested
if ($seed > 0) {
seedDocuments($database, $seed);
}

Console::success("Test environment setup complete.\n");
}

/**
* Seed the collection with a specified number of documents
*/
function seedDocuments(Database $database, int $count): void
{
Console::info("Seeding {$count} documents...");

$batchSize = 100; // Insert in batches for better performance
$batches = (int) ceil($count / $batchSize);

$seedStart = microtime(true);

for ($batch = 0; $batch < $batches; $batch++) {
$docs = [];
$remaining = min($batchSize, $count - ($batch * $batchSize));

for ($i = 0; $i < $remaining; $i++) {
$docNum = ($batch * $batchSize) + $i;
$docs[] = new Document([
'$id' => 'seed_' . $docNum,
'$permissions' => [
Permission::read(Role::any()),
Permission::update(Role::any()),
],
'counter' => rand(0, 1000),
'score' => round(rand(0, 10000) / 100, 2),
'multiplier' => round(rand(50, 200) / 100, 2),
'divider' => round(rand(5000, 15000) / 100, 2),
'modulo_val' => rand(50, 200),
'power_val' => round(rand(100, 300) / 100, 2),
'name' => 'seed_doc_' . $docNum,
'text' => 'Seed text for document ' . $docNum,
'description' => 'This is seed document ' . $docNum . ' with some foo bar baz content',
'active' => (bool) rand(0, 1),
'tags' => ['seed', 'tag' . ($docNum % 10), 'category' . ($docNum % 5)],
'numbers' => [rand(1, 10), rand(11, 20), rand(21, 30)],
'items' => ['item' . ($docNum % 3), 'item' . ($docNum % 7)],
'created_at' => DateTime::now(),
'updated_at' => DateTime::now(),
]);
}

// Bulk insert documents
$database->createDocuments('operators_test', $docs);

// Show progress
$progress = (($batch + 1) * $batchSize);
$current = min($progress, $count);
Console::log(" Seeded {$current}/{$count} documents...");
}

$seedTime = microtime(true) - $seedStart;
Console::success("Seeding completed in " . number_format($seedTime, 2) . "s\n");
}

/**
* Run all operator benchmarks
*/
Expand Down Expand Up @@ -848,13 +913,14 @@ function benchmarkOperatorAcrossOperations(
/**
* Display formatted results table
*/
function displayResults(array $results, string $adapter, int $iterations): void
function displayResults(array $results, string $adapter, int $iterations, int $seed): void
{
Console::info("\n=============================================================");
Console::info(" BENCHMARK RESULTS");
Console::info("=============================================================");
Console::info("Adapter: {$adapter}");
Console::info("Iterations per test: {$iterations}");
Console::info("Seeded documents: {$seed}");
Console::info("=============================================================\n");

// ==================================================================
Expand Down
121 changes: 40 additions & 81 deletions src/Database/Database.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4952,20 +4952,7 @@ public function updateDocument(string $collection, string $id, Document $documen
}
$createdAt = $document->getCreatedAt();

// Extract operators from the document before merging
$documentArray = $document->getArrayCopy();
$extracted = Operator::extractOperators($documentArray);
$operators = $extracted['operators'];
$updates = $extracted['updates'];

$operatorValidator = new OperatorValidator($collection, $old);
foreach ($operators as $attribute => $operator) {
if (!$operatorValidator->isValid($operator)) {
throw new StructureException($operatorValidator->getDescription());
}
}

$document = \array_merge($old->getArrayCopy(), $updates);
$document = \array_merge($old->getArrayCopy(), $document->getArrayCopy());
$document['$collection'] = $old->getAttribute('$collection'); // Make sure user doesn't switch collection ID
$document['$createdAt'] = ($createdAt === null || !$this->preserveDates) ? $old->getCreatedAt() : $createdAt;

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

if (!empty($operators)) {
$shouldUpdate = true;
foreach ($document as $key => $value) {
if (Operator::isOperator($value)) {
$shouldUpdate = true;
break;
}
}

// Compare if the document has any changes
Expand Down Expand Up @@ -5110,7 +5100,8 @@ public function updateDocument(string $collection, string $id, Document $documen
$this->adapter->getIdAttributeType(),
$this->adapter->getMinDateTime(),
$this->adapter->getMaxDateTime(),
$this->adapter->getSupportForAttributes()
$this->adapter->getSupportForAttributes(),
$old
);
if (!$structureValidator->isValid($document)) { // Make sure updated structure still apply collection rules (if any)
throw new StructureException($structureValidator->getDescription());
Expand All @@ -5120,22 +5111,24 @@ public function updateDocument(string $collection, string $id, Document $documen
$document = $this->silent(fn () => $this->updateDocumentRelationships($collection, $old, $document));
}


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

// Re-add operators to document for adapter processing
foreach ($operators as $key => $operator) {
$document->setAttribute($key, $operator);
}

$this->adapter->updateDocument($collection, $id, $document, $skipPermissionsUpdate);

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

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

// If operators were used, refetch document to get computed values
if (!empty($operators)) {
$hasOperators = false;
foreach ($document->getArrayCopy() as $value) {
if (Operator::isOperator($value)) {
$hasOperators = true;
break;
}
}

if ($hasOperators) {
$refetched = $this->refetchDocuments($collection, [$document]);
$document = $refetched[0];
}
Expand Down Expand Up @@ -5258,24 +5251,17 @@ public function updateDocuments(
applyDefaults: false
);

// Separate operators from regular updates for validation
$extracted = Operator::extractOperators($updates->getArrayCopy());
$operators = $extracted['operators'];
$regularUpdates = $extracted['updates'];

// Only validate regular updates, not operators
if (!empty($regularUpdates)) {
$validator = new PartialStructure(
$collection,
$this->adapter->getIdAttributeType(),
$this->adapter->getMinDateTime(),
$this->adapter->getMaxDateTime(),
$this->adapter->getSupportForAttributes()
);
$validator = new PartialStructure(
$collection,
$this->adapter->getIdAttributeType(),
$this->adapter->getMinDateTime(),
$this->adapter->getMaxDateTime(),
$this->adapter->getSupportForAttributes(),
null // No old document available in bulk updates
);

if (!$validator->isValid(new Document($regularUpdates))) {
throw new StructureException($validator->getDescription());
}
if (!$validator->isValid($updates)) {
throw new StructureException($validator->getDescription());
}

$originalLimit = $limit;
Expand Down Expand Up @@ -5311,17 +5297,8 @@ public function updateDocuments(
$currentPermissions = $updates->getPermissions();
sort($currentPermissions);

$this->withTransaction(function () use ($collection, $updates, &$batch, $currentPermissions, $operators) {
$this->withTransaction(function () use ($collection, $updates, &$batch, $currentPermissions) {
foreach ($batch as $index => $document) {
if (!empty($operators)) {
$operatorValidator = new OperatorValidator($collection, $document);
foreach ($operators as $attribute => $operator) {
if (!$operatorValidator->isValid($operator)) {
throw new StructureException($operatorValidator->getDescription());
}
}
}

$skipPermissionsUpdate = true;

if ($updates->offsetExists('$permissions')) {
Expand Down Expand Up @@ -5369,7 +5346,15 @@ public function updateDocuments(

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

if (!empty($operators)) {
$hasOperators = false;
foreach ($updates->getArrayCopy() as $value) {
if (Operator::isOperator($value)) {
$hasOperators = true;
break;
}
}

if ($hasOperators) {
$batch = $this->refetchDocuments($collection, $batch);
}

Expand Down Expand Up @@ -6035,45 +6020,19 @@ public function upsertDocumentsWithIncrease(
}
}

// Extract operators for validation
$documentArray = $document->getArrayCopy();
$extracted = Operator::extractOperators($documentArray);
$operators = $extracted['operators'];
$regularUpdates = $extracted['updates'];

$operatorValidator = new OperatorValidator($collection, $old->isEmpty() ? null : $old);
foreach ($operators as $attribute => $operator) {
if (!$operatorValidator->isValid($operator)) {
throw new StructureException($operatorValidator->getDescription());
}
}

// Create a temporary document with only regular updates for encoding and validation
$tempDocument = new Document($regularUpdates);
$tempDocument->setAttribute('$id', $document->getId());
$tempDocument->setAttribute('$collection', $document->getAttribute('$collection'));
$tempDocument->setAttribute('$createdAt', $document->getAttribute('$createdAt'));
$tempDocument->setAttribute('$updatedAt', $document->getAttribute('$updatedAt'));
$tempDocument->setAttribute('$permissions', $document->getAttribute('$permissions'));
if ($this->adapter->getSharedTables()) {
$tempDocument->setAttribute('$tenant', $document->getAttribute('$tenant'));
}

$encodedTemp = $this->encode($collection, $tempDocument);

$validator = new Structure(
$collection,
$this->adapter->getIdAttributeType(),
$this->adapter->getMinDateTime(),
$this->adapter->getMaxDateTime(),
$this->adapter->getSupportForAttributes()
$this->adapter->getSupportForAttributes(),
$old->isEmpty() ? null : $old
);

if (!$validator->isValid($encodedTemp)) {
if (!$validator->isValid($document)) {
throw new StructureException($validator->getDescription());
}

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

if (!$old->isEmpty()) {
Expand Down
8 changes: 6 additions & 2 deletions src/Database/Validator/Operator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,12 @@ public function getDescription(): string
public function isValid($value): bool
{
if (!$value instanceof DatabaseOperator) {
$this->message = 'Value must be an instance of Operator';
return false;
try {
$value = DatabaseOperator::parse($value);
} catch (\Throwable $e) {
$this->message = 'Invalid operator: ' . $e->getMessage();
return false;
}
}

$method = $value->getMethod();
Expand Down
17 changes: 16 additions & 1 deletion src/Database/Validator/Structure.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
use Utopia\Database\Database;
use Utopia\Database\Document;
use Utopia\Database\Exception as DatabaseException;
use Utopia\Database\Operator;
use Utopia\Database\Validator\Datetime as DatetimeValidator;
use Utopia\Database\Validator\Operator as OperatorValidator;
use Utopia\Validator;
use Utopia\Validator\Boolean;
use Utopia\Validator\FloatValidator;
Expand Down Expand Up @@ -106,7 +108,8 @@ public function __construct(
private readonly string $idAttributeType,
private readonly \DateTime $minAllowedDate = new \DateTime('0000-01-01'),
private readonly \DateTime $maxAllowedDate = new \DateTime('9999-12-31'),
private bool $supportForAttributes = true
private bool $supportForAttributes = true,
private readonly ?Document $currentDocument = null
) {
}

Expand Down Expand Up @@ -305,6 +308,18 @@ protected function checkForUnknownAttributes(array $structure, array $keys): boo
protected function checkForInvalidAttributeValues(array $structure, array $keys): bool
{
foreach ($structure as $key => $value) {
if (Operator::isOperator($value)) {
// Set the attribute name on the operator for validation
$value->setAttribute($key);

$operatorValidator = new OperatorValidator($this->collection, $this->currentDocument);
if (!$operatorValidator->isValid($value)) {
$this->message = $operatorValidator->getDescription();
return false;
}
Comment on lines +311 to +319
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Handle array-backed attribute metadata before invoking OperatorValidator

OperatorValidator still builds its attribute map by calling getAttribute()/getId() on each entry returned from $collection->getAttribute('attributes'). In many code paths (including this test fixture) the collection metadata is hydrated from plain arrays, so as soon as we hit this new branch we instantiate OperatorValidator with array entries and trigger a fatal Call to a member function getAttribute() on array. That turns operator-based updates into hard crashes instead of validation errors. Please normalise the collection attributes to Document instances (or update OperatorValidator to accept arrays) before constructing it here so this code path works for both array- and Document-backed metadata.

🤖 Prompt for AI Agents 
In src/Database/Validator/Structure.php around lines 311 to 319, the new branch
that constructs OperatorValidator assumes collection attribute metadata are
Document objects and crashes when attributes are plain arrays; before creating
OperatorValidator, normalize the collection's 'attributes' entries into Document
instances (or otherwise convert array entries to the same object type
OperatorValidator expects) so getAttribute()/getId() calls are safe — detect
array entries and wrap/convert them into Document objects (preserving original
keys/values and any IDs) and then instantiate OperatorValidator with the
normalized collection metadata; alternatively, if preferred, adjust
OperatorValidator to accept arrays, but the simplest fix here is to coerce the
collection attributes to Document instances prior to constructing
OperatorValidator.

continue;
}

$attribute = $keys[$key] ?? [];
$type = $attribute['type'] ?? '';
$array = $attribute['array'] ?? false;
Expand Down
Loading
Loading