Add a way to create a derived client #97
Conversation
|
@kumai @inch772 PTAL. testDerivedClient() demonstrates this new feature. Please feel free to let me know if there's even better way to achieve this. :-) @inch772 We might want to reduce the construction cost of |
trustin
force-pushed
the
per_request_options
branch
from
b984569
to
01222ec
Compare
|
@inch772 I also find that we cannot use |
|
@trustin LGTM. Thanks! Although it's a tiny thing, I think the javadoc of ProxyClient should be fixed: |
| throw new IllegalArgumentException("not a client: " + client); | ||
| } | ||
|
|
||
| final ClientInvocationHandler parent = |
There was a problem hiding this comment.
I think we can avoid the weird name mangling with something like
ProxiedClient.getClass().getMethod("handler").invoke(client);
The method.getDeclaredClass() == ProxiedClient.class check in the handler will be true for this call and should be false for any other handler() method.
Since this is a reflective Java proxy, I don't think the performance will be any different for the .invoke() call vs this invocation.
There was a problem hiding this comment.
Actually, I found
(ClientInvocationHandler) Proxy.getInvocationHandler(proxy)
From what I can tell, it'll work and remove ProxiedClass completely.
There was a problem hiding this comment.
@anuraaga Ah, nice find. will fix.
trustin
force-pushed
the
per_request_options
branch
3 times, most recently
from
6847aa9
to
a3d29d7
Compare
Related: line#95 Motivation: Given that the creation of a new client is relatively an expensive operation, a user might want to create a client that reuses the RemoteInvoker and ClientCodec of an existing client while using an alternative ClientOptions. A good example is writing an API proxy that forwards a certain set of HTTP headers such as access tokens, as explained in line#95. Modifications: - Add Clients.newDerivedClient() that creates a derived client from an existing client created by ClientBuilder or Clients.newClient() - Add a test case for Clients.newDerivedClient() to ThriftOverHttpClientTest - Add HeaderService to assert that the derived client sends an expected header - Simplify the method comparison of ClientInvocationHandler. It doesn't really need to use Method.equals() that performs rigorous comparison because Object has no overloaded methods. Result: A user can create a derived client at much lower cost, which makes it easy to implement an API proxy-ish server.
|
LGTM - so glad that weird method mangling is gone! |
Add a way to create a derived client
Related: #95
Motivation:
Given that the creation of a new client is relatively an expensive
operation, a user might want to create a client that reuses the
RemoteInvoker and ClientCodec of an existing client while using an
alternative ClientOptions.
A good example is writing an API proxy that forwards a certain set of
HTTP headers such as access tokens, as explained in #95.
Modifications:
existing client created by ClientBuilder or Clients.newClient()
ThriftOverHttpClientTest
expected header
really need to use Method.equals() that performs rigorous comparison
because Object has no overloaded methods.
Result:
A user can create a derived client at much lower cost, which makes it
easy to implement an API proxy-ish server.