From 442c51eb3ca8a146e77208284bab65b4dc429a0d Mon Sep 17 00:00:00 2001 From: Andrew Gaul Date: Mon, 7 May 2012 16:38:03 -0700 Subject: [PATCH] Introduce TransientStorageStrategy This allows code from the filesystem blobstore to be more similar to the transient blobstore. This commit also corrects a bug where blobExists did not throw an exception when the container did not exist. --- .../filesystem/FilesystemAsyncBlobStore.java | 25 +++----- .../FilesystemAsyncBlobStoreTest.java | 8 ++- .../blobstore/TransientAsyncBlobStore.java | 61 +++++++++++++------ 3 files changed, 57 insertions(+), 37 deletions(-) diff --git a/apis/filesystem/src/main/java/org/jclouds/filesystem/FilesystemAsyncBlobStore.java b/apis/filesystem/src/main/java/org/jclouds/filesystem/FilesystemAsyncBlobStore.java index c462b44661..2f7b40e41e 100644 --- a/apis/filesystem/src/main/java/org/jclouds/filesystem/FilesystemAsyncBlobStore.java +++ b/apis/filesystem/src/main/java/org/jclouds/filesystem/FilesystemAsyncBlobStore.java @@ -148,7 +148,7 @@ public class FilesystemAsyncBlobStore extends BaseAsyncBlobStore { public ListenableFuture> list(final String container, ListContainerOptions options) { // Check if the container exists - if (!containerExistsSyncImpl(container)) + if (!storageStrategy.containerExists(container)) return immediateFailedFuture(cnfe(container)); // Loading blobs from container @@ -242,7 +242,9 @@ public class FilesystemAsyncBlobStore extends BaseAsyncBlobStore { } private ContainerNotFoundException cnfe(final String name) { - return new ContainerNotFoundException(name, String.format("container %s not in filesystem", name)); + return new ContainerNotFoundException(name, String.format( + "container %s not in %s", name, + storageStrategy.getAllContainerNames())); } public static MutableBlobMetadata copy(MutableBlobMetadata in) { @@ -304,7 +306,7 @@ public class FilesystemAsyncBlobStore extends BaseAsyncBlobStore { */ @Override public ListenableFuture containerExists(final String containerName) { - return immediateFuture(containerExistsSyncImpl(containerName)); + return immediateFuture(storageStrategy.containerExists(containerName)); } /** @@ -503,6 +505,8 @@ public class FilesystemAsyncBlobStore extends BaseAsyncBlobStore { */ @Override public ListenableFuture blobExists(final String containerName, final String key) { + if (!storageStrategy.containerExists(containerName)) + return immediateFailedFuture(cnfe(containerName)); return immediateFuture(storageStrategy.blobExists(containerName, key)); } @@ -513,7 +517,7 @@ public class FilesystemAsyncBlobStore extends BaseAsyncBlobStore { public ListenableFuture getBlob(final String containerName, final String key, GetOptions options) { logger.debug("Retrieving blob with key %s from container %s", key, containerName); // If the container doesn't exist, an exception is thrown - if (!containerExistsSyncImpl(containerName)) { + if (!storageStrategy.containerExists(containerName)) { logger.debug("Container %s does not exist", containerName); return immediateFailedFuture(cnfe(containerName)); } @@ -601,17 +605,6 @@ public class FilesystemAsyncBlobStore extends BaseAsyncBlobStore { } } - /** - * Each container is a directory, so in order to check if a container exists - * the corresponding directory must exists. Synchronous implementation - * - * @param containerName - * @return - */ - private boolean containerExistsSyncImpl(String containerName) { - return storageStrategy.containerExists(containerName); - } - /** * Calculates the object MD5 and returns it as eTag * @@ -633,7 +626,7 @@ public class FilesystemAsyncBlobStore extends BaseAsyncBlobStore { @Override protected boolean deleteAndVerifyContainerGone(final String container) { storageStrategy.deleteContainer(container); - return containerExistsSyncImpl(container); + return storageStrategy.containerExists(container); } @Override diff --git a/apis/filesystem/src/test/java/org/jclouds/filesystem/FilesystemAsyncBlobStoreTest.java b/apis/filesystem/src/test/java/org/jclouds/filesystem/FilesystemAsyncBlobStoreTest.java index ce936344eb..86860b1796 100644 --- a/apis/filesystem/src/test/java/org/jclouds/filesystem/FilesystemAsyncBlobStoreTest.java +++ b/apis/filesystem/src/test/java/org/jclouds/filesystem/FilesystemAsyncBlobStoreTest.java @@ -565,8 +565,12 @@ public class FilesystemAsyncBlobStoreTest { // when location doesn't exists blobKey = TestUtils.createRandomBlobKey(); - result = blobStore.blobExists(CONTAINER_NAME, blobKey); - assertFalse(result, "Blob exists"); + try { + blobStore.blobExists(CONTAINER_NAME, blobKey); + fail(); + } catch (ContainerNotFoundException cnfe) { + // expected + } // when location exists blobStore.createContainerInLocation(null, CONTAINER_NAME); diff --git a/blobstore/src/main/java/org/jclouds/blobstore/TransientAsyncBlobStore.java b/blobstore/src/main/java/org/jclouds/blobstore/TransientAsyncBlobStore.java index b4cce8a694..6168e8b59a 100644 --- a/blobstore/src/main/java/org/jclouds/blobstore/TransientAsyncBlobStore.java +++ b/blobstore/src/main/java/org/jclouds/blobstore/TransientAsyncBlobStore.java @@ -128,6 +128,7 @@ public class TransientAsyncBlobStore extends BaseAsyncBlobStore { protected final HttpGetOptionsListToGetOptions httpGetOptionsConverter; protected final IfDirectoryReturnNameStrategy ifDirectoryReturnName; protected final Factory blobFactory; + protected final TransientStorageStrategy storageStrategy; @Inject protected TransientAsyncBlobStore(BlobStoreContext context, @@ -152,6 +153,7 @@ public class TransientAsyncBlobStore extends BaseAsyncBlobStore { this.ifDirectoryReturnName = ifDirectoryReturnName; getContainerToLocation().put("stub", defaultLocation.get()); getContainerToBlobs().put("stub", new ConcurrentHashMap()); + this.storageStrategy = new TransientStorageStrategy(); } /** @@ -246,8 +248,9 @@ public class TransientAsyncBlobStore extends BaseAsyncBlobStore { } private ContainerNotFoundException cnfe(final String name) { - return new ContainerNotFoundException(name, String.format("container %s not in %s", name, getContainerToBlobs() - .keySet())); + return new ContainerNotFoundException(name, String.format( + "container %s not in %s", name, + storageStrategy.getAllContainerNames())); } public static MutableBlobMetadata copy(MutableBlobMetadata in) { @@ -285,9 +288,7 @@ public class TransientAsyncBlobStore extends BaseAsyncBlobStore { */ @Override public ListenableFuture removeBlob(final String container, final String key) { - if (getContainerToBlobs().containsKey(container)) { - getContainerToBlobs().get(container).remove(key); - } + storageStrategy.removeBlob(container, key); return immediateFuture(null); } @@ -305,17 +306,15 @@ public class TransientAsyncBlobStore extends BaseAsyncBlobStore { */ @Override public ListenableFuture deleteContainer(final String container) { - if (getContainerToBlobs().containsKey(container)) { - getContainerToBlobs().remove(container); - } + deleteAndVerifyContainerGone(container); return immediateFuture(null); } public ListenableFuture deleteContainerIfEmpty(final String container) { Boolean returnVal = true; - if (getContainerToBlobs().containsKey(container)) { + if (storageStrategy.containerExists(container)) { if (getContainerToBlobs().get(container).size() == 0) - getContainerToBlobs().remove(container); + storageStrategy.deleteContainer(container); else returnVal = false; } @@ -327,7 +326,7 @@ public class TransientAsyncBlobStore extends BaseAsyncBlobStore { */ @Override public ListenableFuture containerExists(final String containerName) { - return immediateFuture(getContainerToBlobs().containsKey(containerName)); + return immediateFuture(storageStrategy.containerExists(containerName)); } /** @@ -335,7 +334,7 @@ public class TransientAsyncBlobStore extends BaseAsyncBlobStore { */ @Override public ListenableFuture> list() { - Iterable containers = getContainerToBlobs().keySet(); + Iterable containers = storageStrategy.getAllContainerNames(); return Futures.> immediateFuture(new PageSetImpl(transform( containers, new Function() { @@ -358,7 +357,7 @@ public class TransientAsyncBlobStore extends BaseAsyncBlobStore { */ @Override public ListenableFuture createContainerInLocation(final Location location, final String name) { - if (getContainerToBlobs().containsKey(name)) { + if (storageStrategy.containerExists(name)) { return immediateFuture(Boolean.FALSE); } getContainerToBlobs().put(name, new ConcurrentHashMap()); @@ -542,10 +541,9 @@ public class TransientAsyncBlobStore extends BaseAsyncBlobStore { */ @Override public ListenableFuture blobExists(final String containerName, final String key) { - if (!getContainerToBlobs().containsKey(containerName)) + if (!storageStrategy.containerExists(containerName)) return immediateFailedFuture(cnfe(containerName)); - Map realContents = getContainerToBlobs().get(containerName); - return immediateFuture(realContents.containsKey(key)); + return immediateFuture(storageStrategy.blobExists(containerName, key)); } /** @@ -555,7 +553,7 @@ public class TransientAsyncBlobStore extends BaseAsyncBlobStore { public ListenableFuture getBlob(final String containerName, final String key, GetOptions options) { logger.debug("Retrieving blob with key %s from container %s", key, containerName); // If the container doesn't exist, an exception is thrown - if (!getContainerToBlobs().containsKey(containerName)) { + if (!storageStrategy.containerExists(containerName)) { logger.debug("Container %s does not exist", containerName); return immediateFailedFuture(cnfe(containerName)); } @@ -660,8 +658,8 @@ public class TransientAsyncBlobStore extends BaseAsyncBlobStore { @Override protected boolean deleteAndVerifyContainerGone(final String container) { - getContainerToBlobs().remove(container); - return getContainerToBlobs().containsKey(container); + storageStrategy.deleteContainer(container); + return storageStrategy.containerExists(container); } private ConcurrentMap getContainerToLocation() { @@ -681,4 +679,29 @@ public class TransientAsyncBlobStore extends BaseAsyncBlobStore { throw new UnsupportedOperationException("publicRead"); return createContainerInLocation(location, container); } + + private class TransientStorageStrategy { + public Iterable getAllContainerNames() { + return getContainerToBlobs().keySet(); + } + + public boolean containerExists(final String containerName) { + return getContainerToBlobs().containsKey(containerName); + } + + public void deleteContainer(final String containerName) { + getContainerToBlobs().remove(containerName); + } + + public boolean blobExists(final String containerName, final String blobName) { + Map map = containerToBlobs.get(containerName); + return map != null && map.containsKey(blobName); + } + + public void removeBlob(final String containerName, final String blobName) { + if (storageStrategy.containerExists(containerName)) { + getContainerToBlobs().get(containerName).remove(blobName); + } + } + } }