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..e35d3be8f 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 */ @@ -11407,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) { 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); + } }