Skip to content

Commit

Permalink
Fixes tests failing on windows filesystems Makes windows behavior mor…
Browse files Browse the repository at this point in the history 
…e consistent, especially for deletes
  • Loading branch information
@zack-shoylev
zack-shoylev committed Nov 12, 2015
1 parent fb63b0e commit 41ce90e
Show file tree
Hide file tree
Showing 9 changed files with 168 additions and 111 deletions.
8 changes: 7 additions & 1 deletion ...esystem/src/main/java/org/jclouds/filesystem/config/FilesystemBlobStoreContextModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
*/
package org.jclouds.filesystem.config;

import static org.jclouds.filesystem.util.Utils.isWindows;

import org.jclouds.blobstore.BlobRequestSigner;
import org.jclouds.blobstore.BlobStore;
import org.jclouds.blobstore.LocalBlobRequestSigner;
Expand All @@ -39,7 +41,11 @@ public class FilesystemBlobStoreContextModule extends AbstractModule {
protected void configure() {
bind(BlobStore.class).to(LocalBlobStore.class);
install(new BlobStoreObjectModule());
bind(ConsistencyModel.class).toInstance(ConsistencyModel.STRICT);
if (isWindows()) {
bind(ConsistencyModel.class).toInstance(ConsistencyModel.EVENTUAL);
} else {
bind(ConsistencyModel.class).toInstance(ConsistencyModel.STRICT);
}
bind(LocalStorageStrategy.class).to(FilesystemStorageStrategyImpl.class);
bind(BlobUtils.class).to(FileSystemBlobUtilsImpl.class);
bind(FilesystemBlobKeyValidator.class).to(FilesystemBlobKeyValidatorImpl.class);
Expand Down
48 changes: 28 additions & 20 deletions ...src/main/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import static java.nio.file.Files.probeContentType;
import static java.nio.file.Files.readAttributes;
import static java.nio.file.Files.setPosixFilePermissions;
import static org.jclouds.filesystem.util.Utils.delete;
import static org.jclouds.filesystem.util.Utils.isPrivate;
import static org.jclouds.filesystem.util.Utils.isWindows;
import static org.jclouds.filesystem.util.Utils.setPrivate;
Expand Down Expand Up @@ -454,7 +455,7 @@ public String putBlob(final String containerName, final Blob blob) throws IOExce
try {
Files.createParentDirs(outputFile);
his = new HashingInputStream(Hashing.md5(), payload.openStream());
outputFile.delete();
delete(outputFile);
Files.asByteSink(outputFile).writeFrom(his);
HashCode actualHashCode = his.hash();
HashCode expectedHashCode = payload.getContentMetadata().getContentMD5AsHashCode();
Expand All @@ -477,8 +478,10 @@ public String putBlob(final String containerName, final Blob blob) throws IOExce
return base16().lowerCase().encode(actualHashCode.asBytes());
} catch (IOException ex) {
if (outputFile != null) {
if (!outputFile.delete()) {
logger.debug("Could not delete %s", outputFile);
try {
delete(outputFile);
} catch (IOException e) {
logger.debug("Could not delete %s: %s", outputFile, e);
}
}
throw ex;
Expand All @@ -497,23 +500,26 @@ public void removeBlob(final String container, final String blobKey) {
String fileName = buildPathStartingFromBaseDir(container, blobKey);
logger.debug("Deleting blob %s", fileName);
File fileToBeDeleted = new File(fileName);
if (!fileToBeDeleted.delete()) {
if (fileToBeDeleted.isDirectory()) {
try {
UserDefinedFileAttributeView view = getUserDefinedFileAttributeView(fileToBeDeleted.toPath());
if (view != null) {
for (String s : view.list()) {
view.delete(s);
}

if (fileToBeDeleted.isDirectory()) {
try {
UserDefinedFileAttributeView view = getUserDefinedFileAttributeView(fileToBeDeleted.toPath());
if (view != null) {
for (String s : view.list()) {
view.delete(s);
}
} catch (IOException e) {
logger.debug("Could not delete attributes from %s", fileToBeDeleted);
}
} else {
logger.debug("Could not delete %s", fileToBeDeleted);
} catch (IOException e) {
logger.debug("Could not delete attributes from %s: %s", fileToBeDeleted, e);
}
}

try {
delete(fileToBeDeleted);
} catch (IOException e) {
logger.debug("Could not delete %s: %s", fileToBeDeleted, e);
}

// now examine if the key of the blob is a complex key (with a directory structure)
// and eventually remove empty directory
removeDirectoriesTreeOfBlobKey(container, blobKey);
Expand Down Expand Up @@ -737,10 +743,10 @@ private static String denormalize(String pathToDenormalize) {
}

/**
* Remove leading and trailing {@link File.separator} character from the string.
* Remove leading and trailing separator character from the string.
*
* @param pathToBeCleaned
* @param remove
* @param onlyTrailing
* only trailing separator char from path
* @return
*/
Expand All @@ -767,7 +773,7 @@ private String removeFileSeparatorFromBorders(String pathToBeCleaned, boolean on
* empty
*
* @param container
* @param normalizedKey
* @param blobKey
*/
private void removeDirectoriesTreeOfBlobKey(String container, String blobKey) {
String normalizedBlobKey = denormalize(blobKey);
Expand All @@ -786,8 +792,10 @@ private void removeDirectoriesTreeOfBlobKey(String container, String blobKey) {
File directory = new File(buildPathStartingFromBaseDir(container, parentPath));
String[] children = directory.list();
if (null == children || children.length == 0) {
if (!directory.delete()) {
logger.debug("Could not delete %s", directory);
try {
delete(directory);
} catch (IOException e) {
logger.debug("Could not delete %s: %s", directory, e);
return;
}
// recursively call for removing other path
Expand Down
54 changes: 47 additions & 7 deletions apis/filesystem/src/main/java/org/jclouds/filesystem/util/Utils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@

import java.io.File;
import java.io.IOException;
import java.nio.file.AccessDeniedException;
import java.nio.file.DirectoryNotEmptyException;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.attribute.AclEntry;
import java.nio.file.attribute.AclEntryPermission;
Expand All @@ -29,6 +32,9 @@
import java.nio.file.attribute.UserPrincipal;
import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.TimeUnit;

import com.google.common.util.concurrent.Uninterruptibles;

/**
* Utilities for the filesystem blobstore.
Expand All @@ -39,6 +45,21 @@ private Utils() {
// Do nothing
}

/**
* Determine if Java is running on a Mac OS
*/
public static boolean isMacOSX() {
String osName = System.getProperty("os.name");
return osName.contains("OS X");
}

/**
* Determine if Java is running on a windows OS
*/
public static boolean isWindows() {
return System.getProperty("os.name", "").toLowerCase().contains("windows");
}

/** Delete a file or a directory recursively. */
public static void deleteRecursively(File file) throws IOException {
if (file.isDirectory()) {
Expand All @@ -49,14 +70,33 @@ public static void deleteRecursively(File file) throws IOException {
}
}
}
Files.delete(file.toPath());

delete(file);
}

/**
* Determine if Java is running on a windows OS
*/
public static boolean isWindows() {
return System.getProperty("os.name", "").toLowerCase().contains("windows");
public static void delete(File file) throws IOException {
for (int n = 0; n < 10; n++) {
try {
Files.delete(file.toPath());
if (Files.exists(file.toPath())) {
Uninterruptibles.sleepUninterruptibly(200, TimeUnit.MILLISECONDS);
continue;
}
return;
} catch (DirectoryNotEmptyException dnee) {
// A previous file delete operation did not finish before this call
Uninterruptibles.sleepUninterruptibly(1, TimeUnit.SECONDS);
continue;
} catch (AccessDeniedException ade) {
// The file was locked by antivirus, indexing, or another operation triggered by previous file modification
Uninterruptibles.sleepUninterruptibly(1, TimeUnit.SECONDS);
continue;
} catch (NoSuchFileException nse) {
return; // The file has been eventually deleted after a previous operation that failed. no-op
}
}
// File could not be deleted multiple times. It is very likely locked in another process
throw new IOException("Could not delete: " + file.toPath());
}

/**
Expand All @@ -65,7 +105,7 @@ public static boolean isWindows() {
*/
public static boolean isPrivate(Path path) throws IOException {
UserPrincipal everyone = getDefault().getUserPrincipalLookupService()
.lookupPrincipalByName("Everyone");
.lookupPrincipalByName("Everyone");
AclFileAttributeView aclFileAttributes = java.nio.file.Files.getFileAttributeView(
path, AclFileAttributeView.class);
for (AclEntry aclEntry : aclFileAttributes.getAcl()) {
Expand Down
10 changes: 8 additions & 2 deletions apis/filesystem/src/test/java/org/jclouds/filesystem/FilesystemBlobStoreTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.jclouds.filesystem;

import static com.google.common.io.BaseEncoding.base16;
import static org.jclouds.filesystem.util.Utils.isMacOSX;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNotNull;
Expand All @@ -31,6 +32,7 @@
import java.util.Iterator;
import java.util.Properties;
import java.util.Set;
import java.util.concurrent.TimeUnit;

import org.jclouds.ContextBuilder;
import org.jclouds.blobstore.BlobRequestSigner;
Expand Down Expand Up @@ -64,6 +66,7 @@
import com.google.common.collect.Sets;
import com.google.common.io.ByteSource;
import com.google.common.io.Files;
import com.google.common.util.concurrent.Uninterruptibles;
import com.google.inject.CreationException;

@Test(groups = "unit", testName = "FilesystemBlobStoreTest", singleThreaded = true)
Expand Down Expand Up @@ -417,7 +420,9 @@ public void testRemoveBlob_TwoComplexBlobKeys() throws IOException {
assertTrue(result, "Blob " + BLOB_KEY2 + " doesn't exist");

// remove first blob
Uninterruptibles.sleepUninterruptibly(5, TimeUnit.SECONDS);
blobStore.removeBlob(CONTAINER_NAME, BLOB_KEY1);
Uninterruptibles.sleepUninterruptibly(5, TimeUnit.SECONDS);
result = blobStore.blobExists(CONTAINER_NAME, BLOB_KEY1);
assertFalse(result, "Blob still exists");
// first file deleted, not the second
Expand Down Expand Up @@ -491,6 +496,7 @@ public void testPutDirectoryBlobs() {
assertTrue(blobStore.blobExists(CONTAINER_NAME, childKey));

blobStore.removeBlob(CONTAINER_NAME, parentKey);
Uninterruptibles.sleepUninterruptibly(1, TimeUnit.SECONDS);
assertFalse(blobStore.blobExists(CONTAINER_NAME, parentKey));
assertTrue(blobStore.blobExists(CONTAINER_NAME, childKey));
}
Expand Down Expand Up @@ -827,7 +833,7 @@ public void testBlobRequestSigner() throws Exception {
* Creates a {@link Blob} object filled with data from a file
*
* @param keyName
* @param fileContent
* @param filePayload
* @return
*/
private Blob createBlob(String keyName, File filePayload) {
Expand Down Expand Up @@ -906,7 +912,7 @@ private void putBlobAndCheckIt(String blobKey) {

@DataProvider
public Object[][] ignoreOnMacOSX() {
return org.jclouds.utils.TestUtils.isMacOSX() ? TestUtils.NO_INVOCATIONS
return isMacOSX() ? TestUtils.NO_INVOCATIONS
: TestUtils.SINGLE_NO_ARG_INVOCATION;
}
}
10 changes: 6 additions & 4 deletions ...ystem/src/test/java/org/jclouds/filesystem/integration/FilesystemBlobIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
*/
package org.jclouds.filesystem.integration;

import static org.jclouds.filesystem.util.Utils.isMacOSX;

import java.io.IOException;
import java.util.Map;
import java.util.Properties;
Expand Down Expand Up @@ -46,7 +48,7 @@ protected Properties setupProperties() {
// https://bugs.openjdk.java.net/browse/JDK-8030048
@Override
public void checkContentMetadata(Blob blob) {
if (!org.jclouds.utils.TestUtils.isMacOSX()) {
if (!isMacOSX()) {
super.checkContentMetadata(blob);
}
}
Expand All @@ -55,7 +57,7 @@ public void checkContentMetadata(Blob blob) {
// https://bugs.openjdk.java.net/browse/JDK-8030048
@Override
protected void checkContentDisposition(Blob blob, String contentDisposition) {
if (!org.jclouds.utils.TestUtils.isMacOSX()) {
if (!isMacOSX()) {
super.checkContentDisposition(blob, contentDisposition);
}
}
Expand All @@ -64,7 +66,7 @@ protected void checkContentDisposition(Blob blob, String contentDisposition) {
// https://bugs.openjdk.java.net/browse/JDK-8030048
@Override
protected void validateMetadata(BlobMetadata metadata) throws IOException {
if (!org.jclouds.utils.TestUtils.isMacOSX()) {
if (!isMacOSX()) {
super.validateMetadata(metadata);
}
}
Expand All @@ -81,7 +83,7 @@ public void testCreateBlobWithExpiry() throws InterruptedException {
* uses to implement user metadata */
@Override
protected void checkUserMetadata(Map<String, String> userMetadata1, Map<String, String> userMetadata2) {
if (!org.jclouds.utils.TestUtils.isMacOSX()) {
if (!isMacOSX()) {
super.checkUserMetadata(userMetadata1, userMetadata2);
}
}
Expand Down
3 changes: 2 additions & 1 deletion .../src/test/java/org/jclouds/filesystem/integration/FilesystemContainerIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.jclouds.filesystem.integration;

import static org.jclouds.blobstore.options.ListContainerOptions.Builder.maxResults;
import static org.jclouds.filesystem.util.Utils.isMacOSX;
import static org.testng.Assert.assertEquals;

import java.io.IOException;
Expand Down Expand Up @@ -164,7 +165,7 @@ public void testDirectory() {

@DataProvider
public Object[][] ignoreOnMacOSX() {
return org.jclouds.utils.TestUtils.isMacOSX() ? TestUtils.NO_INVOCATIONS
return isMacOSX() ? TestUtils.NO_INVOCATIONS
: TestUtils.SINGLE_NO_ARG_INVOCATION;
}

Expand Down
Loading

0 comments on commit 41ce90e

Please sign in to comment.