Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 17 additions & 14 deletions PRDs/20251224-codegen-quality/implementation-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -249,16 +249,16 @@ This PR creates the databind bootstrap infrastructure and regenerates the databi

---

## PR 4: Parser Required Field Validation
## PR 4: Parser Required Field Validation

| Attribute | Value |
|-----------|-------|
| **Files Changed** | ~10 |
| **Files Changed** | ~25 |
| **Risk Level** | Medium |
| **Dependencies** | PR 1 |
| **Target Branch** | develop |
| **Status** | Pending |
| **Pull Request** | |
| **Status** | Completed |
| **Pull Request** | [#593](https://github.com/metaschema-framework/metaschema-java/pull/593) |

This PR adds validation during parsing to emit meaningful errors when required fields are missing, and includes type compatibility validation for collection class overrides.

Expand Down Expand Up @@ -294,15 +294,18 @@ Currently, when a required field/flag is missing from input data, the generated

### Acceptance Criteria

- [ ] Parser validates required fields are present during deserialization
- [ ] Missing required field produces clear error with field name and location
- [ ] Collection class override validates type compatibility (List/Map)
- [ ] Validation is efficient (no per-field overhead)
- [ ] Unit tests for required field validation
- [ ] Unit tests for collection class type validation
- [ ] `mvn checkstyle:check` passes
- [ ] All tests pass: `mvn test`
- [ ] Build succeeds: `mvn clean install -PCI -Prelease`
- [x] Parser validates required fields are present during deserialization
- [x] Missing required field produces clear error with field name and location
- [x] Collection class override validates type compatibility (Collection/Map)
- [x] Validation is efficient (no per-field overhead)
- [x] Choice group support - only error if ALL options in choice are missing
- [x] Unit tests for required field validation
- [x] Unit tests for collection class type validation
- [x] Required field validation enabled by default
- [x] CLI validators disable required field validation (schema handles it)
- [x] `mvn checkstyle:check` passes
- [x] All tests pass: `mvn test`
- [x] Build succeeds: `mvn clean install -PCI -Prelease`

---

Expand All @@ -313,7 +316,7 @@ Currently, when a required field/flag is missing from input data, the generated
| 1 | Code generator improvements + metaschema-testing regeneration | 20 | Low | None | ✅ Completed ([#577](https://github.com/metaschema-framework/metaschema-java/pull/577)) |
| 2 | Collection class override support | ~15 | Low | PR 1 | ✅ Completed ([#584](https://github.com/metaschema-framework/metaschema-java/pull/584)) |
| 3 | Databind bootstrap setup + regeneration | ~55 | Medium | PR 1, PR 2 | ✅ Completed (combined with PR 2) |
| 4 | Parser required field validation | ~10 | Medium | PR 1 | Pending |
| 4 | Parser required field validation | ~25 | Medium | PR 1 | ✅ Completed ([#593](https://github.com/metaschema-framework/metaschema-java/pull/593)) |

**Total Estimated PRs**: 4 (3 actual - PR 2 and PR 3 combined)
**Total Estimated Files**: ~100
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,8 @@ default IConstraintValidator newValidator(
@Nullable IConfiguration<ValidationFeature<?>> config) {
IBoundLoader loader = newBoundLoader();
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);
Expand Down Expand Up @@ -570,6 +572,8 @@ default IValidationResult validateWithConstraints(
throws IOException, ConstraintValidationException {
IBoundLoader loader = newBoundLoader();
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import gov.nist.secauto.metaschema.databind.IBindingContext;
import gov.nist.secauto.metaschema.databind.codegen.ClassUtils;
import gov.nist.secauto.metaschema.databind.config.binding.MetaschemaBindings;
import gov.nist.secauto.metaschema.databind.io.BindingException;
import gov.nist.secauto.metaschema.databind.io.Format;
import gov.nist.secauto.metaschema.databind.io.IDeserializer;

Expand All @@ -25,6 +26,7 @@
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.Path;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -295,8 +297,10 @@ public MetaschemaBindingConfiguration addMetaschemaBindingConfiguration(
* the configuration resource
* @throws IOException
* if an error occurred while reading the {@code file}
* @throws BindingException
* if an error occurred while processing the binding configuration
*/
public void load(Path file) throws IOException {
public void load(Path file) throws IOException, BindingException {
URL resource = file.toAbsolutePath().normalize().toUri().toURL();
load(resource);
}
Expand All @@ -308,8 +312,10 @@ public void load(Path file) throws IOException {
* the configuration resource
* @throws IOException
* if an error occurred while reading the {@code file}
* @throws BindingException
* if an error occurred while processing the binding configuration
*/
public void load(File file) throws IOException {
public void load(File file) throws IOException, BindingException {
load(file.toPath());
}

Expand All @@ -320,8 +326,10 @@ public void load(File file) throws IOException {
* the configuration resource
* @throws IOException
* if an error occurred while reading the {@code resource}
* @throws BindingException
* if an error occurred while processing the binding configuration
*/
public void load(URL resource) throws IOException {
public void load(URL resource) throws IOException, BindingException {
IBindingContext context = IBindingContext.newInstance();
IDeserializer<MetaschemaBindings> deserializer = context.newDeserializer(Format.XML, MetaschemaBindings.class);

Expand Down Expand Up @@ -364,7 +372,7 @@ private void processModelBindingConfig(MetaschemaBindings.ModelBinding model) {
}

private void processMetaschemaBindingConfig(URL configResource, MetaschemaBindings.MetaschemaBinding metaschema)
throws MalformedURLException, URISyntaxException {
throws MalformedURLException, URISyntaxException, BindingException {
String href = metaschema.getHref().toString();
URL moduleUrl = new URL(configResource, href);
String moduleUri = ObjectUtils.notNull(moduleUrl.toURI().normalize().toString());
Expand Down Expand Up @@ -441,14 +449,16 @@ private void processMetaschemaBindingConfig(URL configResource, MetaschemaBindin
* function to extract the Java config from a binding
* @param collectionClassAccessor
* function to extract the collection class name from a Java config
* @throws BindingException
* if the collection class is invalid or cannot be found
*/
private static <P, J> void processPropertyBindings(
@NonNull MetaschemaBindingConfiguration metaschemaConfig,
@NonNull String definitionName,
@Nullable List<P> propertyBindings,
@NonNull Function<P, String> nameAccessor,
@NonNull Function<P, J> javaAccessor,
@NonNull Function<J, String> collectionClassAccessor) {
@NonNull Function<J, String> collectionClassAccessor) throws BindingException {
if (propertyBindings == null) {
return;
}
Expand All @@ -466,13 +476,52 @@ private static <P, J> void processPropertyBindings(

String collectionClassName = collectionClassAccessor.apply(java);
if (collectionClassName != null) {
// Validate the collection class
validateCollectionClass(collectionClassName, definitionName, propertyName);

IMutablePropertyBindingConfiguration config = new DefaultPropertyBindingConfiguration();
config.setCollectionClassName(collectionClassName);
metaschemaConfig.addPropertyBindingConfig(definitionName, propertyName, config);
}
}
}

/**
* Validate that the specified collection class exists and implements a
* supported collection interface (Collection or Map).
*
* @param collectionClassName
* the fully qualified class name to validate
* @param definitionName
* the name of the containing definition (for error messages)
* @param propertyName
* the name of the property (for error messages)
* @throws BindingException
* if the class cannot be found or does not implement a supported
* collection interface
*/
private static void validateCollectionClass(
@NonNull String collectionClassName,
@NonNull String definitionName,
@NonNull String propertyName) throws BindingException {
Class<?> collectionClass;
try {
collectionClass = Class.forName(collectionClassName);
} catch (ClassNotFoundException ex) {
throw new BindingException(String.format(
"Collection class '%s' for property '%s' in definition '%s' could not be found",
collectionClassName, propertyName, definitionName), ex);
}

// Check if the class implements Collection or Map
if (!Collection.class.isAssignableFrom(collectionClass) && !Map.class.isAssignableFrom(collectionClass)) {
throw new BindingException(String.format(
"Collection class '%s' for property '%s' in definition '%s' must implement "
+ "java.util.Collection or java.util.Map",
collectionClassName, propertyName, definitionName));
}
}

/**
* Process property bindings from a define-assembly-binding element.
*
Expand All @@ -482,11 +531,14 @@ private static <P, J> void processPropertyBindings(
* the name of the containing definition
* @param propertyBindings
* the list of property bindings to process
* @throws BindingException
* if the collection class is invalid or cannot be found
*/
private static void processAssemblyPropertyBindings(
@NonNull MetaschemaBindingConfiguration metaschemaConfig,
@NonNull String definitionName,
@Nullable List<MetaschemaBindings.MetaschemaBinding.DefineAssemblyBinding.PropertyBinding> propertyBindings) {
@Nullable List<MetaschemaBindings.MetaschemaBinding.DefineAssemblyBinding.PropertyBinding> propertyBindings)
throws BindingException {
processPropertyBindings(
metaschemaConfig,
definitionName,
Expand All @@ -505,11 +557,14 @@ private static void processAssemblyPropertyBindings(
* the name of the containing definition
* @param propertyBindings
* the list of property bindings to process
* @throws BindingException
* if the collection class is invalid or cannot be found
*/
private static void processFieldPropertyBindings(
@NonNull MetaschemaBindingConfiguration metaschemaConfig,
@NonNull String definitionName,
@Nullable List<MetaschemaBindings.MetaschemaBinding.DefineFieldBinding.PropertyBinding> propertyBindings) {
@Nullable List<MetaschemaBindings.MetaschemaBinding.DefineFieldBinding.PropertyBinding> propertyBindings)
throws BindingException {
processPropertyBindings(
metaschemaConfig,
definitionName,
Expand Down
Loading
Loading