You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tuscany.apache.org by rf...@apache.org on 2012/03/21 22:49:52 UTC

svn commit: r1303591 - /tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java

Author: rfeng
Date: Wed Mar 21 21:49:52 2012
New Revision: 1303591

URL: http://svn.apache.org/viewvc?rev=1303591&view=rev
Log:
Revert the change based on the comment from https://issues.apache.org/jira/browse/TUSCANY-4029

Modified:
    tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java

Modified: tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
URL: http://svn.apache.org/viewvc/tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java?rev=1303591&r1=1303590&r2=1303591&view=diff
==============================================================================
--- tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java (original)
+++ tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java Wed Mar 21 21:49:52 2012
@@ -338,6 +338,11 @@ public class EndpointReferenceBinderImpl
 
         }
         
+        // [rfeng] Setup the target endpoint if the reference uses an explicit binding
+        if (endpointReference.getTargetEndpoint().getBinding() == null) {
+            endpointReference.getTargetEndpoint().setBinding(endpointReference.getBinding());
+        }
+        
         // Now the endpoint reference is resolved check that the binding interfaces contract
         // and the reference contract are compatible
         try {
@@ -500,12 +505,16 @@ public class EndpointReferenceBinderImpl
         } else {
             endpointReference.setTargetEndpoint(matchedEndpoint);
             Binding binding = matchedEndpoint.getBinding();
+            // Reverted the change, see https://issues.apache.org/jira/browse/TUSCANY-4029
+            /*
             try {
 				endpointReference.setBinding((Binding) binding.clone());
 			} catch (CloneNotSupportedException e) {
 				// shouldn't happen
 				throw new RuntimeException(e);
 			}
+	   */
+            endpointReference.setBinding(binding);
             // TUSCANY-3873 - add policy from the service
             //                we don't care about intents at this stage
             endpointReference.getPolicySets().addAll(matchedEndpoint.getPolicySets());
@@ -528,6 +537,7 @@ public class EndpointReferenceBinderImpl
             endpointReference.setStatus(EndpointReference.Status.WIRED_TARGET_FOUND_AND_MATCHED);
             endpointReference.setUnresolved(false);
         }
