setMetadata() method merges existing with new data?

[jgeewax]

Looks like if I call setMetadata on an object, it merges the data with what's in production...

var bucket = ...; // Get a bucket somehow.
var file = bucket.file('my-file');

file.setMetadata({
  metadata: {
    first: 'first'
  }
});

file.setMetadata({
  metadata: {
    second: 'second'
  }
}, function(err, metadata) {
  // metadata here is {first: 'first', second: 'second'}
  // Expected it to be just {second: 'second'} ....
});

I suspect that we can't .. fix this all that much... but maybe we can at least document it ?

---

The options we have so far:

• Make metadata immutable: remove setMetadata
• Operate on a single attribute at a time: getMetadata(key) and setMetadata(key, value)
• Rename setMetadata to patchMetadata or mergeMetadata
• Add some documentation about the behavior of setMetadata

/cc @Capstan @aozarov

[ryanseys]

Remember #459? Yeah we switched to PATCH because using PUT destroyed ACLs with BigQuery, making our datasets inaccessible. I'm not sure if this is also the case with Storage but I feel it would be appropriate to remain consistent across our APIs in this library.

[ryanseys]

We can still document it for our users. If they want to delete an existing value, I believe the correct way is to set it to null.

[aozarov]

Yes, but I doubt if doing it (delete and set) should be done by the library as it can fail in the middle and leave it in an undesired state.

BTW, nulls may be a separate issue which I mentioned to @jgeewax.
In Java nulls are considered as "ignore" in patch operation and instead a special "null" value should be used (e.g. http://javadoc.google-http-java-client.googlecode.com/hg/latest/com/google/api/client/util/Data.html#NULL_BIG_DECIMAL) [which clearly should be done transparently by the library]. Not sure if the same problem applies to node.js apiary client.

[ryanseys]

I doubt if doing it (delete and set) should be done by the library as it can fail in the middle and leave it in an undesired state.

The API call operation is atomic so either the whole thing succeeds or fails.

[ryanseys]

I think I see what you were saying. I meant that using PATCH you can delete existing values by setting them to null. We never make 2 calls for setting metadata. That requires you to know about the ones are set already, which is a getMetadata call away. We leave this as an exercise for the user if they want to do that.

[aozarov]

What I was saying is that we should probably not do the following:

func setMetadata(blob, metadata) {
  rpc.patch(blob, {"metadata": null})
  rpc.patch(blob, {"metadata": metadata})
}

[ryanseys]

Oh we'd never do that. The end result is the same as just rpc.patch(blob, {"metadata": metadata})

I meant if you wanted to set completely new metadata, including destroying the ones currently set, you need to know the existing ones and set them to null so as to achieve the same result as using PUT.

[ryanseys]

Like this:

// metadata is currently { a: '1', b: '2', c: '3' };
rpc.patch(blob, { metadata: { a: 'x', b: null, c: null } }) // need to know b and c exist so you can set them to null
// metadata is now { a: 'x' };

This is the same as:

// metadata is currently { a: '1', b: '2', c: '3' };
rpc.put(blob, { metadata: { a: 'x' } })
// metadata is now { a: 'x' };

[aozarov]

Is it? Is rpc.put doing an update (https://cloud.google.com/storage/docs/json_api/v1/objects/update)?
If so, wouldn't it clear out the other Blob's user-given properties (e.g. contentLanguage)?

[jgeewax]

So back to the main issue:

My concern here is that set typically means "replace the value up there with the value I'm sending from down here". set to us means... "merge this value from down here into whatever data exists up there."

Can we rename anything ? or are we just OK with adding some docs that explain this? Or do we try to get Google to fix something ? What makes the most sense as a user of this library?