Re: [jclouds] Add support for Azure Copy Blob (#514)

[Andrew Gaul]

@demobox Thanks for the feedback and sorry for my delayed responses; please see 
the latest commit.

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Andrew Gaul]

 @@ -254,4 +257,8 @@
  */
 boolean blobExists(String container, String name);
  
 +   /**
 +* @throws ContainerNotFoundException if the container is not present.
 +*/
 +   void copyBlob(URI copySource, String toContainer, String toName, 
 OptionalCopyBlobOptions options);

Changed to use a single non-`Optional` parameter and added 
`CopyBlobOptions.NONE`.

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Andrew Gaul]

 +
 +import java.util.Map;
 +
 +import com.google.common.base.Optional;
 +import com.google.common.collect.ImmutableMap;
 +
 +/**
 + * @see a href=http://msdn.microsoft.com/en-us/library/dd894037.aspx; /
 + */
 +// TODO:
 +// x-ms-source-if-modified-since
 +// x-ms-source-if-unmodified-since
 +// x-ms-source-if-match
 +// x-ms-source-if-none-match
 +public final class CopyBlobOptions {
 +   private final OptionalMapString, String userMetadata;

This is an interesting corner case -- Azure does not seem to allow replacing 
user metadata with an empty map?

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Andrew Gaul]

 +package org.jclouds.azureblob.binders;
 +
 +import java.util.Map;
 +
 +import org.jclouds.azure.storage.reference.AzureStorageHeaders;
 +import org.jclouds.azureblob.options.CopyBlobOptions;
 +import org.jclouds.http.HttpRequest;
 +import org.jclouds.rest.Binder;
 +
 +import com.google.common.base.Optional;
 +
 +/** Binds options to a copyBlob request. */
 +public class BindAzureCopyOptionsToRequest implements Binder {
 +   @Override
 +   public R extends HttpRequest R bindToRequest(R request, Object input) {
 +  OptionalCopyBlobOptions optional = (OptionalCopyBlobOptions) input;

Changed.

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Andrew Gaul]

 +public class BindAzureCopyOptionsToRequest implements Binder {
 +   @Override
 +   public R extends HttpRequest R bindToRequest(R request, Object input) {
 +  OptionalCopyBlobOptions optional = (OptionalCopyBlobOptions) input;
 +  if (!optional.isPresent()) {
 + return request;
 +  }
 +
 +  HttpRequest.Builder builder = request.toBuilder();
 +  CopyBlobOptions options = optional.get();
 +  OptionalMapString, String userMetadata = options.getUserMetadata();
 +  if (userMetadata.isPresent()) {
 + for (Map.EntryString, String entry : 
 userMetadata.get().entrySet()) {
 +builder.addHeader(AzureStorageHeaders.USER_METADATA_PREFIX + 
 entry.getKey(), entry.getValue());
 + }
 +  }

Correct.

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Andrew Gaul]

 + * 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.azureblob.options;
 +
 +import java.util.Map;
 +
 +import com.google.common.base.Optional;
 +import com.google.common.collect.ImmutableMap;
 +
 +/**
 + * @see a href=http://msdn.microsoft.com/en-us/library/dd894037.aspx; /

Done.

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Andrew Gaul]

 @@ -375,4 +380,23 @@ public void testBlockOperations() throws Exception {
assertEquals(1, blocks.getBlocks().get(2).getContentLength());
getApi().deleteContainer(blockContainer);
 }
 +
 +   @Test(timeOut = 5 * 60 * 1000, dependsOnMethods = { 
 testCreateContainer, testCreatePublicContainer })
 +   public void testCopyBlob() throws Exception {
 +  ByteSource byteSource = TestUtils.randomByteSource().slice(0, 1024);
 +
 +  // create blob
 +  AzureBlob object = getApi().newBlob();
 +  object.getProperties().setName(from);
 +  object.setPayload(byteSource.read());
 +  getApi().putBlob(privateContainer, object);
 +
 +  // copy blob
 +  URI copySource = view.getSigner().signGetBlob(privateContainer, 
 from).getEndpoint();
 +  getApi().copyBlob(copySource, privateContainer, to, 
 Optional.CopyBlobOptionsabsent());

Normal test teardown handles this.

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Andrew Gaul]

 + * 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.azureblob.options;
 +
 +import java.util.Map;
 +
 +import com.google.common.base.Optional;
 +import com.google.common.collect.ImmutableMap;
 +
 +/**
 + * @see a href=http://msdn.microsoft.com/en-us/library/dd894037.aspx; /
 + */
 +// TODO:

Removed/implemented.

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Everett Toews]

Thanks for the pull request but it's release week in jclouds and that means 
it's time to clean up the PR queue. This PR will be over 6 months old as of 
April 1. If you intend to continue work on it, please make a comment by April 
2. Otherwise it will be closed on April 3.

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Adrian Cole]

