search

Gau-Yi / google-api-java-client

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

GoogleUtils.useMethodOverride(HttpTransport) and remove GoogleTransport #81

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Version of google-api-java-client (e.g. 1.2.1-alpha)?

Java environment (e.g. Java 6, Android 2.2, App Engine 1.3.7)?
All

Describe the problem.

An developer will call GoogleTransport.create() to create a transport.  Most of 
the time they care about the base functionality of the HTTPTransport, but 
sometimes they have to modify the transport default headers.  Unfortunately, 
HTTPTransport has headers of type HttpHeaders, even though they are actually 
GoogleHeaders.   An application wanting to call GoogleHeaders methods has to do 
a cast on the defaultsHeaders of the transport.  There should be a type safe 
mechanism for a developer to get
the GoogleHeaders instance that GoogleTransport created.

How would you expect it to be fixed?

1 GoogleTransport should extend HTTPTransport.
2. It's create methods should return GoogleTransport objects rather than 
HTTPTransport.
3. GoogleTransport should have a getDefaultHeaders() method which returns a 
GoogleHeaders object.  This puts the class in the base library and out of the 
users code.

Original issue reported on code.google.com by ai...@google.com on 21 Dec 2010 at 9:30

GoogleCodeExporter commented 9 years ago 
By design I don't want to create the unnecessary complication of allowing 
HttpTransport to be subclassed.  We've tried that before in a previous version 
of the library, and it was a nightmare.  In general subclassing in Java is 
dangerously complicated.

Another option is to have GoogleTransport be a wrapper for an instance of 
HttpTransport, but that also has a lot of design complication problems.

So it doesn't appear that there's any painless option, and the option currently 
being used in the library seems the least painful.  The only design problem 
identified so far is that you have to write a cast to GoogleHeaders.  I agree 
with you that this is not ideal.  But it seems to me to hardly be a 
deal-breaker in terms of usage of the library.  Note that this behavior is a 
well-documented in the JavaDoc for GoogleTransport.create() so it is behavior 
that the developer can rely on.  Alternatively, they can simply ignore it and 
set the defaultHeaders to an instance of GoogleHeaders themselves, which would 
allow them to avoid the cast.  Perhaps we should not even set 
HttpTransport.defaultHeaders in GoogleTransport.create(), so as to avoid the 
cast in our recommended usage of the library. That may well be the easiest 
option.

Closing now as "ByDesign", but feel free to continue discussing this issue and 
propose design ideas on the Google Group 
(https://groups.google.com/forum/#forum/google-api-java-client).

Original comment by yan...@google.com on 21 Dec 2010 at 10:16

GoogleCodeExporter commented 9 years ago 
We discussed this in person, and we have a new proposal that there is general 
agreement on:

Remove GoogleTransport.  Instead add a new class GoogleUtils with a method:

public static MethodOverrideIntercepter useMethodOverride(HttpTransport);

This is really the only value that GoogleTransport.create() is currently 
providing.  A big advantage of this solution is that then the developer 
receives an instance of MethodOverrideIntecepter which they can customize to 
select which HTTP methods they actually want to override.

Original comment by yan...@google.com on 22 Dec 2010 at 3:18

GoogleCodeExporter commented 9 years ago

Original comment by yan...@google.com on 22 Dec 2010 at 3:19

GoogleCodeExporter commented 9 years ago

Original comment by yan...@google.com on 7 Jan 2011 at 3:29

GoogleCodeExporter commented 9 years ago

Original comment by yan...@google.com on 8 Jan 2011 at 6:43