Commons JXPath
  1. Commons JXPath
  2. JXPATH-1

getValue() and iterate() are not consistent

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None
    • Environment:

      Operating System: Linux
      Platform: PC

      Description

      On xpath expression which are not simple path (containing * or //), getValue()
      does not always find a value. Using iterate() is a workaround.

      example:

      import java.util.HashMap;
      import java.util.Iterator;
      import java.util.Map;

      import org.apache.commons.jxpath.JXPathContext;

      /**

      • @author Yann Duponchel
        */
        public class JXPathRecursiveMap {

      public static String tests[] =

      { "n2/n3/n6", "n2//n6", // BUG "n2/n3/n6/n7", "n2/*/*/n7", // BUG "n2//n7", // BUG "n2//n7/bytes", }

      ;

      public static void main(String args[]) {
      Map tree = new HashMap();
      Map nodeC, nodeP;
      tree.put("n1", "n1-0");
      nodeC = new HashMap();
      tree.put("n2", nodeC);
      nodeP = nodeC;
      nodeC = new HashMap();
      nodeP.put("n3", nodeC);
      nodeP = nodeC;
      nodeP.put("n4", new HashMap());
      nodeP.put("n5", "n3-1");
      nodeC = new HashMap();
      nodeP.put("n6", nodeC);
      nodeP = nodeC;
      nodeP.put("n7", "n4-0");
      nodeC = new HashMap();
      tree.put("n8", nodeC);
      nodeP = nodeC;
      nodeC = new HashMap();
      nodeP.put("n9", nodeC);
      nodeP = nodeC;
      nodeP.put("n10", "n2-1");

      System.out.println(tree);
      System.out.println();

      JXPathContext context = JXPathContext.newContext(tree);
      for (int i = 0; i < tests.length; i++) {
      Object result = context.getValue(tests[i]);
      if (result != null)
      System.out.println("getValue(" + tests[i] + ") = " + result);
      else
      System.out.println("getValue(" + tests[i] + ") = null");
      System.out.print("iterate(" + tests[i] + ") = ");
      for (Iterator it = context.iterate(tests[i]); it.hasNext()

      { result = it.next(); System.out.print(result + "; "); }

      System.out.println();
      }
      }
      }

      ============================================================

      The previous example produces this output:

      {n1=n1-0, n8={n9={n10=n2-1}}, n2={n3={n6=

      {n7=n4-0}, n4={}, n5=n3-1}}}

      getValue(n2/n3/n6) = {n7=n4-0}

      iterate(n2/n3/n6) =

      {n7=n4-0};

      getValue(n2//n6) = null
      iterate(n2//n6) = {n7=n4-0}

      ;

      getValue(n2/n3/n6/n7) = n4-0
      iterate(n2/n3/n6/n7) = n4-0;

      getValue(n2///n7) = null
      iterate(n2///n7) = n4-0;

      getValue(n2//n7) = null
      iterate(n2//n7) = n4-0;

      getValue(n2//n7/bytes) = [B@16cd7d5
      iterate(n2//n7/bytes) = 110; 52; 45; 48;

      ============================================================
      The problem seems to be the following:
      context.getValue() calls getSingleNodePointerForSteps() which calls
      getSingleNodePointer() and then nextNode() once to get the first matching node,
      while context.iterate().next() calls next() through the ValueIterator.

      And the real problem is that next() and nextNode() are very clearly different!
      next() is defined on Expression while nextNode() is defined on DescendantContext...

      At least, this is my impression.

      Yann Duponchel

      ============================================================

      Here is a trivial patch of org.apache.commons.jxpath.ri.compiler.Path
      (seems to works, but clearly not optimal)

      protected Pointer getSingleNodePointerForSteps(EvalContext context) {
      if (steps.length == 0)

      { return context.getSingleNodePointer(); }

      if (isSimplePath())

      { NodePointer ptr = (NodePointer) context.getSingleNodePointer(); return SimplePathInterpreter.interpretSimpleLocationPath( context, ptr, steps); }

      else

      { // return searchForPath(context); // Identical code as evalSteps() - YD //return searchForPathYD(context).getSingleNodePointer(); // Equivalent to previous code but still buggus - YD context = searchForPathYD(context); return (Pointer) (context.hasNext() ? context.next() : null); // - YD }

      }

      private EvalContext searchForPathYD(EvalContext context) {
      for (int i = 0; i < steps.length; i++) {
      context =
      createContextForStep(
      context,
      steps[i].getAxis(),
      steps[i].getNodeTest());
      Expression predicates[] = steps[i].getPredicates();
      if (predicates != null) {
      for (int j = 0; j < predicates.length; j++)

      { context = new PredicateContext(context, predicates[j]); }

      }
      }
      return context;
      }

      protected EvalContext evalSteps(EvalContext context)

      { return searchForPathYD(context); }

        Activity

        Hide
        Dmitri Plotnikov added a comment -

        The problem turned out to be much deeper than what the provide patch would
        indicate. The crux of the issue was that DynamicPropertyIterator was based on
        the assumption that a Map "contains" any property you would ask for. For
        example, if you have a JavaBean that has no property called "foo", the
        path "/foo" throws an exception - there is no such property. However, if
        instead of a JavaBean we have a Map, the path "/foo" does not throw an
        exception. Rater, it returns null. The property exists, but has not been
        initialized.

        There is nothing wrong with this logic until you start searching with paths
        like "//foo". As soon as the search encounters a Map, it stops because the
        Map "contains" a "property" called "foo". That of course is unacceptable, so I
        had to make some changes here and there to differentiate between regular
        traversal like "/foo" and search like "//foo".

        Show
        Dmitri Plotnikov added a comment - The problem turned out to be much deeper than what the provide patch would indicate. The crux of the issue was that DynamicPropertyIterator was based on the assumption that a Map "contains" any property you would ask for. For example, if you have a JavaBean that has no property called "foo", the path "/foo" throws an exception - there is no such property. However, if instead of a JavaBean we have a Map, the path "/foo" does not throw an exception. Rater, it returns null. The property exists, but has not been initialized. There is nothing wrong with this logic until you start searching with paths like "//foo". As soon as the search encounters a Map, it stops because the Map "contains" a "property" called "foo". That of course is unacceptable, so I had to make some changes here and there to differentiate between regular traversal like "/foo" and search like "//foo".

          People

          • Assignee:
            Unassigned
            Reporter:
            Yann Duponchel
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development