You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Andrew Gaul <no...@github.com> on 2015/04/07 07:58:32 UTC

[jclouds] Allow overriding system metadata during copyBlob (#722)

Add Azure, S3, and Swift variants.
You can view, comment on, or merge this pull request online at:

  https://github.com/jclouds/jclouds/pull/722

-- Commit Summary --

  * JCLOUDS-651: portable copy object system metadata
  * JCLOUDS-651: Azure copy object system metadata
  * JCLOUDS-651: S3 copy object system metadata
  * JCLOUDS-651: Swift copy object system metadata

-- File Changes --

    M apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/blobstore/RegionScopedSwiftBlobStore.java (72)
    M apis/s3/src/main/java/org/jclouds/s3/blobstore/S3BlobStore.java (27)
    M apis/s3/src/main/java/org/jclouds/s3/options/CopyObjectOptions.java (61)
    M blobstore/src/main/java/org/jclouds/blobstore/options/CopyOptions.java (16)
    M blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseBlobIntegrationTest.java (23)
    M providers/azureblob/src/main/java/org/jclouds/azureblob/AzureBlobClient.java (11)
    A providers/azureblob/src/main/java/org/jclouds/azureblob/binders/BindAzureContentMetadataToRequest.java (59)
    M providers/azureblob/src/main/java/org/jclouds/azureblob/blobstore/AzureBlobStore.java (33)

-- Patch Links --

https://github.com/jclouds/jclouds/pull/722.patch
https://github.com/jclouds/jclouds/pull/722.diff

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/722

Re: [jclouds] Allow overriding system metadata during copyBlob (#722)

Posted by Andrew Gaul <no...@github.com>.
> @@ -796,9 +808,8 @@ public void testCopyBlobReplaceMetadata() throws Exception {
>           } finally {
>              Closeables2.closeQuietly(is);
>           }
> -         // TODO: S3 overrideMetadataWith also overrides system metadata
> -         // TODO: Swift does not preserve system metadata
> -         //checkContentMetadata(toBlob);
> +         // TODO: Swift copy object *appends* user metadata, does not overwrite

@zack-shoylev Curious Swift *appends* user metadata so this test fails with:

```
Failed tests:
  CloudFilesUSBlobIntegrationLiveTest>BaseBlobIntegrationTest.testCopyBlobReplaceMetadata:813
Expecting:
 <{"key2"="value2", "key3"="value3", "key1"="value1", "key4"="value4"}>
to be equal to:
 <{"key3"="value3", "key4"="value4"}>
but was not.
```

Swift documents this behavior so this is expected.  I guess we just need to change the test expectations?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/722/files#r27853088

Re: [jclouds] Allow overriding system metadata during copyBlob (#722)

Posted by Zack Shoylev <no...@github.com>.
> @@ -796,9 +808,8 @@ public void testCopyBlobReplaceMetadata() throws Exception {
>           } finally {
>              Closeables2.closeQuietly(is);
>           }
> -         // TODO: S3 overrideMetadataWith also overrides system metadata
> -         // TODO: Swift does not preserve system metadata
> -         //checkContentMetadata(toBlob);
> +         // TODO: Swift copy object *appends* user metadata, does not overwrite

I am not completely sure. Shouldn't we be trying to have the same outcome? The question is if metadata should be completely overridden or appended when using the abstraction, right?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/722/files#r27918060

Re: [jclouds] Allow overriding system metadata during copyBlob (#722)

Posted by Zack Shoylev <no...@github.com>.
> @@ -796,9 +808,8 @@ public void testCopyBlobReplaceMetadata() throws Exception {
>           } finally {
>              Closeables2.closeQuietly(is);
>           }
> -         // TODO: S3 overrideMetadataWith also overrides system metadata
> -         // TODO: Swift does not preserve system metadata
> -         //checkContentMetadata(toBlob);
> +         // TODO: Swift copy object *appends* user metadata, does not overwrite

You could query the source metadata before copy and apply that during the copy operation that overrides it. Otherwise you will have to do two operation that modify an object, instead of a read and a copy. Thoughts?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/722/files#r27923587

Re: [jclouds] Allow overriding content metadata during copyBlob (#722)

Posted by Andrew Gaul <no...@github.com>.
This works pretty well across the major providers -- should we merge as-is and open a JIRA issue for the Swift quirk?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/722#issuecomment-91083960

Re: [jclouds] Allow overriding content metadata during copyBlob (#722)

Posted by Andrew Gaul <no...@github.com>.
Backported all the copyBlob commits to 1.9.x.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/722#issuecomment-91344255

Re: [jclouds] Allow overriding content metadata during copyBlob (#722)

Posted by Andrew Gaul <no...@github.com>.
OK I'll open a JIRA issue and skip the test on Swift before merging.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/722#issuecomment-91118162

Re: [jclouds] Allow overriding content metadata during copyBlob (#722)

Posted by Zack Shoylev <no...@github.com>.
backport?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/722#issuecomment-91339375

Re: [jclouds] Allow overriding system metadata during copyBlob (#722)

Posted by Andrew Gaul <no...@github.com>.
> @@ -796,9 +808,8 @@ public void testCopyBlobReplaceMetadata() throws Exception {
>           } finally {
>              Closeables2.closeQuietly(is);
>           }
> -         // TODO: S3 overrideMetadataWith also overrides system metadata
> -         // TODO: Swift does not preserve system metadata
> -         //checkContentMetadata(toBlob);
> +         // TODO: Swift copy object *appends* user metadata, does not overwrite

Alternatively we could try to fix up the metadata after the copy.  Agreed that the portable abstraction should act the same between providers.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/722/files#r27918213

Re: [jclouds] Allow overriding content metadata during copyBlob (#722)

Posted by Zack Shoylev <no...@github.com>.
@andrewgaul I'm ok with that. Figuring out the swift quirk might need some more work.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/722#issuecomment-91101772

Re: [jclouds] Allow overriding system metadata during copyBlob (#722)

Posted by Andrew Gaul <no...@github.com>.
> @@ -796,9 +808,8 @@ public void testCopyBlobReplaceMetadata() throws Exception {
>           } finally {
>              Closeables2.closeQuietly(is);
>           }
> -         // TODO: S3 overrideMetadataWith also overrides system metadata
> -         // TODO: Swift does not preserve system metadata
> -         //checkContentMetadata(toBlob);
> +         // TODO: Swift copy object *appends* user metadata, does not overwrite

I don't understand how to do this fixup with the current API -- `ObjectApi.updateMetadata` writes user metadata but overwrites content metadata and `ObjectApi.updateRawMetadata` has some strange semantics.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/722/files#r28026132

Re: [jclouds] Allow overriding content metadata during copyBlob (#722)

Posted by Andrew Gaul <no...@github.com>.
Opened [JCLOUDS-883](https://issues.apache.org/jira/browse/JCLOUDS-883).

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/722#issuecomment-91123083