Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GoogleUtils.useMethodOverride(HttpTransport) and remove GoogleTransport #98

Closed
wonderfly opened this issue Jan 10, 2015 · 5 comments
Closed
Assignees
Labels
imported priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@wonderfly
Copy link
Contributor

From ai...@google.com on December 21, 2010 13:30:07

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: http://code.google.com/p/google-api-java-client/issues/detail?id=81

@wonderfly wonderfly added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. imported priority: p2 Moderately-important priority. Fix may not be included in next release. 1 star labels Jan 10, 2015
@wonderfly wonderfly self-assigned this Jan 10, 2015
@wonderfly
Copy link
Contributor Author

From yan...@google.com on December 21, 2010 14:16:32

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 ).

Status: ByDesign
Cc: jasonh...@google.com

@wonderfly
Copy link
Contributor Author

From yan...@google.com on December 22, 2010 07:18:36

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.

Status: Accepted
Labels: Milestone-Version1.3.0

@wonderfly
Copy link
Contributor Author

From yan...@google.com on December 22, 2010 07:19:24

Summary: GoogleUtils.useMethodOverride(HttpTransport) and remove GoogleTransport [Backwards Incompatible]
Owner: yan...@google.com
Cc: -yan...@google.com ai...@google.com

@wonderfly
Copy link
Contributor Author

From yan...@google.com on January 06, 2011 19:29:16

Summary: GoogleUtils.useMethodOverride(HttpTransport) and remove GoogleTransport
Status: Fixed

@wonderfly
Copy link
Contributor Author

From yan...@google.com on January 08, 2011 10:43:08

Labels: Component-Google-APIs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
imported priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

1 participant