Comments inline.
Simon
Jean-Sebastien Delfino wrote:
Simon,
Comments and answers inline.
Simon Nash wrote:
Here are some questions and observations that I came across while
converting the Axis2 binding to the new SPIs.
1. The binding.ws SCDL element is always resolved to Axis2.
What should be the mechanism for making this pluggable
for other binding.ws implementations liske CXF?
Not any more, <binding.ws> is now resolved in a WebServiceBinding
object, independent of Axis2.
The BindingProvider that handles it registered by the
Axis2ModuleActivator. If you wanted to use a different BindingProvider,
you could either come up with a smarter ModuleActivator (which would
select the desired one based on some criteria), or maybe simpler have
two different ModuleActivators for CXF and Axis2 and pick the
ModuleActivator you want. If you go with the option to have
ModuleActivators declared in META-INF/services files on the classpath
(which is not mandatory, it's just one way of doing this) then just
place the one you want on the classpath.
This activation-time selection works as long as we are happy to regard
this as a big global switch that is always set one way or another in a
Tuscany runtime. I'm not sure that we should completely rule out a
"mix and match" scenario where the two stacks could both be active
side by side with some services using one and some using the other.
2. Before the SPI changes, a composite reference or service
was passed to the builder. The new SPIs pass component
references and services. Is this change OK?
Yes. CompositeReferences and CompositeServices are just promotion and
configuration statements, they don't have an existence at runtime.
OK. The new code is working OK and is clearer IMO.
3. Now that TargetInvoker has been replaced by Invoker, we have lost the
optimization to avoid creating a Message object when there are no
other
interceptors in the chain. This seems a lot less efficient.
A lot less efficient == how many microseconds and instructions? :)
I think that we need to take a candid look at the actual use cases:
- Java component to Java component -> why would we even need to go
through a TargetInvoker if the interceptor chain is empty?
- X component to Y component -> why would we need to go through a Java
Proxy? if not, then why would we need to create an Object[] array??
(which is what TargetInvoker expected)
So instead of having a TargetInvoker.invoke(Object[]) a priori in the
general SPI, I'd like to look at the actual use cases, prototype a few
other options, actually measure them and count these instructions :) and
I think we can then do better than this generic TargetInvoker...
I'd like to capture this in a JIRA for now. We can revisit it when
we're not trying to get a release out. I agree that we need to look
at real scenarios and decide what are the paths that need to be
highly optimized.
4. The following line was in Axis2BindingBuilder before the SPI
changes:
URI targetURI = wsBinding.getURI() != null ?
URI.create(wsBinding.getURI()) : URI.create("foo");
targetURI was passed to the ReferenceBindingExtension constructor
and apparently was unused.
Is this code still needed?
Probably not...
OK. I commented it out and things seem to work OK.
5. The following line was in Axis2BindingBuilder before the SPI
changes:
TODO: if <binding.ws> specifies the wsdl service then should
create a service for every port
Is this remoider still needed?
I just checked the WebService spec and couldn't find a statement
confirming this behavior. I don't understand why we would create an
(Axis2) service for every WSDL port...
I'll defer to Ant's comments on this one.
6. WorkContextImpl is in an interface package. I think it should
be in an "impl" package.
Thanks. Good point. Raymond seems to have started to refactor that part.
7. DefaultCompositeActivator has typo in method name
addBindingIntercepor.
Thanks. Looks like it's fixed now.
8. Should Http chunking be on or off? There's code to turn it off,
but I've seen it tunred on when reading wire traces.
In general I think it's better to have it on. Why would we turn it off?
I saw Ant's comment and I'd like to look into this a bit more. I don't
think the code is currently consistent on this point.
9. Why was getCallbackUris() removed from the Message interface?
We do not address the components by URI anymore, and we wouldn't need a
list of URIs anyway as it should be possible to talk to any component in
an assembly without having to know a list of URIs to get to it, as a
component can give its service reference to somebody else, for callbacks
or for any other regular call.
OK.
10. I carried over a lot of code from Axis2ServiceBinding into
Axis2ServiceBinding Provider. I'm not sure how it all gets invoked,
specifically the createTargetInvoker() and invokeTarget() methods.
11. Why was the spi.wire.MessageId class removed?
A lot of dead code was removed. Having the Axis2 binding out of the
build for a while didn't really help track its dependencies... Do you
need it in the Axis2 binding? If you do, and even if we put it back from
the SVN history, IMHO it's broken and needs to be rewritten, as - if I
remember correctly - it was just getting the current time in
milliseconds to form a unique ID... not exactly the best way to form a
unique ID I guess :)
Yes, the Axis2 binding was using this before the SPI refactor. It may
be broken but is better than nothing. At least it's a placeholder for
coming back to revisit later with a better algorithm. I'll restore the
previous implementation as a private class within the Axis2 binding
for now.
Simon
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]