search

cberio / google-api-java-client

Automatically exported from code.google.com/p/google-api-java-client
0 stars 0 forks source link

Generate a deleteX() method for use with PATCH to delete a field #451

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago 
External references, such as a standards document, or specification?

http://googlecode.blogspot.com/2011/07/lightning-fast-performance-tips-for.html

https://developers.google.com/google-apps/tasks/performance#patch

Java environments (e.g. Java 6, Android 2.3, App Engine, or All)?

All

Please describe the feature requested.

See the example from the Tasks API documentation that shows deleting a 
"comment" field from some fictional "demo" API:

PATCH 
https://www.googleapis.com/demo/v1/324?fields=etag,title,comment,characteristics
Authorization: /* Auth token goes here */
Content-Type: application/json
If-Match: "ETagString"

{
  "etag": "ETagString"
  "title": "",                  /* Clear the value of the title by setting it to the empty string. */
  "comment": null,              /* Delete the comment by replacing its value with null. */
  "characteristics": {          /* Omit the length field, since it's not changing. */
    "length": "short",
    "level": "10",              /* Modify the level value. */
    "followers": ["Jo", "Liz"], /* Replace the followers array to delete Will and add Liz. */
    "accuracy": "high"          /* Add a new characteristic. */
  },
}

In order to accomplish this with the generated service-specific library, you 
would have to do something like this:

resource.setComment(Data.NULL_STRING);

but this is not intuitive.  Most users will honestly give up trying by this 
point.

What might be more intuitive is if we generated a method like deleteComment() 
that would encapsulate that logic, e.g.:

/** Deletes the comment field for use with a PATCH request. */
public Resource deleteComment() {
  return setComment(Data.NULL_STRING);
}

Trivial to implement for us, and hopefully be more intuitive to find and use.

Original issue reported on code.google.com by yan...@google.com on 6 Apr 2012 at 9:03

GoogleCodeExporter commented 9 years ago 
This is unfortunately not a trivial implementation. My original logic was to do 
something like:

set{% camel_case param.wireName %}(com.google.api.client.util.Data.NULL_{{ 
param.codeType|upper }});

but this does not work because the type of the parameter may not be a primitive 
type. Eg: For one parameter I ran into List<String>.

I also think it is not that hard for users to figure out 
resource.setComment(Data.NULL_STRING) but to help them out we could (only for 
Patch methods) generate an additional comment in the javadoc pointing them to 
com.google.api.client.util.Data for JSON null values.

What do you think?

Original comment by rmis...@google.com on 9 Apr 2012 at 2:42

GoogleCodeExporter commented 9 years ago 
In general you can call Data.nullOf() to get the JSON null object for any type. 
 I do think it is hard for users to figure it out.  As you say, this is 
"unfortunately not trivial".

Adding a comment to the JavaDoc is reasonable, but my concern is that:

1. Developers often don't read JavaDoc so are less likely to discover it

2. If you do know how it works, it is human nature to accidentally call 
setComment(null) when you meant to call setComment(Data.NULL_STRING).

I do see the down-side of adding more clutter in terms of extra generated 
methods.  This is particularly true in the case where a field cannot be 
modified.  We may be able to solve this problem though: there are plans to add 
information in Discovery about which fields are modifiable.  Perhaps we should 
wait for that before implementing deleteX()?

Original comment by yan...@google.com on 10 Apr 2012 at 12:17

GoogleCodeExporter commented 9 years ago 
Yes it makes sense to wait for that before implementing deleteX(), we do not 
want to provide deleteX() if X cannot be modified, this will lead to confusion.

Do not see how to mark that we have a dependency for this bug, lowering 
priority instead.

Original comment by rmis...@google.com on 16 Apr 2012 at 12:16

GoogleCodeExporter commented 9 years ago

Original comment by rmis...@google.com on 16 May 2012 at 1:24

GoogleCodeExporter commented 9 years ago

Original comment by rmis...@google.com on 30 May 2012 at 7:26

GoogleCodeExporter commented 9 years ago

Original comment by yan...@google.com on 2 Aug 2012 at 2:38

GoogleCodeExporter commented 9 years ago

Original comment by yan...@google.com on 26 Sep 2012 at 12:43