You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Ka-Hing Cheung <no...@github.com> on 2015/01/09 02:12:01 UTC
[jclouds] support directory blobs (#637)
make the filesystem blob store distinguish between a/ and a. a/
must be a directory blob with no content and only metadata
on listing, only directories with md5 attribute is considered a
blob and returned
You can merge this Pull Request by running:
git pull https://github.com/kahing/jclouds filesystem-directory-blobs
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/637
-- Commit Summary --
* support directory blobs
-- File Changes --
M apis/filesystem/src/main/java/org/jclouds/filesystem/predicates/validators/internal/FilesystemBlobKeyValidatorImpl.java (5)
M apis/filesystem/src/main/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImpl.java (135)
M apis/filesystem/src/test/java/org/jclouds/filesystem/FilesystemBlobStoreTest.java (54)
M apis/filesystem/src/test/java/org/jclouds/filesystem/predicates/validators/internal/FilesystemBlobKeyValidatorTest.java (5)
M apis/filesystem/src/test/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImplTest.java (58)
M blobstore/src/main/java/org/jclouds/blobstore/config/LocalBlobStore.java (19)
M blobstore/src/main/java/org/jclouds/blobstore/reference/BlobStoreConstants.java (1)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/637.patch
https://github.com/jclouds/jclouds/pull/637.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/637
Re: [jclouds] support directory blobs (#637)
Posted by Andrew Gaul <no...@github.com>.
Closed #637.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/637#event-216871895
Re: [jclouds] support directory blobs (#637)
Posted by Ka-Hing Cheung <no...@github.com>.
I know what the problem is and working on a fix.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/637#issuecomment-69625983
Re: [jclouds] support directory blobs (#637)
Posted by Andrew Gaul <no...@github.com>.
> @@ -328,7 +376,20 @@ public void removeBlob(final String container, final String blobKey) {
> logger.debug("Deleting blob %s", fileName);
> File fileToBeDeleted = new File(fileName);
> if (!fileToBeDeleted.delete()) {
> - logger.debug("Could not delete %s", fileToBeDeleted);
> + if (fileToBeDeleted.isDirectory()) {
> + try {
> + UserDefinedFileAttributeView view = getUserDefinedFileAttributeView(fileToBeDeleted.toPath());
> + if (view != null) {
> + for (String s : view.list()) {
> + view.delete(s);
Actually I might misunderstand, we have to delete all the attributes before deleting a directory?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/637/files#r22703720
Re: [jclouds] support directory blobs (#637)
Posted by Andrew Gaul <no...@github.com>.
> @@ -328,7 +376,20 @@ public void removeBlob(final String container, final String blobKey) {
> logger.debug("Deleting blob %s", fileName);
> File fileToBeDeleted = new File(fileName);
> if (!fileToBeDeleted.delete()) {
> - logger.debug("Could not delete %s", fileToBeDeleted);
> + if (fileToBeDeleted.isDirectory()) {
> + try {
> + UserDefinedFileAttributeView view = getUserDefinedFileAttributeView(fileToBeDeleted.toPath());
> + if (view != null) {
> + for (String s : view.list()) {
> + view.delete(s);
Does this handle recursive deletes correctly?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/637/files#r22703686
Re: [jclouds] support directory blobs (#637)
Posted by Andrew Gaul <no...@github.com>.
Spurious test failures. Pushed to master as b7ab8b18a090f7ff03c7a25382f8558b0e6d9751.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/637#issuecomment-69430342
Re: [jclouds] support directory blobs (#637)
Posted by Ka-Hing Cheung <no...@github.com>.
> @@ -328,7 +376,20 @@ public void removeBlob(final String container, final String blobKey) {
> logger.debug("Deleting blob %s", fileName);
> File fileToBeDeleted = new File(fileName);
> if (!fileToBeDeleted.delete()) {
> - logger.debug("Could not delete %s", fileToBeDeleted);
> + if (fileToBeDeleted.isDirectory()) {
the proposed change modifies the intended semantic
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/637/files#r22750668
Re: [jclouds] support directory blobs (#637)
Posted by Andrew Gaul <no...@github.com>.
> @@ -328,7 +376,20 @@ public void removeBlob(final String container, final String blobKey) {
> logger.debug("Deleting blob %s", fileName);
> File fileToBeDeleted = new File(fileName);
> if (!fileToBeDeleted.delete()) {
> - logger.debug("Could not delete %s", fileToBeDeleted);
> + if (fileToBeDeleted.isDirectory()) {
Should this be inverted, e.g.,
```java
if (file.isDirectory()) {
// delete attributes
}
if (!file.delete()) {
// log failure
}
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/637/files#r22703742
Re: [jclouds] support directory blobs (#637)
Posted by Jeremy Daggett <no...@github.com>.
@gaul @kahing Hi! I wanted to give you a head's up that on Mac OS X, I am consistently seeing these failures on master:
```
Failed tests:
FilesystemBlobStoreTest.testGetDirectoryBlob:488 expected [true] but found [false]
FilesystemBlobStoreTest.testListDirectoryBlobs:505 expected [true] but found [false]
FilesystemBlobStoreTest.testPutDirectoryBlobs:471 expected [true] but found [false]
FilesystemStorageStrategyImplTest.testDeleteIntermediateDirectoryBlob:399 expected [true] but found [false]
FilesystemStorageStrategyImplTest.testGetDirectoryBlob:360 expected [true] but found [false]
FilesystemStorageStrategyImplTest.testWriteDirectoryBlob:352 expected [true] but found [false]
```
Looking into a fix...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/637#issuecomment-69624045
Re: [jclouds] support directory blobs (#637)
Posted by Ka-Hing Cheung <no...@github.com>.
> @@ -328,7 +376,20 @@ public void removeBlob(final String container, final String blobKey) {
> logger.debug("Deleting blob %s", fileName);
> File fileToBeDeleted = new File(fileName);
> if (!fileToBeDeleted.delete()) {
> - logger.debug("Could not delete %s", fileToBeDeleted);
> + if (fileToBeDeleted.isDirectory()) {
> + try {
> + UserDefinedFileAttributeView view = getUserDefinedFileAttributeView(fileToBeDeleted.toPath());
> + if (view != null) {
> + for (String s : view.list()) {
> + view.delete(s);
No, delete() would fail if it's not empty, in that case we need to delete all the attributes, which will cause us to no longer treat it as a blob
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/637/files#r22750631
Re: [jclouds] support directory blobs (#637)
Posted by Andrew Gaul <no...@github.com>.
Thanks for addressing this @kahing, the jclouds directory support generally is a little wonky due to differences between filesystem-based providers like Atmos and fake path-based providers like S3.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/637#issuecomment-69300655