+        
     }
 
     private void build(EndpointReference endpointReference) {



Re: svn commit: r1303591 - /tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java

Posted by ant elder <an...@gmail.com>.
Come on Raymond be a bit nice.  The Web Service binding worked, there
are tests that uses it with @target, the Hazelcast binding too.
Whether or not its actually possible with the protocol i doubt anyone
would ever use the jsonp binding like that as it makes no sense, atom
is likely similar, and i doubt there are many real world use cases
where JMS would be used like that either as the client and service
ends usually would need quite different configuration. So they don't
ALL need updating. In fact i vaguely remember when @target came up for
discussion on the ML in the past there were some strong arguments made
for only supporting it with binding.sca anyway. And presently there is
nothing to change as you've reverted the update, if you're agreeing to
put it back I'm happy to help make any other updates.

   ...ant

On Thu, Mar 22, 2012 at 5:56 PM, Raymond Feng <en...@gmail.com> wrote:
> I'm making one final clarification before wasting more time:
>
> As I pointed out, the break you introduced applied to most if not all reference binding invokers. Please don't try to hint it's only RESTBindingInvoker.
>
> Now I'll go to fix ALL reference binding invokers.
>
> Raymond Feng
> Sent from my iPhone
>
> On Mar 22, 2012, at 10:11 AM, ant elder <an...@gmail.com> wrote:
>
>> Come on Raymond if anyone is playing games it is you by reverting
>> changes, comments inline below:
>>
>>   ...ant
>>
>> On Thu, Mar 22, 2012 at 4:38 PM, Raymond Feng <en...@gmail.com> wrote:
>>> Ant, please STOP playing the "veto" game here.
>>>
>>> 1. Your change broke most if not all the binding invokers that make outbound
>>> connections using the binding uri even though the failures were not directly
>>> exposed due to lack of test coverage which we should improve.
>>>
>>
>> The change did not break the Tuscany build, or any tests. If you have
>> a particular use case that is not part of that you should add some
>> tests to Tuscany for it. At the very least you should be less upset
>> when a trunk change impacts your non-tested use cases while you don't
>> have tests for them in the build.
>>
>>> 2. The Endpoint concept was introduced later in the cycle. As a result, most
>>> of the reference binding invokers still depend on the behavior that the
>>> reference binding is set with the deployed service endpoint uri. We can
>>> argue if it's good or bad but we need to fix them before another bug fix
>>> regressed the code so much. I will try to fix the binding invokers but it
>>> will take a bit time.
>>>
>>
>> Ok finally that gives a hint at what your issue is - so are you
>> agreeing now that the Rest invoker just hasn't kept up with trunk dev
>> and now has a bug that needs fixing along the lines of what i've
>> suggested in TUSCANY-4029?
>>
>>> 3. I have the same veto right too. The only thing is that my revert didn't
>>> completely roll back all your changes including the failing compliance test
>>> if it's added recently. I explained in the e-mail why I need to revert your
>>> change. Maybe I should say -1 or veto your change too to make it clear.
>>>
>>
>> Yes you could, but you need a technical reason to support it and then
>> you leave the actual reverting to be done by the person you're
>> vetoing. And the technical reason for this objection here is seeming
>> very hard to get you or Luciano to actually give precise details on.
>> And BTW, the compliance test thats now broken is not a new one its
>> been there for ages, even if it was new thats shouldn't make a
>> difference - trunk is where we do new development.
>>
>>> 4. Please don't abuse/stain the Apache hat when/if you try to push for some
>>> 'private' agenda.
>>>
>>> Sorry for being harsh here but otherwise we'll be painted as anti-Apache if
>>> we have technical disagreement.
>>>
>>
>> I don't know about anti-Apache but it doesn't feel particularly
>> community spirited with this aggressive over reaction that i'm getting
>> from you and Luciano for a change that didn't even break anything
>> within Tuscany. If we're ever going to encourage others to come
>> participate in development here, especially as volunteers in their own
>> time, we need to have a more friendly approach when you don't like or
>> understand something.
>>
>>   ...ant

Re: svn commit: r1303591 - /tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java

Posted by Raymond Feng <en...@gmail.com>.
I'm making one final clarification before wasting more time:

As I pointed out, the break you introduced applied to most if not all reference binding invokers. Please don't try to hint it's only RESTBindingInvoker.

Now I'll go to fix ALL reference binding invokers.

Raymond Feng
Sent from my iPhone

On Mar 22, 2012, at 10:11 AM, ant elder <an...@gmail.com> wrote:

> Come on Raymond if anyone is playing games it is you by reverting
> changes, comments inline below:
> 
>   ...ant
> 
> On Thu, Mar 22, 2012 at 4:38 PM, Raymond Feng <en...@gmail.com> wrote:
>> Ant, please STOP playing the "veto" game here.
>> 
>> 1. Your change broke most if not all the binding invokers that make outbound
>> connections using the binding uri even though the failures were not directly
>> exposed due to lack of test coverage which we should improve.
>> 
> 
> The change did not break the Tuscany build, or any tests. If you have
> a particular use case that is not part of that you should add some
> tests to Tuscany for it. At the very least you should be less upset
> when a trunk change impacts your non-tested use cases while you don't
> have tests for them in the build.
> 
>> 2. The Endpoint concept was introduced later in the cycle. As a result, most
>> of the reference binding invokers still depend on the behavior that the
>> reference binding is set with the deployed service endpoint uri. We can
>> argue if it's good or bad but we need to fix them before another bug fix
>> regressed the code so much. I will try to fix the binding invokers but it
>> will take a bit time.
>> 
> 
> Ok finally that gives a hint at what your issue is - so are you
> agreeing now that the Rest invoker just hasn't kept up with trunk dev
> and now has a bug that needs fixing along the lines of what i've
> suggested in TUSCANY-4029?
> 
>> 3. I have the same veto right too. The only thing is that my revert didn't
>> completely roll back all your changes including the failing compliance test
>> if it's added recently. I explained in the e-mail why I need to revert your
>> change. Maybe I should say -1 or veto your change too to make it clear.
>> 
> 
> Yes you could, but you need a technical reason to support it and then
> you leave the actual reverting to be done by the person you're
> vetoing. And the technical reason for this objection here is seeming
> very hard to get you or Luciano to actually give precise details on.
> And BTW, the compliance test thats now broken is not a new one its
> been there for ages, even if it was new thats shouldn't make a
> difference - trunk is where we do new development.
> 
>> 4. Please don't abuse/stain the Apache hat when/if you try to push for some
>> 'private' agenda.
>> 
>> Sorry for being harsh here but otherwise we'll be painted as anti-Apache if
>> we have technical disagreement.
>> 
> 
> I don't know about anti-Apache but it doesn't feel particularly
> community spirited with this aggressive over reaction that i'm getting
> from you and Luciano for a change that didn't even break anything
> within Tuscany. If we're ever going to encourage others to come
> participate in development here, especially as volunteers in their own
> time, we need to have a more friendly approach when you don't like or
> understand something.
> 
>   ...ant

Re: svn commit: r1303591 - /tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java

Posted by ant elder <an...@gmail.com>.
Come on Raymond if anyone is playing games it is you by reverting
changes, comments inline below:

   ...ant

On Thu, Mar 22, 2012 at 4:38 PM, Raymond Feng <en...@gmail.com> wrote:
> Ant, please STOP playing the "veto" game here.
>
> 1. Your change broke most if not all the binding invokers that make outbound
> connections using the binding uri even though the failures were not directly
> exposed due to lack of test coverage which we should improve.
>

The change did not break the Tuscany build, or any tests. If you have
a particular use case that is not part of that you should add some
tests to Tuscany for it. At the very least you should be less upset
when a trunk change impacts your non-tested use cases while you don't
have tests for them in the build.

> 2. The Endpoint concept was introduced later in the cycle. As a result, most
> of the reference binding invokers still depend on the behavior that the
> reference binding is set with the deployed service endpoint uri. We can
> argue if it's good or bad but we need to fix them before another bug fix
> regressed the code so much. I will try to fix the binding invokers but it
> will take a bit time.
>

Ok finally that gives a hint at what your issue is - so are you
agreeing now that the Rest invoker just hasn't kept up with trunk dev
and now has a bug that needs fixing along the lines of what i've
suggested in TUSCANY-4029?

> 3. I have the same veto right too. The only thing is that my revert didn't
> completely roll back all your changes including the failing compliance test
> if it's added recently. I explained in the e-mail why I need to revert your
> change. Maybe I should say -1 or veto your change too to make it clear.
>

Yes you could, but you need a technical reason to support it and then
you leave the actual reverting to be done by the person you're
vetoing. And the technical reason for this objection here is seeming
very hard to get you or Luciano to actually give precise details on.
And BTW, the compliance test thats now broken is not a new one its
been there for ages, even if it was new thats shouldn't make a
difference - trunk is where we do new development.

> 4. Please don't abuse/stain the Apache hat when/if you try to push for some
> 'private' agenda.
>
> Sorry for being harsh here but otherwise we'll be painted as anti-Apache if
> we have technical disagreement.
>

I don't know about anti-Apache but it doesn't feel particularly
community spirited with this aggressive over reaction that i'm getting
from you and Luciano for a change that didn't even break anything
within Tuscany. If we're ever going to encourage others to come
participate in development here, especially as volunteers in their own
time, we need to have a more friendly approach when you don't like or
understand something.

   ...ant

Re: svn commit: r1303591 - /tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java

Posted by Raymond Feng <en...@gmail.com>.
Ant, please STOP playing the "veto" game here.

1. Your change broke most if not all the binding invokers that make
outbound connections using the binding uri even though the failures were
not directly exposed due to lack of test coverage which we should improve.

2. The Endpoint concept was introduced later in the cycle. As a result,
most of the reference binding invokers still depend on the behavior that
the reference binding is set with the deployed service endpoint uri. We can
argue if it's good or bad but we need to fix them before another bug fix
regressed the code so much. I will try to fix the binding invokers but it
will take a bit time.

3. I have the same veto right too. The only thing is that my revert didn't
completely roll back all your changes including the failing compliance test
if it's added recently. I explained in the e-mail why I need to revert your
change. Maybe I should say -1 or veto your change too to make it clear.

4. Please don't abuse/stain the Apache hat when/if you try to push for some
'private' agenda.

Sorry for being harsh here but otherwise we'll be painted as anti-Apache if
we have technical disagreement.

Thanks,
Raymond

On Thu, Mar 22, 2012 at 7:50 AM, ant elder <an...@gmail.com> wrote:

> On Thu, Mar 22, 2012 at 2:46 PM, Luciano Resende <lu...@gmail.com>
> wrote:
> >
> >
> > On Thursday, March 22, 2012, ant elder <an...@gmail.com> wrote:
> >> On Wed, Mar 21, 2012 at 9:49 PM,  <rf...@apache.org> wrote:
> >>> Author: rfeng
> >>> Date: Wed Mar 21 21:49:52 2012
> >>> New Revision: 1303591
> >>>
> >>> URL: http://svn.apache.org/viewvc?rev=1303591&view=rev
> >>> Log:
> >>> Revert the change based on the comment from
> >>> https://issues.apache.org/jira/browse/TUSCANY-4029
> >>>
> >>> Modified:
> >>>
> >>>
>  tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
> >>>
> >>> Modified:
> >>>
> tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
> >>> URL:
> >>>
> http://svn.apache.org/viewvc/tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java?rev=1303591&r1=1303590&r2=1303591&view=diff
> >>>
> >>>
> ==============================================================================
> >>> ---
> >>>
> tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
> >>> (original)
> >>> +++
> >>>
> tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
> >>> Wed Mar 21 21:49:52 2012
> >>> @@ -338,6 +338,11 @@ public class EndpointReferenceBinderImpl
> >>>
> >>>         }
> >>>
> >>> +        // [rfeng] Setup the target endpoint if the reference uses an
> >>> explicit binding
> >>> +        if (endpointReference.getTargetEndpoint().getBinding() ==
> null)
> >>> {
> >>> +
> >>>
>  endpointReference.getTargetEndpoint().setBinding(endpointReference.getBinding());
> >>> +        }
> >>> +
> >>>         // Now the endpoint reference is resolved check that the
> binding
> >>> interfaces contract
> >>>         // and the reference contract are compatible
> >>>         try {
> >>> @@ -500,12 +505,16 @@ public class EndpointReferenceBinderImpl
> >>>         } else {
> >>>             endpointReference.setTargetEndpoint(matchedEndpoint);
> >>>             Binding binding = matchedEndpoint.getBinding();
> >>> +            // Reverted the change, see
> >>> https://issues.apache.org/jira/browse/TUSCANY-4029
> >>> +            /*
> >>>             try {
> >>>                                endpointReference.setBinding((Binding)
> >>> binding.clone());
> >>>                        } catch (CloneNotSupportedException e) {
> >>>                                // shouldn't happen
> >>>                                throw new RuntimeException(e);
> >>>                        }
> >>> +          */
> >>> +            endpointReference.setBinding(binding);
> >>>             // TUSCANY-3873 - add policy from the service
> >>>             //                we don't care about intents at this stage
> >>>
> >>>
> endpointReference.getPolicySets().addAll(matchedEndpoint.getPolicySets());
> >>> @@ -528,6 +537,7 @@ public class EndpointReferenceBinderImpl
> >>>
> >>>
> endpointReference.setStatus(EndpointReference.Status.WIRED_TARGET_FOUND_AND_MATCHED);
> >>>          Raymond, as discussed in TUSCANY-4029 and on the IM chat we
> had
> >> yesterday i'm -1 on your commit. It doesn't fix the problem which
> >> TUSCANY-4029 addresses and it also breaks the OASIS compliance tests.
> >>
> >> If there are any issues with the approach suggested in TUSCANY-4029
> >> then lets discuss it here or you're welcome to IM ping me to more
> >> quickly find a solution that works for everyone.
> >>
> >>   ...ant
> >>
> >
> > Please, let's keep these technical discussions on the mailing list.
> >
> > BTW, how come it breakes the compliance test ? It has been like this
> forever
> > and the tests were passing fine.
> >
> > In the same way, your fix seems to break other stuff as well as
> commented on
> > the other thread, which I'm -1 on having all that broken.
> >
>
> Can you provide more details about what other stuff is broken? The
> build works, does the update suggested in TUSCANY-4029 fix the problem
> with the rest invoker?
>
>   ...ant
>

Re: svn commit: r1303591 - /tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java

Posted by ant elder <an...@gmail.com>.
On Thu, Mar 22, 2012 at 2:46 PM, Luciano Resende <lu...@gmail.com> wrote:
>
>
> On Thursday, March 22, 2012, ant elder <an...@gmail.com> wrote:
>> On Wed, Mar 21, 2012 at 9:49 PM,  <rf...@apache.org> wrote:
>>> Author: rfeng
>>> Date: Wed Mar 21 21:49:52 2012
>>> New Revision: 1303591
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1303591&view=rev
>>> Log:
>>> Revert the change based on the comment from
>>> https://issues.apache.org/jira/browse/TUSCANY-4029
>>>
>>> Modified:
>>>
>>>  tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
>>>
>>> Modified:
>>> tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
>>> URL:
>>> http://svn.apache.org/viewvc/tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java?rev=1303591&r1=1303590&r2=1303591&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
>>> (original)
>>> +++
>>> tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
>>> Wed Mar 21 21:49:52 2012
>>> @@ -338,6 +338,11 @@ public class EndpointReferenceBinderImpl
>>>
>>>         }
>>>
>>> +        // [rfeng] Setup the target endpoint if the reference uses an
>>> explicit binding
>>> +        if (endpointReference.getTargetEndpoint().getBinding() == null)
>>> {
>>> +
>>>  endpointReference.getTargetEndpoint().setBinding(endpointReference.getBinding());
>>> +        }
>>> +
>>>         // Now the endpoint reference is resolved check that the binding
>>> interfaces contract
>>>         // and the reference contract are compatible
>>>         try {
>>> @@ -500,12 +505,16 @@ public class EndpointReferenceBinderImpl
>>>         } else {
>>>             endpointReference.setTargetEndpoint(matchedEndpoint);
>>>             Binding binding = matchedEndpoint.getBinding();
>>> +            // Reverted the change, see
>>> https://issues.apache.org/jira/browse/TUSCANY-4029
>>> +            /*
>>>             try {
>>>                                endpointReference.setBinding((Binding)
>>> binding.clone());
>>>                        } catch (CloneNotSupportedException e) {
>>>                                // shouldn't happen
>>>                                throw new RuntimeException(e);
>>>                        }
>>> +          */
>>> +            endpointReference.setBinding(binding);
>>>             // TUSCANY-3873 - add policy from the service
>>>             //                we don't care about intents at this stage
>>>
>>> endpointReference.getPolicySets().addAll(matchedEndpoint.getPolicySets());
>>> @@ -528,6 +537,7 @@ public class EndpointReferenceBinderImpl
>>>
>>> endpointReference.setStatus(EndpointReference.Status.WIRED_TARGET_FOUND_AND_MATCHED);
>>>          Raymond, as discussed in TUSCANY-4029 and on the IM chat we had
>> yesterday i'm -1 on your commit. It doesn't fix the problem which
>> TUSCANY-4029 addresses and it also breaks the OASIS compliance tests.
>>
>> If there are any issues with the approach suggested in TUSCANY-4029
>> then lets discuss it here or you're welcome to IM ping me to more
>> quickly find a solution that works for everyone.
>>
>>   ...ant
>>
>
> Please, let's keep these technical discussions on the mailing list.
>
> BTW, how come it breakes the compliance test ? It has been like this forever
> and the tests were passing fine.
>
> In the same way, your fix seems to break other stuff as well as commented on
> the other thread, which I'm -1 on having all that broken.
>

Can you provide more details about what other stuff is broken? The
build works, does the update suggested in TUSCANY-4029 fix the problem
with the rest invoker?

   ...ant

Re: svn commit: r1303591 - /tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java

Posted by Luciano Resende <lu...@gmail.com>.
On Thursday, March 22, 2012, ant elder <an...@gmail.com> wrote:
> On Wed, Mar 21, 2012 at 9:49 PM,  <rf...@apache.org> wrote:
>> Author: rfeng
>> Date: Wed Mar 21 21:49:52 2012
>> New Revision: 1303591
>>
>> URL: http://svn.apache.org/viewvc?rev=1303591&view=rev
>> Log:
>> Revert the change based on the comment from
https://issues.apache.org/jira/browse/TUSCANY-4029
>>
>> Modified:
>>
 tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
>>
>> Modified:
tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
>> URL:
http://svn.apache.org/viewvc/tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java?rev=1303591&r1=1303590&r2=1303591&view=diff
>>
==============================================================================
>> ---
tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
(original)
>> +++
tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
Wed Mar 21 21:49:52 2012
>> @@ -338,6 +338,11 @@ public class EndpointReferenceBinderImpl
>>
>>         }
>>
>> +        // [rfeng] Setup the target endpoint if the reference uses an
explicit binding
>> +        if (endpointReference.getTargetEndpoint().getBinding() == null)
{
>> +
 endpointReference.getTargetEndpoint().setBinding(endpointReference.getBinding());
>> +        }
>> +
>>         // Now the endpoint reference is resolved check that the binding
interfaces contract
>>         // and the reference contract are compatible
>>         try {
>> @@ -500,12 +505,16 @@ public class EndpointReferenceBinderImpl
>>         } else {
>>             endpointReference.setTargetEndpoint(matchedEndpoint);
>>             Binding binding = matchedEndpoint.getBinding();
>> +            // Reverted the change, see
https://issues.apache.org/jira/browse/TUSCANY-4029
>> +            /*
>>             try {
>>                                endpointReference.setBinding((Binding)
binding.clone());
>>                        } catch (CloneNotSupportedException e) {
>>                                // shouldn't happen
>>                                throw new RuntimeException(e);
>>                        }
>> +          */
>> +            endpointReference.setBinding(binding);
>>             // TUSCANY-3873 - add policy from the service
>>             //                we don't care about intents at this stage
>>
endpointReference.getPolicySets().addAll(matchedEndpoint.getPolicySets());
>> @@ -528,6 +537,7 @@ public class EndpointReferenceBinderImpl
>>
endpointReference.setStatus(EndpointReference.Status.WIRED_TARGET_FOUND_AND_MATCHED);
>>          Raymond, as discussed in TUSCANY-4029 and on the IM chat we had
> yesterday i'm -1 on your commit. It doesn't fix the problem which
> TUSCANY-4029 addresses and it also breaks the OASIS compliance tests.
>
> If there are any issues with the approach suggested in TUSCANY-4029
> then lets discuss it here or you're welcome to IM ping me to more
> quickly find a solution that works for everyone.
>
>   ...ant
>

Please, let's keep these technical discussions on the mailing list.

BTW, how come it breakes the compliance test ? It has been like this
forever and the tests were passing fine.

In the same way, your fix seems to break other stuff as well as commented
on the other thread, which I'm -1 on having all that broken.


-- 
Luciano Resende
http://people.apache.org/~lresende
http://twitter.com/lresende1975
http://lresende.blogspot.com/

Re: svn commit: r1303591 - /tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java

Posted by ant elder <an...@gmail.com>.
On Wed, Mar 21, 2012 at 9:49 PM,  <rf...@apache.org> wrote:
> Author: rfeng
> Date: Wed Mar 21 21:49:52 2012
> New Revision: 1303591
>
> URL: http://svn.apache.org/viewvc?rev=1303591&view=rev
> Log:
> Revert the change based on the comment from https://issues.apache.org/jira/browse/TUSCANY-4029
>
> Modified:
>    tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
>
> Modified: tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java
> URL: http://svn.apache.org/viewvc/tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java?rev=1303591&r1=1303590&r2=1303591&view=diff
> ==============================================================================
> --- tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java (original)
> +++ tuscany/sca-java-2.x/trunk/modules/core/src/main/java/org/apache/tuscany/sca/core/runtime/impl/EndpointReferenceBinderImpl.java Wed Mar 21 21:49:52 2012
> @@ -338,6 +338,11 @@ public class EndpointReferenceBinderImpl
>
>         }
>
> +        // [rfeng] Setup the target endpoint if the reference uses an explicit binding
> +        if (endpointReference.getTargetEndpoint().getBinding() == null) {
> +            endpointReference.getTargetEndpoint().setBinding(endpointReference.getBinding());
> +        }
> +
>         // Now the endpoint reference is resolved check that the binding interfaces contract
>         // and the reference contract are compatible
>         try {
> @@ -500,12 +505,16 @@ public class EndpointReferenceBinderImpl
>         } else {
>             endpointReference.setTargetEndpoint(matchedEndpoint);
>             Binding binding = matchedEndpoint.getBinding();
> +            // Reverted the change, see https://issues.apache.org/jira/browse/TUSCANY-4029
> +            /*
>             try {
>                                endpointReference.setBinding((Binding) binding.clone());
>                        } catch (CloneNotSupportedException e) {
>                                // shouldn't happen
>                                throw new RuntimeException(e);
>                        }
> +          */
> +            endpointReference.setBinding(binding);
>             // TUSCANY-3873 - add policy from the service
>             //                we don't care about intents at this stage
>             endpointReference.getPolicySets().addAll(matchedEndpoint.getPolicySets());
> @@ -528,6 +537,7 @@ public class EndpointReferenceBinderImpl
>             endpointReference.setStatus(EndpointReference.Status.WIRED_TARGET_FOUND_AND_MATCHED);
>             endpointReference.setUnresolved(false);
>         }
> +
>     }
>
>     private void build(EndpointReference endpointReference) {
>
>

Raymond, as discussed in TUSCANY-4029 and on the IM chat we had
yesterday i'm -1 on your commit. It doesn't fix the problem which
TUSCANY-4029 addresses and it also breaks the OASIS compliance tests.

If there are any issues with the approach suggested in TUSCANY-4029
then lets discuss it here or you're welcome to IM ping me to more
quickly find a solution that works for everyone.

   ...ant