From 7c524da3b1e76838ab05596558444bae3e522d97 Mon Sep 17 00:00:00 2001 From: David Waltermire Date: Mon, 6 Jan 2025 20:28:53 -0500 Subject: [PATCH] Apply suggestions from code review. --- .../core/metapath/DynamicContext.java | 17 +++++++- .../cst/items/AbstractLiteralExpression.java | 5 +-- .../core/metapath/cst/items/Except.java | 12 +++--- .../core/metapath/cst/items/Intersect.java | 5 +-- .../core/metapath/cst/items/Quantified.java | 41 ++++++++----------- .../metapath/cst/path/RootSlashOnlyPath.java | 34 ++++++++------- .../core/metapath/cst/type/InstanceOf.java | 3 +- 7 files changed, 65 insertions(+), 52 deletions(-) diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/DynamicContext.java b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/DynamicContext.java index ae90f1027..6ee46ee5a 100644 --- a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/DynamicContext.java +++ b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/DynamicContext.java @@ -34,6 +34,7 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.Nullable; @@ -326,7 +327,9 @@ public void pushExecutionStack(@NonNull IExpression expression) { */ public void popExecutionStack(@NonNull IExpression expression) { IExpression popped = this.sharedState.executionStack.pop(); - assert expression.equals(popped); + if (!expression.equals(popped)) { + throw new IllegalStateException("Popped expression does not match expected expression"); + } } /** @@ -339,6 +342,18 @@ public List getExecutionStack() { return CollectionUtil.unmodifiableList(new ArrayList<>(this.sharedState.executionStack)); } + /** + * Provides a formatted stack trace. + * + * @return the formatted stack trace + */ + @NonNull + public String formatExecutionStackTrace() { + return ObjectUtils.notNull(getExecutionStack().stream() + .map(IExpression::toCSTString) + .collect(Collectors.joining("\n-> "))); + } + private class CachingLoader implements IDocumentLoader { @NonNull private final IDocumentLoader proxy; diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/AbstractLiteralExpression.java b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/AbstractLiteralExpression.java index 9cb0d817a..f5ae54f88 100644 --- a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/AbstractLiteralExpression.java +++ b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/AbstractLiteralExpression.java @@ -13,8 +13,7 @@ import edu.umd.cs.findbugs.annotations.NonNull; /** - * A base class for all Metapath - * literal value + * A base class for all Metapath literal value * expressions. * * @param @@ -45,7 +44,7 @@ public RESULT_TYPE getValue() { } @Override - protected ISequence evaluate(DynamicContext dynamicContext, ISequence focus) { + protected ISequence evaluate(DynamicContext dynamicContext, ISequence focus) { return ISequence.of(getValue()); } diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/Except.java b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/Except.java index 5ed903ebb..7b73e3cb4 100644 --- a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/Except.java +++ b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/Except.java @@ -11,21 +11,22 @@ import gov.nist.secauto.metaschema.core.metapath.item.ISequence; import gov.nist.secauto.metaschema.core.util.ObjectUtils; +import java.util.HashSet; import java.util.List; +import java.util.Set; import edu.umd.cs.findbugs.annotations.NonNull; /** - * The CST node for a Metapath - * except + * The CST node for a Metapath except * expression. */ public class Except extends AbstractFilterExpression { /** - * Construct a except filter expression, which removes the items resulting from - * the filter expression from the items expression. + * Construct a except filter expression, which removes the items resulting from the filter + * expression from the items expression. * * @param text * the parsed text of the expression @@ -43,8 +44,9 @@ public Except( @Override protected ISequence applyFilterTo(@NonNull List source, @NonNull List items) { + Set filterSet = new HashSet<>(items); return ISequence.of(ObjectUtils.notNull(source.stream() - .filter(item -> !items.contains(item)))); + .filter(item -> !filterSet.contains(item)))); } @Override diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/Intersect.java b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/Intersect.java index cce0b8a4d..c9c1e2c45 100644 --- a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/Intersect.java +++ b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/Intersect.java @@ -16,15 +16,14 @@ import edu.umd.cs.findbugs.annotations.NonNull; /** - * The CST node for a Metapath - * intersect + * The CST node for a Metapath intersect * expression. */ public class Intersect extends AbstractFilterExpression { /** - * Construct a new Metapath except expression CST node. + * Construct a new Metapath intersect expression CST node. * * @param text * the parsed text of the expression diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/Quantified.java b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/Quantified.java index c072f2e31..753ec3f45 100644 --- a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/Quantified.java +++ b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/items/Quantified.java @@ -30,8 +30,7 @@ import edu.umd.cs.findbugs.annotations.NonNull; /** - * An XPath 3.1 quantified + * An XPath 3.1 quantified * expression. */ public class Quantified @@ -41,23 +40,21 @@ public class Quantified */ public enum Quantifier { /** - * The quantified expression is {@code true} if at least one evaluation of the - * test expression has the - * effective boolean value - * {@code true}; otherwise the quantified expression is {@code false}. + * The quantified expression is {@code true} if at least one evaluation of the test expression has + * the effective boolean value {@code true}; + * otherwise the quantified expression is {@code false}. *

- * This rule implies that, if the in-clauses generate zero binding tuples, the - * value of the quantified expression is {@code false}. + * This rule implies that, if the in-clauses generate zero binding tuples, the value of the + * quantified expression is {@code false}. */ SOME, /** - * the quantified expression is {@code true} if every evaluation of the test - * expression has the effective - * boolean value {@code true}; otherwise the quantified expression is - * {@code false}. + * the quantified expression is {@code true} if every evaluation of the test expression has the + * effective boolean value {@code true}; + * otherwise the quantified expression is {@code false}. *

- * This rule implies that, if the in-clauses generate zero binding tuples, the - * value of the quantified expression is {@code true}. + * This rule implies that, if the in-clauses generate zero binding tuples, the value of the + * quantified expression is {@code true}. */ EVERY; } @@ -77,11 +74,10 @@ public enum Quantifier { * @param quantifier * the quantifier operation * @param inClauses - * the set of expressions that define the variables to use for - * determining the Cartesian product for evaluation + * the set of expressions that define the variables to use for determining the Cartesian + * product for evaluation * @param satisfies - * the expression used for evaluation using the Cartesian product of - * the variables + * the expression used for evaluation using the Cartesian product of the variables */ public Quantified( @NonNull String text, @@ -105,8 +101,8 @@ public Quantifier getQuantifier() { } /** - * Get the set of expressions that define the variables to use for determining - * the Cartesian product for evaluation. + * Get the set of expressions that define the variables to use for determining the Cartesian product + * for evaluation. * * @return the variable names mapped to the associated Metapath expression */ @@ -116,8 +112,7 @@ public Map getInClauses() { } /** - * Get the expression used for evaluation using the Cartesian product of the - * variables. + * Get the expression used for evaluation using the Cartesian product of the variables. * * @return the evaluation expression */ @@ -133,7 +128,7 @@ public List getChildren() { } @Override - protected ISequence evaluate(DynamicContext dynamicContext, ISequence focus) { + protected ISequence evaluate(DynamicContext dynamicContext, ISequence focus) { Map> clauses = getInClauses().entrySet().stream() .map(entry -> Map.entry( entry.getKey(), diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/path/RootSlashOnlyPath.java b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/path/RootSlashOnlyPath.java index 83dd46255..a8b17595e 100644 --- a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/path/RootSlashOnlyPath.java +++ b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/path/RootSlashOnlyPath.java @@ -23,13 +23,11 @@ /** * An expression that gets the document root. *

- * Based on the XPath 3.1 - * path + * Based on the XPath 3.1 path * operator. *

- * This class handles the root path expression "/", which selects the document - * root node when evaluated. The evaluation follows the XPath specification for - * absolute paths. + * This class handles the root path expression "/", which selects the document root node when + * evaluated. The evaluation follows the XPath specification for absolute paths. */ public class RootSlashOnlyPath extends AbstractPathExpression { @@ -63,15 +61,21 @@ public RESULT accept(IExpressionVisitor visit protected ISequence evaluate(DynamicContext dynamicContext, ISequence focus) { return ISequence.of(ObjectUtils.notNull(focus.stream() .map(ItemUtils::checkItemIsNodeItemForStep) - .map(item -> Axis.ANCESTOR_OR_SELF.execute(ObjectUtils.notNull(item)) - .findFirst() - .orElseThrow(() -> new DynamicMetapathException(DynamicMetapathException.TREAT_DOES_NOT_MATCH_TYPE, - "Root node not found"))) - .peek(item -> { - if (!(item instanceof IDocumentNodeItem)) { - throw new DynamicMetapathException(DynamicMetapathException.TREAT_DOES_NOT_MATCH_TYPE, - "The head of the tree is not a document node."); - } - }))); + .map(RootSlashOnlyPath::findAndValidateDocumentRoot))); + } + + private static INodeItem findAndValidateDocumentRoot(INodeItem item) { + INodeItem root = Axis.ANCESTOR_OR_SELF.execute(ObjectUtils.notNull(item)) + .findFirst() + .orElseThrow(() -> new DynamicMetapathException( + DynamicMetapathException.TREAT_DOES_NOT_MATCH_TYPE, + "Root node not found")); + + if (!(root instanceof IDocumentNodeItem)) { + throw new DynamicMetapathException( + DynamicMetapathException.TREAT_DOES_NOT_MATCH_TYPE, + "The head of the tree is not a document node."); + } + return root; } } diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/type/InstanceOf.java b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/type/InstanceOf.java index 8cb85d597..b4e1aa370 100644 --- a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/type/InstanceOf.java +++ b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/type/InstanceOf.java @@ -24,8 +24,7 @@ */ /** * A compact syntax tree node that supports the Metapath - * "instance of" - * operator. + * "instance of" operator. */ public class InstanceOf extends AbstractExpression {