From 19667f78a92e00fa203a7a35bd4f4f13e2175851 Mon Sep 17 00:00:00 2001 From: fogelito Date: Thu, 7 Nov 2024 17:23:46 +0200 Subject: [PATCH] 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()); } /**