+1 Sent from a mobile device Am 22.05.2012 10:03 schrieb "Claus Ibsen" <claus.ib...@gmail.com>:
> Hi > > Thanks for adding the unit test. We should have these tests, that > helps against regressions. > > XML nodes with and without attributes. I dont see an optional solution > that fits all use-cases. > > I suggest to add a note in the Camel 2.10 release notes about this change. > And then keep the old behavior in the 2.9 and 2.8 patch releases. Then > we dont introduce any surprises for people in production etc. > And for ppl upgrading to 2.10 they would be more okay with us > introducing this change in a minor release. But not in a patch > release. > > > > On Mon, May 21, 2012 at 4:05 PM, Willem Jiang <willem.ji...@gmail.com> > wrote: > > 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,<ningji...@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 > > > > -- > Claus Ibsen > ----------------- > CamelOne 2012 Conference, May 15-16, 2012: http://camelone.com > FuseSource > Email: cib...@fusesource.com > Web: http://fusesource.com > Twitter: davsclaus, fusenews > Blog: http://davsclaus.blogspot.com/ > Author of Camel in Action: http://www.manning.com/ibsen/ >
