From 590a7848743e06971b421291679c9275d2a4f166 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 9 Jun 2025 22:42:32 -0400 Subject: [PATCH 1/4] Return new value atomically --- src/Database/Adapter.php | 10 +- src/Database/Adapter/MariaDB.php | 18 +- src/Database/Database.php | 206 +++++++++++---------- tests/e2e/Adapter/Scopes/DocumentTests.php | 8 +- 4 files changed, 136 insertions(+), 106 deletions(-) diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index 5f3bde320..d3584a437 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -1153,7 +1153,15 @@ protected function escapeWildcards(string $value): string * @return bool * @throws Exception */ - abstract public function increaseDocumentAttribute(string $collection, string $id, string $attribute, int|float $value, string $updatedAt, int|float|null $min = null, int|float|null $max = null): bool; + abstract public function increaseDocumentAttribute( + string $collection, + string $id, + string $attribute, + int|float $value, + string $updatedAt, + int|float|null $min = null, + int|float|null $max = null + ): bool; /** * Returns the connection ID identifier diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index bbb6eee52..bab2eb267 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -1382,8 +1382,15 @@ public function createOrUpdateDocuments( * @return bool * @throws DatabaseException */ - public function increaseDocumentAttribute(string $collection, string $id, string $attribute, int|float $value, string $updatedAt, int|float|null $min = null, int|float|null $max = null): bool - { + public function increaseDocumentAttribute( + string $collection, + string $id, + string $attribute, + int|float $value, + string $updatedAt, + int|float|null $min = null, + int|float|null $max = null + ): bool { $name = $this->filter($collection); $attribute = $this->filter($attribute); @@ -1412,7 +1419,12 @@ public function increaseDocumentAttribute(string $collection, string $id, string $stmt->bindValue(':_tenant', $this->tenant); } - $stmt->execute() || throw new DatabaseException('Failed to update attribute'); + try { + $stmt->execute(); + } catch (PDOException $e) { + throw $this->processException($e); + } + return true; } diff --git a/src/Database/Database.php b/src/Database/Database.php index 568322282..3bc91f5d2 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -5091,44 +5091,32 @@ public function createOrUpdateDocumentsWithIncrease( /** * Increase a document attribute by a value * - * @param string $collection - * @param string $id - * @param string $attribute - * @param int|float $value - * @param int|float|null $max - * @return bool - * + * @param string $collection The collection ID + * @param string $id The document ID + * @param string $attribute The attribute to increase + * @param int|float $value The value to increase the attribute by, can be a float + * @param int|float|null $max The maximum value the attribute can reach after the increase, null means no limit + * @return int The new value of the attribute after the increase * @throws AuthorizationException * @throws DatabaseException - * @throws Exception + * @throws LimitException + * @throws NotFoundException + * @throws TypeException + * @throws \Throwable */ - public function increaseDocumentAttribute(string $collection, string $id, string $attribute, int|float $value = 1, int|float|null $max = null): bool - { + public function increaseDocumentAttribute( + string $collection, + string $id, + string $attribute, + int|float $value = 1, + int|float|null $max = null + ): int|float { if ($value <= 0) { // Can be a float throw new DatabaseException('Value must be numeric and greater than 0'); } - $validator = new Authorization(self::PERMISSION_UPDATE); - - /* @var $document Document */ - $document = Authorization::skip(fn () => $this->silent(fn () => $this->getDocument($collection, $id))); // Skip ensures user does not need read permission for this - - if ($document->isEmpty()) { - return false; - } - $collection = $this->silent(fn () => $this->getCollection($collection)); - if ($collection->getId() !== self::METADATA) { - $documentSecurity = $collection->getAttribute('documentSecurity', false); - if (!$validator->isValid([ - ...$collection->getUpdate(), - ...($documentSecurity ? $document->getUpdate() : []) - ])) { - throw new AuthorizationException($validator->getDescription()); - } - } - $attr = \array_filter($collection->getAttribute('attributes', []), function ($a) use ($attribute) { return $a['$id'] === $attribute; }); @@ -5137,46 +5125,63 @@ public function increaseDocumentAttribute(string $collection, string $id, string throw new NotFoundException('Attribute not found'); } - $whiteList = [self::VAR_INTEGER, self::VAR_FLOAT]; + $whiteList = [ + self::VAR_INTEGER, + self::VAR_FLOAT + ]; - /** - * @var Document $attr - */ + /** @var Document $attr */ $attr = \end($attr); if (!in_array($attr->getAttribute('type'), $whiteList)) { throw new TypeException('Attribute type must be one of: ' . implode(',', $whiteList)); } - if ($max && ($document->getAttribute($attribute) + $value > $max)) { - throw new LimitException('Attribute value exceeds maximum limit: ' . $max); - } + $document = $this->withTransaction(function () use ($collection, $id, $attribute, $value, $max) { + /* @var $document Document */ + $document = Authorization::skip(fn () => $this->silent(fn () => $this->getDocument($collection->getId(), $id, forUpdate: true))); // Skip ensures user does not need read permission for this - $time = DateTime::now(); - $updatedAt = $document->getUpdatedAt(); - $updatedAt = (empty($updatedAt) || !$this->preserveDates) ? $time : $updatedAt; + if ($document->isEmpty()) { + return false; + } - // Check if document was updated after the request timestamp - $oldUpdatedAt = new \DateTime($document->getUpdatedAt()); - if (!is_null($this->timestamp) && $oldUpdatedAt > $this->timestamp) { - throw new ConflictException('Document was updated after the request timestamp'); - } + $validator = new Authorization(self::PERMISSION_UPDATE); + + if ($collection->getId() !== self::METADATA) { + $documentSecurity = $collection->getAttribute('documentSecurity', false); + if (!$validator->isValid([ + ...$collection->getUpdate(), + ...($documentSecurity ? $document->getUpdate() : []) + ])) { + throw new AuthorizationException($validator->getDescription()); + } + } - $max = $max ? $max - $value : null; + if ($max && ($document->getAttribute($attribute) + $value > $max)) { + throw new LimitException('Attribute value exceeds maximum limit: ' . $max); + } - $result = $this->adapter->increaseDocumentAttribute( - $collection->getId(), - $id, - $attribute, - $value, - $updatedAt, - max: $max - ); + $time = DateTime::now(); + $updatedAt = $document->getUpdatedAt(); + $updatedAt = (empty($updatedAt) || !$this->preserveDates) ? $time : $updatedAt; + $max = $max ? $max - $value : null; + + $this->adapter->increaseDocumentAttribute( + $collection->getId(), + $id, + $attribute, + $value, + $updatedAt, + max: $max + ); + + return $document; + }); $this->purgeCachedDocument($collection->getId(), $id); $this->trigger(self::EVENT_DOCUMENT_INCREASE, $document); - return $result; + return $document->getAttribute($attribute) + $value; } @@ -5188,38 +5193,24 @@ public function increaseDocumentAttribute(string $collection, string $id, string * @param string $attribute * @param int|float $value * @param int|float|null $min - * @return bool + * @return int|float * * @throws AuthorizationException * @throws DatabaseException */ - public function decreaseDocumentAttribute(string $collection, string $id, string $attribute, int|float $value = 1, int|float|null $min = null): bool - { + public function decreaseDocumentAttribute( + string $collection, + string $id, + string $attribute, + int|float $value = 1, + int|float|null $min = null + ): int|float { if ($value <= 0) { // Can be a float throw new DatabaseException('Value must be numeric and greater than 0'); } - $validator = new Authorization(self::PERMISSION_UPDATE); - - /* @var $document Document */ - $document = Authorization::skip(fn () => $this->silent(fn () => $this->getDocument($collection, $id))); // Skip ensures user does not need read permission for this - - if ($document->isEmpty()) { - return false; - } - $collection = $this->silent(fn () => $this->getCollection($collection)); - if ($collection->getId() !== self::METADATA) { - $documentSecurity = $collection->getAttribute('documentSecurity', false); - if (!$validator->isValid([ - ...$collection->getUpdate(), - ...($documentSecurity ? $document->getUpdate() : []) - ])) { - throw new AuthorizationException($validator->getDescription()); - } - } - $attr = \array_filter($collection->getAttribute('attributes', []), function ($a) use ($attribute) { return $a['$id'] === $attribute; }); @@ -5228,7 +5219,10 @@ public function decreaseDocumentAttribute(string $collection, string $id, string throw new NotFoundException('Attribute not found'); } - $whiteList = [self::VAR_INTEGER, self::VAR_FLOAT]; + $whiteList = [ + self::VAR_INTEGER, + self::VAR_FLOAT + ]; /** * @var Document $attr @@ -5238,36 +5232,52 @@ public function decreaseDocumentAttribute(string $collection, string $id, string throw new TypeException('Attribute type must be one of: ' . \implode(',', $whiteList)); } - if ($min && ($document->getAttribute($attribute) - $value < $min)) { - throw new LimitException('Attribute value exceeds minimum limit: ' . $min); - } + $document = $this->withTransaction(function () use ($collection, $id, $attribute, $value, $min) { + /* @var $document Document */ + $document = Authorization::skip(fn () => $this->silent(fn () => $this->getDocument($collection->getId(), $id, forUpdate: true))); // Skip ensures user does not need read permission for this - $time = DateTime::now(); - $updatedAt = $document->getUpdatedAt(); - $updatedAt = (empty($updatedAt) || !$this->preserveDates) ? $time : $updatedAt; + if ($document->isEmpty()) { + return false; + } - // Check if document was updated after the request timestamp - $oldUpdatedAt = new \DateTime($document->getUpdatedAt()); - if (!is_null($this->timestamp) && $oldUpdatedAt > $this->timestamp) { - throw new ConflictException('Document was updated after the request timestamp'); - } + $validator = new Authorization(self::PERMISSION_UPDATE); - $min = $min ? $min + $value : null; + if ($collection->getId() !== self::METADATA) { + $documentSecurity = $collection->getAttribute('documentSecurity', false); + if (!$validator->isValid([ + ...$collection->getUpdate(), + ...($documentSecurity ? $document->getUpdate() : []) + ])) { + throw new AuthorizationException($validator->getDescription()); + } + } - $result = $this->adapter->increaseDocumentAttribute( - $collection->getId(), - $id, - $attribute, - $value * -1, - $updatedAt, - min: $min - ); + if ($min && ($document->getAttribute($attribute) - $value < $min)) { + throw new LimitException('Attribute value exceeds minimum limit: ' . $min); + } + + $time = DateTime::now(); + $updatedAt = $document->getUpdatedAt(); + $updatedAt = (empty($updatedAt) || !$this->preserveDates) ? $time : $updatedAt; + $min = $min ? $min + $value : null; + + $this->adapter->increaseDocumentAttribute( + $collection->getId(), + $id, + $attribute, + $value * -1, + $updatedAt, + min: $min + ); + + return $document; + }); $this->purgeCachedDocument($collection->getId(), $id); $this->trigger(self::EVENT_DOCUMENT_DECREASE, $document); - return $result; + return $document->getAttribute($attribute) - $value; } /** diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index 290d7c4d1..9d7415e63 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -992,21 +992,21 @@ public function testIncreaseDecrease(): Document $updatedAt = $document->getUpdatedAt(); - $this->assertEquals(true, $database->increaseDocumentAttribute($collection, $document->getId(), 'increase', 1, 101)); + $this->assertEquals(101, $database->increaseDocumentAttribute($collection, $document->getId(), 'increase', 1, 101)); $document = $database->getDocument($collection, $document->getId()); $this->assertEquals(101, $document->getAttribute('increase')); $this->assertNotEquals($updatedAt, $document->getUpdatedAt()); - $this->assertEquals(true, $database->decreaseDocumentAttribute($collection, $document->getId(), 'decrease', 1, 98)); + $this->assertEquals(99, $database->decreaseDocumentAttribute($collection, $document->getId(), 'decrease', 1, 98)); $document = $database->getDocument($collection, $document->getId()); $this->assertEquals(99, $document->getAttribute('decrease')); - $this->assertEquals(true, $database->increaseDocumentAttribute($collection, $document->getId(), 'increase_float', 5.5, 110)); + $this->assertEquals(105.5, $database->increaseDocumentAttribute($collection, $document->getId(), 'increase_float', 5.5, 110)); $document = $database->getDocument($collection, $document->getId()); $this->assertEquals(105.5, $document->getAttribute('increase_float')); - $this->assertEquals(true, $database->decreaseDocumentAttribute($collection, $document->getId(), 'increase_float', 1.1, 100)); + $this->assertEquals(104.4, $database->decreaseDocumentAttribute($collection, $document->getId(), 'increase_float', 1.1, 100)); $document = $database->getDocument($collection, $document->getId()); $this->assertEquals(104.4, $document->getAttribute('increase_float')); From 1c685bb8f8d65ddefa827951116cd44ef87978c1 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 9 Jun 2025 23:15:02 -0400 Subject: [PATCH 2/4] Fix ql --- src/Database/Database.php | 4 ++-- src/Database/Mirror.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 3bc91f5d2..4876654c0 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -5141,7 +5141,7 @@ public function increaseDocumentAttribute( $document = Authorization::skip(fn () => $this->silent(fn () => $this->getDocument($collection->getId(), $id, forUpdate: true))); // Skip ensures user does not need read permission for this if ($document->isEmpty()) { - return false; + throw new NotFoundException('Document not found'); } $validator = new Authorization(self::PERMISSION_UPDATE); @@ -5237,7 +5237,7 @@ public function decreaseDocumentAttribute( $document = Authorization::skip(fn () => $this->silent(fn () => $this->getDocument($collection->getId(), $id, forUpdate: true))); // Skip ensures user does not need read permission for this if ($document->isEmpty()) { - return false; + throw new NotFoundException('Document not found'); } $validator = new Authorization(self::PERMISSION_UPDATE); diff --git a/src/Database/Mirror.php b/src/Database/Mirror.php index 168a227ee..bd16696f1 100644 --- a/src/Database/Mirror.php +++ b/src/Database/Mirror.php @@ -948,12 +948,12 @@ public function renameIndex(string $collection, string $old, string $new): bool return $this->delegate(__FUNCTION__, \func_get_args()); } - public function increaseDocumentAttribute(string $collection, string $id, string $attribute, int|float $value = 1, int|float|null $max = null): bool + public function increaseDocumentAttribute(string $collection, string $id, string $attribute, int|float $value = 1, int|float|null $max = null): int|float { return $this->delegate(__FUNCTION__, \func_get_args()); } - public function decreaseDocumentAttribute(string $collection, string $id, string $attribute, int|float $value = 1, int|float|null $min = null): bool + public function decreaseDocumentAttribute(string $collection, string $id, string $attribute, int|float $value = 1, int|float|null $min = null): int|float { return $this->delegate(__FUNCTION__, \func_get_args()); } From ae2055c0e826a57eb289c3f853a534953aa5a789 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 9 Jun 2025 23:21:48 -0400 Subject: [PATCH 3/4] Return document --- src/Database/Database.php | 22 ++++++++++++++-------- src/Database/Mirror.php | 4 ++-- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 4876654c0..13ba9930d 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -5096,7 +5096,7 @@ public function createOrUpdateDocumentsWithIncrease( * @param string $attribute The attribute to increase * @param int|float $value The value to increase the attribute by, can be a float * @param int|float|null $max The maximum value the attribute can reach after the increase, null means no limit - * @return int The new value of the attribute after the increase + * @return Document * @throws AuthorizationException * @throws DatabaseException * @throws LimitException @@ -5110,7 +5110,7 @@ public function increaseDocumentAttribute( string $attribute, int|float $value = 1, int|float|null $max = null - ): int|float { + ): Document { if ($value <= 0) { // Can be a float throw new DatabaseException('Value must be numeric and greater than 0'); } @@ -5174,14 +5174,17 @@ public function increaseDocumentAttribute( max: $max ); - return $document; + return $document->setAttribute( + $attribute, + $document->getAttribute($attribute) - $value + ); }); $this->purgeCachedDocument($collection->getId(), $id); $this->trigger(self::EVENT_DOCUMENT_INCREASE, $document); - return $document->getAttribute($attribute) + $value; + return $document; } @@ -5193,7 +5196,7 @@ public function increaseDocumentAttribute( * @param string $attribute * @param int|float $value * @param int|float|null $min - * @return int|float + * @return Document * * @throws AuthorizationException * @throws DatabaseException @@ -5204,7 +5207,7 @@ public function decreaseDocumentAttribute( string $attribute, int|float $value = 1, int|float|null $min = null - ): int|float { + ): Document { if ($value <= 0) { // Can be a float throw new DatabaseException('Value must be numeric and greater than 0'); } @@ -5270,14 +5273,17 @@ public function decreaseDocumentAttribute( min: $min ); - return $document; + return $document->setAttribute( + $attribute, + $document->getAttribute($attribute) - $value + ); }); $this->purgeCachedDocument($collection->getId(), $id); $this->trigger(self::EVENT_DOCUMENT_DECREASE, $document); - return $document->getAttribute($attribute) - $value; + return $document; } /** diff --git a/src/Database/Mirror.php b/src/Database/Mirror.php index bd16696f1..ea11784b3 100644 --- a/src/Database/Mirror.php +++ b/src/Database/Mirror.php @@ -948,12 +948,12 @@ public function renameIndex(string $collection, string $old, string $new): bool return $this->delegate(__FUNCTION__, \func_get_args()); } - public function increaseDocumentAttribute(string $collection, string $id, string $attribute, int|float $value = 1, int|float|null $max = null): int|float + public function increaseDocumentAttribute(string $collection, string $id, string $attribute, int|float $value = 1, int|float|null $max = null): Document { return $this->delegate(__FUNCTION__, \func_get_args()); } - public function decreaseDocumentAttribute(string $collection, string $id, string $attribute, int|float $value = 1, int|float|null $min = null): int|float + public function decreaseDocumentAttribute(string $collection, string $id, string $attribute, int|float $value = 1, int|float|null $min = null): Document { return $this->delegate(__FUNCTION__, \func_get_args()); } From 376945c2a81e683bf72d4952d1d4bf29373b2f21 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 9 Jun 2025 23:45:54 -0400 Subject: [PATCH 4/4] Fix tests --- src/Database/Database.php | 2 +- tests/e2e/Adapter/Scopes/DocumentTests.php | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 13ba9930d..d9a15730b 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -5176,7 +5176,7 @@ public function increaseDocumentAttribute( return $document->setAttribute( $attribute, - $document->getAttribute($attribute) - $value + $document->getAttribute($attribute) + $value ); }); diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index 9d7415e63..11bfc85f4 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -981,7 +981,7 @@ public function testIncreaseDecrease(): Document 'increase' => 100, 'decrease' => 100, 'increase_float' => 100, - 'increase_text' => "some text", + 'increase_text' => 'some text', '$permissions' => [ Permission::read(Role::any()), Permission::create(Role::any()), @@ -992,21 +992,25 @@ public function testIncreaseDecrease(): Document $updatedAt = $document->getUpdatedAt(); - $this->assertEquals(101, $database->increaseDocumentAttribute($collection, $document->getId(), 'increase', 1, 101)); + $doc = $database->increaseDocumentAttribute($collection, $document->getId(), 'increase', 1, 101); + $this->assertEquals(101, $doc->getAttribute('increase')); $document = $database->getDocument($collection, $document->getId()); $this->assertEquals(101, $document->getAttribute('increase')); $this->assertNotEquals($updatedAt, $document->getUpdatedAt()); - $this->assertEquals(99, $database->decreaseDocumentAttribute($collection, $document->getId(), 'decrease', 1, 98)); + $doc = $database->decreaseDocumentAttribute($collection, $document->getId(), 'decrease', 1, 98); + $this->assertEquals(99, $doc->getAttribute('decrease')); $document = $database->getDocument($collection, $document->getId()); $this->assertEquals(99, $document->getAttribute('decrease')); - $this->assertEquals(105.5, $database->increaseDocumentAttribute($collection, $document->getId(), 'increase_float', 5.5, 110)); + $doc = $database->increaseDocumentAttribute($collection, $document->getId(), 'increase_float', 5.5, 110); + $this->assertEquals(105.5, $doc->getAttribute('increase_float')); $document = $database->getDocument($collection, $document->getId()); $this->assertEquals(105.5, $document->getAttribute('increase_float')); - $this->assertEquals(104.4, $database->decreaseDocumentAttribute($collection, $document->getId(), 'increase_float', 1.1, 100)); + $doc = $database->decreaseDocumentAttribute($collection, $document->getId(), 'increase_float', 1.1, 100); + $this->assertEquals(104.4, $doc->getAttribute('increase_float')); $document = $database->getDocument($collection, $document->getId()); $this->assertEquals(104.4, $document->getAttribute('increase_float'));