camel-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Willem Jiang <willem.ji...@gmail.com>
Subject Re: svn commit: r1339962 - /camel/trunk/camel-core/src/main/java/org/apache/camel/util/ObjectHelper.java
Date Mon, 21 May 2012 14:05:14 GMT
Sorry, I didn't bake the idea with enough tests.

I dig the first change of ObjectHelper about checking the empty DOM[1]
But I don't think the change on the ObjectHelper is related to the patch 
as when I revert the patch on the ObjectHelper, the 
XPathContentBasedRouterTest is passed.

I just committed a patch[2] of the unit test to show an interesting example

<person name="claus"/>
<person>claus</person>

I think the below two element is represent same person but the first one 
will be treat as false without applying the patch.

Any thought?


[1]https://issues.apache.org/jira/browse/CAMEL-3531
[2]http://svn.apache.org/viewvc?rev=1341029&view=rev

On Sat May 19 17:22:25 2012, Claus Ibsen wrote:
> Hi
>
> 1)
> There *should* be an unit test when we change behavior in a *core*
> feature such as this.
>
> 2)
> A lot of Camel users parses XML files and whatnot, and evaluates
> predicates and expressions.
> So this is a change in behavior. And this behavior has been running
> for this for a long time.
>
> I do not like that this is backported to patch releases.
> The patch releases *must* be stable and rely on current behavior.
>
> 3)
> And I think this should be disussed a bit more before committing a
> fix. As its a *core* change.
> Why is an attribute on a root tag suddenly matter if its regarded as
> true or false?
>
> If you have tags as follows.
>
> a)
> <users>
> </users>
>
> b)
> <users type="admin">
> </users>
>
> In my mind both of these is empty, and therefore false.
>
> I think the old behavior is the correct. This is how Camel have run
> all the time.
>
> Personally I think we should consider reverting and keeping the old behavior.
> Unless there is a good use-case saying otherwise.
> I have not yet seen this.
>
>
>
>
> On Fri, May 18, 2012 at 5:13 AM,<ningjiang@apache.org>  wrote:
>> Author: ningjiang
>> Date: Fri May 18 03:13:35 2012
>> New Revision: 1339962
>>
>> URL: http://svn.apache.org/viewvc?rev=1339962&view=rev
>> Log:
>> CAMEL-5276 Fix the issue that the ObjectHelper will return false for a node without
children
>>
>> Modified:
>>     camel/trunk/camel-core/src/main/java/org/apache/camel/util/ObjectHelper.java
>>
>> Modified: camel/trunk/camel-core/src/main/java/org/apache/camel/util/ObjectHelper.java
>> URL: http://svn.apache.org/viewvc/camel/trunk/camel-core/src/main/java/org/apache/camel/util/ObjectHelper.java?rev=1339962&r1=1339961&r2=1339962&view=diff
>> ==============================================================================
>> --- camel/trunk/camel-core/src/main/java/org/apache/camel/util/ObjectHelper.java
(original)
>> +++ camel/trunk/camel-core/src/main/java/org/apache/camel/util/ObjectHelper.java
Fri May 18 03:13:35 2012
>> @@ -1227,7 +1227,10 @@ public final class ObjectHelper {
>>                  return false;
>>              }
>>          } else if (value instanceof NodeList) {
>> -            // is it an empty dom
>> +            // is it an empty dom with empty attributes
>> +            if (value instanceof Node&&  ((Node)value).hasAttributes())
{
>> +                return true;
>> +            }
>>              NodeList list = (NodeList) value;
>>              return list.getLength()>  0;
>>          } else if (value instanceof Collection) {
>>
>>
>
>
>


-- 
Willem
----------------------------------
CamelOne 2012 Conference, May 15-16, 2012: http://camelone.com
FuseSource
Web: http://www.fusesource.com
Blog:    http://willemjiang.blogspot.com (English)
          http://jnn.javaeye.com (Chinese)
Twitter: willemjiang
Weibo: willemjiang

Mime
View raw message