Improve exception reporting for null values on optional field values

[aj-stein-gsa]

User Story

As a developer using this library and related tooling, when encountering an optional field value, I would like a more precise error message and controlled error behavior when such a null value is visited. I do not want a null pointer exception.

NOTE: This is a fix in the core library that is the root cause of the bug reported in https://github.com/GSA/fedramp-automation/issues/787.

Goals

• [ ] Improve error handling for users of metaschema-java based tools related to null field values
• [ ] Gracefully fail in the occurrence of null values for a field value

Dependencies

N/A

Acceptance Criteria

• [ ] All website and readme documentation affected by the changes in this issue have been updated.
• [ ] A Pull Request (PR) is submitted that fully addresses the goals of this User Story. This issue is referenced in the PR.
  • [ ] Remove the ObjectUtils.requireNonNull() check on https://github.com/metaschema-framework/metaschema-java/blob/ebdf350362e3c75f2fb39969a407089464a9b7c0/databind/src/main/java/gov/nist/secauto/metaschema/databind/model/IBoundDefinitionModelFieldComplex.java#L77 and other applicable areas in the code base.
  • [ ] Properly implement null-checking and throw the correct exception type in place of the NPE generated from the replaced null check above
  • [ ] Implement tests to check for correctness of implementation and gaps as approprriate
• [ ] The CI-CD build process runs without any reported errors on the PR. This can be confirmed by reviewing that all checks have passed in the PR.

Revisions

No response

[aj-stein-gsa]

@david-waltermire do we need to wrap this into the upcoming release? I do not know if we are planning that for the next day or two or we have some wiggle room. Let me know, thanks.

[david-waltermire]

I can include this in the 2.0.0 release.

There are two ways to handle this.

1. Turn this into an error when parsing, since this is caused by missing expected information in the instance. This would keep the non-null requirement on the field value. This might have a minor impact to parsing performance. (low effort)
2. Relax the non-null requirement on field values and add checks where this value is accessed. This will still result in Metapath processing errors, but will make the API a bit more robust, potentially at the expense of performance, since additional null checks will need to be added to the code paths. (moderate effort)

I am leaning towards option 1, but would like to consider other thoughts.

[aj-stein-gsa]

I can include this in the 2.0.0 release.

There are two ways to handle this.

I am leaning towards option 1, but would like to consider other thoughts.

I guess it depends on what is more accessible to developers or engineers extending tools on top of it. I guess it depends on what you want to present to the user with either option 1. Can it give relevant information to the developer/engineer sufficient context to know where in the document the issue occurs without more work and surfacing it specifically as a Metapath processing error? Can such context be given without Metapath processing and presumably Metapath location?

Generally, I am fine with Option 1, but wanted to understand a little more detail on that.

[david-waltermire]

To handle this at parse time null checking needs to be handled in the following locations:

• JSON/YAML: https://github.com/metaschema-framework/metaschema-java/blob/98a2c539ffc689e4be8c6c9b9221e0ffa93f9aa3/databind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.java#L247
• XML: https://github.com/metaschema-framework/metaschema-java/blob/98a2c539ffc689e4be8c6c9b9221e0ffa93f9aa3/databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java#L465

[david-waltermire]

I created a feature branch with a test for this.

[david-waltermire]

227 summarizes why this is not possible to completely fix right now. Adding to backlog.