[
https://issues.apache.org/jira/browse/THRIFT-3112?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15137318#comment-15137318
]
John Sirois commented on THRIFT-3112:
-------------------------------------
The fundamental problem here is that the async client stack is parametrized
over its implementation. An example from the tutorial "correctly" parametrized
today:
{code:java}
package tutorial;
public class Calculator {
public interface AsyncIface extends shared.SharedService .AsyncIface {
public void
ping(org.apache.thrift.async.AsyncMethodCallback<AsyncClient.ping_call>
resultHandler) throws org.apache.thrift.TException;
public void add(int num1, int num2,
org.apache.thrift.async.AsyncMethodCallback<AsyncClient.add_call>
resultHandler) throws org.apache.thrift.TException;
...
}
public static class AsyncClient extends shared.SharedService.AsyncClient
implements AsyncIface {
public void ping(org.apache.thrift.async.AsyncMethodCallback<ping_call>
resultHandler) throws org.apache.thrift.TException {
...
}
public static class ping_call extends
org.apache.thrift.async.TAsyncMethodCall<ping_call> {
...
public void getResult() throws org.apache.thrift.TException {
...
}
}
...
}
public static class AsyncProcessor<I extends AsyncIface> extends
shared.SharedService.AsyncProcessor<I> {
public static class ping<I extends AsyncIface> extends
org.apache.thrift.AsyncProcessFunction<I, ping_args, Void> {
public AsyncMethodCallback<Void> getResultHandler(final AsyncFrameBuffer
fb, final int seqid) {
...
}
}
public static class add<I extends AsyncIface> extends
org.apache.thrift.AsyncProcessFunction<I, add_args, Integer> {
public AsyncMethodCallback<Integer> getResultHandler(final
AsyncFrameBuffer fb, final int seqid) {
...
}
}
...
}
}
{code}
Notice in the {{AsyncIface}} that:
# The client-side AsyncMethodCallback's are not parametrized over the sync
interface method return type as you'd expect, they are parametrized by the
client implementation of the method call.
# The server-side AsyncMethodCallback's are parametrized as you'd expect - over
the sync interface method return type.
The combination of 1 and 2 is confusing, but worse 1 mixes the implementation -
{{AsyncClient}} - into the interface - {{AsyncIface}} and it forces clients to
both call {{getResult}} in their {{onComplete}} handler as well as deal with
errors. This leaves error handling split between {{onComplete}} and
{{onError}}.
It seems to me the right long-term answer is to fix the generated client code
to match the generated server code and pass the successfull completed result to
{{onComplete}} and not the {{TAsyncMethodCall}} implementation. This would be
a breaking change though and anyone using AsyncIface today would need to fix
there code on upgrade. A [github
search|https://github.com/search?utf8=%E2%9C%93&q=new+AsyncMethodCallback+language%3AJava+extension%3Ajava&type=Code&ref=advsearch&l=Java&l=]
does reveal folks actually using {{AsyncIface}} today. That said - in the 1st
page of results alone there is evidence of folks doing it "wrong" and
parametrizing in the natural way. That code must either be unused or else
written against an Apache Thrift fork.
> [Java] AsyncMethodCallback should be typed in generated AsyncIface
> ------------------------------------------------------------------
>
> Key: THRIFT-3112
> URL: https://issues.apache.org/jira/browse/THRIFT-3112
> Project: Thrift
> Issue Type: Improvement
> Components: Java - Compiler
> Affects Versions: 0.9.2
> Reporter: Sergei Egorov
> Assignee: John Sirois
> Labels: easyfix
>
> AsyncMethodCallback is generic, but current Java code generator is not adding
> type info to it, and instead of:
> {code:java}
> public interface AsyncIface {
> public void ping(org.apache.thrift.async.AsyncMethodCallback<String>
> resultHandler) throws org.apache.thrift.TException;
> }
> {code}
> we have:
> {code:java}
> public interface AsyncIface {
> public void ping(org.apache.thrift.async.AsyncMethodCallback
> resultHandler) throws org.apache.thrift.TException;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)