Commons JXPath
  1. Commons JXPath
  2. JXPATH-50

does not properly handle NodeSet returned by extension function

    Details

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

      Operating System: other
      Platform: All

      Description

      Per the documentation, my function is returning a BasicNodeSet containing zero
      or more pointers:

      public static NodeSet observations(ExpressionContext context) {
      // the cast below shouldn't break, as this is the only pointer type that
      // makes sense in this context
      List<NodePointer> ptrs = extractObservations(
      (NodePointer)context.getContextNodePointer(),
      new ArrayList<NodePointer>());
      BasicNodeSet result = new BasicNodeSet();
      for (NodePointer ptr : ptrs)

      { result.add(ptr); }

      return result;
      }

      However, if I call JXPathContext.selectNodes("ems:observations()"), I'm getting
      a single node containing the BasicNodeSet. I notice that there is a testcase for
      functions that return NodeSets, but that it uses expressions that actually
      return the children of the NodeSet ("test:nodeSet()/name").

      There appear to be two problems. First, Expression.iterate() and
      Expression.iteratePointers() do not correctly recognize a NodeSet as something
      iterable. I've resolved this by reaching into the NodeSet and getting an
      iterator over its pointers.

      Second, Expression.PointerIterator doesn't recognize when it already has a
      pointer, and instead tries to wrap it in a new pointer. This ends up treating
      the pointer as a bean.

      I've made these changes, and written a testcase that uses an unadorned NodeSet
      function. I also found a class that used a variable named "enum", and changed
      this so that it would compile under 1.5.

      The patch is attached. It's relative to "commons-jxpath-1.2" (root of extract
      directory).

        Activity

        Hide
        Matt Benson added a comment -

        applied to SVN HEAD. Thanks, Keith!

        Show
        Matt Benson added a comment - applied to SVN HEAD. Thanks, Keith!
        Hide
        Keith D Gregory added a comment -

        No problem. I hereby grant the Apache Software Foundation rights to this patch as described by "Individual Contributor License Agreement V2.0" ( http://www.apache.org/licenses/icla.txt ), submitted separately.

        Show
        Keith D Gregory added a comment - No problem. I hereby grant the Apache Software Foundation rights to this patch as described by "Individual Contributor License Agreement V2.0" ( http://www.apache.org/licenses/icla.txt ), submitted separately.
        Hide
        Matt Benson added a comment -

        Keith, since you're watching... right, only 2 files. Apparently there was no licensing at the time you submitted your original patch. When I just submitted my rework, I mistakenly selected the grant option, but since the work is actually yours for the most part that was wrong. Probably if you clarify your intent that the original patch be incorporated (sounds silly, huh?) it would be for the best here.

        -Matt

        Show
        Matt Benson added a comment - Keith, since you're watching... right, only 2 files. Apparently there was no licensing at the time you submitted your original patch. When I just submitted my rework, I mistakenly selected the grant option, but since the work is actually yours for the most part that was wrong. Probably if you clarify your intent that the original patch be incorporated (sounds silly, huh?) it would be for the best here. -Matt
        Hide
        Keith D Gregory added a comment -

        Sorry 'bout that I think there were only 1 or 2 files that got changed, particularly if you delete the "enum" patch.

        Glad to see that JxPath development seems to be moving again.

        Show
        Keith D Gregory added a comment - Sorry 'bout that I think there were only 1 or 2 files that got changed, particularly if you delete the "enum" patch. Glad to see that JxPath development seems to be moving again.
        Hide
        Matt Benson added a comment -

        my kingdom for a unified diff... nevertheless, I will persevere.

        Show
        Matt Benson added a comment - my kingdom for a unified diff... nevertheless, I will persevere.
        Hide
        Matt Benson added a comment -

        RE the use of "enum" as a variable name, Martin van den Bemt fixed this in SVN revision 374128.

        Show
        Matt Benson added a comment - RE the use of "enum" as a variable name, Martin van den Bemt fixed this in SVN revision 374128.
        Hide
        Keith D Gregory added a comment -

        Created an attachment (id=17828)
        Appears to fix the problem; unit tests remain green

        Show
        Keith D Gregory added a comment - Created an attachment (id=17828) Appears to fix the problem; unit tests remain green

          People

          • Assignee:
            Unassigned
            Reporter:
            Keith D Gregory
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development