sorry that unasyncing probably screwed up this branch. let me know if you want 
a hand rebasing.

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Andrew Phillips]

 +package org.jclouds.azureblob.binders;
 +
 +import java.util.Map;
 +
 +import org.jclouds.azure.storage.reference.AzureStorageHeaders;
 +import org.jclouds.azureblob.options.CopyBlobOptions;
 +import org.jclouds.http.HttpRequest;
 +import org.jclouds.rest.Binder;
 +
 +import com.google.common.base.Optional;
 +
 +/** Binds options to a copyBlob request. */
 +public class BindAzureCopyOptionsToRequest implements Binder {
 +   @Override
 +   public R extends HttpRequest R bindToRequest(R request, Object input) {
 +  OptionalCopyBlobOptions optional = (OptionalCopyBlobOptions) input;

We don't usually have something like a `checkArg` or `checkState` to verify the 
type here, I guess?

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Andrew Phillips]

 +package org.jclouds.azureblob.binders;
 +
 +import java.util.Map;
 +
 +import org.jclouds.azure.storage.reference.AzureStorageHeaders;
 +import org.jclouds.azureblob.options.CopyBlobOptions;
 +import org.jclouds.http.HttpRequest;
 +import org.jclouds.rest.Binder;
 +
 +import com.google.common.base.Optional;
 +
 +/** Binds options to a copyBlob request. */
 +public class BindAzureCopyOptionsToRequest implements Binder {
 +   @Override
 +   public R extends HttpRequest R bindToRequest(R request, Object input) {
 +  OptionalCopyBlobOptions optional = (OptionalCopyBlobOptions) input;

[minor] `optionalOptions` or so?

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Andrew Phillips]

 +public class BindAzureCopyOptionsToRequest implements Binder {
 +   @Override
 +   public R extends HttpRequest R bindToRequest(R request, Object input) {
 +  OptionalCopyBlobOptions optional = (OptionalCopyBlobOptions) input;
 +  if (!optional.isPresent()) {
 + return request;
 +  }
 +
 +  HttpRequest.Builder builder = request.toBuilder();
 +  CopyBlobOptions options = optional.get();
 +  OptionalMapString, String userMetadata = options.getUserMetadata();
 +  if (userMetadata.isPresent()) {
 + for (Map.EntryString, String entry : 
 userMetadata.get().entrySet()) {
 +builder.addHeader(AzureStorageHeaders.USER_METADATA_PREFIX + 
 entry.getKey(), entry.getValue());
 + }
 +  }

Just to confirm: what we're doing here is copying the user metadata from 
`options` into the request as headers..?

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Andrew Phillips]

 + * 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.azureblob.options;
 +
 +import java.util.Map;
 +
 +import com.google.common.base.Optional;
 +import com.google.common.collect.ImmutableMap;
 +
 +/**
 + * @see a href=http://msdn.microsoft.com/en-us/library/dd894037.aspx; /

Remove external URLs?

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Andrew Phillips]

 +
 +import java.util.Map;
 +
 +import com.google.common.base.Optional;
 +import com.google.common.collect.ImmutableMap;
 +
 +/**
 + * @see a href=http://msdn.microsoft.com/en-us/library/dd894037.aspx; /
 + */
 +// TODO:
 +// x-ms-source-if-modified-since
 +// x-ms-source-if-unmodified-since
 +// x-ms-source-if-match
 +// x-ms-source-if-none-match
 +public final class CopyBlobOptions {
 +   private final OptionalMapString, String userMetadata;

Instead of an optional, how about simply an empty map by default? That would 
have the same effect - no headers getting added to the request - unless I'm 
missing something? And would avoid a few checks?

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Andrew Phillips]

 @@ -375,4 +380,23 @@ public void testBlockOperations() throws Exception {
assertEquals(1, blocks.getBlocks().get(2).getContentLength());
getApi().deleteContainer(blockContainer);
 }
 +
 +   @Test(timeOut = 5 * 60 * 1000, dependsOnMethods = { 
 testCreateContainer, testCreatePublicContainer })
 +   public void testCopyBlob() throws Exception {
 +  ByteSource byteSource = TestUtils.randomByteSource().slice(0, 1024);
 +
 +  // create blob
 +  AzureBlob object = getApi().newBlob();
 +  object.getProperties().setName(from);
 +  object.setPayload(byteSource.read());
 +  getApi().putBlob(privateContainer, object);
 +
 +  // copy blob
 +  URI copySource = view.getSigner().signGetBlob(privateContainer, 
 from).getEndpoint();
 +  getApi().copyBlob(copySource, privateContainer, to, 
 Optional.CopyBlobOptionsabsent());

Do we need to clean this up afterwards? Or is that already done somewhere?

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Jeremy Daggett]

 @@ -325,4 +330,16 @@
 ListenableFutureListBlobBlocksResponse 
 getBlockList(@PathParam(container) 
 @ParamValidators(ContainerNameValidator.class) String container,
   @PathParam(name) 
 String name);
  
 +   /**
 +* @see AzureBlobClient#copyBlob
 +*/
 +   @Named(CopyBlob)
 +   @PUT
 +   @Path({toContainer}/{toName})
 +   @OverrideRequestFilters
 +   @RequestFilters(value = { AddCopySourceHeader.class, 
 SharedKeyLiteAuthentication.class })

@andrewgaul That syntax is a bit strange, and I know what your intent is here. 
Rather than using a `@RequestFilter`, what about just adding this simple header 
annotation? `@Headers(keys = COPY_SOURCE, values = 
/{fromContainer}/{fromName})``


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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Andrew Gaul]

 @@ -325,4 +330,16 @@
 ListenableFutureListBlobBlocksResponse 
 getBlockList(@PathParam(container) 
 @ParamValidators(ContainerNameValidator.class) String container,
   @PathParam(name) 
 String name);
  
 +   /**
 +* @see AzureBlobClient#copyBlob
 +*/
 +   @Named(CopyBlob)
 +   @PUT
 +   @Path({toContainer}/{toName})
 +   @OverrideRequestFilters
 +   @RequestFilters(value = { AddCopySourceHeader.class, 
 SharedKeyLiteAuthentication.class })

Unfortunately `RestAnnotationProcessor.buildHeaders` does not replace headers 
with headerParams.  The following:

```java
   @Named(CopyBlob)
   @PUT
   @Path({toContainer}/{toName})
   @Headers(keys = AzureStorageHeaders.COPY_SOURCE, values = 
/{fromContainer}/{fromName})
   ListenableFutureVoid copyBlob(
 @HeaderParam(fromContainer) 
@ParamValidators(ContainerNameValidator.class) String fromContainer, 
@HeaderParam(fromName) String fromName,
 @PathParam(toContainer) 
@ParamValidators(ContainerNameValidator.class) String toContainer, 
@PathParam(toName) String toName,
 OptionalCopyBlobOptions options);
```

results in:

```
fromContainer: fromcontainer
fromName: fromblob
x-ms-copy-source: /{fromContainer}/{fromName}
x-ms-version: 2012-02-12
```

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Jeremy Daggett]

 @@ -325,4 +330,16 @@
 ListenableFutureListBlobBlocksResponse 
 getBlockList(@PathParam(container) 
 @ParamValidators(ContainerNameValidator.class) String container,
   @PathParam(name) 
 String name);
  
 +   /**
 +* @see AzureBlobClient#copyBlob
 +*/
 +   @Named(CopyBlob)
 +   @PUT
 +   @Path({toContainer}/{toName})
 +   @OverrideRequestFilters
 +   @RequestFilters(value = { AddCopySourceHeader.class, 
 SharedKeyLiteAuthentication.class })

What if you instead use `@PathParam` instead of `@HeaderParam` on the `from*` 
args?
```java
@Named(CopyBlob)
@PUT
@Path({toContainer}/{toName})
@Headers(keys = AzureStorageHeaders.COPY_SOURCE, values = 
/{fromContainer}/{fromName})
ListenableFutureVoid copyBlob(
  @PathParam(fromContainer) 
@ParamValidators(ContainerNameValidator.class) String fromContainer,
  @PathParam(fromName) String fromName,
  @PathParam(toContainer) @ParamValidators(ContainerNameValidator.class) 
String toContainer,
  @PathParam(toName) String toName,
  OptionalCopyBlobOptions options);
```

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Ignasi Barrera]

 @@ -325,4 +330,16 @@
 ListenableFutureListBlobBlocksResponse 
 getBlockList(@PathParam(container) 
 @ParamValidators(ContainerNameValidator.class) String container,
   @PathParam(name) 
 String name);
  
 +   /**
 +* @see AzureBlobClient#copyBlob
 +*/
 +   @Named(CopyBlob)
 +   @PUT
 +   @Path({toContainer}/{toName})
 +   @OverrideRequestFilters
 +   @RequestFilters(value = { AddCopySourceHeader.class, 
 SharedKeyLiteAuthentication.class })

@gaul mind opening a JIRA to improve that and assign it to me?

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Andrew Gaul]

Going with a URI-based copy source.  This allows copies between different 
accounts via signed URLs.

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[CloudBees pull request builder plugin]

[jclouds-pull-requests-java-6 
#69](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-6/69/) 
SUCCESS
This pull request looks good

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[CloudBees pull request builder plugin]

[jclouds-pull-requests 
#1158](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1158/) SUCCESS
This pull request looks good

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[CloudBees pull request builder plugin]

[jclouds-pull-requests 
#1155](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1155/) FAILURE
Looks like there's a problem with this pull request

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[BuildHive]

[jclouds » jclouds 
#1609](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1609/) FAILURE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)

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

[jclouds] Add support for Azure Copy Blob (#514)

[Andrew Gaul]

Presently lacks support for CopyBlobOptions.  API reference:

http://msdn.microsoft.com/en-us/library/dd894037.aspx
You can merge this Pull Request by running:

  git pull https://github.com/andrewgaul/jclouds azure-copy-blob

Or you can view, comment on it, or merge it online at:

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

-- Commit Summary --

  * Add support for Azure Copy Blob

-- File Changes --

M 
common/azure/src/main/java/org/jclouds/azure/storage/reference/AzureStorageHeaders.java
 (2)
M 
providers/azureblob/src/main/java/org/jclouds/azureblob/AzureBlobAsyncClient.java
 (18)
M 
providers/azureblob/src/main/java/org/jclouds/azureblob/AzureBlobClient.java (6)
A 
providers/azureblob/src/main/java/org/jclouds/azureblob/filters/AddCopySourceHeader.java
 (45)
M 
providers/azureblob/src/test/java/org/jclouds/azureblob/AzureBlobAsyncClientTest.java
 (20)
M 
providers/azureblob/src/test/java/org/jclouds/azureblob/AzureBlobClientLiveTest.java
 (23)

-- Patch Links --

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

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[CloudBees pull request builder plugin]

[jclouds-pull-requests-java-6 
#66](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-6/66/) 
SUCCESS
This pull request looks good

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Andrew Gaul]

 @@ -325,4 +330,16 @@
 ListenableFutureListBlobBlocksResponse 
 getBlockList(@PathParam(container) 
 @ParamValidators(ContainerNameValidator.class) String container,
   @PathParam(name) 
 String name);
  
 +   /**
 +* @see AzureBlobClient#copyBlob
 +*/
 +   @Named(CopyBlob)
 +   @PUT
 +   @Path({toContainer}/{toName})
 +   @OverrideRequestFilters
 +   @RequestFilters(value = { AddCopySourceHeader.class, 
 SharedKeyLiteAuthentication.class })

Unsure about `AddCopySourceHeader`; this feels like a clumsy way to implement a 
compound header.  I modeled the interface after the S3 equivalent but perhaps 
this is the wrong approach. If I change `fromContainer` and `fromName` to 
`fromUrl` I can avoid this entirely and expose the full power of the Azure 
interface.  Thoughts?

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[Andrew Gaul]

I am still digesting the variants of `CopyBlobOptions`; it seems like Azure and 
S3 implement the same ETag variants and copy/replace user metadata operations.

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[CloudBees pull request builder plugin]

[jclouds-pull-requests-java-6 
#67](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-6/67/) 
SUCCESS
This pull request looks good

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

Re: [jclouds] Add support for Azure Copy Blob (#514)

[BuildHive]

[jclouds » jclouds 
#1608](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1608/) FAILURE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)

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