http://gwt-code-reviews.appspot.com/1107801/diff/1/7 File user/src/com/google/gwt/user/client/rpc/impl/AbstractSerializationStream.java (right):
http://gwt-code-reviews.appspot.com/1107801/diff/1/7#newcode52 user/src/com/google/gwt/user/client/rpc/impl/AbstractSerializationStream.java:52: public static final int SERIALIZATION_STREAM_MIN_VERSION = 5; On 2010/11/15 16:02:21, jat wrote:
I think the version number needs to be bumped, since I don't believe
the
implementation fails if any unknown flag bits are set.
Version bumped.
To avoid having to bump the version in similar cases in the future, it
should
probably check if any unknown flags values are set and fail the same
was as a
protocol version mismatch.
Added checks for unknown flags.
However, I remember complications when Dan Rice increased the version
number for
changing the long representation, so we should look back over those
and see if
we can avoid the problems.
Any pointers for this? http://gwt-code-reviews.appspot.com/1107801/diff/3001/4002 File user/src/com/google/gwt/user/client/rpc/RpcToken.java (right): http://gwt-code-reviews.appspot.com/1107801/diff/3001/4002#newcode37 user/src/com/google/gwt/user/client/rpc/RpcToken.java:37: public @interface Class { On 2010/11/15 16:02:21, jat wrote:
Since usually people will have this imported, I would prefer a better
name -
@Class on an interface isn't going to be clear what it refers to. How
about
@RpcTokenImplementation?
Done. http://gwt-code-reviews.appspot.com/1107801/diff/3001/4008 File user/src/com/google/gwt/user/client/rpc/impl/RequestCallbackAdapter.java (right): http://gwt-code-reviews.appspot.com/1107801/diff/3001/4008#newcode170 user/src/com/google/gwt/user/client/rpc/impl/RequestCallbackAdapter.java:170: String methodName, RpcStatsContext statsContext, AsyncCallback<T> callback, On 2010/11/15 16:02:21, jat wrote:
Line length > 80.
Done. http://gwt-code-reviews.appspot.com/1107801/diff/3001/4008#newcode172 user/src/com/google/gwt/user/client/rpc/impl/RequestCallbackAdapter.java:172: this(streamFactory, methodName, statsContext, callback, null, responseReader); On 2010/11/15 16:02:21, jat wrote:
Line length > 80.
Done. http://gwt-code-reviews.appspot.com/1107801/diff/3001/4008#newcode230 user/src/com/google/gwt/user/client/rpc/impl/RequestCallbackAdapter.java:230: } else if (tokenExceptionHandler != null && caught instanceof RpcTokenException) { On 2010/11/15 16:02:21, jat wrote:
Line length.
Done. http://gwt-code-reviews.appspot.com/1107801/diff/3001/4009 File user/src/com/google/gwt/user/rebind/rpc/ProxyCreator.java (right): http://gwt-code-reviews.appspot.com/1107801/diff/3001/4009#newcode189 user/src/com/google/gwt/user/rebind/rpc/ProxyCreator.java:189: stob.addRootType(logger, rteType); On 2010/11/15 16:02:21, jat wrote:
Should this be conditional on finding an RpcToken subtype?
Done. http://gwt-code-reviews.appspot.com/1107801/diff/3001/4009#newcode497 user/src/com/google/gwt/user/rebind/rpc/ProxyCreator.java:497: // setRpcToken() On 2010/11/15 16:02:21, jat wrote:
Should this check instead be done at setRpcToken time? Perhaps have
setRpcToken
call protected boolean checkRpcTokenType(RpcToken) which by default
returns
true, and generate a check there?
I know I previously said over chat that a cast was fine, but thinking
about it
more I think we need a better error here. So, I think you want an
instanceof
check and then throw an exception with more details about the problem.
Done. http://gwt-code-reviews.appspot.com/1107801/diff/3001/4009#newcode593 user/src/com/google/gwt/user/rebind/rpc/ProxyCreator.java:593: returnStatement.append("return new " + FailingRequestBuilder.class.getName() + "(" On 2010/11/15 16:02:21, jat wrote:
If you are going to use a StringBuffer here, don't mix regular + --
use append
to add the pieces together.
Done. http://gwt-code-reviews.appspot.com/1107801/diff/3001/4009#newcode605 user/src/com/google/gwt/user/rebind/rpc/ProxyCreator.java:605: w.println("} catch(ClassCastException " + exceptionName + " ) {"); On 2010/11/15 16:02:21, jat wrote:
I think if the RpcToken type is checked at setRpcToken time, all of
these
changes aren't needed, right?
Done. http://gwt-code-reviews.appspot.com/1107801/diff/3001/4010 File user/src/com/google/gwt/user/server/rpc/HybridServiceServlet.java (right): http://gwt-code-reviews.appspot.com/1107801/diff/3001/4010#newcode137 user/src/com/google/gwt/user/server/rpc/HybridServiceServlet.java:137: log("An RpcTokenException was thrown while processing this call.", tokenException); On 2010/11/15 16:02:21, jat wrote:
Line length.
Done. http://gwt-code-reviews.appspot.com/1107801/diff/3001/4018 File user/test/com/google/gwt/user/client/rpc/RpcTokenTest.java (right): http://gwt-code-reviews.appspot.com/1107801/diff/3001/4018#newcode67 user/test/com/google/gwt/user/client/rpc/RpcTokenTest.java:67: fail(); On 2010/11/15 16:02:21, jat wrote:
Should include the exception to help debugging a failure just from the
logs. Done. http://gwt-code-reviews.appspot.com/1107801/diff/3001/4018#newcode133 user/test/com/google/gwt/user/client/rpc/RpcTokenTest.java:133: ((ServiceDefTarget) service).setRpcToken(token); On 2010/11/15 16:02:21, jat wrote:
As mentioned earlier, it really seems like this error should be caught
here,
rather than when the service is actually called.
Done. http://gwt-code-reviews.appspot.com/1107801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
