Simon Laws wrote:
Comments inline....
snip...
and that's OK I think :) as binding.ws and binding.jsonrpc could end up
on different port numbers for example (depending on their node
configuration).
And they might not.
Actually, I was struggling to understand why we needed this test for
duplicate names at all.
Does the spec forbids two bindings with the same name?
No but it says that if the name is used as the default binding URI then
only one binding for a service can use this default.
By trying hard to detect duplicate names early on (without having all
the info about what the binding URIs will end up to be), won't we raise
errors even for cases that should work?
I agree that we don't have all of the information required to carry out this
test. Hence the TODO in the code.
One example of the issues I was running into:
<service...>
<binding.jsonrpc/>
<binding.ws uri="/whatever">
</service>
throwing a duplicate name exception as the two bindings had the same name.
On a fun note, I think that:
<service name="aservice">
<binding.jsonrpc name="whatever"/>
<binding.ws uri="/whatever">
</service>
would probably not have thrown an exception, but should have :)
Hmmm. not sure but you could be right.
So my recommendation would be to not try too hard to detect these
duplicates early on based on binding names (as the detection algorithm
is too fragile at that point), instead detect duplicates at the very end
when we see that we end up with duplicate URIs.
+1. I'll add a new method to scan the model for duplicate URI that can be
called independently.
+1 this will be simpler and more reliable.
Eventually all the code that calculates binding URIs (in the
Axis2ServiceBindingProvider for example) will have to move to the model
processing phase as well, as already discussed.
Also can you give a quick example the problems you found relating to
"avoid
adding binding name to itself, and consider binding URI when specified
in
the composite in the single service case too" so I can understand the
other
code changes.
One issue was that the code was not considering the binding URI when
there was only one service on a component, for example IIRC:
<component name="Store">
<service name="Widget">
<binding.http uri="/ui"/>
</service>
</component>
was bound to /Store (the component URI) instead of /ui.
I understand that the binding name should be omitted from the computed
URI in the single service case, but the binding URI should be considered
if specified, leading to the requirement to distinguish between binding
name (always know, not always considered) and binding URI (not always
specified, always considered when specified).
Ok. Thank you. I had interpreted the spec incorrectly here. I had mistakenly
read assembly spec line 2375 as "Where a component has only a single
service, the value of the Service Binding URI is null" instead of what it
actually says, which is "Where a component has only a single service, the *
default* value of the Service Binding URI is null".
About the "adding the binding name to itself" part, this is a little
complicated. IIRC the issue was that CompositeConfigurationBuilder would
first convert:
<component name="Foo">
<service...>
<binding.sca/>
</service>
</component>
into:
<component name="Foo">
<service...>
<binding.sca uri="Foo"/>
</service>
</component>
Then when an SCANode would consume that, CompositeBuilder.build would
turn it into:
<component name="Foo">
<service...>
<binding.sca uri="Foo/Foo"/>
</service>
</component>
as I think the old CompositeConfigurationBuilder code used in that case
used to concatenate the component URI and the binding URI.
OK, I see. So this was a combination of the old and the new causing
problems. I see that you now call the old after the new has run so am
assuming that your changes are making this behave now. Let me know if not.
Yes the latest code handles that correctly now.
Thanks
Simon
--
Jean-Sebastien
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
