From a6779caa5ebf0eca8a080061fca91931a1e9842b Mon Sep 17 00:00:00 2001 From: fogelito Date: Thu, 7 Nov 2024 14:28:39 +0200 Subject: [PATCH 1/7] Trigger sentry --- src/Appwrite/Platform/Workers/Databases.php | 24 ++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/src/Appwrite/Platform/Workers/Databases.php b/src/Appwrite/Platform/Workers/Databases.php index f697e7be13..1974a0b959 100644 --- a/src/Appwrite/Platform/Workers/Databases.php +++ b/src/Appwrite/Platform/Workers/Databases.php @@ -92,6 +92,7 @@ class Databases extends Action * @throws Authorization * @throws Conflict * @throws \Exception + * @throws \Throwable */ private function createAttribute(Document $database, Document $collection, Document $attribute, Document $project, Database $dbForConsole, Database $dbForProject): void { @@ -134,7 +135,6 @@ class Databases extends Action $options = $attribute->getAttribute('options', []); $project = $dbForConsole->getDocument('projects', $projectId); - try { switch ($type) { case Database::VAR_RELATIONSHIP: @@ -170,7 +170,6 @@ class Databases extends Action $dbForProject->updateDocument('attributes', $attribute->getId(), $attribute->setAttribute('status', 'available')); } catch (\Throwable $e) { - // TODO: Send non DatabaseExceptions to Sentry Console::error($e->getMessage()); if ($e instanceof DatabaseException) { @@ -193,6 +192,9 @@ class Databases extends Action $relatedAttribute->setAttribute('status', 'failed') ); } + + // TODO: Send non DatabaseExceptions to Sentry + throw $e; } finally { $this->trigger($database, $collection, $attribute, $project, $projectId, $events); } @@ -215,6 +217,7 @@ class Databases extends Action * @throws Authorization * @throws Conflict * @throws \Exception + * @throws \Throwable **/ private function deleteAttribute(Document $database, Document $collection, Document $attribute, Document $project, Database $dbForConsole, Database $dbForProject): void { @@ -273,7 +276,6 @@ class Databases extends Action $dbForProject->deleteDocument('attributes', $relatedAttribute->getId()); } } catch (\Throwable $e) { - // TODO: Send non DatabaseExceptions to Sentry Console::error($e->getMessage()); if ($e instanceof DatabaseException) { @@ -294,6 +296,9 @@ class Databases extends Action $relatedAttribute->setAttribute('status', 'stuck') ); } + + // TODO: Send non DatabaseExceptions to Sentry + throw $e; } finally { $this->trigger($database, $collection, $attribute, $project, $projectId, $events); } @@ -370,6 +375,7 @@ class Databases extends Action * @throws Conflict * @throws Structure * @throws DatabaseException + * @throws \Throwable */ private function createIndex(Document $database, Document $collection, Document $index, Document $project, Database $dbForConsole, Database $dbForProject): void { @@ -401,7 +407,7 @@ class Databases extends Action } $dbForProject->updateDocument('indexes', $index->getId(), $index->setAttribute('status', 'available')); } catch (\Throwable $e) { - // TODO: Send non DatabaseExceptions to Sentry + Console::error($e->getMessage()); if ($e instanceof DatabaseException) { @@ -412,6 +418,9 @@ class Databases extends Action $index->getId(), $index->setAttribute('status', 'failed') ); + + // TODO: Send non DatabaseExceptions to Sentry + throw $e; } finally { $this->trigger($database, $collection, $index, $project, $projectId, $events); } @@ -431,6 +440,7 @@ class Databases extends Action * @throws Conflict * @throws Structure * @throws DatabaseException + * @throws \Throwable */ private function deleteIndex(Document $database, Document $collection, Document $index, Document $project, Database $dbForConsole, Database $dbForProject): void { @@ -459,7 +469,6 @@ class Databases extends Action $dbForProject->deleteDocument('indexes', $index->getId()); $index->setAttribute('status', 'deleted'); } catch (\Throwable $e) { - // TODO: Send non DatabaseExceptions to Sentry Console::error($e->getMessage()); if ($e instanceof DatabaseException) { @@ -470,10 +479,15 @@ class Databases extends Action $index->getId(), $index->setAttribute('status', 'stuck') ); + + // TODO: Send non DatabaseExceptions to Sentry ? + throw $e; + } finally { $this->trigger($database, $collection, $index, $project, $projectId, $events); } + // Do we want this in finally? $dbForProject->purgeCachedDocument('database_' . $database->getInternalId(), $collection->getId()); } From 19667f78a92e00fa203a7a35bd4f4f13e2175851 Mon Sep 17 00:00:00 2001 From: fogelito Date: Thu, 7 Nov 2024 17:23:46 +0200 Subject: [PATCH 2/7] Add try catch on delete Attribute --- src/Appwrite/Platform/Workers/Databases.php | 180 ++++++++++---------- 1 file changed, 90 insertions(+), 90 deletions(-) diff --git a/src/Appwrite/Platform/Workers/Databases.php b/src/Appwrite/Platform/Workers/Databases.php index 1974a0b959..388dd3b88c 100644 --- a/src/Appwrite/Platform/Workers/Databases.php +++ b/src/Appwrite/Platform/Workers/Databases.php @@ -251,115 +251,118 @@ class Databases extends Action // - stuck: attribute was available but cannot be removed try { - if ($status !== 'failed') { - if ($type === Database::VAR_RELATIONSHIP) { - if ($options['twoWay']) { - $relatedCollection = $dbForProject->getDocument('database_' . $database->getInternalId(), $options['relatedCollection']); - if ($relatedCollection->isEmpty()) { - throw new DatabaseException('Collection not found'); + try { + if ($status !== 'failed') { + if ($type === Database::VAR_RELATIONSHIP) { + if ($options['twoWay']) { + $relatedCollection = $dbForProject->getDocument('database_' . $database->getInternalId(), $options['relatedCollection']); + if ($relatedCollection->isEmpty()) { + throw new DatabaseException('Collection not found'); + } + $relatedAttribute = $dbForProject->getDocument('attributes', $database->getInternalId() . '_' . $relatedCollection->getInternalId() . '_' . $options['twoWayKey']); } - $relatedAttribute = $dbForProject->getDocument('attributes', $database->getInternalId() . '_' . $relatedCollection->getInternalId() . '_' . $options['twoWayKey']); - } - if (!$dbForProject->deleteRelationship('database_' . $database->getInternalId() . '_collection_' . $collection->getInternalId(), $key)) { - $dbForProject->updateDocument('attributes', $relatedAttribute->getId(), $relatedAttribute->setAttribute('status', 'stuck')); - throw new DatabaseException('Failed to delete Relationship'); + if (!$dbForProject->deleteRelationship('database_' . $database->getInternalId() . '_collection_' . $collection->getInternalId(), $key)) { + $dbForProject->updateDocument('attributes', $relatedAttribute->getId(), $relatedAttribute->setAttribute('status', 'stuck')); + throw new DatabaseException('Failed to delete Relationship'); + } + } elseif (!$dbForProject->deleteAttribute('database_' . $database->getInternalId() . '_collection_' . $collection->getInternalId(), $key)) { + throw new DatabaseException('Failed to delete Attribute'); } - } elseif (!$dbForProject->deleteAttribute('database_' . $database->getInternalId() . '_collection_' . $collection->getInternalId(), $key)) { - throw new DatabaseException('Failed to delete Attribute'); } - } - $dbForProject->deleteDocument('attributes', $attribute->getId()); + $dbForProject->deleteDocument('attributes', $attribute->getId()); - if (!$relatedAttribute->isEmpty()) { - $dbForProject->deleteDocument('attributes', $relatedAttribute->getId()); - } - } catch (\Throwable $e) { - Console::error($e->getMessage()); - - if ($e instanceof DatabaseException) { - $attribute->setAttribute('error', $e->getMessage()); if (!$relatedAttribute->isEmpty()) { - $relatedAttribute->setAttribute('error', $e->getMessage()); + $dbForProject->deleteDocument('attributes', $relatedAttribute->getId()); + } + } catch (\Throwable $e) { + Console::error($e->getMessage()); + + if ($e instanceof DatabaseException) { + $attribute->setAttribute('error', $e->getMessage()); + if (!$relatedAttribute->isEmpty()) { + $relatedAttribute->setAttribute('error', $e->getMessage()); + } } - } - $dbForProject->updateDocument( - 'attributes', - $attribute->getId(), - $attribute->setAttribute('status', 'stuck') - ); - if (!$relatedAttribute->isEmpty()) { $dbForProject->updateDocument( 'attributes', - $relatedAttribute->getId(), - $relatedAttribute->setAttribute('status', 'stuck') + $attribute->getId(), + $attribute->setAttribute('status', 'stuck') ); + if (!$relatedAttribute->isEmpty()) { + $dbForProject->updateDocument( + 'attributes', + $relatedAttribute->getId(), + $relatedAttribute->setAttribute('status', 'stuck') + ); + } + + // TODO: Send non DatabaseExceptions to Sentry + throw $e; + } finally { + $this->trigger($database, $collection, $attribute, $project, $projectId, $events); } - // TODO: Send non DatabaseExceptions to Sentry - throw $e; - } finally { - $this->trigger($database, $collection, $attribute, $project, $projectId, $events); - } + // The underlying database removes/rebuilds indexes when attribute is removed + // Update indexes table with changes + /** @var Document[] $indexes */ + $indexes = $collection->getAttribute('indexes', []); - // The underlying database removes/rebuilds indexes when attribute is removed - // Update indexes table with changes - /** @var Document[] $indexes */ - $indexes = $collection->getAttribute('indexes', []); + foreach ($indexes as $index) { + /** @var string[] $attributes */ + $attributes = $index->getAttribute('attributes'); + $lengths = $index->getAttribute('lengths'); + $orders = $index->getAttribute('orders'); - foreach ($indexes as $index) { - /** @var string[] $attributes */ - $attributes = $index->getAttribute('attributes'); - $lengths = $index->getAttribute('lengths'); - $orders = $index->getAttribute('orders'); + $found = \array_search($key, $attributes); - $found = \array_search($key, $attributes); + if ($found !== false) { + // If found, remove entry from attributes, lengths, and orders + // array_values wraps array_diff to reindex array keys + // when found attribute is removed from array + $attributes = \array_values(\array_diff($attributes, [$attributes[$found]])); + $lengths = \array_values(\array_diff($lengths, isset($lengths[$found]) ? [$lengths[$found]] : [])); + $orders = \array_values(\array_diff($orders, isset($orders[$found]) ? [$orders[$found]] : [])); - if ($found !== false) { - // If found, remove entry from attributes, lengths, and orders - // array_values wraps array_diff to reindex array keys - // when found attribute is removed from array - $attributes = \array_values(\array_diff($attributes, [$attributes[$found]])); - $lengths = \array_values(\array_diff($lengths, isset($lengths[$found]) ? [$lengths[$found]] : [])); - $orders = \array_values(\array_diff($orders, isset($orders[$found]) ? [$orders[$found]] : [])); - - if (empty($attributes)) { - $dbForProject->deleteDocument('indexes', $index->getId()); - } else { - $index - ->setAttribute('attributes', $attributes, Document::SET_TYPE_ASSIGN) - ->setAttribute('lengths', $lengths, Document::SET_TYPE_ASSIGN) - ->setAttribute('orders', $orders, Document::SET_TYPE_ASSIGN); - - // Check if an index exists with the same attributes and orders - $exists = false; - foreach ($indexes as $existing) { - if ( - $existing->getAttribute('key') !== $index->getAttribute('key') // Ignore itself - && $existing->getAttribute('attributes') === $index->getAttribute('attributes') - && $existing->getAttribute('orders') === $index->getAttribute('orders') - ) { - $exists = true; - break; - } - } - - if ($exists) { // Delete the duplicate if created, else update in db - $this->deleteIndex($database, $collection, $index, $project, $dbForConsole, $dbForProject); + if (empty($attributes)) { + $dbForProject->deleteDocument('indexes', $index->getId()); } else { - $dbForProject->updateDocument('indexes', $index->getId(), $index); + $index + ->setAttribute('attributes', $attributes, Document::SET_TYPE_ASSIGN) + ->setAttribute('lengths', $lengths, Document::SET_TYPE_ASSIGN) + ->setAttribute('orders', $orders, Document::SET_TYPE_ASSIGN); + + // Check if an index exists with the same attributes and orders + $exists = false; + foreach ($indexes as $existing) { + if ( + $existing->getAttribute('key') !== $index->getAttribute('key') // Ignore itself + && $existing->getAttribute('attributes') === $index->getAttribute('attributes') + && $existing->getAttribute('orders') === $index->getAttribute('orders') + ) { + $exists = true; + break; + } + } + + if ($exists) { // Delete the duplicate if created, else update in db + $this->deleteIndex($database, $collection, $index, $project, $dbForConsole, $dbForProject); + } else { + $dbForProject->updateDocument('indexes', $index->getId(), $index); + } } } } - } - $dbForProject->purgeCachedDocument('database_' . $database->getInternalId(), $collectionId); - $dbForProject->purgeCachedCollection('database_' . $database->getInternalId() . '_collection_' . $collection->getInternalId()); + } finally { + $dbForProject->purgeCachedDocument('database_' . $database->getInternalId(), $collectionId); + $dbForProject->purgeCachedCollection('database_' . $database->getInternalId() . '_collection_' . $collection->getInternalId()); - if (!$relatedCollection->isEmpty() && !$relatedAttribute->isEmpty()) { - $dbForProject->purgeCachedDocument('database_' . $database->getInternalId(), $relatedCollection->getId()); - $dbForProject->purgeCachedCollection('database_' . $database->getInternalId() . '_collection_' . $relatedCollection->getInternalId()); + if (!$relatedCollection->isEmpty() && !$relatedAttribute->isEmpty()) { + $dbForProject->purgeCachedDocument('database_' . $database->getInternalId(), $relatedCollection->getId()); + $dbForProject->purgeCachedCollection('database_' . $database->getInternalId() . '_collection_' . $relatedCollection->getInternalId()); + } } } @@ -480,15 +483,12 @@ class Databases extends Action $index->setAttribute('status', 'stuck') ); - // TODO: Send non DatabaseExceptions to Sentry ? throw $e; } finally { $this->trigger($database, $collection, $index, $project, $projectId, $events); + $dbForProject->purgeCachedDocument('database_' . $database->getInternalId(), $collection->getId()); } - - // Do we want this in finally? - $dbForProject->purgeCachedDocument('database_' . $database->getInternalId(), $collection->getId()); } /** From cd0d1d94dbb56936091d07cd380180e4b857562d Mon Sep 17 00:00:00 2001 From: fogelito Date: Thu, 7 Nov 2024 17:27:56 +0200 Subject: [PATCH 3/7] Add to finally --- src/Appwrite/Platform/Workers/Databases.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Appwrite/Platform/Workers/Databases.php b/src/Appwrite/Platform/Workers/Databases.php index 388dd3b88c..8c26ca6cea 100644 --- a/src/Appwrite/Platform/Workers/Databases.php +++ b/src/Appwrite/Platform/Workers/Databases.php @@ -197,13 +197,12 @@ class Databases extends Action throw $e; } finally { $this->trigger($database, $collection, $attribute, $project, $projectId, $events); - } + if ($type === Database::VAR_RELATIONSHIP && $options['twoWay']) { + $dbForProject->purgeCachedDocument('database_' . $database->getInternalId(), $relatedCollection->getId()); + } - if ($type === Database::VAR_RELATIONSHIP && $options['twoWay']) { - $dbForProject->purgeCachedDocument('database_' . $database->getInternalId(), $relatedCollection->getId()); + $dbForProject->purgeCachedDocument('database_' . $database->getInternalId(), $collectionId); } - - $dbForProject->purgeCachedDocument('database_' . $database->getInternalId(), $collectionId); } /** @@ -426,9 +425,9 @@ class Databases extends Action throw $e; } finally { $this->trigger($database, $collection, $index, $project, $projectId, $events); + $dbForProject->purgeCachedDocument('database_' . $database->getInternalId(), $collectionId); } - $dbForProject->purgeCachedDocument('database_' . $database->getInternalId(), $collectionId); } /** From fb40970eba10f55596db980d89f0e1598d71ab43 Mon Sep 17 00:00:00 2001 From: fogelito Date: Thu, 7 Nov 2024 17:35:03 +0200 Subject: [PATCH 4/7] line --- src/Appwrite/Platform/Workers/Databases.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Appwrite/Platform/Workers/Databases.php b/src/Appwrite/Platform/Workers/Databases.php index 8c26ca6cea..01f73018f4 100644 --- a/src/Appwrite/Platform/Workers/Databases.php +++ b/src/Appwrite/Platform/Workers/Databases.php @@ -197,6 +197,7 @@ class Databases extends Action throw $e; } finally { $this->trigger($database, $collection, $attribute, $project, $projectId, $events); + if ($type === Database::VAR_RELATIONSHIP && $options['twoWay']) { $dbForProject->purgeCachedDocument('database_' . $database->getInternalId(), $relatedCollection->getId()); } From 06274764c645a7678e071fc17fa8512b4e206a75 Mon Sep 17 00:00:00 2001 From: fogelito Date: Fri, 8 Nov 2024 07:12:23 +0200 Subject: [PATCH 5/7] Remove extra lines --- src/Appwrite/Platform/Workers/Databases.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Appwrite/Platform/Workers/Databases.php b/src/Appwrite/Platform/Workers/Databases.php index 01f73018f4..7cff9f264a 100644 --- a/src/Appwrite/Platform/Workers/Databases.php +++ b/src/Appwrite/Platform/Workers/Databases.php @@ -354,7 +354,6 @@ class Databases extends Action } } } - } finally { $dbForProject->purgeCachedDocument('database_' . $database->getInternalId(), $collectionId); $dbForProject->purgeCachedCollection('database_' . $database->getInternalId() . '_collection_' . $collection->getInternalId()); @@ -410,10 +409,14 @@ class Databases extends Action } $dbForProject->updateDocument('indexes', $index->getId(), $index->setAttribute('status', 'available')); } catch (\Throwable $e) { - Console::error($e->getMessage()); - + Console::error('shmuel::createIndex'); if ($e instanceof DatabaseException) { + Console::error('shmuel::createIndex' . $e->getMessage()); + Console::error('shmuel::createIndex' . $e->getMessage()); + Console::error('shmuel::createIndex' . $e->getMessage()); + Console::error('shmuel::createIndex' . $e->getMessage()); + Console::error('shmuel::createIndex' . $e->getMessage()); $index->setAttribute('error', $e->getMessage()); } $dbForProject->updateDocument( @@ -428,7 +431,6 @@ class Databases extends Action $this->trigger($database, $collection, $index, $project, $projectId, $events); $dbForProject->purgeCachedDocument('database_' . $database->getInternalId(), $collectionId); } - } /** From e5730506ebf5c8f8cda94434d89b3d9013c0b66d Mon Sep 17 00:00:00 2001 From: fogelito Date: Sun, 10 Nov 2024 16:37:44 +0200 Subject: [PATCH 6/7] Remove var dumps --- src/Appwrite/Platform/Workers/Databases.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/Appwrite/Platform/Workers/Databases.php b/src/Appwrite/Platform/Workers/Databases.php index 7cff9f264a..accc003976 100644 --- a/src/Appwrite/Platform/Workers/Databases.php +++ b/src/Appwrite/Platform/Workers/Databases.php @@ -410,13 +410,7 @@ class Databases extends Action $dbForProject->updateDocument('indexes', $index->getId(), $index->setAttribute('status', 'available')); } catch (\Throwable $e) { Console::error($e->getMessage()); - Console::error('shmuel::createIndex'); if ($e instanceof DatabaseException) { - Console::error('shmuel::createIndex' . $e->getMessage()); - Console::error('shmuel::createIndex' . $e->getMessage()); - Console::error('shmuel::createIndex' . $e->getMessage()); - Console::error('shmuel::createIndex' . $e->getMessage()); - Console::error('shmuel::createIndex' . $e->getMessage()); $index->setAttribute('error', $e->getMessage()); } $dbForProject->updateDocument( From 8cca88f2b8180944e0fb2a8170d431047de0805c Mon Sep 17 00:00:00 2001 From: fogelito Date: Mon, 11 Nov 2024 18:52:15 +0200 Subject: [PATCH 7/7] Remove Sentry comment --- src/Appwrite/Platform/Workers/Databases.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Appwrite/Platform/Workers/Databases.php b/src/Appwrite/Platform/Workers/Databases.php index accc003976..fe81114ea0 100644 --- a/src/Appwrite/Platform/Workers/Databases.php +++ b/src/Appwrite/Platform/Workers/Databases.php @@ -193,7 +193,6 @@ class Databases extends Action ); } - // TODO: Send non DatabaseExceptions to Sentry throw $e; } finally { $this->trigger($database, $collection, $attribute, $project, $projectId, $events); @@ -298,7 +297,6 @@ class Databases extends Action ); } - // TODO: Send non DatabaseExceptions to Sentry throw $e; } finally { $this->trigger($database, $collection, $attribute, $project, $projectId, $events); @@ -419,7 +417,6 @@ class Databases extends Action $index->setAttribute('status', 'failed') ); - // TODO: Send non DatabaseExceptions to Sentry throw $e; } finally { $this->trigger($database, $collection, $index, $project, $projectId, $events);