From 40220a81e8e0db8e01c681bab48c061aa5e7be72 Mon Sep 17 00:00:00 2001 From: David Waltermire Date: Sun, 3 Nov 2024 11:40:24 -0500 Subject: [PATCH] Ensuring runtime exceptions are caught that occur during validation. Fixes metaschema-framework/oscal-cli#67. --- .../DefaultConstraintValidator.java | 75 ++++++++++++++----- .../core/model/constraint/IConstraint.java | 3 + .../nist/secauto/metaschema/cli/CLITest.java | 28 ++++--- 3 files changed, 72 insertions(+), 34 deletions(-) diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/DefaultConstraintValidator.java b/core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/DefaultConstraintValidator.java index 71d57aef3..314f636b7 100644 --- a/core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/DefaultConstraintValidator.java +++ b/core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/DefaultConstraintValidator.java @@ -226,15 +226,18 @@ protected void validateAssembly( * the Metapath dynamic execution context to use for Metapath * evaluation */ + @SuppressWarnings("PMD.AvoidCatchingGenericException") private void validateHasCardinality( // NOPMD false positive @NonNull List constraints, @NonNull IAssemblyNodeItem item, @NonNull DynamicContext dynamicContext) { for (ICardinalityConstraint constraint : constraints) { - ISequence> targets = constraint.matchTargets(item, dynamicContext); + assert constraint != null; + try { + ISequence> targets = constraint.matchTargets(item, dynamicContext); validateHasCardinality(constraint, item, targets, dynamicContext); - } catch (MetapathException ex) { + } catch (RuntimeException ex) { handleError(constraint, item, ex, dynamicContext); } } @@ -292,15 +295,18 @@ private void validateHasCardinality( * the Metapath dynamic execution context to use for Metapath * evaluation */ + @SuppressWarnings("PMD.AvoidCatchingGenericException") private void validateIndex( @NonNull List constraints, @NonNull IAssemblyNodeItem item, @NonNull DynamicContext dynamicContext) { for (IIndexConstraint constraint : constraints) { - ISequence> targets = constraint.matchTargets(item, dynamicContext); + assert constraint != null; + try { + ISequence> targets = constraint.matchTargets(item, dynamicContext); validateIndex(constraint, item, targets, dynamicContext); - } catch (MetapathException ex) { + } catch (RuntimeException ex) { handleError(constraint, item, ex, dynamicContext); } } @@ -367,7 +373,7 @@ private void handlePass( private void handleError( @NonNull IConstraint constraint, @NonNull INodeItem node, - @NonNull MetapathException ex, + @NonNull Throwable ex, @NonNull DynamicContext dynamicContext) { getConstraintValidationHandler() .handleError(constraint, node, toErrorMessage(constraint, node, ex), ex, dynamicContext); @@ -377,7 +383,7 @@ private void handleError( private static String toErrorMessage( @NonNull IConstraint constraint, @NonNull INodeItem item, - @NonNull MetapathException ex) { + @NonNull Throwable ex) { StringBuilder builder = new StringBuilder(128); builder.append("A ") .append(constraint.getClass().getName()) @@ -413,15 +419,18 @@ private static String toErrorMessage( * the Metapath dynamic execution context to use for Metapath * evaluation */ + @SuppressWarnings("PMD.AvoidCatchingGenericException") private void validateUnique( @NonNull List constraints, @NonNull IAssemblyNodeItem item, @NonNull DynamicContext dynamicContext) { for (IUniqueConstraint constraint : constraints) { - ISequence> targets = constraint.matchTargets(item, dynamicContext); + assert constraint != null; + try { + ISequence> targets = constraint.matchTargets(item, dynamicContext); validateUnique(constraint, item, targets, dynamicContext); - } catch (MetapathException ex) { + } catch (RuntimeException ex) { handleError(constraint, item, ex, dynamicContext); } } @@ -482,16 +491,19 @@ private void validateUnique( * the Metapath dynamic execution context to use for Metapath * evaluation */ + @SuppressWarnings("PMD.AvoidCatchingGenericException") private void validateMatches( // NOPMD false positive @NonNull List constraints, @NonNull IDefinitionNodeItem item, @NonNull DynamicContext dynamicContext) { for (IMatchesConstraint constraint : constraints) { - ISequence> targets = constraint.matchTargets(item, dynamicContext); + assert constraint != null; + try { + ISequence> targets = constraint.matchTargets(item, dynamicContext); validateMatches(constraint, item, targets, dynamicContext); - } catch (MetapathException ex) { + } catch (RuntimeException ex) { handleError(constraint, item, ex, dynamicContext); } } @@ -567,14 +579,21 @@ private void validateMatchesItem( * the Metapath dynamic execution context to use for Metapath * evaluation */ + @SuppressWarnings("PMD.AvoidCatchingGenericException") private void validateIndexHasKey( // NOPMD false positive @NonNull List constraints, @NonNull IDefinitionNodeItem item, @NonNull DynamicContext dynamicContext) { for (IIndexHasKeyConstraint constraint : constraints) { - ISequence> targets = constraint.matchTargets(item, dynamicContext); - validateIndexHasKey(constraint, item, targets); + assert constraint != null; + + try { + ISequence> targets = constraint.matchTargets(item, dynamicContext); + validateIndexHasKey(constraint, item, targets); + } catch (RuntimeException ex) { + handleError(constraint, item, ex, dynamicContext); + } } } @@ -619,13 +638,20 @@ private void validateIndexHasKey( * the Metapath dynamic execution context to use for Metapath * evaluation */ + @SuppressWarnings("PMD.AvoidCatchingGenericException") private void validateExpect( @NonNull List constraints, @NonNull IDefinitionNodeItem item, @NonNull DynamicContext dynamicContext) { for (IExpectConstraint constraint : constraints) { - ISequence> targets = constraint.matchTargets(item, dynamicContext); - validateExpect(constraint, item, targets, dynamicContext); + assert constraint != null; + + try { + ISequence> targets = constraint.matchTargets(item, dynamicContext); + validateExpect(constraint, item, targets, dynamicContext); + } catch (RuntimeException ex) { + handleError(constraint, item, ex, dynamicContext); + } } } @@ -686,13 +712,19 @@ private void validateExpect( * the Metapath dynamic execution context to use for Metapath * evaluation */ + @SuppressWarnings("PMD.AvoidCatchingGenericException") private void validateAllowedValues( @NonNull List constraints, @NonNull IDefinitionNodeItem item, @NonNull DynamicContext dynamicContext) { for (IAllowedValuesConstraint constraint : constraints) { - ISequence> targets = constraint.matchTargets(item, dynamicContext); - validateAllowedValues(constraint, item, targets, dynamicContext); + assert constraint != null; + try { + ISequence> targets = constraint.matchTargets(item, dynamicContext); + validateAllowedValues(constraint, item, targets, dynamicContext); + } catch (RuntimeException ex) { + handleError(constraint, item, ex, dynamicContext); + } } } @@ -712,6 +744,7 @@ private void validateAllowedValues( * the Metapath dynamic execution context to use for Metapath * evaluation */ + @SuppressWarnings("PMD.AvoidCatchingGenericException") private void validateAllowedValues( @NonNull IAllowedValuesConstraint constraint, @NonNull IDefinitionNodeItem node, @@ -722,7 +755,7 @@ private void validateAllowedValues( if (item.hasValue()) { try { updateValueStatus(item, constraint, node); - } catch (MetapathException ex) { + } catch (RuntimeException ex) { handleError(constraint, item, ex, dynamicContext); } } @@ -774,6 +807,7 @@ protected void handleAllowedValues( } } + @SuppressWarnings("PMD.AvoidCatchingGenericException") @Override public void finalizeValidation(DynamicContext dynamicContext) { // key references @@ -790,8 +824,11 @@ public void finalizeValidation(DynamicContext dynamicContext) { List targets = keyRef.getTargets(); for (INodeItem item : targets) { assert item != null; - - validateKeyRef(constraint, node, item, indexName, index, dynamicContext); + try { + validateKeyRef(constraint, node, item, indexName, index, dynamicContext); + } catch (RuntimeException ex) { + handleError(constraint, item, ex, dynamicContext); + } } } } diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraint.java b/core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraint.java index 67fa20d38..35f5912be 100644 --- a/core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraint.java +++ b/core/src/main/java/gov/nist/secauto/metaschema/core/model/constraint/IConstraint.java @@ -8,6 +8,7 @@ import gov.nist.secauto.metaschema.core.datatype.markup.MarkupMultiline; import gov.nist.secauto.metaschema.core.metapath.DynamicContext; import gov.nist.secauto.metaschema.core.metapath.ISequence; +import gov.nist.secauto.metaschema.core.metapath.MetapathException; import gov.nist.secauto.metaschema.core.metapath.item.node.IDefinitionNodeItem; import gov.nist.secauto.metaschema.core.model.IAttributable; import gov.nist.secauto.metaschema.core.model.IDescribable; @@ -110,6 +111,8 @@ enum Level { * @param dynamicContext * the Metapath evaluation context to use * @return the matching nodes as a sequence + * @throws MetapathException + * if an error occurred during evaluation * @see #getTarget() */ @NonNull diff --git a/metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/CLITest.java b/metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/CLITest.java index dd6985b07..673fef7ea 100644 --- a/metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/CLITest.java +++ b/metaschema-cli/src/test/java/gov/nist/secauto/metaschema/cli/CLITest.java @@ -5,14 +5,13 @@ package gov.nist.secauto.metaschema.cli; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; -import static org.assertj.core.api.Assertions.assertThat; import gov.nist.secauto.metaschema.cli.processor.ExitCode; import gov.nist.secauto.metaschema.cli.processor.ExitStatus; -import nl.altindag.log.LogCaptor; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -24,6 +23,7 @@ import java.util.stream.Stream; import edu.umd.cs.findbugs.annotations.NonNull; +import nl.altindag.log.LogCaptor; /** * Unit test for simple CLI. @@ -212,19 +212,17 @@ void test() { }; CLI.runCli(cliArgs); assertThat(captor.getErrorLogs().toString()) - .contains(new String[] { - "expect-default-non-zero: Expect constraint '. > 0' did not match the data", - "expect-custom-non-zero: No default message, custom error message for expect-custom-non-zero constraint.", - "matches-default-regex-letters-only: Value '1' did not match the pattern", - "matches-custom-regex-letters-only: No default message, custom error message for matches-custom-regex-letters-only constraint.", - "cardinality-default-two-minimum: The cardinality '1' is below the required minimum '2' for items matching", - "index-items-default: Index 'index-items-default' has duplicate key for items", - "index-items-custom: No default message, custom error message for index-item-custom.", - "is-unique-default: Unique constraint violation at paths", - "is-unique-custom: No default message, custom error message for is-unique-custom.", - "index-has-key-default: Key reference [2] not found in index 'index-items-default' for item", - "index-has-key-custom: No default message, custom error message for index-has-key-custom." - }); + .contains("expect-default-non-zero: Expect constraint '. > 0' did not match the data", + "expect-custom-non-zero: No default message, custom error message for expect-custom-non-zero constraint.", + "matches-default-regex-letters-only: Value '1' did not match the pattern", + "matches-custom-regex-letters-only: No default message, custom error message for matches-custom-regex-letters-only constraint.", + "cardinality-default-two-minimum: The cardinality '1' is below the required minimum '2' for items matching", + "index-items-default: Index 'index-items-default' has duplicate key for items", + "index-items-custom: No default message, custom error message for index-item-custom.", + "is-unique-default: Unique constraint violation at paths", + "is-unique-custom: No default message, custom error message for is-unique-custom.", + "index-has-key-default: Key reference [2] not found in index 'index-items-default' for item", + "index-has-key-custom: No default message, custom error message for index-has-key-custom."); } } }