From b660af1c35ccb5674cae08cfff952a6a3590fef8 Mon Sep 17 00:00:00 2001 From: David Waltermire Date: Sun, 28 Dec 2025 21:50:47 -0500 Subject: [PATCH] refactor(databind): add permissive document loading and fix error messages Refinements to PR #593 (parser validation for required fields): Permissive Loading: - Add newPermissiveBoundLoader() to IBindingContext for creating loaders with DESERIALIZE_VALIDATE_REQUIRED_FIELDS disabled - Update constraint validation to use permissive loaders for referenced documents (enables doc() function during constraint evaluation) - Update CLI convert command to use permissive loading - Update CLI metapath eval command to use permissive loading and set document loader on DynamicContext to enable doc() function support - Add tests for permissive loading behavior Error Message Fixes: - Add plural parameter to getFormatPropertyTypeName() and getPropertyTypeName() - Fix plural forms: 'properties' instead of 'propertys' for JSON/YAML - Consolidate getFormatPropertyGroupLabel() to delegate to getFormatPropertyTypeName() and capitalize, reducing code duplication --- .../metaschema/databind/IBindingContext.java | 33 +++++-- .../databind/io/AbstractDeserializer.java | 3 +- .../databind/io/AbstractProblemHandler.java | 46 +++++----- .../io/ValidationErrorMessageTest.java | 85 +++++++++++++++++++ .../commands/AbstractConvertSubcommand.java | 3 +- .../metapath/EvaluateMetapathCommand.java | 15 +++- 6 files changed, 152 insertions(+), 33 deletions(-) diff --git a/databind/src/main/java/gov/nist/secauto/metaschema/databind/IBindingContext.java b/databind/src/main/java/gov/nist/secauto/metaschema/databind/IBindingContext.java index 8e2f0c762..6615116d0 100644 --- a/databind/src/main/java/gov/nist/secauto/metaschema/databind/IBindingContext.java +++ b/databind/src/main/java/gov/nist/secauto/metaschema/databind/IBindingContext.java @@ -411,6 +411,27 @@ default IBoundLoader newBoundLoader() { return new DefaultBoundLoader(this); } + /** + * Get a new {@link IBoundLoader} instance configured for permissive loading. + *

