From fc91f64290bbfcd5f67a9f764f3e2b611025ff7b Mon Sep 17 00:00:00 2001 From: Zack Shoylev Date: Fri, 21 Oct 2016 17:15:50 -0500 Subject: [PATCH] Ensure that jclouds consistently uses /, works with \\, and uses File.separator on the filesystem level. --- .../FilesystemBlobKeyValidatorImpl.java | 7 +-- .../FilesystemContainerNameValidatorImpl.java | 7 +-- .../FilesystemStorageStrategyImpl.java | 44 +++++++-------- .../filesystem/FilesystemBlobStoreTest.java | 56 +++++++++++-------- .../FilesystemStorageStrategyImplTest.java | 4 +- 5 files changed, 57 insertions(+), 61 deletions(-) diff --git a/apis/filesystem/src/main/java/org/jclouds/filesystem/predicates/validators/internal/FilesystemBlobKeyValidatorImpl.java b/apis/filesystem/src/main/java/org/jclouds/filesystem/predicates/validators/internal/FilesystemBlobKeyValidatorImpl.java index 5b92bcc3eb..9855e528c8 100644 --- a/apis/filesystem/src/main/java/org/jclouds/filesystem/predicates/validators/internal/FilesystemBlobKeyValidatorImpl.java +++ b/apis/filesystem/src/main/java/org/jclouds/filesystem/predicates/validators/internal/FilesystemBlobKeyValidatorImpl.java @@ -16,8 +16,6 @@ */ package org.jclouds.filesystem.predicates.validators.internal; -import java.io.File; - import org.jclouds.filesystem.predicates.validators.FilesystemBlobKeyValidator; import com.google.inject.Singleton; @@ -38,9 +36,8 @@ public class FilesystemBlobKeyValidatorImpl extends FilesystemBlobKeyValidator { throw new IllegalArgumentException("Blob key can't be null or empty"); //blobkey cannot start with / (or \ in Windows) character - if (name.startsWith(File.separator)) - throw new IllegalArgumentException(String.format( - "Blob key '%s' cannot start with character %s", name, File.separator)); + if (name.startsWith("\\") || name.startsWith("/")) + throw new IllegalArgumentException("Blob key '%s' cannot start with \\ or /"); } } diff --git a/apis/filesystem/src/main/java/org/jclouds/filesystem/predicates/validators/internal/FilesystemContainerNameValidatorImpl.java b/apis/filesystem/src/main/java/org/jclouds/filesystem/predicates/validators/internal/FilesystemContainerNameValidatorImpl.java index accdf109b4..0d48ec99dd 100644 --- a/apis/filesystem/src/main/java/org/jclouds/filesystem/predicates/validators/internal/FilesystemContainerNameValidatorImpl.java +++ b/apis/filesystem/src/main/java/org/jclouds/filesystem/predicates/validators/internal/FilesystemContainerNameValidatorImpl.java @@ -16,8 +16,6 @@ */ package org.jclouds.filesystem.predicates.validators.internal; -import java.io.File; - import org.jclouds.filesystem.predicates.validators.FilesystemContainerNameValidator; import com.google.inject.Singleton; @@ -38,9 +36,8 @@ public class FilesystemContainerNameValidatorImpl extends FilesystemContainerNam throw new IllegalArgumentException("Container name can't be null or empty"); //container name cannot contains / (or \ in Windows) character - if (name.contains(File.separator)) - throw new IllegalArgumentException(String.format( - "Container name '%s' cannot contain character %s", name, File.separator)); + if (name.contains("\\") || name.contains("/")) + throw new IllegalArgumentException("Container name '%s' cannot contain \\ or /"); } } diff --git a/apis/filesystem/src/main/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImpl.java b/apis/filesystem/src/main/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImpl.java index 319633b315..0c0a59a8a7 100644 --- a/apis/filesystem/src/main/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImpl.java +++ b/apis/filesystem/src/main/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImpl.java @@ -109,8 +109,6 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy { private static final byte[] DIRECTORY_MD5 = Hashing.md5().hashBytes(new byte[0]).asBytes(); - private static final String BACK_SLASH = "\\"; - @Resource protected Logger logger = Logger.NULL; @@ -315,7 +313,7 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy { populateBlobKeysInContainer(containerFile, blobNames, new Function() { @Override public String apply(String string) { - return string.substring(containerPathLength); + return denormalize(string.substring(containerPathLength)); } }); return blobNames; @@ -614,7 +612,7 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy { @Override public String getSeparator() { - return File.separator; + return "/"; } public boolean createContainer(String container) { @@ -745,28 +743,26 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy { } /** - * Substitutes all the file separator occurrences in the path with a file separator for the - * current operative system + * Convert path to the current OS filesystem standard * - * @param pathToBeNormalized + * @param path * @return */ - private static String normalize(String pathToBeNormalized) { - if (null != pathToBeNormalized && pathToBeNormalized.contains(BACK_SLASH)) { - if (!BACK_SLASH.equals(File.separator)) { - return pathToBeNormalized.replace(BACK_SLASH, File.separator); - } + private static String normalize(String path) { + if (null != path) { + return path.replace("/", File.separator).replace("\\", File.separator); } - return pathToBeNormalized; + return path; } - private static String denormalize(String pathToDenormalize) { - if (null != pathToDenormalize && pathToDenormalize.contains("/")) { - if (BACK_SLASH.equals(File.separator)) { - return pathToDenormalize.replace("/", BACK_SLASH); - } + /** + * Convert path to jclouds standard (/) + */ + private static String denormalize(String path) { + if (null != path) { + return path.replace("\\", "/"); } - return pathToDenormalize; + return path; } /** @@ -786,10 +782,11 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy { // search for separator chars if (!onlyTrailing) { - if (pathToBeCleaned.substring(0, 1).equals(File.separator)) + if (pathToBeCleaned.charAt(0) == '/' || pathToBeCleaned.charAt(0) == '\\') beginIndex = 1; } - if (pathToBeCleaned.substring(pathToBeCleaned.length() - 1).equals(File.separator)) + if (pathToBeCleaned.charAt(pathToBeCleaned.length() - 1) == '/' || + pathToBeCleaned.charAt(pathToBeCleaned.length() - 1) == '\\') endIndex--; return pathToBeCleaned.substring(beginIndex, endIndex); @@ -803,10 +800,7 @@ public class FilesystemStorageStrategyImpl implements LocalStorageStrategy { * @param blobKey */ private void removeDirectoriesTreeOfBlobKey(String container, String blobKey) { - String normalizedBlobKey = denormalize(blobKey); - // exists is no path is present in the blobkey - if (!normalizedBlobKey.contains(File.separator)) - return; + String normalizedBlobKey = normalize(blobKey); File file = new File(normalizedBlobKey); // TODO diff --git a/apis/filesystem/src/test/java/org/jclouds/filesystem/FilesystemBlobStoreTest.java b/apis/filesystem/src/test/java/org/jclouds/filesystem/FilesystemBlobStoreTest.java index d27924f1ff..e23bebd138 100644 --- a/apis/filesystem/src/test/java/org/jclouds/filesystem/FilesystemBlobStoreTest.java +++ b/apis/filesystem/src/test/java/org/jclouds/filesystem/FilesystemBlobStoreTest.java @@ -173,9 +173,11 @@ public class FilesystemBlobStoreTest { checkForContainerContent(CONTAINER_NAME, null); // creates blobs in first container - Set blobsExpected = TestUtils.createBlobsInContainer(CONTAINER_NAME, "bbb" + File.separator + "ccc" - + File.separator + "ddd" + File.separator + "1234.jpg", "4rrr.jpg", "rrr" + File.separator + "sss" - + File.separator + "788.jpg", "xdc" + File.separator + "wert.kpg"); + Set blobsExpected = TestUtils.createBlobsInContainer(CONTAINER_NAME, + "bbb/ccc/ddd/1234.jpg", + "4rrr.jpg", + "rrr/sss/788.jpg", + "xdc/wert.kpg"); checkForContainerContent(CONTAINER_NAME, blobsExpected); } @@ -220,17 +222,19 @@ public class FilesystemBlobStoreTest { // creates blobs in first container - Set blobNamesCreatedInContainer1 = TestUtils.createBlobsInContainer(CONTAINER_NAME, "bbb" - + File.separator + "ccc" + File.separator + "ddd" + File.separator + "1234.jpg", - TestUtils.createRandomBlobKey(), "rrr" + File.separator + "sss" + File.separator + "788.jpg", "xdc" - + File.separator + "wert.kpg"); + Set blobNamesCreatedInContainer1 = TestUtils.createBlobsInContainer(CONTAINER_NAME, + "bbb/ccc/ddd/1234.jpg", + TestUtils.createRandomBlobKey(), + "rrr/sss/788.jpg", + "xdc/wert.kpg"); // creates blobs in second container blobStore.createContainerInLocation(null, CONTAINER_NAME2); - Set blobNamesCreatedInContainer2 = TestUtils.createBlobsInContainer(CONTAINER_NAME2, "asd" - + File.separator + "bbb" + File.separator + "ccc" + File.separator + "ddd" + File.separator + "1234.jpg", - TestUtils.createRandomBlobKey(), "rrr" + File.separator + "sss" + File.separator + "788.jpg", "xdc" - + File.separator + "wert.kpg"); + Set blobNamesCreatedInContainer2 = TestUtils.createBlobsInContainer(CONTAINER_NAME2, + "asd/bbb/ccc/ddd/1234.jpg", + TestUtils.createRandomBlobKey(), + "rrr/sss/788.jpg", + "xdc/wert.kpg"); // test blobs in first container checkForContainerContent(CONTAINER_NAME, blobNamesCreatedInContainer1); @@ -244,12 +248,14 @@ public class FilesystemBlobStoreTest { checkForContainerContent(CONTAINER_NAME, null); // creates blobs in first container - Set blobsExpected = TestUtils.createBlobsInContainer(CONTAINER_NAME, "bbb" + File.separator + "ccc" - + File.separator + "ddd" + File.separator + "1234.jpg", "4rrr.jpg", "rrr" + File.separator + "sss" - + File.separator + "788.jpg", "rrr" + File.separator + "wert.kpg"); + Set blobsExpected = TestUtils.createBlobsInContainer(CONTAINER_NAME, + "bbb/ccc/ddd/1234.jpg", + "4rrr.jpg", + "rrr/sss/788.jpg", + "rrr/wert.kpg"); // remove not expected values - blobsExpected.remove("bbb" + File.separator + "ccc" + File.separator + "ddd" + File.separator + "1234.jpg"); + blobsExpected.remove("bbb/ccc/ddd/1234.jpg"); blobsExpected.remove("4rrr.jpg"); checkForContainerContent(CONTAINER_NAME, "rrr", blobsExpected); @@ -271,17 +277,19 @@ public class FilesystemBlobStoreTest { blobStore.createContainerInLocation(null, CONTAINER_NAME2); // creates blobs in first container - Set blobNamesCreatedInContainer1 = TestUtils.createBlobsInContainer(CONTAINER_NAME, "bbb" - + File.separator + "ccc" + File.separator + "ddd" + File.separator + "1234.jpg", - TestUtils.createRandomBlobKey(), "rrr" + File.separator + "sss" + File.separator + "788.jpg", "xdc" - + File.separator + "wert.kpg"); + Set blobNamesCreatedInContainer1 = TestUtils.createBlobsInContainer(CONTAINER_NAME, + "bbb/ccc/ddd/1234.jpg", + TestUtils.createRandomBlobKey(), + "rrr/sss/788.jpg", + "xdc/wert.kpg"); // creates blobs in second container blobStore.createContainerInLocation(null, CONTAINER_NAME2); - Set blobNamesCreatedInContainer2 = TestUtils.createBlobsInContainer(CONTAINER_NAME2, "asd" - + File.separator + "bbb" + File.separator + "ccc" + File.separator + "ddd" + File.separator + "1234.jpg", - TestUtils.createRandomBlobKey(), "rrr" + File.separator + "sss" + File.separator + "788.jpg", "xdc" - + File.separator + "wert.kpg"); + Set blobNamesCreatedInContainer2 = TestUtils.createBlobsInContainer(CONTAINER_NAME2, + "asd/bbb/ccc/ddd/1234.jpg", + TestUtils.createRandomBlobKey(), + "rrr/sss/788.jpg", + "xdc/wert.kpg"); // test blobs in containers checkForContainerContent(CONTAINER_NAME, blobNamesCreatedInContainer1); @@ -329,7 +337,7 @@ public class FilesystemBlobStoreTest { blobStore.removeBlob(CONTAINER_NAME, BLOB_KEY); result = blobStore.blobExists(CONTAINER_NAME, BLOB_KEY); assertFalse(result, "Blob still exists"); - TestUtils.fileExists(TARGET_CONTAINER_NAME + File.separator + BLOB_KEY, false); + TestUtils.fileExists(TARGET_CONTAINER_NAME + "/" + BLOB_KEY, false); } public void testRemoveBlob_TwoSimpleBlobKeys() throws IOException { diff --git a/apis/filesystem/src/test/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImplTest.java b/apis/filesystem/src/test/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImplTest.java index 9079fa0bb2..6eb4238251 100644 --- a/apis/filesystem/src/test/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImplTest.java +++ b/apis/filesystem/src/test/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImplTest.java @@ -605,8 +605,8 @@ public class FilesystemStorageStrategyImplTest { Set createBlobKeys = TestUtils.createBlobsInContainer(CONTAINER_NAME, new String[] { TestUtils.createRandomBlobKey("GetBlobKeys-", ".jpg"), TestUtils.createRandomBlobKey("GetBlobKeys-", ".jpg"), - TestUtils.createRandomBlobKey("563" + FS + "g3sx2" + FS + "removeBlob-", ".jpg"), - TestUtils.createRandomBlobKey("563" + FS + "g3sx2" + FS + "removeBlob-", ".jpg") }); + TestUtils.createRandomBlobKey("563" + "/" + "g3sx2" + "/" + "removeBlob-", ".jpg"), + TestUtils.createRandomBlobKey("563" + "/" + "g3sx2" + "/" + "removeBlob-", ".jpg") }); storageStrategy.getBlobKeysInsideContainer(CONTAINER_NAME); List retrievedBlobKeys = Lists.newArrayList();