From 75b31deda1dc05e38aace9c056407336cc81e58c Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 16 Oct 2026 21:52:37 +1300 Subject: [PATCH 1/2] fix(documents): hold an increase or decrease to the column's own range in the write itself increaseDocumentAttribute() and decreaseDocumentAttribute() checked only the bound passed in the call, so a column whose format holds it to 0..10 was incremented from 5 by 6 to 11, decremented below 0, and accepted an explicit maximum above its own. The tighter of the call's bound and the Range the column's format validator is now bounds the change, both in the check against the value read and in the conditional write. Adapters return false when that condition left the row unchanged, and the change is refused with Exception\Limit instead of reporting a value that was never stored, so concurrent changes near the bound cannot carry a counter past it. Co-Authored-By: Claude Opus 5.5 --- src/Database/Adapter.php | 5 +- src/Database/Adapter/Memory.php | 14 +- src/Database/Adapter/Mongo.php | 6 +- src/Database/Adapter/Redis.php | 8 +- src/Database/Adapter/SQL.php | 2 +- src/Database/Trait/Documents.php | 83 +++++++- tests/e2e/Adapter/Scopes/DocumentTests.php | 138 +++++++++++++ tests/unit/Adapter/MemoryWritePathsTest.php | 6 +- tests/unit/Adapter/RedisAdapterPathsTest.php | 4 +- tests/unit/Documents/CounterRangeRaceTest.php | 191 ++++++++++++++++++ .../Documents/Interleaving/Interleaves.php | 24 ++- 11 files changed, 446 insertions(+), 35 deletions(-) create mode 100644 tests/unit/Documents/CounterRangeRaceTest.php diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index 612d4507f..7d6b0ff7c 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -757,7 +757,10 @@ abstract public function updateDocument(Document $collection, string $id, Docume abstract public function updateDocuments(Document $collection, Document $updates, array $documents, array $skipPermissions = []): int; /** - * Increase or decrease attribute value + * Increase or decrease attribute value, in one atomic write that changes nothing when the stored value is + * above $max or below $min + * + * @return bool False when the stored value was outside $min or $max, so nothing changed * * @throws Exception */ diff --git a/src/Database/Adapter/Memory.php b/src/Database/Adapter/Memory.php index 8ba136a0c..9ead7a952 100644 --- a/src/Database/Adapter/Memory.php +++ b/src/Database/Adapter/Memory.php @@ -1912,30 +1912,24 @@ public function increaseDocumentAttribute(Document $collection, string $id, stri $exact = (\is_int($current) || (\is_string($current) && BigInt::isIntegerString($current))) && (\is_int($value) || (\is_string($value) && BigInt::isIntegerString($value))); - // MariaDB encodes the bound check as part of the WHERE clause against - // the current column value (`attr <= :max` / `attr >= :min`); when the - // bound is violated the UPDATE simply matches zero rows and the call - // still returns true. Mirror that — silent no-op on bound violation. - // The Database layer pre-subtracts $value from $max (and adds it to - // $min), so the comparison stays against the pre-update value. if ($exact) { $current = BigInt::toNative($current); $value = BigInt::toNative($value); if (! is_null($min) && BigInt::compare($current, $min) < 0) { - return true; + return false; } if (! is_null($max) && BigInt::compare($current, $max) > 0) { - return true; + return false; } $result = BigInt::add($current, $value); } else { $current = $this->numericValue($current, 0) ?? 0; $value = $this->numericValue($value, 0) ?? 0; if (! is_null($min) && $current < $min) { - return true; + return false; } if (! is_null($max) && $current > $max) { - return true; + return false; } $result = $current + $value; } diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index e43499183..fdc80a2e1 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -2365,17 +2365,15 @@ public function increaseDocumentAttribute(Document $collection, string $id, stri ]]]; try { - $this->updateWithPipeline( + return $this->updateWithPipeline( $this->getNamespace().'_'.$this->filter($collectionId), $filters, $pipeline, $this->getTransactionOptions(), - ); + ) > 0; } catch (MongoException $e) { throw $this->processException($e); } - - return true; } private function normalizeAtomicNumber(int|float|string $value, string $name): int|float diff --git a/src/Database/Adapter/Redis.php b/src/Database/Adapter/Redis.php index 429dc767a..52211398a 100644 --- a/src/Database/Adapter/Redis.php +++ b/src/Database/Adapter/Redis.php @@ -1618,20 +1618,20 @@ public function increaseDocumentAttribute(Document $collection, string $id, stri $current = BigInt::toNative($current); $value = BigInt::toNative($value); if (! \is_null($min) && BigInt::compare($current, $min) < 0) { - return true; + return false; } if (! \is_null($max) && BigInt::compare($current, $max) > 0) { - return true; + return false; } $result = BigInt::add($current, $value); } else { $current = $this->numericOr($current, 0); $value = $this->numericOr($value, 0); if (! \is_null($min) && $current < $min) { - return true; + return false; } if (! \is_null($max) && $current > $max) { - return true; + return false; } $result = $current + $value; } diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index 36ef14a21..b1db4dc4e 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -1452,7 +1452,7 @@ public function increaseDocumentAttribute( throw $this->processException($e); } - return true; + return $statement->rowCount() > 0; } #[\Override] diff --git a/src/Database/Trait/Documents.php b/src/Database/Trait/Documents.php index 6793dcc9a..bf9c7e138 100644 --- a/src/Database/Trait/Documents.php +++ b/src/Database/Trait/Documents.php @@ -70,6 +70,7 @@ use Utopia\Query\OrderDirection; use Utopia\Query\Schema\ColumnType; use Utopia\Query\Schema\IndexType; +use Utopia\Validator\Range; use WeakMap; trait Documents @@ -287,6 +288,60 @@ private function numericAttribute(Collection $collection, string $key): ?Attribu return $attribute; } + /** + * The lower of the maximum an increase is given and the maximum the attribute's format holds it to. + */ + private function tighterMaximum(int|float|string|null $max, ?Attribute $attribute): int|float|string|null + { + $limit = $this->formatRange($attribute)?->getMax(); + if ($limit === null || (\is_float($limit) && ! \is_finite($limit))) { + return $max; + } + + if ($this->isDeclaredInteger($attribute)) { + $limit = $this->integerBound(Rounding::Floor->round($limit), 'Max'); + + return $max === null || BigInt::compare($limit, $max) < 0 ? $limit : $max; + } + + return $max === null || $limit < $this->getNativeNumber($max) ? $limit : $max; + } + + /** + * The higher of the minimum a decrease is given and the minimum the attribute's format holds it to. + */ + private function tighterMinimum(int|float|string|null $min, ?Attribute $attribute): int|float|string|null + { + $limit = $this->formatRange($attribute)?->getMin(); + if ($limit === null || (\is_float($limit) && ! \is_finite($limit))) { + return $min; + } + + if ($this->isDeclaredInteger($attribute)) { + $limit = $this->integerBound(Rounding::Ceil->round($limit), 'Min'); + + return $min === null || BigInt::compare($limit, $min) > 0 ? $limit : $min; + } + + return $min === null || $limit > $this->getNativeNumber($min) ? $limit : $min; + } + + /** + * The range an attribute's format holds its values to, when its format validator is a Range, as the + * `intRange`, `bigintRange` and `floatRange` formats Appwrite registers are. + */ + private function formatRange(?Attribute $attribute): ?Range + { + $format = $attribute?->format; + if ($attribute === null || $format === null || ! Structure::hasFormat($format->name, $attribute->type)) { + return null; + } + + $validator = Structure::getFormat($format->name, $attribute->type)['callback']($attribute->toDocument()->getArrayCopy()); + + return $validator instanceof Range ? $validator : null; + } + private function isDeclaredInteger(?Attribute $attribute): bool { return $attribute !== null && ! $attribute->array && $attribute->isInteger(); @@ -2742,14 +2797,16 @@ public function increaseDocumentAttribute( $collection = $this->silent(fn () => $this->getCollection($collection)); $numericAttribute = $this->numericAttribute($collection, $attribute); + $declared = $numericAttribute ?? $this->declaredAttribute($collection, $attribute); - if ($this->isDeclaredInteger($numericAttribute ?? $this->declaredAttribute($collection, $attribute))) { + if ($this->isDeclaredInteger($declared)) { $value = Rounding::HalfAwayFromZero->round($value); $this->assertIntegerChange($value); if ($max !== null) { $max = $this->integerBound(Rounding::Floor->round($max), 'Max'); } } + $max = $this->tighterMaximum($max, $declared); $cacheTarget = $collection->getId() === self::METADATA ? new Document([Document::ID => $id, Document::COLLECTION => self::METADATA]) @@ -2802,13 +2859,14 @@ public function increaseDocumentAttribute( $time = DateTime::nowAfter($document->getUpdatedAt()); $updatedAt = $document->getUpdatedAt(); $updatedAt = (empty($updatedAt) || ! $this->datePreservation()->get()) ? $time : DateTime::setTimezone($updatedAt); + $storedMaximum = null; if ($max !== null) { - $max = $numericAttribute?->isInteger() === true + $storedMaximum = $numericAttribute?->isInteger() === true ? BigInt::subtract($max, $value) : $this->getNativeNumber($max) - $this->getNativeNumber($value); } - $this->adapter->increaseDocumentAttribute( + $increased = $this->adapter->increaseDocumentAttribute( $collection, $id, $attribute, @@ -2816,8 +2874,11 @@ public function increaseDocumentAttribute( ? BigInt::toNative($value) : $this->getNativeNumber($value), $updatedAt, - max: $max + max: $storedMaximum ); + if (! $increased && $storedMaximum !== null) { + throw new LimitException('Attribute value exceeds maximum limit: '.$max); + } $this->purgeCachedDocumentInternal($collection->getId(), $id); $this->queueDocumentPurge($collection->getId(), $id); @@ -2873,14 +2934,16 @@ public function decreaseDocumentAttribute( $collection = $this->silent(fn () => $this->getCollection($collection)); $numericAttribute = $this->numericAttribute($collection, $attribute); + $declared = $numericAttribute ?? $this->declaredAttribute($collection, $attribute); - if ($this->isDeclaredInteger($numericAttribute ?? $this->declaredAttribute($collection, $attribute))) { + if ($this->isDeclaredInteger($declared)) { $value = Rounding::HalfAwayFromZero->round($value); $this->assertIntegerChange($value); if ($min !== null) { $min = $this->integerBound(Rounding::Ceil->round($min), 'Min'); } } + $min = $this->tighterMinimum($min, $declared); $cacheTarget = $collection->getId() === self::METADATA ? new Document([Document::ID => $id, Document::COLLECTION => self::METADATA]) @@ -2933,13 +2996,14 @@ public function decreaseDocumentAttribute( $time = DateTime::nowAfter($document->getUpdatedAt()); $updatedAt = $document->getUpdatedAt(); $updatedAt = (empty($updatedAt) || ! $this->datePreservation()->get()) ? $time : DateTime::setTimezone($updatedAt); + $storedMinimum = null; if ($min !== null) { - $min = $numericAttribute?->isInteger() === true + $storedMinimum = $numericAttribute?->isInteger() === true ? BigInt::add($min, $value) : $this->getNativeNumber($min) + $this->getNativeNumber($value); } - $this->adapter->increaseDocumentAttribute( + $decreased = $this->adapter->increaseDocumentAttribute( $collection, $id, $attribute, @@ -2947,8 +3011,11 @@ public function decreaseDocumentAttribute( ? BigInt::negate($value) : $this->getNativeNumber($value) * -1, $updatedAt, - min: $min + min: $storedMinimum ); + if (! $decreased && $storedMinimum !== null) { + throw new LimitException('Attribute value exceeds minimum limit: '.$min); + } $this->purgeCachedDocumentInternal($collection->getId(), $id); $this->queueDocumentPurge($collection->getId(), $id); diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index 6a18359f9..9999066f7 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -36,6 +36,7 @@ use Utopia\Database\Exception\Type as TypeException; use Utopia\Database\Exception\Unique as UniqueException; use Utopia\Database\Filter; +use Utopia\Database\Format; use Utopia\Database\Id; use Utopia\Database\Index; use Utopia\Database\IntegerWidth; @@ -46,14 +47,22 @@ use Utopia\Database\Relationship; use Utopia\Database\Role; use Utopia\Database\SetType; +use Utopia\Database\Validator\Structure; use Utopia\Query\CursorDirection; use Utopia\Query\Method; use Utopia\Query\OrderDirection; use Utopia\Query\Schema\ColumnType; use Utopia\Query\Schema\IndexType; +use Utopia\Validator\Range; trait DocumentTests { + private const string COUNTER_INTEGER_RANGE = 'counterIntegerRange'; + + private const string COUNTER_BIG_INTEGER_RANGE = 'counterBigIntegerRange'; + + private const string COUNTER_DOUBLE_RANGE = 'counterDoubleRange'; + private static string $moviesCollection = ''; private static string $documentsCollection = ''; @@ -11363,6 +11372,135 @@ public function testIncreaseAndDecreaseFloorAMaximumAndCeilAMinimumOnAnInteger() } } + public function testIncreaseAndDecreaseKeepACounterWithinItsColumnRange(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + $collection = $this->createCounterRangeCollection($database); + + $refusals = [ + 'an increase past the column maximum' => [fn () => $database->increaseDocumentAttribute($collection, 'row', 'b', 6), 'maximum limit: 10'], + 'an increase with a maximum above the column maximum' => [fn () => $database->increaseDocumentAttribute($collection, 'row', 'b', 6, 20), 'maximum limit: 10'], + 'an increase with a maximum below the column maximum' => [fn () => $database->increaseDocumentAttribute($collection, 'row', 'b', 4, 8), 'maximum limit: 8'], + 'a decrease past the column minimum' => [fn () => $database->decreaseDocumentAttribute($collection, 'row', 'b', 8), 'minimum limit: 0'], + 'a decrease with a minimum below the column minimum' => [fn () => $database->decreaseDocumentAttribute($collection, 'row', 'b', 8, -10), 'minimum limit: 0'], + 'a decrease with a minimum above the column minimum' => [fn () => $database->decreaseDocumentAttribute($collection, 'row', 'b', 4, 2), 'minimum limit: 2'], + 'an increase of a 64-bit integer past its column maximum' => [fn () => $database->increaseDocumentAttribute($collection, 'row', 'wide', 2), 'maximum limit: '.(PHP_INT_MAX - 1)], + 'a decrease of a 64-bit integer past its column minimum' => [fn () => $database->decreaseDocumentAttribute($collection, 'low', 'wide', 2), 'minimum limit: '.(PHP_INT_MIN + 1)], + 'an increase of a big integer past its column maximum' => [fn () => $database->increaseDocumentAttribute($collection, 'row', 'big', 2), 'maximum limit: '.(PHP_INT_MAX - 1)], + 'a decrease of a big integer past its column minimum' => [fn () => $database->decreaseDocumentAttribute($collection, 'low', 'big', 2), 'minimum limit: '.(PHP_INT_MIN + 1)], + 'an increase of a double past its column maximum' => [fn () => $database->increaseDocumentAttribute($collection, 'row', 'f', 0.75), 'maximum limit: 2.5'], + 'a decrease of a double past its column minimum' => [fn () => $database->decreaseDocumentAttribute($collection, 'row', 'f', 3.75), 'minimum limit: -1.5'], + ]; + foreach ($refusals as $name => [$change, $message]) { + try { + $change(); + $this->fail($name.' was accepted'); + } catch (LimitException $error) { + $this->assertSame('Attribute value exceeds '.$message, $error->getMessage(), $name); + } + } + + $stored = $database->getDocument($collection, 'row'); + $this->assertSame(5, $stored->getAttribute('b')); + $this->assertSame(PHP_INT_MAX - 2, $stored->getAttribute('wide')); + $this->assertSame(PHP_INT_MAX - 2, $this->storedNumber($stored->getAttribute('big'))); + $this->assertSame(2.0, $stored->getAttribute('f')); + $low = $database->getDocument($collection, 'low'); + $this->assertSame(PHP_INT_MIN + 2, $low->getAttribute('wide')); + $this->assertSame(PHP_INT_MIN + 2, $this->storedNumber($low->getAttribute('big'))); + + $this->assertSame(10, $database->increaseDocumentAttribute($collection, 'row', 'b', 5)->getAttribute('b')); + $this->assertSame(0, $database->decreaseDocumentAttribute($collection, 'row', 'b', 10, -5)->getAttribute('b')); + $this->assertSame(PHP_INT_MAX - 1, $database->increaseDocumentAttribute($collection, 'row', 'wide', 1)->getAttribute('wide')); + $this->assertSame(PHP_INT_MAX - 1, $this->storedNumber($database->increaseDocumentAttribute($collection, 'row', 'big', 1)->getAttribute('big'))); + $this->assertSame(PHP_INT_MIN + 1, $database->decreaseDocumentAttribute($collection, 'low', 'wide', 1)->getAttribute('wide')); + $this->assertSame(PHP_INT_MIN + 1, $this->storedNumber($database->decreaseDocumentAttribute($collection, 'low', 'big', 1)->getAttribute('big'))); + $this->assertSame(2.5, $database->increaseDocumentAttribute($collection, 'row', 'f', 0.5, 100)->getAttribute('f')); + $this->assertSame(-1.5, $database->decreaseDocumentAttribute($collection, 'row', 'f', 4.0)->getAttribute('f')); + + $stored = $database->getDocument($collection, 'row'); + $this->assertSame(0, $stored->getAttribute('b')); + $this->assertSame(PHP_INT_MAX - 1, $stored->getAttribute('wide')); + $this->assertSame(PHP_INT_MAX - 1, $this->storedNumber($stored->getAttribute('big'))); + $this->assertSame(-1.5, $stored->getAttribute('f')); + $low = $database->getDocument($collection, 'low'); + $this->assertSame(PHP_INT_MIN + 1, $low->getAttribute('wide')); + $this->assertSame(PHP_INT_MIN + 1, $this->storedNumber($low->getAttribute('big'))); + } + + public function testAnIncreaseTheStoredValueAlreadyPassedTheBoundOfChangesNothing(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + $collection = $this->createCounterRangeCollection($database); + $definition = $database->getCollection($collection); + $adapter = $database->getAdapter(); + $stored = static fn (string $attribute): mixed => $adapter->getDocument($definition, 'row')->getAttribute($attribute); + + $database->updateDocument($collection, 'row', new Document(['b' => 9])); + $now = DateTime::now(); + + $this->assertTrue($adapter->increaseDocumentAttribute($definition, 'row', 'b', 1, $now, max: 9), 'the first of two increases read at 9 reaches the maximum'); + $this->assertFalse($adapter->increaseDocumentAttribute($definition, 'row', 'b', 1, $now, max: 9), 'the second of two increases read at 9 passes the maximum'); + $this->assertSame(10, $this->storedNumber($stored('b'))); + + $this->assertTrue($adapter->increaseDocumentAttribute($definition, 'row', 'b', -10, $now, min: 10)); + $this->assertFalse($adapter->increaseDocumentAttribute($definition, 'row', 'b', -10, $now, min: 10)); + $this->assertSame(0, $this->storedNumber($stored('b'))); + + $this->assertFalse($adapter->increaseDocumentAttribute($definition, 'row', 'f', 1.0, $now, max: 1.5)); + $this->assertFalse($adapter->increaseDocumentAttribute($definition, 'row', 'f', -4.0, $now, min: 2.5)); + $this->assertSame(2.0, (float) $this->storedNumber($stored('f'))); + } + + /** + * A number as an adapter or a big integer attribute hands it back: an integer string as its integer. + */ + private function storedNumber(mixed $value): int|float + { + if (! \is_numeric($value)) { + $this->fail('Expected a stored number, got '.\get_debug_type($value)); + } + + return $value + 0; + } + + private function createCounterRangeCollection(Database $database): string + { + Structure::addFormat(self::COUNTER_INTEGER_RANGE, static fn (array $attribute): Range => self::counterRange($attribute, Range::TYPE_INTEGER), ColumnType::Integer); + Structure::addFormat(self::COUNTER_BIG_INTEGER_RANGE, static fn (array $attribute): Range => self::counterRange($attribute, Range::TYPE_INTEGER), ColumnType::BigInteger); + Structure::addFormat(self::COUNTER_DOUBLE_RANGE, static fn (array $attribute): Range => self::counterRange($attribute, Range::TYPE_FLOAT), ColumnType::Double); + + $collection = 'counter_range_'.uniqid(); + $database->createCollection(Collection::create(id: $collection, permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any()), + Permission::update(Role::any()), + ], documentSecurity: false)); + $database->createAttribute($collection, Attribute::integer(key: 'b', format: new Format(self::COUNTER_INTEGER_RANGE, ['min' => 0, 'max' => 10]))); + $database->createAttribute($collection, Attribute::integer(key: 'wide', width: IntegerWidth::Bits64, format: new Format(self::COUNTER_INTEGER_RANGE, ['min' => PHP_INT_MIN + 1, 'max' => PHP_INT_MAX - 1]))); + $database->createAttribute($collection, Attribute::bigInteger(key: 'big', format: new Format(self::COUNTER_BIG_INTEGER_RANGE, ['min' => PHP_INT_MIN + 1, 'max' => PHP_INT_MAX - 1]))); + $database->createAttribute($collection, Attribute::double(key: 'f', format: new Format(self::COUNTER_DOUBLE_RANGE, ['min' => -1.5, 'max' => 2.5]))); + $database->createDocument($collection, new Document(['$id' => 'row', 'b' => 5, 'wide' => PHP_INT_MAX - 2, 'big' => PHP_INT_MAX - 2, 'f' => 2.0])); + $database->createDocument($collection, new Document(['$id' => 'low', 'b' => 0, 'wide' => PHP_INT_MIN + 2, 'big' => PHP_INT_MIN + 2, 'f' => 0.0])); + + return $collection; + } + + /** + * The range Appwrite's intRange, bigintRange and floatRange formats hold a column to. + * + * @param array $attribute + */ + private static function counterRange(array $attribute, string $type): Range + { + /** @var array{min?: int|float, max?: int|float} $options */ + $options = $attribute['formatOptions'] ?? []; + + return new Range($options['min'] ?? -INF, $options['max'] ?? INF, $type); + } + public function testIncreaseAndDecreaseRoundTheChangeValueOnAnInteger(): void { /** @var Database $database */ diff --git a/tests/unit/Adapter/MemoryWritePathsTest.php b/tests/unit/Adapter/MemoryWritePathsTest.php index 7a176b2f4..2b8908b75 100644 --- a/tests/unit/Adapter/MemoryWritePathsTest.php +++ b/tests/unit/Adapter/MemoryWritePathsTest.php @@ -188,15 +188,15 @@ public function testACursorWithoutAnOrderPagesBySequence(): void $this->assertSame(['first'], $ids($adapter->find($this->collection(), cursor: $cursor, cursorDirection: CursorDirection::Before))); } - public function testIncrementIsANoOpWhenTheStoredValueAlreadyViolatesTheBound(): void + public function testIncrementChangesNothingAndSaysSoWhenTheStoredValueAlreadyViolatesTheBound(): void { $adapter = $this->adapter(); $this->storeAddress($adapter, 'whole', 'x', 10); $this->storeAddress($adapter, 'fraction', 'y', 10.5); foreach (['whole' => 10, 'fraction' => 10.5] as $id => $stored) { - $this->assertTrue($adapter->increaseDocumentAttribute(new Document(['$id' => self::COLLECTION]), $id, 'visits', 1, self::UPDATED_AT, max: 5)); - $this->assertTrue($adapter->increaseDocumentAttribute(new Document(['$id' => self::COLLECTION]), $id, 'visits', -1, self::UPDATED_AT, min: 20)); + $this->assertFalse($adapter->increaseDocumentAttribute(new Document(['$id' => self::COLLECTION]), $id, 'visits', 1, self::UPDATED_AT, max: 5)); + $this->assertFalse($adapter->increaseDocumentAttribute(new Document(['$id' => self::COLLECTION]), $id, 'visits', -1, self::UPDATED_AT, min: 20)); $this->assertSame($stored, $adapter->getDocument($this->collection(), $id)->getAttribute('visits'), $id); } } diff --git a/tests/unit/Adapter/RedisAdapterPathsTest.php b/tests/unit/Adapter/RedisAdapterPathsTest.php index f7bd135b4..ca0b209ae 100644 --- a/tests/unit/Adapter/RedisAdapterPathsTest.php +++ b/tests/unit/Adapter/RedisAdapterPathsTest.php @@ -344,8 +344,8 @@ public function testIncrementGuardsOfTheAdapter(): void $adapter->createDocument($this->notes(), new Document(['$id' => 'fraction', '$permissions' => [], 'count' => 10.5])); foreach (['whole' => 10, 'fraction' => 10.5] as $id => $stored) { - $this->assertTrue($adapter->increaseDocumentAttribute(new Document(['$id' => self::NOTES]), $id, 'count', 1, '2026-01-01 00:00:00.000', max: 5)); - $this->assertTrue($adapter->increaseDocumentAttribute(new Document(['$id' => self::NOTES]), $id, 'count', -1, '2026-01-01 00:00:00.000', min: 20)); + $this->assertFalse($adapter->increaseDocumentAttribute(new Document(['$id' => self::NOTES]), $id, 'count', 1, '2026-01-01 00:00:00.000', max: 5)); + $this->assertFalse($adapter->increaseDocumentAttribute(new Document(['$id' => self::NOTES]), $id, 'count', -1, '2026-01-01 00:00:00.000', min: 20)); $this->assertSame($stored, $adapter->getDocument($this->notes(), $id)->getAttribute('count'), $id); } diff --git a/tests/unit/Documents/CounterRangeRaceTest.php b/tests/unit/Documents/CounterRangeRaceTest.php new file mode 100644 index 000000000..88c3f8c49 --- /dev/null +++ b/tests/unit/Documents/CounterRangeRaceTest.php @@ -0,0 +1,191 @@ + self::range($attribute, Range::TYPE_INTEGER), ColumnType::Integer); + Structure::addFormat(self::DOUBLE_RANGE, static fn (array $attribute): Range => self::range($attribute, Range::TYPE_FLOAT), ColumnType::Double); + } + + #[\Override] + public static function tearDownAfterClass(): void + { + Structure::removeFormat(self::INTEGER_RANGE); + Structure::removeFormat(self::DOUBLE_RANGE); + } + + /** + * @return iterable + */ + public static function adapters(): iterable + { + yield 'memory' => [static fn (): Memory => new Memory()]; + yield 'without transactions' => [static fn (): Standalone => new Standalone()]; + } + + /** + * @param Closure(): (Memory|Standalone) $adapter + */ + #[DataProvider('adapters')] + public function testAnIncreaseThatAnotherIncreaseCarriedPastTheColumnMaximumIsRefused(Closure $adapter): void + { + [$database, $engine] = $this->database($adapter(), ['b' => 9, 'f' => 2.0]); + $engine->beforeIncrease = $this->concurrently($engine, 'b', 1); + + try { + $database->increaseDocumentAttribute(self::COLLECTION, 'row', 'b', 1); + $this->fail('An increase past the column maximum another increase reached was accepted'); + } catch (LimitException $error) { + $this->assertSame('Attribute value exceeds maximum limit: 10', $error->getMessage()); + } + + $this->assertSame(10, $database->getDocument(self::COLLECTION, 'row')->getAttribute('b')); + } + + /** + * @param Closure(): (Memory|Standalone) $adapter + */ + #[DataProvider('adapters')] + public function testADecreaseThatAnotherDecreaseCarriedPastTheColumnMinimumIsRefused(Closure $adapter): void + { + [$database, $engine] = $this->database($adapter(), ['b' => 1, 'f' => -1.0]); + $engine->beforeIncrease = $this->concurrently($engine, 'f', -0.5); + + try { + $database->decreaseDocumentAttribute(self::COLLECTION, 'row', 'f', 0.25); + $this->fail('A decrease past the column minimum another decrease reached was accepted'); + } catch (LimitException $error) { + $this->assertSame('Attribute value exceeds minimum limit: -1.5', $error->getMessage()); + } + + $this->assertSame(-1.5, $database->getDocument(self::COLLECTION, 'row')->getAttribute('f')); + } + + /** + * @param Closure(): (Memory|Standalone) $adapter + */ + #[DataProvider('adapters')] + public function testAnIncreaseThatAnotherIncreaseCarriedPastTheGivenMaximumIsRefused(Closure $adapter): void + { + [$database, $engine] = $this->database($adapter(), ['b' => 0, 'f' => 0.0]); + $engine->beforeIncrease = $this->concurrently($engine, 'n', 5); + + try { + $database->increaseDocumentAttribute(self::COLLECTION, 'row', 'n', 3, 6); + $this->fail('An increase past the given maximum another increase reached was accepted'); + } catch (LimitException $error) { + $this->assertSame('Attribute value exceeds maximum limit: 6', $error->getMessage()); + } + + $this->assertSame(5, $database->getDocument(self::COLLECTION, 'row')->getAttribute('n')); + } + + /** + * @param Closure(): (Memory|Standalone) $adapter + */ + #[DataProvider('adapters')] + public function testAnIncreaseThatStillFitsAfterAnotherIncreaseIsApplied(Closure $adapter): void + { + [$database, $engine] = $this->database($adapter(), ['b' => 7, 'f' => 2.0]); + $engine->beforeIncrease = $this->concurrently($engine, 'b', 1); + + $database->increaseDocumentAttribute(self::COLLECTION, 'row', 'b', 2); + + $this->assertSame(10, $database->getDocument(self::COLLECTION, 'row')->getAttribute('b')); + } + + /** + * @return Closure(Document, string): void + */ + private function concurrently(Memory|Standalone $engine, string $attribute, int|float $change): Closure + { + return static function (Document $collection, string $id) use ($engine, $attribute, $change): void { + $write = static fn (): bool => $engine->increaseDocumentAttribute($collection, $id, $attribute, $change, DateTime::now()); + if ($engine instanceof Memory) { + $engine->detached($write); + + return; + } + $write(); + }; + } + + /** + * @param array $values + * @return array{Database, Memory|Standalone} + */ + private function database(Memory|Standalone $adapter, array $values): array + { + $authorization = new Authorization(); + $database = new Database($adapter, new Cache(new None())); + $database + ->setAuthorization($authorization) + ->setDatabase('race') + ->setNamespace('race_'.\uniqid()); + $database->create(); + + $database->createCollection(Collection::create( + id: self::COLLECTION, + attributes: [ + Attribute::integer(key: 'b', format: new Format(self::INTEGER_RANGE, ['min' => 0, 'max' => 10])), + Attribute::double(key: 'f', format: new Format(self::DOUBLE_RANGE, ['min' => -1.5, 'max' => 2.5])), + Attribute::integer(key: 'n'), + ], + permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any()), + Permission::update(Role::any()), + ], + documentSecurity: false, + )); + $database->createDocument(self::COLLECTION, new Document(['$id' => 'row', 'n' => 0, ...$values])); + + return [$database, $adapter]; + } + + /** + * @param array $attribute + */ + private static function range(array $attribute, string $type): Range + { + /** @var array{min?: int|float, max?: int|float} $options */ + $options = $attribute['formatOptions'] ?? []; + + return new Range($options['min'] ?? -INF, $options['max'] ?? INF, $type); + } +} diff --git a/tests/unit/Documents/Interleaving/Interleaves.php b/tests/unit/Documents/Interleaving/Interleaves.php index c564b20a4..05ec10d6c 100644 --- a/tests/unit/Documents/Interleaving/Interleaves.php +++ b/tests/unit/Documents/Interleaving/Interleaves.php @@ -8,8 +8,9 @@ use Utopia\Query\CursorDirection; /** - * Runs a write of the test's choosing right after a find() returns, or right before a bulk delete reaches the - * engine, as another connection whose write lands in between would, and counts the finds and bulk deletes. + * Runs a write of the test's choosing right after a find() returns, or right before a bulk delete or an increase + * reaches the engine, as another connection whose write lands in between would, and counts the finds and bulk + * deletes. */ trait Interleaves { @@ -23,6 +24,13 @@ trait Interleaves */ public ?Closure $beforeDelete = null; + /** + * Runs once, before the next increase or decrease reaches the engine. + * + * @var (Closure(Document, string): void)|null + */ + public ?Closure $beforeIncrease = null; + public int $finds = 0; public int $deletes = 0; @@ -52,4 +60,16 @@ public function deleteDocuments(Document $collection, array $sequences, array $p return parent::deleteDocuments($collection, $sequences, $permissionIds); } + + #[\Override] + public function increaseDocumentAttribute(Document $collection, string $id, string $attribute, int|float|string $value, string $updatedAt, int|float|string|null $min = null, int|float|string|null $max = null): bool + { + $beforeIncrease = $this->beforeIncrease; + $this->beforeIncrease = null; + if ($beforeIncrease !== null) { + $beforeIncrease($collection, $id); + } + + return parent::increaseDocumentAttribute($collection, $id, $attribute, $value, $updatedAt, $min, $max); + } } From a8a31c10ce2c43e61da293ba6ac94c66dfe43db3 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 16 Oct 2026 21:51:36 +1300 Subject: [PATCH 2/2] test(find): give each distinct row its own creation time Two documents created in the same millisecond share $createdAt, so a distinct read of k and $createdAt returned three rows instead of four on fast runs. Co-Authored-By: Claude Opus 5.5 --- tests/e2e/Adapter/Scopes/DocumentTests.php | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index 9999066f7..e35d3be8f 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -11545,9 +11545,16 @@ public function testDistinctRowsCarryOnlyWhatTheySelectAndRebuildIntoADocument() ], permissions: [Permission::create(Role::any()), Permission::read(Role::any())], )); - foreach ([['a', 1], ['a', 1], ['b', 2], [null, null]] as [$k, $n]) { - $database->createDocument($collection, new Document(['k' => $k, 'n' => $n, '$permissions' => [Permission::read(Role::any())]])); - } + $database->withPreserveDates(true, function () use ($database, $collection): void { + foreach ([['a', 1], ['a', 1], ['b', 2], [null, null]] as $second => [$k, $n]) { + $database->createDocument($collection, new Document([ + 'k' => $k, + 'n' => $n, + '$createdAt' => '2026-01-01T00:00:0'.$second.'.000+00:00', + '$permissions' => [Permission::read(Role::any())], + ])); + } + }); $roundTrip = function (array $rows, string $case): void { foreach ($rows as $row) {