From 8677ffcb213f4182eee03039431a9562c2eee16a Mon Sep 17 00:00:00 2001 From: Timur Alperovich Date: Wed, 24 Jun 2015 16:52:28 -0700 Subject: [PATCH] JCLOUDS-930: Implement prefix for LocalBlobStore. Implements prefix support for the local blob store. The patch allows for correctly parsing prefixes that may not terminate with a delimiter (i.e. foo with delimiter "/" and a key foobar/key, should return foobar/ as the common path) and ones that do (i.e. foo/). NOTE: there is a small change in behavior in this patch. LocalBlobStore used to return the common prefixes without the delimiter character ("/"). However, other providers do include the delimiter (I checked S3 and Google Cloud Storage) and LocalBlobStore should include it as well. --- .../blobstore/config/LocalBlobStore.java | 68 +++++++--- .../internal/BaseContainerLiveTest.java | 4 +- .../strategy/internal/ListContainerTest.java | 128 ++++++++++++++++++ 3 files changed, 180 insertions(+), 20 deletions(-) create mode 100644 blobstore/src/test/java/org/jclouds/blobstore/strategy/internal/ListContainerTest.java diff --git a/blobstore/src/main/java/org/jclouds/blobstore/config/LocalBlobStore.java b/blobstore/src/main/java/org/jclouds/blobstore/config/LocalBlobStore.java index 831e83afea..a244a4b1df 100644 --- a/blobstore/src/main/java/org/jclouds/blobstore/config/LocalBlobStore.java +++ b/blobstore/src/main/java/org/jclouds/blobstore/config/LocalBlobStore.java @@ -216,6 +216,9 @@ public final class LocalBlobStore implements BlobStore { */ @Override public PageSet list(final String containerName, ListContainerOptions options) { + if (options.getDir() != null && options.getPrefix() != null) { + throw new IllegalArgumentException("Cannot set both prefix and directory"); + } // Check if the container exists if (!storageStrategy.containerExists(containerName)) @@ -254,6 +257,8 @@ public final class LocalBlobStore implements BlobStore { if (options.getDir() != null && !options.getDir().isEmpty()) { contents = filterDirectory(contents, options); + } else if (options.getPrefix() != null) { + contents = filterPrefix(contents, options); } else if (!options.isRecursive()) { contents = extractCommonPrefixes(contents, storageStrategy.getSeparator(), null); } @@ -324,7 +329,22 @@ public final class LocalBlobStore implements BlobStore { })); if (!options.isRecursive()) { - return extractCommonPrefixes(contents, storageStrategy.getSeparator(), prefix); + return extractCommonPrefixes(contents, storageStrategy.getSeparator(), dirPrefix); + } + + return contents; + } + + private SortedSet filterPrefix(SortedSet contents, final ListContainerOptions + options) { + contents = newTreeSet(filter(contents, new Predicate() { + public boolean apply(StorageMetadata o) { + return o != null && o.getName().replace(File.separatorChar, '/').startsWith(options.getPrefix()); + } + })); + + if (!options.isRecursive()) { + return extractCommonPrefixes(contents, storageStrategy.getSeparator(), options.getPrefix()); } return contents; @@ -341,12 +361,9 @@ public final class LocalBlobStore implements BlobStore { for (String o : commonPrefixes) { MutableStorageMetadata md = new MutableStorageMetadataImpl(); md.setType(StorageType.RELATIVE_PATH); - if (prefix != null && !prefix.isEmpty()) { - if (!prefix.endsWith(delimiter)) { - o = prefix + delimiter + o; - } else { - o = prefix + o; - } + + if (prefix != null) { + o = prefix + o; } md.setName(o); contents.add(md); @@ -450,12 +467,23 @@ public final class LocalBlobStore implements BlobStore { } public boolean apply(StorageMetadata metadata) { + String name = metadata.getName(); + if (metadata.getType() == StorageType.RELATIVE_PATH) { + // For directories, we need to make sure to include the separator character, as that is what the common + // prefix will include. + name += '/'; + } if (prefix == null || prefix.isEmpty()) - return metadata.getName().indexOf(delimiter) == -1; - // ensure we don't accidentally append twice - String toMatch = prefix.endsWith("/") ? prefix : prefix + delimiter; - if (metadata.getName().startsWith(toMatch)) { - String unprefixedName = metadata.getName().replaceFirst(Pattern.quote(toMatch), ""); + return name.indexOf(delimiter) == -1; + String prefixMatch; + if (prefix.endsWith(delimiter)) { + prefixMatch = "^" + Pattern.quote(prefix) + ".*"; + } else { + // We should correctly match strings like "foobar/" where the prefix is only "foo" + prefixMatch = "^" + Pattern.quote(prefix) + ".*" + Pattern.quote(delimiter) + ".*"; + } + if (name.matches(prefixMatch)) { + String unprefixedName = name.replaceFirst(prefix, ""); if (unprefixedName.equals("")) { // we are the prefix in this case, return false return false; @@ -479,16 +507,18 @@ public final class LocalBlobStore implements BlobStore { public String apply(StorageMetadata metadata) { String working = metadata.getName(); if (prefix != null) { - // ensure we don't accidentally append twice - String toMatch = prefix.endsWith("/") ? prefix : prefix + delimiter; - if (working.startsWith(toMatch)) { - working = working.replaceFirst(Pattern.quote(toMatch), ""); + if (working.startsWith(prefix)) { + working = working.replaceFirst(Pattern.quote(prefix), ""); + } else { + return NO_PREFIX; } } - if (working.contains(delimiter)) { - return working.substring(0, working.indexOf(delimiter)); + if (working.indexOf(delimiter) >= 0) { + // include the delimiter in the result + return working.substring(0, working.indexOf(delimiter) + 1); + } else { + return NO_PREFIX; } - return NO_PREFIX; } } diff --git a/blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseContainerLiveTest.java b/blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseContainerLiveTest.java index 7c620b222a..b11b5ddf18 100644 --- a/blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseContainerLiveTest.java +++ b/blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseContainerLiveTest.java @@ -17,6 +17,8 @@ package org.jclouds.blobstore.integration.internal; import static java.util.concurrent.TimeUnit.SECONDS; + +import static org.assertj.core.api.Assertions.assertThat; import static org.jclouds.blobstore.options.CreateContainerOptions.Builder.publicRead; import static org.jclouds.util.Predicates2.retry; import static org.testng.Assert.assertEquals; @@ -141,7 +143,7 @@ public class BaseContainerLiveTest extends BaseBlobStoreIntegrationTest { names.add(sm.getName()); } - assertEquals(expectedSet, names); + assertThat(names).containsExactlyElementsOf(expectedSet); } private void runCreateContainerInLocation(String payload, Location nonDefault) throws InterruptedException, diff --git a/blobstore/src/test/java/org/jclouds/blobstore/strategy/internal/ListContainerTest.java b/blobstore/src/test/java/org/jclouds/blobstore/strategy/internal/ListContainerTest.java new file mode 100644 index 0000000000..435df30405 --- /dev/null +++ b/blobstore/src/test/java/org/jclouds/blobstore/strategy/internal/ListContainerTest.java @@ -0,0 +1,128 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jclouds.blobstore.strategy.internal; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.google.common.collect.Iterables; +import com.google.inject.Injector; + +import org.jclouds.ContextBuilder; +import org.jclouds.blobstore.BlobStore; +import org.jclouds.blobstore.domain.StorageMetadata; +import org.jclouds.blobstore.domain.StorageType; +import org.jclouds.blobstore.options.ListContainerOptions; +import org.jclouds.util.Closeables2; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +@Test(testName = "PrefixTest", singleThreaded = true) +public class ListContainerTest { + private BlobStore blobStore; + private ConcatenateContainerLists concatter; + + @BeforeClass + public void setupBlobStore() { + Injector injector = ContextBuilder.newBuilder("transient").buildInjector(); + blobStore = injector.getInstance(BlobStore.class); + concatter = injector.getInstance(ConcatenateContainerLists.class); + } + + @AfterClass + public void closeBlobSore() { + if (blobStore != null) { + Closeables2.closeQuietly(blobStore.getContext()); + } + } + + public void testListWithPrefix() { + String containerName = "prefix"; + String prefix = "foo"; + blobStore.createContainerInLocation(null, containerName); + blobStore.putBlob(containerName, blobStore.blobBuilder(prefix).payload("").build()); + blobStore.putBlob(containerName, blobStore.blobBuilder(prefix + "bar").payload("").build()); + blobStore.putBlob(containerName, blobStore.blobBuilder(prefix + "baz").payload("").build()); + blobStore.putBlob(containerName, blobStore.blobBuilder("bar").payload("").build()); + + Iterable results = concatter.execute(containerName, + ListContainerOptions.Builder.prefix(prefix).recursive()); + assertThat(results).hasSize(3); + assertThat(Iterables.get(results, 0).getName()).isEqualTo(prefix); + assertThat(Iterables.get(results, 0).getType()).isEqualTo(StorageType.BLOB); + assertThat(Iterables.get(results, 1).getName()).isEqualTo(prefix + "bar"); + assertThat(Iterables.get(results, 1).getType()).isEqualTo(StorageType.BLOB); + assertThat(Iterables.get(results, 2).getName()).isEqualTo(prefix + "baz"); + assertThat(Iterables.get(results, 2).getType()).isEqualTo(StorageType.BLOB); + } + + public void testListWithPrefixAndSeparator() { + String containerName = "prefixWithSeparator"; + String prefix = "foo"; + blobStore.createContainerInLocation(null, containerName); + blobStore.putBlob(containerName, blobStore.blobBuilder(prefix + "/object").payload("").build()); + blobStore.putBlob(containerName, blobStore.blobBuilder(prefix + "bar/object").payload("") + .build()); + blobStore.putBlob(containerName, blobStore.blobBuilder(prefix + "baz/object").payload("") + .build()); + blobStore.putBlob(containerName, blobStore.blobBuilder("bar/object").payload("").build()); + + Iterable results = concatter.execute(containerName, + ListContainerOptions.Builder.prefix(prefix)); + assertThat(Iterables.size(results)).isEqualTo(3); + assertThat(Iterables.get(results, 0).getType()).isEqualTo(StorageType.RELATIVE_PATH); + assertThat(Iterables.get(results, 0).getName()).isEqualTo(prefix + "/"); + assertThat(Iterables.get(results, 1).getName()).isEqualTo(prefix + "bar/"); + assertThat(Iterables.get(results, 1).getType()).isEqualTo(StorageType.RELATIVE_PATH); + assertThat(Iterables.get(results, 2).getName()).isEqualTo(prefix + "baz/"); + assertThat(Iterables.get(results, 2).getType()).isEqualTo(StorageType.RELATIVE_PATH); + } + + public void testListRecursivePrefix() { + String containerName = "testListRecursive"; + String prefix = "foo"; + blobStore.createContainerInLocation(null, containerName); + blobStore.putBlob(containerName, blobStore.blobBuilder(prefix + "/object").payload("").build()); + blobStore.putBlob(containerName, blobStore.blobBuilder(prefix + "bar/object").payload("") + .build()); + blobStore.putBlob(containerName, blobStore.blobBuilder(prefix + "baz/object").payload("") + .build()); + blobStore.putBlob(containerName, blobStore.blobBuilder("bar/object").payload("").build()); + + Iterable results = concatter.execute(containerName, + ListContainerOptions.Builder.prefix(prefix).recursive()); + assertThat(results).hasSize(3); + assertThat(Iterables.get(results, 0).getType()).isEqualTo(StorageType.BLOB); + assertThat(Iterables.get(results, 0).getName()).isEqualTo(prefix + "/object"); + assertThat(Iterables.get(results, 1).getType()).isEqualTo(StorageType.BLOB); + assertThat(Iterables.get(results, 1).getName()).isEqualTo(prefix + "bar/object"); + assertThat(Iterables.get(results, 2).getType()).isEqualTo(StorageType.BLOB); + assertThat(Iterables.get(results, 2).getName()).isEqualTo(prefix + "baz/object"); + } + + public void testListDirectory() { + String containerName = "testListDir"; + String directory = "dir"; + blobStore.createContainerInLocation(null, containerName); + blobStore.createDirectory(containerName, directory); + blobStore.putBlob(containerName, blobStore.blobBuilder(directory + "/foo").payload("").build()); + blobStore.putBlob(containerName, blobStore.blobBuilder(directory + "/bar").payload("").build()); + Iterable results = concatter.execute(containerName, ListContainerOptions.NONE); + assertThat(results).hasSize(1); + assertThat(Iterables.get(results, 0).getName()).isEqualTo(directory + '/'); + } +}