The result of a patch metadata is a union rather than replacement.

[aozarov]

It looks like patch operations do not work well with Map attributes (such as metadata) and instead of replacing the map content (which is backed by repeated message) it merges it.

[jgeewax]

@aozarov can you add in the description the code snippet that reproduces this for us ?

[jgeewax]

Here is the bug for gcloud-node: googleapis/google-cloud-node#597

We haven't decided whether this is just something we document, or whether we find a way to make setMetadata delete the entries and re-add them, or something entirely different....

[aozarov]

The issue is in the Apiary level and there is a plan to fix that (so once fix current code will work as expected).

[aozarov]

Here is a code snippet to reproduce the problem:

StorageServiceOptions options = StorageServiceOptions.defaultInstance();
StorageService storage = StorageServiceFactory.instance().get(options);
Blob blob = storage.get("bucket", "blobName");
Map<String, String> metadata = new HashMap<>();
metadata.put("name", "value");
storage.update(blob.toBuilder().metadata(metadata).build());

[mziccard]

I double checked and this "problem" persists.
gcloud-node guys ended up documenting that the setMetadata method actually merges metadata.

Merge the given metadata with the current remote file's metadata. This will set metadata if it was previously unset or update previously set metadata. To unset previously set metadata, set its value to null.

We could do something similar for update. Notice that in our case setting metadata to null does not unset remote metadata.

[mziccard]

@aozarov @jgeewax Sorry for pinging, do you have any thoughts on this?

[jgeewax]

Might want to take a look at this discussion we had over in gcloud-node: googleapis/google-cloud-node#597

[mziccard]

I actually checked, this is why I was proposing to document this behavior as it has been done for gcloud-node.

[aozarov]

The internal bug for apiary is still open. There are various ways that we can deal with it but I would rather avoid implementing the patch logic ourselves (via get, merge and set if not modified).

I am not sure that in Java we have an option to null/clear the metadata property and still use the patch operation.

Some options:

1. Replace patch with update and document that update operation will replace all the blob's metadata with values from the given BlobInfo.

2. Provide to the user both patch and update options (documenting the differences) and
   also mentioning that if a new metadata is given its values will be merged with the previous version
   and if that is not desired update should be used instead.

Option (2) is actually an add-on to option (1).

[aozarov]

BTW, @BrandonY suggests that setting the metadata to null does work for him (and clears it).

[aozarov]

@mziccard I think that the reason you were not able to set metadata to null was because we need to change the BlobInfo.Builder.metadata function to use Data.nullOf when user explicitly sets it to null (otherwise for PATCH null means keep-previous-value).

If that is indeed the case than from the API perspective I would suggest a different approach from the one we took in #276. Basically I would suggest that we only change the documentation to mention that an update of metadata will be merged with any previously set metadata and setting it to null will clear it. That means for replacing metadata with new content the workaround would be an update with null following another update with the new map.

If we decide to go with this route, than we will need to create our own "ImmutableEmptyMap" class (public and public empty constructor) to be used with Data.nullOf.