From 2f686903689946073cefa066720020ffdf6e0e1c Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 14 Jan 2025 18:50:14 +1300 Subject: [PATCH 1/6] Ensure collection key is purged --- src/Database/Database.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Database/Database.php b/src/Database/Database.php index 0350a658b..95b58bf26 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -5291,6 +5291,8 @@ public function purgeCachedCollection(string $collectionId): bool foreach ($documentKeys as $documentKey) { $this->cache->purge($documentKey); } + + $this->cache->purge($collectionKey); return true; } From 19f17a8e7cfb5a8993c97ac9163357013050f01f Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 14 Jan 2025 18:50:46 +1300 Subject: [PATCH 2/6] Simplify checks --- src/Database/Database.php | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 95b58bf26..c16d34094 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -1462,17 +1462,8 @@ public function createAttribute(string $collection, string $id, string $type, in throw new DatabaseException("Attribute of type: $type requires the following filters: " . implode(",", $requiredFilters)); } - if ( - $this->adapter->getLimitForAttributes() > 0 && - $this->adapter->getCountOfAttributes($collection) >= $this->adapter->getLimitForAttributes() - ) { - throw new LimitException('Column limit reached. Cannot create new attribute.'); - } - - if ($format) { - if (!Structure::hasFormat($format, $type)) { - throw new DatabaseException('Format ("' . $format . '") not available for this attribute type ("' . $type . '")'); - } + if ($format && !Structure::hasFormat($format, $type)) { + throw new DatabaseException('Format ("' . $format . '") not available for this attribute type ("' . $type . '")'); } $attribute = new Document([ @@ -1489,14 +1480,13 @@ public function createAttribute(string $collection, string $id, string $type, in 'filters' => $filters, ]); - $collection->setAttribute('attributes', $attribute, Document::SET_TYPE_APPEND); + $this->checkAttribute($collection, $attribute); - if ( - $this->adapter->getDocumentSizeLimit() > 0 && - $this->adapter->getAttributeWidth($collection) >= $this->adapter->getDocumentSizeLimit() - ) { - throw new LimitException('Row width limit reached. Cannot create new attribute.'); - } + $collection->setAttribute( + 'attributes', + $attribute, + Document::SET_TYPE_APPEND + ); switch ($type) { case self::VAR_STRING: From 41e7a08fb59bae4edbf5bc9e1ec7d06c4653cefc Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 14 Jan 2025 18:51:30 +1300 Subject: [PATCH 3/6] Relationship metadata updates in transaction with cleanup --- src/Database/Database.php | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index c16d34094..aecf00712 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -1494,7 +1494,6 @@ public function createAttribute(string $collection, string $id, string $type, in throw new DatabaseException('Max size allowed for string is: ' . number_format($this->adapter->getLimitForString())); } break; - case self::VAR_INTEGER: $limit = ($signed) ? $this->adapter->getLimitForInt() / 2 : $this->adapter->getLimitForInt(); if ($size > $limit) { @@ -2249,8 +2248,24 @@ public function createRelationship( } $this->silent(function () use ($collection, $relatedCollection, $type, $twoWay, $id, $twoWayKey) { - $this->updateDocument(self::METADATA, $collection->getId(), $collection); - $this->updateDocument(self::METADATA, $relatedCollection->getId(), $relatedCollection); + try { + $this->withTransaction(function () use ($collection, $relatedCollection) { + $this->updateDocument(self::METADATA, $collection->getId(), $collection); + $this->updateDocument(self::METADATA, $relatedCollection->getId(), $relatedCollection); + }); + } catch (\Throwable $e) { + $this->adapter->deleteRelationship( + $collection->getId(), + $relatedCollection->getId(), + $type, + $twoWay, + $id, + $twoWayKey, + Database::RELATION_SIDE_PARENT + ); + + throw new DatabaseException('Failed to create relationship: ' . $e->getMessage()); + } $indexKey = '_index_' . $id; $twoWayIndexKey = '_index_' . $twoWayKey; @@ -2535,8 +2550,14 @@ public function deleteRelationship(string $collection, string $id): bool $relatedCollection->setAttribute('attributes', \array_values($relatedAttributes)); $this->silent(function () use ($collection, $relatedCollection, $type, $twoWay, $id, $twoWayKey, $side) { - $this->updateDocument(self::METADATA, $collection->getId(), $collection); - $this->updateDocument(self::METADATA, $relatedCollection->getId(), $relatedCollection); + try { + $this->withTransaction(function () use ($collection, $relatedCollection) { + $this->updateDocument(self::METADATA, $collection->getId(), $collection); + $this->updateDocument(self::METADATA, $relatedCollection->getId(), $relatedCollection); + }); + } catch (\Throwable $e) { + throw new DatabaseException('Failed to delete relationship: ' . $e->getMessage()); + } $indexKey = '_index_' . $id; $twoWayIndexKey = '_index_' . $twoWayKey; From 1fd16cdfef14dbfaa8ceb3f0cd815e220be8064d Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 14 Jan 2025 18:51:40 +1300 Subject: [PATCH 4/6] Update messages for relationships --- src/Database/Database.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index aecf00712..acbee79b5 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -2333,13 +2333,13 @@ public function updateRelationship( !\is_null($newKey) && \in_array($newKey, \array_map(fn ($attribute) => $attribute['key'], $attributes)) ) { - throw new DuplicateException('Attribute already exists'); + throw new DuplicateException('Relationship already exists'); } $attributeIndex = array_search($id, array_map(fn ($attribute) => $attribute['$id'], $attributes)); if ($attributeIndex === false) { - throw new NotFoundException('Attribute not found'); + throw new NotFoundException('Relationship not found'); } $attribute = $attributes[$attributeIndex]; @@ -2526,7 +2526,7 @@ public function deleteRelationship(string $collection, string $id): bool } if (\is_null($relationship)) { - throw new NotFoundException('Attribute not found'); + throw new NotFoundException('Relationship not found'); } $collection->setAttribute('attributes', \array_values($attributes)); From 101b43851a9aca14e74c65eee82eb0111eedbb13 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 14 Jan 2025 20:50:39 +1300 Subject: [PATCH 5/6] Remove all relationship caching --- src/Database/Database.php | 65 +------------------------------------- tests/e2e/Adapter/Base.php | 14 ++++---- 2 files changed, 9 insertions(+), 70 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index acbee79b5..b2e6f632c 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -3003,35 +3003,9 @@ public function getDocument(string $collection, string $id, array $queries = [], $attribute['type'] === Database::VAR_RELATIONSHIP ); - $hasTwoWayRelationship = false; - foreach ($relationships as $relationship) { - if ($relationship['options']['twoWay']) { - $hasTwoWayRelationship = true; - break; - } - } - - /** - * Bug with function purity in PHPStan means it thinks $this->map is always empty - * @phpstan-ignore-next-line - */ - foreach ($this->map as $key => $value) { - [$k, $v] = \explode('=>', $key); - $ck = $this->cacheName . '-cache-' . $this->getNamespace() . ':' . $this->adapter->getTenant() . ':map:' . $k; - $cache = $this->cache->load($ck, self::TTL, $ck); - if (empty($cache)) { - $cache = []; - } - if (!\in_array($v, $cache)) { - $cache[] = $v; - $this->cache->save($ck, $cache, $ck); - } - } - // Don't save to cache if it's part of a relationship - if (!$hasTwoWayRelationship && empty($relationships)) { + if (empty($relationships)) { $this->cache->save($documentCacheKey, $document->getArrayCopy(), $documentCacheHash); - // Add document reference to the collection key $this->cache->save($collectionCacheKey, 'empty', $documentCacheKey); } @@ -3959,7 +3933,6 @@ public function updateDocument(string $collection, string $id, Document $documen $document = $this->decode($collection, $document); - $this->purgeRelatedDocuments($collection, $id); $this->purgeCachedDocument($collection->getId(), $id); $this->trigger(self::EVENT_DOCUMENT_UPDATE, $document); @@ -4114,7 +4087,6 @@ public function updateDocuments(string $collection, Document $updates, array $qu } foreach ($documents as $document) { - $this->purgeRelatedDocuments($collection, $document->getId()); $this->purgeCachedDocument($collection->getId(), $document->getId()); } @@ -4763,7 +4735,6 @@ public function deleteDocument(string $collection, string $id): bool return $this->adapter->deleteDocument($collection->getId(), $id); }); - $this->purgeRelatedDocuments($collection, $id); $this->purgeCachedDocument($collection->getId(), $id); $this->trigger(self::EVENT_DOCUMENT_DELETE, $document); @@ -5257,7 +5228,6 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba throw new ConflictException('Document was updated after the request timestamp'); } - $this->purgeRelatedDocuments($collection, $document->getId()); $this->purgeCachedDocument($collection->getId(), $document->getId()); } @@ -6006,39 +5976,6 @@ public static function convertQueries(Document $collection, array $queries): arr return $queries; } - /** - * @param Document $collection - * @param string $id - * @return void - * @throws DatabaseException - */ - private function purgeRelatedDocuments(Document $collection, string $id): void - { - if ($collection->getId() === self::METADATA) { - return; - } - - $relationships = \array_filter( - $collection->getAttribute('attributes', []), - fn ($attribute) => - $attribute['type'] === Database::VAR_RELATIONSHIP - ); - - if (empty($relationships)) { - return; - } - - $key = $this->cacheName . '-cache-' . $this->getNamespace() . ':map:' . $collection->getId() . ':' . $id; - $cache = $this->cache->load($key, self::TTL, $key); - if (!empty($cache)) { - foreach ($cache as $v) { - list($collectionId, $documentId) = explode(':', $v); - $this->purgeCachedDocument($collectionId, $documentId); - } - $this->cache->purge($key); - } - } - /** * @return array> */ diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index ad117acd7..d5d172291 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -7897,7 +7897,7 @@ public function testIdenticalTwoWayKeyRelationship(): void ); $this->fail('Failed to throw Exception'); } catch (Exception $e) { - $this->assertEquals('Attribute already exists', $e->getMessage()); + $this->assertEquals('Relationship already exists', $e->getMessage()); } try { @@ -12955,10 +12955,12 @@ public function testDeleteMissingRelationship(): void return; } - $this->expectException(Exception::class); - $this->expectExceptionMessage('Attribute not found'); - - static::getDatabase()->deleteRelationship('test', 'test2'); + try { + static::getDatabase()->deleteRelationship('test', 'test2'); + $this->fail('Failed to throw exception'); + } catch (\Throwable $e) { + $this->assertEquals('Relationship not found', $e->getMessage()); + } } public function testCreateInvalidIntValueRelationship(): void @@ -13321,7 +13323,7 @@ public function testUpdateRelationshipToExistingKey(): void static::getDatabase()->updateRelationship('ovens', 'cakes', newKey: 'owner'); $this->fail('Failed to throw exception'); } catch (DuplicateException $e) { - $this->assertEquals('Attribute already exists', $e->getMessage()); + $this->assertEquals('Relationship already exists', $e->getMessage()); } try { From e783760408e57066260d45617f24460ab2024ca9 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Tue, 14 Jan 2025 21:13:19 +1300 Subject: [PATCH 6/6] Format --- src/Database/Database.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index b2e6f632c..c30294d7f 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -5272,7 +5272,7 @@ public function purgeCachedCollection(string $collectionId): bool foreach ($documentKeys as $documentKey) { $this->cache->purge($documentKey); } - + $this->cache->purge($collectionKey); return true;