Provide a way to unset values in blob metadata

mziccard · mziccard · commit 87bc1a112eac · 2015-10-23T13:46:11.000+02:00
- Use HashMap instead of ImmutableMap in builder.metadata
- Use Collections.unmodifiableMap in blobInfo.metadata getter
- Convert null values in metadata to Data.nullOf in blobInfo.toPb
- Add integration tests
- Add javadoc to mark BlobInfo.ImmutableEmptyMap as internal
diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/BlobInfo.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/BlobInfo.java
@@ -27,13 +27,14 @@
 import com.google.common.base.Function;
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 
 import java.io.Serializable;
 import java.math.BigInteger;
 import java.util.AbstractMap;
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -84,6 +85,9 @@ public StorageObject apply(BlobInfo blobInfo) {
   private final String contentLanguage;
   private final Integer componentCount;
 
+  /**
+   * This class is meant for internal use only. Users are discouraged from using this class.
+   */
   public static final class ImmutableEmptyMap<K, V> extends AbstractMap<K, V> {
 
     @Override
@@ -200,7 +204,7 @@ Builder mediaLink(String mediaLink) {
 
     public Builder metadata(Map<String, String> metadata) {
       this.metadata = metadata != null ?
-          ImmutableMap.copyOf(metadata) : Data.<Map>nullOf(ImmutableEmptyMap.class);
+          new HashMap(metadata) : Data.<Map>nullOf(ImmutableEmptyMap.class);
       return this;
     }
 
@@ -327,7 +331,7 @@ public String mediaLink() {
   }
 
   public Map<String, String> metadata() {
-    return Data.isNull(metadata) ? null : metadata;
+    return metadata == null || Data.isNull(metadata) ? null : Collections.unmodifiableMap(metadata);
   }
 
   public Long generation() {
@@ -414,14 +418,21 @@ public ObjectAccessControl apply(Acl acl) {
     if (owner != null) {
       storageObject.setOwner(new Owner().setEntity(owner.toPb()));
     }
+    Map<String, String> pbMetadata = metadata;
+    if (metadata != null && !Data.isNull(metadata)) {
+      pbMetadata = new HashMap<>();
+      for (String key : metadata.keySet()) {
+        pbMetadata.put(key, firstNonNull(metadata.get(key), Data.<String>nullOf(String.class)));
+      }
+    }
+    storageObject.setMetadata(pbMetadata);
     storageObject.setCacheControl(cacheControl);
     storageObject.setContentEncoding(contentEncoding);
     storageObject.setCrc32c(crc32c);
     storageObject.setContentType(contentType);
     storageObject.setGeneration(generation);
     storageObject.setMd5Hash(md5);
     storageObject.setMediaLink(mediaLink);
-    storageObject.setMetadata(metadata);
     storageObject.setMetageneration(metageneration);
     storageObject.setContentDisposition(contentDisposition);
     storageObject.setComponentCount(componentCount);
diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/ITStorageTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/ITStorageTest.java
@@ -37,8 +37,10 @@
 import java.net.URLConnection;
 import java.nio.ByteBuffer;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
@@ -174,17 +176,57 @@ public void testUpdateBlob() {
   @Test
   public void testUpdateBlobReplaceMetadata() {
     String blobName = "test-update-blob-replace-metadata";
+    ImmutableMap<String, String> metadata = ImmutableMap.of("k1", "a");
+    ImmutableMap<String, String> newMetadata = ImmutableMap.of("k2", "b");
     BlobInfo blob = BlobInfo.builder(bucket, blobName)
         .contentType(CONTENT_TYPE)
-        .metadata(ImmutableMap.of("k1", "a"))
+        .metadata(metadata)
         .build();
     assertNotNull(storage.create(blob));
     BlobInfo updatedBlob = storage.update(blob.toBuilder().metadata(null).build());
     assertNotNull(updatedBlob);
     assertNull(updatedBlob.metadata());
-    updatedBlob = storage.update(blob.toBuilder().metadata(ImmutableMap.of("k2", "b")).build());
+    updatedBlob = storage.update(blob.toBuilder().metadata(newMetadata).build());
     assertEquals(blob.blobId(), updatedBlob.blobId());
-    assertEquals(ImmutableMap.of("k2", "b"), updatedBlob.metadata());
+    assertEquals(newMetadata, updatedBlob.metadata());
+    assertTrue(storage.delete(bucket, blobName));
+  }
+
+  @Test
+  public void testUpdateBlobMergeMetadata() {
+    String blobName = "test-update-blob-merge-metadata";
+    ImmutableMap<String, String> metadata = ImmutableMap.of("k1", "a");
+    ImmutableMap<String, String> newMetadata = ImmutableMap.of("k2", "b");
+    ImmutableMap<String, String> expectedMetadata = ImmutableMap.of("k1", "a", "k2", "b");
+    BlobInfo blob = BlobInfo.builder(bucket, blobName)
+        .contentType(CONTENT_TYPE)
+        .metadata(metadata)
+        .build();
+    assertNotNull(storage.create(blob));
+    BlobInfo updatedBlob = storage.update(blob.toBuilder().metadata(newMetadata).build());
+    assertNotNull(updatedBlob);
+    assertEquals(blob.blobId(), updatedBlob.blobId());
+    assertEquals(expectedMetadata, updatedBlob.metadata());
+    assertTrue(storage.delete(bucket, blobName));
+  }
+
+  @Test
+  public void testUpdateBlobUnsetMetadata() {
+    String blobName = "test-update-blob-unset-metadata";
+    ImmutableMap<String, String> metadata = ImmutableMap.of("k1", "a", "k2", "b");
+    Map<String, String> newMetadata = new HashMap<>();
+    newMetadata.put("k1", "a");
+    newMetadata.put("k2", null);
+    ImmutableMap<String, String> expectedMetadata = ImmutableMap.of("k1", "a");
+    BlobInfo blob = BlobInfo.builder(bucket, blobName)
+        .contentType(CONTENT_TYPE)
+        .metadata(metadata)
+        .build();
+    assertNotNull(storage.create(blob));
+    BlobInfo updatedBlob = storage.update(blob.toBuilder().metadata(newMetadata).build());
+    assertNotNull(updatedBlob);
+    assertEquals(blob.blobId(), updatedBlob.blobId());
+    assertEquals(expectedMetadata, updatedBlob.metadata());
     assertTrue(storage.delete(bucket, blobName));
   }
 