+ * This loader has + * {@link DeserializationFeature#DESERIALIZE_VALIDATE_REQUIRED_FIELDS} disabled, + * making it suitable for use with Metapath functions like {@code fn:doc()} + * where documents may be incomplete or under construction. + *

+ * Use this method when setting up a {@link DynamicContext} for Metapath + * evaluation to ensure that referenced documents can be loaded without strict + * required field validation. + * + * @return a permissive loader instance + */ + @NonNull + default IBoundLoader newPermissiveBoundLoader() { + IBoundLoader loader = newBoundLoader(); + loader.disableFeature(DeserializationFeature.DESERIALIZE_VALIDATE_REQUIRED_FIELDS); + return loader; + } + /** * Create a deep copy of the provided bound object. * @@ -445,10 +466,10 @@ CLASS deepCopy(@NonNull CLASS other, IBoundObject p default IConstraintValidator newValidator( @NonNull IConstraintValidationHandler handler, @Nullable IConfiguration> config) { - IBoundLoader loader = newBoundLoader(); + // Use permissive loader for referenced documents + IBoundLoader loader = newPermissiveBoundLoader(); + // Also disable constraint validation for referenced documents loader.disableFeature(DeserializationFeature.DESERIALIZE_VALIDATE_CONSTRAINTS); - // Disable required field validation since schema validation handles this - loader.disableFeature(DeserializationFeature.DESERIALIZE_VALIDATE_REQUIRED_FIELDS); DynamicContext context = new DynamicContext(); context.setDocumentLoader(loader); @@ -570,10 +591,10 @@ default IValidationResult validateWithConstraints( @NonNull URI target, @Nullable IConfiguration> config) throws IOException, ConstraintValidationException { - IBoundLoader loader = newBoundLoader(); + // Use permissive loader for the target document and any referenced documents + IBoundLoader loader = newPermissiveBoundLoader(); + // Also disable constraint validation during loading loader.disableFeature(DeserializationFeature.DESERIALIZE_VALIDATE_CONSTRAINTS); - // Disable required field validation since schema validation handles this - loader.disableFeature(DeserializationFeature.DESERIALIZE_VALIDATE_REQUIRED_FIELDS); IDocumentNodeItem nodeItem = loader.loadAsNodeItem(target); return validate(nodeItem, loader, config); diff --git a/databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractDeserializer.java b/databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractDeserializer.java index 6587cd92d..a9e9aae36 100644 --- a/databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractDeserializer.java +++ b/databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractDeserializer.java @@ -147,7 +147,8 @@ private void validate(@NonNull INodeItem nodeItem) throws ConstraintValidationEx } DynamicContext dynamicContext = new DynamicContext(nodeItem.getStaticContext()); - dynamicContext.setDocumentLoader(getBindingContext().newBoundLoader()); + // Use permissive loader for documents referenced in constraint expressions + dynamicContext.setDocumentLoader(getBindingContext().newPermissiveBoundLoader()); DefaultConstraintValidator validator = new DefaultConstraintValidator(getConstraintValidationHandler()); validator.validate(definitionNodeItem, dynamicContext); validator.finalizeValidation(dynamicContext); diff --git a/databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.java b/databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.java index 62f3e95f6..a0d1fc527 100644 --- a/databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.java +++ b/databind/src/main/java/gov/nist/secauto/metaschema/databind/io/AbstractProblemHandler.java @@ -211,7 +211,7 @@ private String formatMissingPropertiesMessage( IBoundProperty missing = !missingFlags.isEmpty() ? missingFlags.get(0) : !missingFields.isEmpty() ? missingFields.get(0) : missingAssemblies.get(0); - String type = getPropertyTypeName(missing, format); + String type = getPropertyTypeName(missing, format, false); String name = getInstanceName(missing, context); message.append(String.format("Missing required %s '%s' in '%s'", type, name, parentName)); } else if (hasSingleType(missingFlags, missingFields, missingAssemblies)) { @@ -219,9 +219,9 @@ private String formatMissingPropertiesMessage( List> list = !missingFlags.isEmpty() ? missingFlags : !missingFields.isEmpty() ? missingFields : missingAssemblies; - String type = getPropertyTypeName(list.get(0), format); + String type = getPropertyTypeName(list.get(0), format, true); String names = formatNameList(list, context); - message.append(String.format("Missing required %ss in '%s': %s", type, parentName, names)); + message.append(String.format("Missing required %s in '%s': %s", type, parentName, names)); } else { // Multiple properties of different types message.append(String.format("Missing required properties in '%s':", parentName)); @@ -268,19 +268,24 @@ private String formatMissingPropertiesMessage( * {@code true} if the property is a flag instance * @param format * the format being parsed + * @param plural + * {@code true} for plural form, {@code false} for singular * @return the user-friendly type name */ @NonNull - private static String getFormatPropertyTypeName(boolean isFlag, @NonNull Format format) { + private static String getFormatPropertyTypeName(boolean isFlag, @NonNull Format format, boolean plural) { switch (format) { case XML: - return isFlag ? "attribute" : "element"; + if (isFlag) { + return plural ? "attributes" : "attribute"; + } + return plural ? "elements" : "element"; case JSON: case YAML: - return "property"; + return plural ? "properties" : "property"; default: // Fallback for any future formats - use generic "property" - return "property"; + return plural ? "properties" : "property"; } } @@ -288,6 +293,8 @@ private static String getFormatPropertyTypeName(boolean isFlag, @NonNull Format * Get the user-friendly label for a group of properties in the given format. *

* Used when listing multiple properties of the same type in error messages. + * Delegates to {@link #getFormatPropertyTypeName(boolean, Format, boolean)} and + * capitalizes the result. * * @param isFlags * {@code true} if listing flag instances @@ -297,16 +304,8 @@ private static String getFormatPropertyTypeName(boolean isFlag, @NonNull Format */ @NonNull private static String getFormatPropertyGroupLabel(boolean isFlags, @NonNull Format format) { - switch (format) { - case XML: - return isFlags ? "Attributes" : "Elements"; - case JSON: - case YAML: - return "Properties"; - default: - // Fallback for any future formats - return "Properties"; - } + String typeName = getFormatPropertyTypeName(isFlags, format, true); + return Character.toUpperCase(typeName.charAt(0)) + typeName.substring(1); } /** @@ -332,19 +331,24 @@ private static boolean hasSingleType( /** * Get the property type name for error messages in format-appropriate terms. *

- * Delegates to {@link #getFormatPropertyTypeName(boolean, Format)} based on - * whether the instance is a flag. + * Delegates to {@link #getFormatPropertyTypeName(boolean, Format, boolean)} + * based on whether the instance is a flag. * * @param instance * the property instance * @param format * the format being parsed + * @param plural + * {@code true} for plural form, {@code false} for singular * @return the user-friendly type name appropriate for the format */ @NonNull - private static String getPropertyTypeName(@NonNull IBoundProperty instance, @NonNull Format format) { + private static String getPropertyTypeName( + @NonNull IBoundProperty instance, + @NonNull Format format, + boolean plural) { boolean isFlag = instance instanceof IFlagInstance; - return getFormatPropertyTypeName(isFlag, format); + return getFormatPropertyTypeName(isFlag, format, plural); } /** diff --git a/databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java b/databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java index 05006c138..6f51cefda 100644 --- a/databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java +++ b/databind/src/test/java/gov/nist/secauto/metaschema/databind/io/ValidationErrorMessageTest.java @@ -460,4 +460,89 @@ void testNullFieldValueDoesNotThrowNpe() throws IOException { }); } } + + /** + * Tests for permissive document loading via newPermissiveBoundLoader. + *

+ * These tests verify that permissive loaders skip required field validation, + * which is useful for the fn:doc() function when loading incomplete documents. + */ + @Nested + class PermissiveLoadingTest { + + @Test + void testPermissiveLoaderSkipsRequiredFieldValidation() throws IOException { + // Document missing all required fields + String xml = "" + + ""; + + IBindingContext bindingContext = newBindingContext(); + // Create deserializer and apply permissive configuration + IDeserializer deserializer = bindingContext.newDeserializer(Format.XML, rootClass); + deserializer.disableFeature(DeserializationFeature.DESERIALIZE_VALIDATE_REQUIRED_FIELDS); + + // Should not throw - permissive loading skips required field validation + assertDoesNotThrow(() -> { + deserializer.deserialize(new StringReader(xml), URI.create("test://incomplete.xml")); + }); + } + + @Test + void testStrictLoaderValidatesRequiredFields() throws IOException { + // Document missing all required fields + String xml = "" + + ""; + + IBindingContext bindingContext = newBindingContext(); + // Use deserializer with explicit strict validation enabled + IDeserializer deserializer = bindingContext.newDeserializer(Format.XML, rootClass); + deserializer.enableFeature(DeserializationFeature.DESERIALIZE_VALIDATE_REQUIRED_FIELDS); + + // Should throw because required fields are missing + assertThrows(IOException.class, () -> { + deserializer.deserialize(new StringReader(xml), URI.create("test://incomplete.xml")); + }); + } + + @Test + void testPermissiveLoaderAllowsPartialDocuments() throws IOException { + // Document with some fields but not all required ones + String xml = "" + + "some value" + + ""; + + IBindingContext bindingContext = newBindingContext(); + IDeserializer deserializer = bindingContext.newDeserializer(Format.XML, rootClass); + deserializer.disableFeature(DeserializationFeature.DESERIALIZE_VALIDATE_REQUIRED_FIELDS); + + // Should load successfully despite missing required-flag, required-field, and + // required-assembly + Object result = assertDoesNotThrow( + () -> deserializer.deserialize(new StringReader(xml), URI.create("test://partial.xml"))); + + assertNotNull(result, "Permissive loading should return a result even for partial documents"); + } + + @Test + void testNewPermissiveBoundLoaderHasValidationDisabled() throws IOException { + IBindingContext bindingContext = newBindingContext(); + IBoundLoader permissiveLoader = bindingContext.newPermissiveBoundLoader(); + + // Verify that the permissive loader has DESERIALIZE_VALIDATE_REQUIRED_FIELDS + // disabled + assertFalse(permissiveLoader.isFeatureEnabled(DeserializationFeature.DESERIALIZE_VALIDATE_REQUIRED_FIELDS), + "Permissive loader should have DESERIALIZE_VALIDATE_REQUIRED_FIELDS disabled"); + } + + @Test + void testNewBoundLoaderHasValidationEnabledByDefault() throws IOException { + IBindingContext bindingContext = newBindingContext(); + IBoundLoader regularLoader = bindingContext.newBoundLoader(); + + // Verify that the regular loader has DESERIALIZE_VALIDATE_REQUIRED_FIELDS + // enabled by default + assertTrue(regularLoader.isFeatureEnabled(DeserializationFeature.DESERIALIZE_VALIDATE_REQUIRED_FIELDS), + "Regular loader should have DESERIALIZE_VALIDATE_REQUIRED_FIELDS enabled by default"); + } + } } diff --git a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractConvertSubcommand.java b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractConvertSubcommand.java index dc92a9b98..84ce28428 100644 --- a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractConvertSubcommand.java +++ b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractConvertSubcommand.java @@ -127,7 +127,8 @@ public void execute() throws CommandExecutionException { try { IBindingContext bindingContext = getBindingContext(); - IBoundLoader loader = bindingContext.newBoundLoader(); + // Use permissive loader since convert is not a validation command + IBoundLoader loader = bindingContext.newPermissiveBoundLoader(); if (LOGGER.isInfoEnabled()) { LOGGER.info("Converting '{}'.", source); } diff --git a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/EvaluateMetapathCommand.java b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/EvaluateMetapathCommand.java index ce501a6c4..e3c918810 100644 --- a/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/EvaluateMetapathCommand.java +++ b/metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/EvaluateMetapathCommand.java @@ -126,9 +126,13 @@ private void executeCommand( IModule module = null; INodeItem item = null; + IBoundLoader loader = null; if (cmdLine.hasOption(MetaschemaCommands.METASCHEMA_OPTIONAL_OPTION)) { IBindingContext bindingContext = MetaschemaCommands.newBindingContextWithDynamicCompilation(); + // Use permissive loader since eval is not a validation command + loader = bindingContext.newPermissiveBoundLoader(); + try { module = bindingContext.registerModule(MetaschemaCommands.loadModule( cmdLine, @@ -142,9 +146,6 @@ private void executeCommand( // determine if the query is evaluated against the module or the instance if (cmdLine.hasOption(CONTENT_OPTION)) { // load the content - - IBoundLoader loader = bindingContext.newBoundLoader(); - String contentLocation = ObjectUtils.requireNonNull(cmdLine.getOptionValue(CONTENT_OPTION)); URI contentResource; try { @@ -196,10 +197,16 @@ private void executeCommand( String.format("Must use '%s' to specify the Metapath expression.", EXPRESSION_OPTION.getArgName())); } + // Setup dynamic context with document loader for doc() function support + DynamicContext dynamicContext = new DynamicContext(staticContext); + if (loader != null) { + dynamicContext.setDocumentLoader(loader); + } + try { // Parse and compile the Metapath expression ISequence sequence = IMetapathExpression.compile(expression, staticContext) - .evaluate(item, new DynamicContext(staticContext)); + .evaluate(item, dynamicContext); // handle the metapath results try (Writer stringWriter = new StringWriter()) {