From 627ea06e2980eebfbdd24cce097c6911c848754f Mon Sep 17 00:00:00 2001 From: Jamie Hannaford Date: Mon, 1 Dec 2014 10:46:50 +0000 Subject: [PATCH 1/5] Making batch object deletion more robust --- .../ObjectStore/Resource/Container.php | 34 ++++++++++++++++--- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/lib/OpenCloud/ObjectStore/Resource/Container.php b/lib/OpenCloud/ObjectStore/Resource/Container.php index 3f47c2227..c0b93de73 100644 --- a/lib/OpenCloud/ObjectStore/Resource/Container.php +++ b/lib/OpenCloud/ObjectStore/Resource/Container.php @@ -184,15 +184,39 @@ public function delete($deleteObjects = false) */ public function deleteAllObjects() { - $requests = array(); + $paths = array(); - $list = $this->objectList(); + $objects = $this->objectList(); - foreach ($list as $object) { - $requests[] = $this->getClient()->delete($object->getUrl()); + foreach ($objects as $object) { + $i++; + $paths[] = sprintf('/%s/%s', $this->getName(), $object->getName()); } - return $this->getClient()->send($requests); + // Batch delete can only handle 10000 paths per request + $chunks = array_chunk($paths, 10000); + foreach ($chunks as $chunk) { + $this->getService()->bulkDelete($chunk); + } + + // Poll the container for state change + $timeout = 60; + $currentTime = 0; + + while (true) { + ++$currentTime; + + if ($currentTime >= $timeout) { + return false; + } + + $metadata = $this->retrieveMetadata(); + if ($metadata->getProperty('Object-Count') === 0) { + return false; + } + } + + return true; } /** From 26d980e59ed08d581918210704a4d66dd7ac593d Mon Sep 17 00:00:00 2001 From: Jamie Hannaford Date: Mon, 1 Dec 2014 10:53:58 +0000 Subject: [PATCH 2/5] No need for counter :thought_balloon: --- lib/OpenCloud/ObjectStore/Resource/Container.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/OpenCloud/ObjectStore/Resource/Container.php b/lib/OpenCloud/ObjectStore/Resource/Container.php index c0b93de73..9b970f3f5 100644 --- a/lib/OpenCloud/ObjectStore/Resource/Container.php +++ b/lib/OpenCloud/ObjectStore/Resource/Container.php @@ -189,7 +189,6 @@ public function deleteAllObjects() $objects = $this->objectList(); foreach ($objects as $object) { - $i++; $paths[] = sprintf('/%s/%s', $this->getName(), $object->getName()); } From 87797b083015a34314c105f5aa3f9aa3e6284131 Mon Sep 17 00:00:00 2001 From: Jamie Hannaford Date: Mon, 1 Dec 2014 14:56:54 +0000 Subject: [PATCH 3/5] Use proper timeout and class constant --- .../ObjectStore/Resource/Container.php | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/lib/OpenCloud/ObjectStore/Resource/Container.php b/lib/OpenCloud/ObjectStore/Resource/Container.php index 9b970f3f5..0f3bb9b13 100644 --- a/lib/OpenCloud/ObjectStore/Resource/Container.php +++ b/lib/OpenCloud/ObjectStore/Resource/Container.php @@ -43,6 +43,7 @@ class Container extends AbstractContainer { const METADATA_LABEL = 'Container'; + const BATCH_DELETE_MAX = 10000; /** * This is the object that holds all the CDN functionality. This Container therefore acts as a simple wrapper and is @@ -193,26 +194,19 @@ public function deleteAllObjects() } // Batch delete can only handle 10000 paths per request - $chunks = array_chunk($paths, 10000); + $chunks = array_chunk($paths, self::BATCH_DELETE_MAX); foreach ($chunks as $chunk) { $this->getService()->bulkDelete($chunk); } // Poll the container for state change - $timeout = 60; - $currentTime = 0; - - while (true) { - ++$currentTime; - - if ($currentTime >= $timeout) { + $timeout = time() + 60; + while (time() < $timeout) { + if ($this->retrieveMetadata()->getProperty('Object-Count') === 0) { return false; } - $metadata = $this->retrieveMetadata(); - if ($metadata->getProperty('Object-Count') === 0) { - return false; - } + sleep(1); } return true; From cf46cbf5a36d30f09b5aea091ec0f4c90f35e180 Mon Sep 17 00:00:00 2001 From: Jamie Hannaford Date: Mon, 1 Dec 2014 16:17:55 +0000 Subject: [PATCH 4/5] Moving chunking functionality to service client; deprecate bulkDelete; add container waiter --- .../ObjectStore/Resource/Container.php | 34 ++++++++----- lib/OpenCloud/ObjectStore/Service.php | 50 +++++++++++++++++-- .../ObjectStore/Resource/ContainerTest.php | 18 ++++++- .../Tests/ObjectStore/ServiceTest.php | 14 ++++++ 4 files changed, 98 insertions(+), 18 deletions(-) diff --git a/lib/OpenCloud/ObjectStore/Resource/Container.php b/lib/OpenCloud/ObjectStore/Resource/Container.php index 0f3bb9b13..6a4673e59 100644 --- a/lib/OpenCloud/ObjectStore/Resource/Container.php +++ b/lib/OpenCloud/ObjectStore/Resource/Container.php @@ -43,7 +43,6 @@ class Container extends AbstractContainer { const METADATA_LABEL = 'Container'; - const BATCH_DELETE_MAX = 10000; /** * This is the object that holds all the CDN functionality. This Container therefore acts as a simple wrapper and is @@ -193,23 +192,32 @@ public function deleteAllObjects() $paths[] = sprintf('/%s/%s', $this->getName(), $object->getName()); } - // Batch delete can only handle 10000 paths per request - $chunks = array_chunk($paths, self::BATCH_DELETE_MAX); - foreach ($chunks as $chunk) { - $this->getService()->bulkDelete($chunk); - } + $this->getService()->batchDelete($paths); - // Poll the container for state change - $timeout = time() + 60; - while (time() < $timeout) { - if ($this->retrieveMetadata()->getProperty('Object-Count') === 0) { - return false; + return $this->waitUntilEmpty(); + } + + /** + * This is a method that makes batch deletions more convenient. It continually + * polls the resource, waiting for its state to change. If the loop exceeds the + * provided timeout, it breaks and returns FALSE. + * + * @param int $secondsToWait The number of seconds to run the loop + * @return bool + */ + public function waitUntilEmpty($secondsToWait = 60, $interval = 1) + { + $endTime = time() + $secondsToWait; + + while (time() < $endTime) { + if ((int) $this->retrieveMetadata()->getProperty('Object-Count') === 0) { + return true; } - sleep(1); + sleep($interval); } - return true; + return false; } /** diff --git a/lib/OpenCloud/ObjectStore/Service.php b/lib/OpenCloud/ObjectStore/Service.php index b35cb3c31..a2d01a479 100644 --- a/lib/OpenCloud/ObjectStore/Service.php +++ b/lib/OpenCloud/ObjectStore/Service.php @@ -36,6 +36,7 @@ class Service extends AbstractService { const DEFAULT_NAME = 'cloudFiles'; const DEFAULT_TYPE = 'object-store'; + const BATCH_DELETE_MAX = 10000; /** * This holds the associated CDN service (for Rackspace public cloud) @@ -169,14 +170,55 @@ public function bulkExtract($path = '', $archive, $archiveType = UrlType::TAR_GZ } /** - * This method will delete multiple objects or containers from their account with a single request. + * @deprecated Please use {@see batchDelete()} instead. + */ + public function bulkDelete(array $paths) + { + $this->getLogger()->deprecated(__METHOD__, '::batchDelete()'); + + return $this->executeBatchDeleteRequest($paths); + } + + /** + * Batch delete will delete an array of object paths. By default, + * the API will only accept a maximum of 10,000 object deletions + * per request - so for arrays that exceed this size, it is chunked + * and sent as individual requests. + * + * @param array $paths The objects you want to delete. Each path needs + * be formatted as /{containerName}/{objectName}. If + * you are deleting object_1 and object_2 from the + * photos_container, the array will be: + * + * array( + * '/photos_container/object_1', + * '/photos_container/object_2' + * ) * - * @param array $paths A two-dimensional array of paths: - * array('container_a/file_1', 'container_b/file_78', 'container_c/file_40582') + * @return array The array of responses from the API + * @throws Exception\BulkOperationException + */ + public function batchDelete(array $paths) + { + $chunks = array_chunk($paths, self::BATCH_DELETE_MAX); + + $responses = array(); + + foreach ($chunks as $chunk) { + $responses[] = $this->executeBatchDeleteRequest($chunk); + } + + return $responses; + } + + /** + * Internal method for dispatching single batch delete requests. + * + * @param array $paths * @return \Guzzle\Http\Message\Response * @throws Exception\BulkOperationException */ - public function bulkDelete(array $paths) + private function executeBatchDeleteRequest(array $paths) { $entity = EntityBody::factory(implode(PHP_EOL, $paths)); diff --git a/tests/OpenCloud/Tests/ObjectStore/Resource/ContainerTest.php b/tests/OpenCloud/Tests/ObjectStore/Resource/ContainerTest.php index 8dfd1da5f..719550e49 100644 --- a/tests/OpenCloud/Tests/ObjectStore/Resource/ContainerTest.php +++ b/tests/OpenCloud/Tests/ObjectStore/Resource/ContainerTest.php @@ -335,4 +335,20 @@ public function test_Object_Exists_False() { $this->assertFalse($this->container->objectExists('test.foo')); } -} + + public function test_Waiter_Returns_True_For_Empty_Container() + { + $response = new Response(204, array('X-Container-Object-Count' => 0)); + $this->addMockSubscriber($response); + + $this->assertTrue($this->container->waitUntilEmpty(2, 0)); + } + + public function test_Waiter_Returns_False_On_Timeout() + { + $response = new Response(204, array('X-Container-Object-Count' => 10)); + $this->addMockSubscriber($response); + + $this->assertFalse($this->container->waitUntilEmpty(0.1, 1)); + } +} \ No newline at end of file diff --git a/tests/OpenCloud/Tests/ObjectStore/ServiceTest.php b/tests/OpenCloud/Tests/ObjectStore/ServiceTest.php index e5a898712..dfb62e63c 100644 --- a/tests/OpenCloud/Tests/ObjectStore/ServiceTest.php +++ b/tests/OpenCloud/Tests/ObjectStore/ServiceTest.php @@ -25,6 +25,7 @@ */ namespace OpenCloud\Tests\ObjectStore; +use Guzzle\Http\Message\Response; use OpenCloud\ObjectStore\Constants\UrlType; use OpenCloud\ObjectStore\Service; @@ -137,6 +138,19 @@ public function test_Bad_Bulk_Delete() $this->service->bulkDelete(array('nonEmptyContainer')); } + public function test_Batch_Delete_Returns_Array_Of_Responses() + { + $responses = array_fill(0, 2, new Response(200)); + + foreach ($responses as $response) { + $this->addMockSubscriber($response); + } + + $paths = array_fill(0, 15000, '/foo/bar'); + + $this->assertEquals($responses, $this->service->batchDelete($paths)); + } + public function test_Accounts() { $account = $this->service->getAccount(); From 4f095fc8f66cb2ca1f8677d3173ecd522ce5c0af Mon Sep 17 00:00:00 2001 From: Jamie Hannaford Date: Mon, 1 Dec 2014 17:20:05 +0000 Subject: [PATCH 5/5] PSR-2 fixes --- tests/OpenCloud/Tests/ObjectStore/Resource/ContainerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/OpenCloud/Tests/ObjectStore/Resource/ContainerTest.php b/tests/OpenCloud/Tests/ObjectStore/Resource/ContainerTest.php index 719550e49..db9e04f12 100644 --- a/tests/OpenCloud/Tests/ObjectStore/Resource/ContainerTest.php +++ b/tests/OpenCloud/Tests/ObjectStore/Resource/ContainerTest.php @@ -351,4 +351,4 @@ public function test_Waiter_Returns_False_On_Timeout() $this->assertFalse($this->container->waitUntilEmpty(0.1, 1)); } -} \ No newline at end of file +}