TEDEFO-5150: Preserve predicates and always return a valid path when adding an axis

src/main/java/eu/europa/ted/eforms/xpath/XPathListenerImpl.java

@@ -18,6 +18,11 @@
 import eu.europa.ted.eforms.xpath.XPath20Parser.AxisstepContext;
 import eu.europa.ted.eforms.xpath.XPath20Parser.FilterexprContext;
 import eu.europa.ted.eforms.xpath.XPath20Parser.PredicateContext;
+import eu.europa.ted.eforms.xpath.XPath20Parser.PredicatelistContext;
+import eu.europa.ted.eforms.xpath.XPath20Parser.ForwardaxisContext;
+import eu.europa.ted.eforms.xpath.XPath20Parser.ForwardstepContext;
+import eu.europa.ted.eforms.xpath.XPath20Parser.NodetestContext;
+import eu.europa.ted.eforms.xpath.XPath20Parser.ReversestepContext;
 
 class XPathListenerImpl extends XPath20BaseListener {
   private XPathInfo xpathInfo;
@@ -44,10 +49,7 @@ public XPathInfo parse(String xpathInput) {
     final ParseTreeWalker walker = new ParseTreeWalker();
     walker.walk(this, tree);
 
-    steps.stream().forEach(s -> {
-      XPathStep step = new XPathStep(s.stepText, s.predicates);
-      xpathInfo.addStep(step);
-    });
+    steps.stream().forEach(s -> xpathInfo.addStep(s.step));
 
     if (!xpathInfo.isAttribute()) {
       // The XPath does not point to an attribute, so it is the path to the last element
@@ -163,26 +165,89 @@ private Boolean inPredicateMode() {
     return inPredicate > 0;
   }
 
+  /**
+   * The step an axis step is written as, told apart by whether what it looks for could be looked
+   * for along another axis. A step is read the same way however it is spelled: {@code b} is
+   * {@code child::b}, {@code @x} is {@code attribute::x} and {@code ..} is {@code parent::node()}.
+   */
+  private XPathStep readAxisStep(final AxisstepContext ctx, final List<String> predicates) {
+    final ForwardstepContext forward = ctx.forwardstep();
+    if (forward != null) {
+      final AbbrevforwardstepContext abbreviated = forward.abbrevforwardstep();
+      if (abbreviated != null) {
+        return abbreviated.AT() != null
+            ? XPathStep.opaque(getInputText(forward), predicates)
+            : XPathStep.retargetable(getInputText(forward),
+                getInputText(abbreviated.nodetest()), predicates);
+      }
+
+      final ForwardaxisContext axis = forward.forwardaxis();
+      if (axis.KW_ATTRIBUTE() != null || axis.KW_NAMESPACE() != null) {
+        // An attribute and a namespace are found on the axis leading to them and nowhere else, so
+        // a node test naming one means nothing along another axis.
+        return XPathStep.opaque(getInputText(forward), predicates);
+      }
+      if (axis.KW_SELF() != null && namesAnyNode(forward.nodetest())) {
+        return XPathStep.navigation(getInputText(forward), predicates);
+      }
+      return XPathStep.retargetable(getInputText(forward), getInputText(forward.nodetest()),
+          predicates);
+    }
+
+    final ReversestepContext reverse = ctx.reversestep();
+    if (reverse.reverseaxis() == null) {
+      return XPathStep.navigation(getInputText(reverse), predicates);
+    }
+    if (reverse.reverseaxis().KW_PARENT() != null && namesAnyNode(reverse.nodetest())) {
+      return XPathStep.navigation(getInputText(reverse), predicates);
+    }
+    return XPathStep.retargetable(getInputText(reverse), getInputText(reverse.nodetest()),
+        predicates);
+  }
+
+  /**
+   * The step a filter expression is written as. The context item is the one of them that only moves
+   * about; the rest are evaluated for the nodes they return and stay where they are.
+   */
+  private XPathStep readFilterStep(final FilterexprContext ctx, final List<String> predicates) {
+    if (ctx.primaryexpr().contextitemexpr() != null) {
+      return XPathStep.navigation(getInputText(ctx.primaryexpr()), predicates);
+    }
+    return XPathStep.opaque(getInputText(ctx.primaryexpr()), predicates);
+  }
+
+  /**
+   * Whether the node test takes any node at all, which is what the steps that only move about look
+   * for. It is the {@code node()} of {@code self::node()} and {@code parent::node()}, written short
+   * as {@code .} and {@code ..}.
+   */
+  private static boolean namesAnyNode(final NodetestContext ctx) {
+    return ctx.kindtest() != null && ctx.kindtest().anykindtest() != null;
+  }
+
+  private static List<String> predicatesOf(final PredicatelistContext ctx,
+      final Function<ParserRuleContext, String> getInputText) {
+    return ctx.predicate().stream().map(getInputText).collect(Collectors.toList());
+  }
+
+
   private class StepInfo {
-    String stepText;
-    List<String> predicates;
+    XPathStep step;
     int a;
     int b;
 
     private StepInfo(AxisstepContext ctx, Function<ParserRuleContext, String> getInputText) {
-      this(ctx.reversestep() != null ? getInputText.apply(ctx.reversestep()) : getInputText.apply(ctx.forwardstep()), 
-          ctx.predicatelist().predicate().stream().map(getInputText).collect(Collectors.toList()),
+      this(readAxisStep(ctx, predicatesOf(ctx.predicatelist(), getInputText)),
           ctx.getSourceInterval());
     }
+
     private StepInfo(FilterexprContext ctx, Function<ParserRuleContext, String> getInputText) {
-      this(getInputText.apply(ctx.primaryexpr()),
-          ctx.predicatelist().predicate().stream().map(getInputText).collect(Collectors.toList()),
+      this(readFilterStep(ctx, predicatesOf(ctx.predicatelist(), getInputText)),
           ctx.getSourceInterval());
     }
 
-    private StepInfo(String stepText, List<String> predicates, Interval interval) {
-      this.stepText = stepText;
-      this.predicates = predicates;
+    private StepInfo(XPathStep step, Interval interval) {
+      this.step = step;
       this.a = interval.a;
       this.b = interval.b;
     }

src/main/java/eu/europa/ted/eforms/xpath/XPathProcessor.java

@@ -15,14 +15,35 @@ public static XPathInfo parse(String xpathInput) {
     return parser.parse(xpathInput);
   }
 
-  public static String addAxis(String axis, String path) {
-    LinkedList<XPathStep> steps = new LinkedList<>(parse(path).getSteps());
-
-    while (steps.getFirst().getStepText().equals("..")) {
+  /**
+   * Rewrites a path so that it looks along the given axis instead of along the one it was written
+   * for.
+   *
+   * <p>
+   * This serves the axis that can be written on an EFX-1 field reference, and nothing more. The
+   * axis is expected to be one that XPath knows, and the path to be relative to the context the
+   * axis is applied from; an absolute path cannot keep its anchor, because an axis cannot be
+   * followed by a separator, so it is read the same way. It is not a general way of rewriting
+   * XPath.
+   */
+  public static String addAxis(final String axis, final String path) {
+    final LinkedList<XPathStep> steps = new LinkedList<>(parse(path).getSteps());
+
+    // Moving about before the axis makes no difference to what it finds, since it searches from the
+    // context node wherever the path would have gone first. Such steps are dropped, except for the
+    // one the axis is put on, and except where a predicate says which node was arrived at.
+    while (steps.size() > 1 && steps.getFirst().isNavigationStep()
+        && steps.getFirst().getPredicates().isEmpty()) {
       steps.removeFirst();
     }
 
-    return axis + "::" + steps.stream().map(s -> s.getStepText()).collect(Collectors.joining("/"));
+    if (steps.isEmpty()) {
+      return XPathStep.anyNodeOn(axis).toString();
+    }
+
+    steps.addAll(0, steps.removeFirst().onAxis(axis));
+
+    return steps.stream().map(s -> s.toString()).collect(Collectors.joining("/"));
   }
 
   public static String join(final String first, final String second) {

src/main/java/eu/europa/ted/eforms/xpath/XPathStep.java

@@ -1,20 +1,127 @@
 package eu.europa.ted.eforms.xpath;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Objects;
 
 import org.apache.commons.lang3.StringUtils;
 
+/**
+ * One step of a path.
+ *
+ * <p>
+ * A step read from a path is known well enough to say whether it can be looked for along another
+ * axis, which is all {@link XPathProcessor#addAxis} needs of it. A step built from its text alone
+ * says nothing of the sort and is taken at its word.
+ *
+ * <p>
+ * How a step was read is not part of what it is worth: two steps written the same way, carrying the
+ * same predicates, are the same step.
+ */
 public class XPathStep implements Comparable<XPathStep> {
+  /** The node test that matches any node, which the steps that only move about look for. */
+  private static final String ANY_NODE = "node()";
+
+  /**
+   * As much as is needed to know whether a step can be looked for along a different axis. This is
+   * not a reading of XPath's own grammar, which distinguishes far more than this: an explicit
+   * {@code child::b} and a plain {@code b} are told apart there and are the same thing here.
+   */
+  private enum StepKind {
+    /** The step names something that can be looked for along another axis. */
+    RETARGETABLE,
+
+    /** The step only moves about, naming nothing: the current node, or the parent node. */
+    NAVIGATION,
+
+    /**
+     * Anything else. An attribute, a namespace and an expression all name something that is only
+     * found where it already is, and a step built from text alone is not known at all. All of them
+     * stay where they are.
+     */
+    OPAQUE
+  }
+
   private final String stepText;
   private final List<String> predicates;
+  private final StepKind kind;
+
+  /** What the step looks for, where it looks for anything. */
+  private final String nodeTest;
 
+  /**
+   * Builds a step from the text it is written as. Steps read from a path are built by the parser,
+   * which knows more about them than their text says.
+   */
   public XPathStep(String stepText, List<String> predicates) {
+    this(stepText, predicates, StepKind.OPAQUE, null);
+  }
+
+  private XPathStep(final String stepText, final List<String> predicates, final StepKind kind,
+      final String nodeTest) {
     this.stepText = StringUtils.strip(stepText);
-    this.predicates = predicates;
+    this.predicates = predicates == null ? Collections.emptyList() : predicates;
+    this.kind = kind;
+    this.nodeTest = nodeTest;
+  }
+
+  /**
+   * A step that names what it looks for, which can therefore be looked for along another axis. The
+   * node test is what it looks for, apart from however the step happens to be written.
+   */
+  static XPathStep retargetable(final String stepText, final String nodeTest,
+      final List<String> predicates) {
+    return new XPathStep(stepText, predicates, StepKind.RETARGETABLE,
+        StringUtils.strip(nodeTest));
+  }
+
+  /**
+   * A step that only moves about: {@code .} or {@code ..}. It names nothing, so what it arrives at
+   * is any node at all, and any predicate it carries describes that node.
+   */
+  static XPathStep navigation(final String stepText, final List<String> predicates) {
+    return new XPathStep(stepText, predicates, StepKind.NAVIGATION, ANY_NODE);
+  }
+
+  /**
+   * A step that has to be left where it is: an attribute, a namespace, or an expression evaluated
+   * for the nodes it returns.
+   */
+  static XPathStep opaque(final String stepText, final List<String> predicates) {
+    return new XPathStep(stepText, predicates, StepKind.OPAQUE, null);
+  }
+
+  /**
+   * A step that walks the given axis and takes whatever it finds.
+   */
+  static XPathStep anyNodeOn(final String axis) {
+    return retargetable(axis + "::" + ANY_NODE, ANY_NODE, Collections.emptyList());
+  }
+
+  /**
+   * The same step, looked for along the given axis instead.
+   *
+   * <p>
+   * A step that names what it looks for is simply looked for elsewhere, and one step comes back. A
+   * step that has to stay where it is keeps its place behind a step that walks the axis, and two
+   * come back.
+   */
+  List<XPathStep> onAxis(final String axis) {
+    if (this.kind == StepKind.OPAQUE) {
+      return Arrays.asList(anyNodeOn(axis), this);
+    }
+    return Collections
+        .singletonList(retargetable(axis + "::" + this.nodeTest, this.nodeTest, this.predicates));
+  }
+
+  /**
+   * Whether the step only moves about, without naming anything to look for.
+   */
+  boolean isNavigationStep() {
+    return this.kind == StepKind.NAVIGATION;
   }
 
   public String getStepText() {
@@ -132,7 +239,7 @@ public boolean isTheSameAs(final XPathStep other) {
    * This method was renamed for clarity. It is marked as deprecated so that the
    * library interface does not change. It will be removed in the next major
    * version of the library.
-   * 
+   *
    */
   @Deprecated(since = "1.3.0", forRemoval = true)
   public boolean isSimilarTo(final XPathStep other) {

src/test/java/eu/europa/ted/eforms/xpath/XPathProcessorTest.java

@@ -206,6 +206,55 @@ void testAddAxis() {
     assertEquals("descendant::b/c", XPathProcessor.addAxis("descendant", "../../b/c"));
   }
 
+  @Test
+  void testAddAxis_MustPreserveThePredicates() {
+    assertEquals("preceding::b[x = 'y']/c",
+        XPathProcessor.addAxis("preceding", "b[x = 'y']/c"));
+    assertEquals("preceding::b/c[x = 'y']",
+        XPathProcessor.addAxis("preceding", "b/c[x = 'y']"));
+    assertEquals("preceding::b[e][f]/c[g]",
+        XPathProcessor.addAxis("preceding", "b[e][f]/c[g]"));
+    assertEquals("descendant::b[x = 'y']/c",
+        XPathProcessor.addAxis("descendant", "../../b[x = 'y']/c"));
+  }
+
+  @Test
+  void testAddAxis_MustAimTheStepTheAxisLandsOn() {
+    assertEquals("preceding::node()", XPathProcessor.addAxis("preceding", "."));
+    assertEquals("preceding::node()", XPathProcessor.addAxis("preceding", ".."));
+    assertEquals("preceding::node()", XPathProcessor.addAxis("preceding", "../.."));
+    assertEquals("preceding::b", XPathProcessor.addAxis("preceding", "./b"));
+    assertEquals("preceding::node()[x]/b", XPathProcessor.addAxis("preceding", ".[x]/b"));
+    assertEquals("preceding::node()[x]/b", XPathProcessor.addAxis("preceding", "..[x]/b"));
+    assertEquals("preceding::b/c", XPathProcessor.addAxis("preceding", "child::b/c"));
+    assertEquals("preceding::b/c", XPathProcessor.addAxis("preceding", "following::b/c"));
+    assertEquals("preceding::text()/b", XPathProcessor.addAxis("preceding", "text()/b"));
+  }
+
+  @Test
+  void testAddAxis_MustKeepAStepThatCannotBeAimed() {
+    assertEquals("preceding::node()/@x", XPathProcessor.addAxis("preceding", "@x"));
+    assertEquals("preceding::node()/$var/b", XPathProcessor.addAxis("preceding", "$var/b"));
+    assertEquals("preceding::node()/doc('x')/b", XPathProcessor.addAxis("preceding", "doc('x')/b"));
+    assertEquals("preceding::node()/id('x')/b", XPathProcessor.addAxis("preceding", "id('x')/b"));
+    assertEquals("preceding::node()/(a | b)/c", XPathProcessor.addAxis("preceding", "(a | b)/c"));
+    assertEquals("preceding::node()/namespace::x",
+        XPathProcessor.addAxis("preceding", "namespace::x"));
+  }
+
+  @Test
+  void testJoin_MustNotRewriteTheStepsItWasGiven() {
+    assertEquals("child::a/attribute::x", XPathProcessor.join("child::a", "attribute::x"));
+    assertEquals("self::node()/b", XPathProcessor.join("self::node()", "b"));
+  }
+
+  @Test
+  void testAddAxis_MustReadAnAbsolutePathFromTheContext() {
+    assertEquals("preceding::a/b", XPathProcessor.addAxis("preceding", "/a/b"));
+    assertEquals("preceding::a/b", XPathProcessor.addAxis("preceding", "//a/b"));
+    assertEquals("preceding::a[x]/b", XPathProcessor.addAxis("preceding", "/a[x]/b"));
+  }
+
   @Test
   void testJoin() {
     assertEquals("a/b/c/d", XPathProcessor.join("a/b", "c/d"));