Issue 389, RPC delegation support - active?
Rob Jellinghaus
http://code.google.com/p/google-web-toolkit/issues/detail?id=389
is listed as "low" priority, assigned to gwt.team.mmendez.
I am interested in working on this in order to facilitate a better
GWT-to-JSF coupling. (Basically I want to be able to specify a JSF
managed bean as the service endpoint for a GWT component via JSF,
without needing to write a per-bean RemoteServiceServlet, or do CGLIB
magic as in http://g.georgovassilis.googlepages.com/usingthegwthandler
. Just checked out the GWT source with an eye towards making a patch
to support this.
So my question is mainly for mmendez: is any active development
underway on this that I am likely to collide with, or is this pretty
much going to stay on the back burner for the next month or two? It'd
be dispiriting to get it working only to find out someone else already
got it into the latest release candidate :-)
Cheers!
Rob Jellinghaus
Rob Jellinghaus
relatively quick review of RemoteServiceServlet.
My plan is to refactor RemoteServiceServlet into an abstract superclass
DelegatingServiceServlet, and a concrete subclass RemoteServiceServlet
which will be 100% compatible with the current version. Basically
DelegatingServiceServlet will have an abstract method something like
"protected abstract Object fetchServiceDelegate (HttpRequest request);"
which RemoteServiceServlet implements as "return this;".
Then all the existing interface lookup logic will live in
DelegatingServiceServlet and will do all the method lookup off the
result of fetchServiceDelegate.
Net result: RemoteServiceServlet works exactly as it did previously,
but any server-side implementors who want to can instead subclass
DelegatingServiceServlet, and override fetchServiceDelegate to return
whatever existing server-side component they like. Should let George
get rid of the 70% of his Spring GWTHandler code that he doesn't want,
and should allow g4jsf to have a much more GWT-like RPC mechanism (I
will be patching g4jsf to prove that concept as part of this work).
Obviously I'll make sure any patches I submit pass the full GWT test
suite and conform to the GWT style guidelines.
Comments welcome!
Cheers,
Rob Jellinghaus
Miguel Méndez
I'm glad that you are looking at this since it is not high on our priority list right now. It sounds like you have already read the Making GWT Better document just don't forget to follow the steps in the Contributor License Agreement section of that document so you can submit patches.
The current infrastructure allows you to achieve the same delegating effect by simply creating a RemoteServiceServlet instance that implements the service interface and then delegates to some other concrete implementation of the same interface. This is a simple and effective pattern. The question is why is this not sufficient again?
Thanks,
g.georgo...@gmail.com
I think Rob's idea is good though I would propose something slightly
more radical. Our main goal is to allow people to write POJO services
with no dependencies on GWT (not even RemoteService because we may
already be using them elsewhere) and map RPC invocations to its
methods. It is a good practice in modern web applications to separate
concerns and dependencies - my service doesn't do anything GWT
specific, why must it inherit it's API?
The problem with the current implementation is that one has to extend
RemoteServiceServlet in order to obtain RPC functionality - you are
tying the RPC protocol implementation to the servlet API a little more
than necessary - from an application design point of view the necessity
for this constraint is not obvious.
One can easily picture this with an alternative RPC API:
Method m = RPC.decodeMethod(httpRequest);
Object[] arguments = RPC.decodeArguments(httpRequest);
try{
Object result = m.invoke(service, arguments);
RPC.post(httpResponse, result);
}
catch (SerializableException e){
RPC.throw(e);
}
In this example I don't have to extend a servlet, my dependencies are
limited to the HttpRequest/Response. For Spring, this way I could write
elegant URL mappers, handler and Service Exporters (just like the SOAP,
Burlap, Hessian etc exporters that already exist with Spring).
I would love an officially documented API to whatever RPC
implementation GWT provides which I can use for my own X-over-RPC
integrations - just like the GWTHandler Rob mentioned which indeed
would shrink considerably if RPC was factored out.
Rob Jellinghaus
> The current infrastructure allows you to achieve the same delegating effect
> by simply creating a RemoteServiceServlet instance that implements the
> service interface
The point is that if you already have server-side components that
implement the service interfaces you want to expose to GWT, then having
to subclass RemoteServiceServlet on a per-interface basis is pure
overhead. The servlet-subclass code is total boilerplate, adding no
useful functionality and increasing your maintenance cost.
It also obfuscates the server-side logic if you want to select
server-side service components on a per-request basis (as g4jsf
allows), or generically intercept service methods (for pruning object
graphs, or for converting JDK 1.5 business objects into JDK 1.4 data
transfer objects, for example -- when will GWT support Java 5 again?
;-).
Additionally, server-side frameworks like Seam make it very easy to
implement server-side components as POJOs, with no servlet tweaking
necessary at all. You write a JSF template that calls methods on your
server-side POJOs, and you never need to see or think about servlets in
the middle. I would like to be able to write a Seam POJO component and
expose it directly to a GWT component in a JSF page, without needing to
muck about with servlets or the details of the request plumbing in any
way. (This is pretty much what g4jsf is for, except it's not hooked up
to Seam yet. Once GWT doesn't require a servlet implementation per
interface, coupling it with Seam will be much easier.)
Basically, classes that consist only of forwarding methods are pretty
much always a maintenance headache, and -- given reflection -- are
often unnecessary. A lot of the power of Seam and other modern
server-side frameworks is being able to generically interpose on an
entire interface. The GWT server-side plumbing is *so close* to
supporting this cleanly that there seems no reason not to just do it.
The short version: if you already have server code implementing your
service interfaces, then why have one implementation of
RemoteServiceServlet per service interface, when you could just have
*one* servlet that does all the work?
Cheers!
Rob
Rob Jellinghaus
> In this example I don't have to extend a servlet, my dependencies are
> limited to the HttpRequest/Response. For Spring, this way I could write
> elegant URL mappers, handler and Service Exporters (just like the SOAP,
> Burlap, Hessian etc exporters that already exist with Spring).
Hm, interesting. Yes, that is a bit more radical than what I had in
mind, but I see your point: why not make the GWT RPC server-side code
be a utility library that the existing servlet framework can call,
instead of forcing the GWT servlet to be the base class for your own
servlet entry point? Don't burn your base class if you can avoid it.
( http://www.artima.com/intv/dotnet.html midway down the page --
doesn't apply 100% but you can get the basic idea.)
I'll look into this, George.
Cheers!
Rob
Bruce Johnson
I'm glad Miguel played devil's advocate by asking the question "why isn't simple delegation good enough?" for two main reasons:
1) It's a good idea to be highly circumspect whenever we make changes to key GWT classes that are hard to revert due to compatibility/integration issues. This change falls squarely into that category. Too often, we've seen cases where making an "obvious" change sounds great the first 30 minutes that you think about, then much later you realize it was a mistake. I'd like to minimize those situations by belaboring the analysis (which is cheap) rather than having to undo work or live with a mistake (which is very expensive and annoying for everyone). Our biggest headaches come from "obvious" design decisions that weren't scrutinized enough before they shipped.
2) When there is a clear, reasonable rationale for a change (as there seems to be in this case), forcing people to fully discuss the reasons for a change gives us a great paper-trail as to how we came to a decision. This will be an extremely valuable resource to the community over time, because we can easily answer questions such as "why does this work this way?" by pointing folks (including ourselves) back at these high-value "rationale threads".
</meta-commentary>
So, all that said, I can't think of any downside to the basic idea if we can design and implement it properly. Here are some follow-up questions:
1) A single RemoteServiceServlet-derived class can implement any number of RPC interfaces. Since part of the goal of this change is to increase flexibility in how you stitch things together, wouldn't people expect to be able to delegate to multiple POJO services, one per RPC interface that is implemented? For example, suppose
- I have 2 GWT RPC interfaces: EmailService and UserSettingsService
- These are both implemented in the server-side class EmailAndUserSettingsServiceImpl which extends RemoteServiceServlet
- I want to delegate to 2 POJO services: EmailBackEnd and UserSettingsBackEnd
It seems like fetchServiceDelegate() actually needs to be capable of handling multiple delegate types, one per service interface. Thoughts?
2) What threading model should implementers of a delegate POJO service expect to operate within? "Everyone knows" that servlets need to be thread-safe, so forcing people to extend RemoteServiceServlet is a strong reminder that "hey, you'd better be careful". When you abstract that out one level so that your service POJOs are getting called automatically, will the need for thread safety be as apparent? Actually, I think I know a good answer for this one: your subclass of DelegatingServiceServlet can return either a "singleton" delegate (in the case when the POJO service is known to be thread-safe) or it could return a dedicated instance for the calling thread (in the case when the POJO service may be single-threaded).
3) Does the POJO service actually need to (A) implement the RPC service interface explicitly, or (B) is delegation done on the basis of the delegate having a compatible method signature alone? (A) seems preferable from a correctness standpoint because it avoids latent problems due to missing methods but then again it makes the POJO service less "PO" :-) But then (B) would be a false freedom anyway: your method must have a method declaration that matches the RPC interface exactly.
I think if we can work through all these issues and other that may come up, this could be a very positive change.
-- Bruce
Rob Jellinghaus
> I'm glad Miguel played devil's advocate by asking the question "why isn't
> simple delegation good enough?" for two main reasons:
+1 to everything you said. One thing I will definitely do is post the
modified classes here before submitting them in any way, so as to get
comprehensive code review and public discussion of the code itself
before landing it. That way, not only is the discussion public and
reviewable, but so is the code itself, prior to hitting the mainline.
> 1) A single RemoteServiceServlet-derived class can implement any number of
> RPC interfaces. Since part of the goal of this change is to increase
> flexibility in how you stitch things together, wouldn't people expect to be
> able to delegate to multiple POJO services, one per RPC interface that is
> implemented?
That is indeed part of the goal, but really it's up to the server-side
implementor. After all, you could right now have two different
RemoteServiceServlet subclasses, one per interface, with different
service endpoints, and get the same effect as having a single
RemoteServiceServlet that delegated to two different POJOs. (modulo
having two endpoints rather than one.)
Note that endpoints are a bit more flexible than I originally
appreciated, especially when you start thinking about flexible
server-side delegation. g4jsf appends a "clientId" request parameter
to the service endpoint of a GWT component instantiated in a JSF page.
This lets the g4jsf servlet dispatcher associate an inbound GWT request
with a particular server-side object that is to handle the request. In
other words, this basically creates one logical endpoint per GWT
component on the client page, and the server can use that to do
per-GWT-component delegation! The endpoint request parameters become a
sort of "backchannel" for conveying additional service information
beyond that bundled in the RPC itself.
(Digression: I wish there were a similar backchannel for responses,
because I have nebulous plans to integrate Seam conversations with GWT
components in a way that is transparent to the GWT RPC mechanism. But
I can't see how to do it at the moment (there's no such thing as
"response parameters" AFAIK). This is not germane to my immediate
plans, so please don't get hung up on my ill-formed brainstorming ;-)
> It seems like fetchServiceDelegate() actually needs to be capable of
> handling multiple delegate types, one per service interface. Thoughts?
Yes, I agree, fetchServiceDelegate should probably have an extended
signature, so that it gets not only the whole request to look at but
also some amount of extracted RPC signature / interface information.
George's suggestion somewhat argues for not implementing
DelegatingServiceServlet at all, but rather for refactoring
RemoteServiceServlet so as to make the RPC-handling functionality
therein be more reusable in other servlets altogether. I might wind up
doing both -- e.g. refactor the RPC code so George could use it in a
reimplemented GWTRequestHandler, but *also* implement
DelegatingServiceServlet as a superclass of RemoteServiceServlet, just
to simplify things for other users who want to write their own
dispatcher logic as conveniently as possible (and who don't mind
subclassing a GWT servlet class).
> 2) What threading model should implementers of a delegate POJO service
> expect to operate within?
This is really up to the overall threading model of the servlet
framework. Seam, for example, has application-scoped components (which
need to be synchronized on explicitly), sesssion-scoped and
conversation-scoped components (which the framework implements mutual
exclusion on), and page- and request-scoped components (which are
instantiated anew per request). Spring has a similar taxonomy AFAIK.
When writing Seam applications, the programmer already needs to be
aware of what components can and can't be safely invoked from various
kinds of JSF pages or other server components; GWT fundamentally brings
no new issues to the table.
(By the way, I've mentioned Seam a lot without any actual linkage.
http://docs.jboss.com/seam/1.0.0.GA/reference/en/html/tutorial.html is
probably a good starting point for those who are wondering what I'm
talking about.)
So all the delegating servlet needs to do is to invoke the framework
properly such that the framework can apply its normal synchronization
logic on the service component. I'll be implementing this as part of
integrating g4jsf with Seam -- the existing AJAX remoting for Seam
provides a good starting point.
You're correct that if you're rolling your own servlet framework for
your POJO services, you'll need to be aware of the synchronization
issues. How should this best be documented? Probably at least as part
of the javadoc of DelegatingServiceServlet, which is what roll-your-own
programmers would likely be using...?
> 3) Does the POJO service actually need to (A) implement the RPC service
> interface explicitly,
I would say, yes, absolutely, definitely. Otherwise we're effectively
using structural binding, not name binding, which is very contrary to
Java practice. It also would break much of the existing
interface-binding logic in RemoteServiceServlet, that I would like to
move around without actually modifying.
> I think if we can work through all these issues and other that may come up,
> this could be a very positive change.
Glad to hear it. Thanks for continuing the discussion! More, please
:-)
Cheers!
Rob
g.georgo...@gmail.com
architecturally alter neither the way RPC functions nor how existing
RemoteServiceServlets operate - it is only a refactoring of existing
code. If RPC was a discrete (external?) dependency the
RemoteServiceServlet would still import it and do it's work exactly the
same way it does now.
If and how delegate signatures, thread scopes and an application
environment affect RPC would then be in the design contract of the
specific Seam/Spring/Servlet adapter. The client still communicates via
a proxy over RPC with the service and it's the service provider
responsibility (not GWT's anymore!) to provide for correct invocation
of the service delegates.
Rob Jellinghaus
I do not want to pre-empt any further discussion on this thread (quite
the contrary!), but I read through the RemoteServiceServlet code in
detail and realized that a straightforward refactoring actually
wouldn't be that much work. So I went ahead and did it. It passes
GWT's "ant test". (I love refactoring....)
I have posted the files as attachments to issue 389:
http://code.google.com/p/google-web-toolkit/issues/detail?id=389
I didn't post a patch to RemoteServiceServlet -- I just posted my
refactored version wholesale -- because at this point I am really only
asking for code review, not actually submitting this for inclusion in
GWT. (For one thing, I only just mailed my Google contributor's
release form today, so until you guys get it early next week, I can't
land it anyhow!) But hopefully interested parties (Bruce, Miguel,
George, Musachy?) can take a look and comment.
I split RemoteServiceServlet into four classes:
- RPC: Simple static class with helper methods for reflection. (Is
this contrary to Google style? I know that static-only classes tend to
become dumping grounds for random code, but conversely, with these
kinds of reflection helpers it's often good to have them in one central
place since they're so generic.)
- RPCCall: Encapsulates a payload with accessors that let you retrieve
the interface, method, parameter types, and parameter values.
Encapsulates the serialization stream readers and writers. It also
supports invoking the service method on a given target object. This
seems like a reasonable abstraction, while still supporting the
pre-existing "validate the payload piece by piece" control flow.
- DelegatingServiceServlet: This is pretty much as I suggested in my
post above; it handles pulling the payload out of the servlet request,
validating it piecemeal, and then using RPCCall to actually invoke the
method. It has abstract hooks for obtaining the actual delegate. This
would be quite a bit simpler if it parsed the whole payload before
doing any validation, but it seemed worth keeping the original code's
validate-as-you-go approach.
- RemoteServiceServlet: This has boiled down to just three methods now,
one of which is a helper method, and another of which is "return this;"
:-)
George, I'm fairly sure the RPC / RPCCall classes are mostly what you
want. Musachy, I'm fairly sure you could hugely simplify your
strut2gwtplugin GWTServlet class by subclassing
DelegatingServiceServlet instead. Let me know whether I'm on the ball
or not. If not, we'll iterate!
Some concerns and questions:
- RPCCall now encapsulates the ServerSerializableTypeOracle. (In fact,
it encapsulates all the rpc.impl classes.) However, the lifecycle of
RPCCall is per-request. Previously the RemoteServiceServlet owned the
ServerSerializableTypeOracle, and used it for all requests. On
inspection, it looks like all the performance-sensitive state of
ServerSerializableTypeOracle is statically cached (and carefully
synchronized) already, so I don't think this is any kind of performance
problem, but let me know if I'm missing something.
- DelegatingServiceServlet has a fair amount of plumbing that might be
useful to other servlet implementations, even if they don't want to
subclass DelegatingServiceServlet itself. Pretty much all the private
methods of DelegatingServiceServlet are just about handling the request
and the response. Should those be exposed as static methods, or moved
into another helper class, or something, to allow people like George to
reuse that code in their unrelated servlets?
- There are a couple of very minor questions I had as I went through
this. In the attached sources, I flagged these with comments: "//
[RobJ: question question]" I'll remove these comments before final
submission.
- First, the comment to isImplementedRemoteServiceInterface says "Used
to determine whether the specified interface name is implemented by
this class, without loading the class (for security)." But the method
actually does "getClass()" which effectively *does* load the class. So
I was confused by this comment. Should it read, "without deserializing
the full payload"?
- Second, in RPCCall.createResponse (previously
RemoteServiceServlet.createResponse), there is a line "responseObj =
e;" that is apparently a no-op. Am I missing the point of that line
somehow, or could it just be removed?
Finally, are there any other stylistic, semantic, conceptual, or
procedural concerns anyone has with this code as it stands, or with how
I'm going about developing this patch?
As I said, I'm more than happy to keep discussing this and iterating on
it; I just got excited and wanted to hack :-) But I'm glad to revise
things, perhaps extensively, if that's desired. Also, now that I have
this apparently mostly working, I'll start modifying g4jsf to use it as
a first test case of how it can be integrated, and some further changes
might come out of that.
Bruce, Miguel, et al., please let me know when you get my contributor's
release form? Also, please don't apply this patch yet! Ideally I'd
like another week to two weeks to actually get it working in g4jsf. (I
also assume this is too major a change to get into 1.3rc2.)
I'm just glad you guys didn't get to this yet, so I got to play :-D
Cheers!
Rob Jellinghaus
Rob Jellinghaus
signature of RPCCall.invoke.
The problem is that RPCCall wants to encapsulate the invocation, since
that enables it to encapsulate the serializing stream writer that's
used to serialize the method return value.
But if it encapsulates the invocation, it has to deal with issues
around exception handling. And if the invocation throws an exception
that the method doesn't expect, RPCCall.invoke somehow has to signal
that to the code that's calling RPCCall.invoke (so it can be server
logged or whatever else is right). RPCCall.invoke partly wants to
return the serialized payload, but it also needs to return the
unexpected exception if any.
Since the exception could be anything, RPCCall.invoke could "throw
Throwable" -- but that's so horrible! And it also makes it impossible
for it to throw SerializedException. So I took the path you see here,
where RPCCall.invoke returns Throwable (the unexpected exception, if
any), and you later get the payload via RPCCall.getResponsePayload.
But like I said, it still smells funny. Any other ideas welcome.
Cheers!
Rob Jellinghaus
Scott Blum
> Since the exception could be anything, RPCCall.invoke could "throw
> Throwable" -- but that's so horrible! And it also makes it impossible
> for it to throw SerializedException. So I took the path you see here,
> where RPCCall.invoke returns Throwable (the unexpected exception, if
> any), and you later get the payload via RPCCall.getResponsePayload.
>
> But like I said, it still smells funny. Any other ideas welcome.
What if you always throw an InvocationTargetException wrapping the
real exception, and it's the caller's job to unwrap it?
Scott
Rob Jellinghaus
> What if you always throw an InvocationTargetException wrapping the
> real exception, and it's the caller's job to unwrap it?
That sounds good, I'll do that. Thanks!