You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tuscany.apache.org by Jean-Sebastien Delfino <js...@apache.org> on 2010/07/20 06:46:19 UTC

Re: Regression for local method overloading, Re: svn commit: r963624 [2/2]

Raymond Feng wrote:
> Hi,
> 
> The following change breaks the local interface with overloaded methods. I add the the compatibility check back for the local operations. This is done under r964980.
> 
> Thanks,
> Raymond
> 
> @@ -224,7 +226,11 @@ public class RuntimeEndpointImpl extends
>             for (InvocationChain chain : getInvocationChains()) {
>                 Operation op = chain.getTargetOperation();
> 
> -                if (interfaceContractMapper.isCompatible(operation, op, Compatibility.SUBSET)) {
> +                // We used to check compatibility here but this is now validated when the 
> +                // chain is created. As the chain operations are the real interface types 
> +                // they may be incompatible just because they are described in different 
> +                // IDLs
> +                if (operation.getName().equals(op.getName())) {
>                     invocationChainMap.put(operation, chain);
>                     return chain;
>                 }
> 

Also, these recent changes break the support for dynamic interfaces, in 
particular:

 > +                if (operation.getName().equals(op.getName())) {

The operation name should be ignored on a dynamic interface.

The InterfaceContractMapper correctly did that. See the small change I 
just made in the 'dynamic' experimental branch [1] to make dynamic 
interfaces work again.

[1] http://www.mail-archive.com/commits@tuscany.apache.org/msg11135.html
--
Jean-Sebastien

Re: Regression for local method overloading, Re: svn commit: r963624 [2/2]

Posted by Simon Laws <si...@googlemail.com>.
On Tue, Jul 20, 2010 at 5:46 AM, Jean-Sebastien Delfino
<js...@apache.org> wrote:
> Raymond Feng wrote:
>>
>> Hi,
>>
>> The following change breaks the local interface with overloaded methods. I
>> add the the compatibility check back for the local operations. This is done
>> under r964980.
>>
>> Thanks,
>> Raymond
>>
>> @@ -224,7 +226,11 @@ public class RuntimeEndpointImpl extends
>>            for (InvocationChain chain : getInvocationChains()) {
>>                Operation op = chain.getTargetOperation();
>>
>> -                if (interfaceContractMapper.isCompatible(operation, op,
>> Compatibility.SUBSET)) {
>> +                // We used to check compatibility here but this is now
>> validated when the +                // chain is created. As the chain
>> operations are the real interface types +                // they may be
>> incompatible just because they are described in different +
>>  // IDLs
>> +                if (operation.getName().equals(op.getName())) {
>>                    invocationChainMap.put(operation, chain);
>>                    return chain;
>>                }
>>
>
> Also, these recent changes break the support for dynamic interfaces, in
> particular:
>
>> +                if (operation.getName().equals(op.getName())) {
>
> The operation name should be ignored on a dynamic interface.
>
> The InterfaceContractMapper correctly did that. See the small change I just
> made in the 'dynamic' experimental branch [1] to make dynamic interfaces
> work again.
>
> [1] http://www.mail-archive.com/commits@tuscany.apache.org/msg11135.html
> --
> Jean-Sebastien
>

Hi Raymond

I asked about the test that highlighed this issue for you on the
commit message then noticed this thread so I'm repeating the question
here. Just want to make sure we've got a full set of tests as I missed
the issue when I made the change which worries me.

Simon

-- 
Apache Tuscany committer: tuscany.apache.org
Co-author of a book about Tuscany and SCA: tuscanyinaction.com