Improve NPE error messages for missing data (#309)

* Some work towards improving NullPointerExceptions to provide more context when missing data is the cause. * Apply suggestions from code review Co-authored-by: A.J. Stein <alexander.stein@nist.gov> * Code formatting. * Fixed misnamed variable reference. --------- Co-authored-by: A.J. Stein <alexander.stein@nist.gov>
metaschema-framework · Feb 23, 2024 · 6c65ce7 · 6c65ce7
1 parent fe02173
commit 6c65ce7
Show file tree

Hide file tree

Showing 12 changed files with 125 additions and 39 deletions.
diff --git a/.../java/gov/nist/secauto/metaschema/core/datatype/markup/flexmark/AbstractMarkupWriter.java b/.../java/gov/nist/secauto/metaschema/core/datatype/markup/flexmark/AbstractMarkupWriter.java
@@ -126,10 +126,10 @@ public abstract class AbstractMarkupWriter<T, E extends Throwable> // NOPMD not
   @NonNull
   private final ListOptions options;
 
-  public AbstractMarkupWriter(@NonNull String namespace, @NonNull ListOptions options, T stream) {
+  public AbstractMarkupWriter(@NonNull String namespace, @NonNull ListOptions options, @NonNull T stream) {
     this.namespace = namespace;
     this.options = options;
-    this.stream = ObjectUtils.requireNonNull(stream);
+    this.stream = stream;
   }
 
   @NonNull
@@ -303,7 +303,7 @@ public void writeLink(
     }
 
     if (!node.getTitle().isBlank()) {
-      String title = ObjectUtils.requireNonNull(node.getTitle().unescape());
+      String title = ObjectUtils.notNull(node.getTitle().unescape());
       attributes.put("title", title);
     }
 

diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/BuildCSTVisitor.java b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/BuildCSTVisitor.java
@@ -753,8 +753,7 @@ protected IExpression handleForexpr(ForexprContext ctx) {
     // for SimpleForBinding ("," SimpleForBinding)*
     int bindingCount = simpleForClause.getChildCount() / 2;
 
-    @NonNull
-    IExpression retval = ObjectUtils.notNull(ctx.exprsingle().accept(this));
+    @NonNull IExpression retval = ObjectUtils.notNull(ctx.exprsingle().accept(this));
 
     // step through in reverse
     for (int idx = bindingCount - 1; idx >= 0; idx--) {
@@ -783,8 +782,7 @@ protected IExpression handleForexpr(ForexprContext ctx) {
 
   @Override
   protected IExpression handleLet(LetexprContext context) {
-    @NonNull
-    IExpression retval = ObjectUtils.notNull(context.exprsingle().accept(this));
+    @NonNull IExpression retval = ObjectUtils.notNull(context.exprsingle().accept(this));
 
     SimpleletclauseContext letClause = context.simpleletclause();
     List<SimpleletbindingContext> clauses = letClause.simpleletbinding();

diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/FunctionService.java b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/FunctionService.java
@@ -77,7 +77,7 @@ public IFunctionLibrary load() {
         .flatMap(library -> {
           return library.getFunctionsAsStream();
         })
-        .forEachOrdered(function -> functionLibrary.registerFunction(ObjectUtils.requireNonNull(function)));
+        .forEachOrdered(function -> functionLibrary.registerFunction(ObjectUtils.notNull(function)));
 
     synchronized (this) {
       this.library = functionLibrary;

diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/model/xml/impl/ModelFactory.java b/core/src/main/java/gov/nist/secauto/metaschema/core/model/xml/impl/ModelFactory.java
@@ -131,8 +131,14 @@ private static Map<String, IAllowedValue> toAllowedValues(
     Map<String, IAllowedValue> allowedValues // NOPMD - intentional
         = new LinkedHashMap<>(xmlObject.sizeOfEnumArray());
     for (AllowedValueType xmlEnum : xmlObject.getEnumList()) {
+      String value = xmlEnum.getValue();
+      if (value == null) {
+        throw new IllegalStateException(String.format("Null value found in allowed value enumeration: %s",
+            xmlObject.xmlText()));
+      }
+
       IAllowedValue allowedValue = IAllowedValue.of(
-          ObjectUtils.requireNonNull(xmlEnum.getValue()),
+          value,
           MarkupStringConverter.toMarkupString(xmlEnum));
       allowedValues.put(allowedValue.getValue(), allowedValue);
     }

diff --git a/.../main/java/gov/nist/secauto/metaschema/core/model/xml/impl/XmlAssemblyModelContainer.java b/.../main/java/gov/nist/secauto/metaschema/core/model/xml/impl/XmlAssemblyModelContainer.java
@@ -182,7 +182,19 @@ private static void handleChoiceGroup( // NOPMD false positive
     XmlChoiceGroupInstance instance = new XmlChoiceGroupInstance(
         (GroupedChoiceType) obj,
         ObjectUtils.notNull(state.getLeft()));
-    ObjectUtils.notNull(state.getRight()).append(instance);
+    XmlAssemblyModelContainer container = ObjectUtils.notNull(state.getRight());
+
+    String groupAsName = instance.getGroupAsName();
+    if (groupAsName == null) {
+      String location = XmlObjectParser.toLocation(obj);
+      String locationCtx = location == null ? "" : " at location " + location;
+      throw new IllegalArgumentException(
+          String.format("Missing group-as for a choice group within the definition '%s'%s.",
+              instance.getContainingDefinition().getName(),
+              locationCtx));
+    }
+    container.getChoiceGroupInstanceMap().put(groupAsName, instance);
+    container.getModelInstances().add(instance);
   }
 
   public void append(@NonNull IFieldInstanceAbsolute instance) {
@@ -203,9 +215,4 @@ public void append(@NonNull IChoiceInstance instance) {
     getChoiceInstances().add(instance);
     getModelInstances().add(instance);
   }
-
-  public void append(@NonNull IChoiceGroupInstance instance) {
-    getChoiceGroupInstanceMap().put(ObjectUtils.requireNonNull(instance.getGroupAsName()), instance);
-    getModelInstances().add(instance);
-  }
 }
diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/model/xml/impl/XmlObjectParser.java b/core/src/main/java/gov/nist/secauto/metaschema/core/model/xml/impl/XmlObjectParser.java
@@ -107,8 +107,14 @@ private String getXpath() {
     return xpath;
   }
 
+  @SuppressWarnings({ "resource", "null" })
   @Nullable
-  public static String getLocation(@NonNull XmlCursor cursor) {
+  public static String toLocation(@NonNull XmlObject obj) {
+    return toLocation(obj.newCursor());
+  }
+
+  @Nullable
+  public static String toLocation(@NonNull XmlCursor cursor) {
     String retval = null;
     XmlBookmark bookmark = cursor.getBookmark(XmlLineNumber.class);
     if (bookmark != null) {
@@ -150,7 +156,7 @@ protected Handler<T> identifyHandler(@NonNull XmlCursor cursor, @NonNull XmlObje
     QName qname = cursor.getName();
     Handler<T> retval = getElementNameToHandlerMap().get(qname);
     if (retval == null) {
-      String location = getLocation(cursor);
+      String location = toLocation(cursor);
       if (location == null) {
         location = "";
       } else {

diff --git a/databind/src/main/java/gov/nist/secauto/metaschema/databind/DefaultBindingContext.java b/databind/src/main/java/gov/nist/secauto/metaschema/databind/DefaultBindingContext.java
@@ -145,10 +145,14 @@ public IBindingMatcher registerBindingMatcher(@NonNull IBoundDefinitionModelAsse
   @Override
   public IBindingMatcher registerBindingMatcher(@NonNull Class<?> clazz) {
     IBoundDefinitionModelComplex definition = getBoundDefinitionForClass(clazz);
+    if (definition == null) {
+      throw new IllegalArgumentException(String.format("Unable to find bound definition for class '%s'.",
+          clazz.getName()));
+    }
 
     try {
       IBoundDefinitionModelAssembly assemblyDefinition = IBoundDefinitionModelAssembly.class.cast(definition);
-      return registerBindingMatcher(ObjectUtils.requireNonNull(assemblyDefinition));
+      return registerBindingMatcher(ObjectUtils.notNull(assemblyDefinition));
     } catch (ClassCastException ex) {
       throw new IllegalArgumentException(
           String.format("The provided class '%s' is not a root assembly.", clazz.getName()), ex);
@@ -265,7 +269,7 @@ public IBindingContext registerModule(
             }
           })
           .forEachOrdered(clazz -> {
-            IBoundModule boundModule = registerModule(ObjectUtils.requireNonNull(clazz));
+            IBoundModule boundModule = registerModule(ObjectUtils.notNull(clazz));
             // force the binding matchers to load
             boundModule.getRootAssemblyDefinitions();
           });

diff --git a/...bind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.java b/...bind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.java
@@ -394,8 +394,15 @@ public Object readChoiceGroupItem(Object parentItem, IBoundInstanceModelChoiceGr
       JsonParser parser = getReader();
       ObjectNode node = parser.readValueAsTree();
 
-      JsonNode descriminatorNode = ObjectUtils.requireNonNull(node.get(instance.getJsonDiscriminatorProperty()));
-      String discriminator = ObjectUtils.requireNonNull(descriminatorNode.asText());
+      String discriminatorProperty = instance.getJsonDiscriminatorProperty();
+      JsonNode discriminatorNode = node.get(discriminatorProperty);
+      if (discriminatorNode == null) {
+        throw new IllegalArgumentException(String.format(
+            "Unable to find discriminator property '%s' for object at '%s'.",
+            discriminatorProperty,
+            JsonUtil.toString(parser)));
+      }
+      String discriminator = ObjectUtils.requireNonNull(discriminatorNode.asText());
 
       IBoundInstanceModelGroupedNamed actualInstance = instance.getGroupedModelInstance(discriminator);
       assert actualInstance != null;
@@ -432,7 +439,8 @@ private JsonKeyBodyHandler(
       @Override
       public void accept(IBoundDefinitionModelComplex definition, Object parentItem, IJsonProblemHandler problemHandler)
           throws IOException {
-        @SuppressWarnings("resource") JsonParser parser = getReader();
+        @SuppressWarnings("resource")
+        JsonParser parser = getReader();
         JsonUtil.assertCurrent(parser, JsonToken.FIELD_NAME);
 
         // the field will be the JSON key
@@ -469,7 +477,8 @@ public void accept(
           Object parentItem,
           IJsonProblemHandler problemHandler)
           throws IOException {
-        @SuppressWarnings("resource") JsonParser parser = getReader();
+        @SuppressWarnings("resource")
+        JsonParser parser = getReader();
 
         // advance past the start object
         JsonUtil.assertAndAdvance(parser, JsonToken.START_OBJECT);
@@ -593,7 +602,8 @@ public boolean handleUnknownProperty(IBoundDefinitionModelComplex definition, Ob
         if (foundJsonValueKey) {
           retval = delegate.handleUnknownProperty(definition, parentItem, fieldName, reader);
         } else {
-          @SuppressWarnings("resource") JsonParser parser = getReader();
+          @SuppressWarnings("resource")
+          JsonParser parser = getReader();
           // handle JSON value key
           String key = ObjectUtils.notNull(parser.getCurrentName());
           Object keyValue = jsonValueKyeFlag.getJavaTypeAdapter().parse(key);
@@ -693,7 +703,8 @@ public List<?> readList() throws IOException {
 
       IBoundInstanceModel instance = getCollectionInfo().getInstance();
 
-      @SuppressWarnings("PMD.UseConcurrentHashMap") Map<String, Object> items = new LinkedHashMap<>();
+      @SuppressWarnings("PMD.UseConcurrentHashMap")
+      Map<String, Object> items = new LinkedHashMap<>();
 
       // A map value is always wrapped in a START_OBJECT, since fields are used for
       // the keys
@@ -711,7 +722,12 @@ public List<?> readList() throws IOException {
         IBoundInstanceFlag jsonKey = instance.getItemJsonKey(item);
         assert jsonKey != null;
 
-        String key = ObjectUtils.requireNonNull(jsonKey.getValue(item)).toString();
+        Object keyValue = jsonKey.getValue(item);
+        if (keyValue == null) {
+          throw new IOException(String.format("Null value for json-key for definition '%s'",
+              jsonKey.getContainingDefinition().toCoordinates()));
+        }
+        String key = jsonKey.getJavaTypeAdapter().asString(keyValue);
         items.put(key, item);
 
         // the next item will be a FIELD_NAME, or we will encounter an END_OBJECT if all

diff --git a/...bind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonWriter.java b/...bind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonWriter.java
@@ -29,7 +29,6 @@
 import com.fasterxml.jackson.core.JsonGenerator;
 
 import gov.nist.secauto.metaschema.core.model.JsonGroupAsBehavior;
-import gov.nist.secauto.metaschema.core.util.ObjectUtils;
 import gov.nist.secauto.metaschema.databind.model.IBoundDefinitionModelAssembly;
 import gov.nist.secauto.metaschema.databind.model.IBoundDefinitionModelFieldComplex;
 import gov.nist.secauto.metaschema.databind.model.IBoundFieldValue;
@@ -128,8 +127,13 @@ public void writeItemFieldValue(Object parentItem, IBoundFieldValue fieldValue)
 
       String valueKeyName;
       if (jsonValueKey != null) {
+        Object keyValue = jsonValueKey.getValue(parentItem);
+        if (keyValue == null) {
+          throw new IOException(String.format("Null value for json-value-key for definition '%s'",
+              jsonValueKey.getContainingDefinition().toCoordinates()));
+        }
         // this is the JSON value key case
-        valueKeyName = ObjectUtils.requireNonNull(jsonValueKey.getValue(parentItem)).toString();
+        valueKeyName = jsonValueKey.getJavaTypeAdapter().asString(keyValue);
       } else {
         valueKeyName = fieldValue.getParentFieldDefinition().getEffectiveJsonValueKeyName();
       }
@@ -253,8 +257,9 @@ public void writeChoiceGroupItem(Object item, IBoundInstanceModelChoiceGroup ins
       actualInstance.writeItem(item, this);
     }
 
-    private void writeScalarItem(Object item, IFeatureScalarItemValueHandler handler) throws IOException {
-      handler.getJavaTypeAdapter().writeJsonValue(ObjectUtils.requireNonNull(item), writer);
+    private void writeScalarItem(@NonNull Object item, @NonNull IFeatureScalarItemValueHandler handler)
+        throws IOException {
+      handler.getJavaTypeAdapter().writeJsonValue(item, writer);
     }
 
     private <T extends IBoundInstanceModelGroupedNamed> void writeDiscriminatorProperty(
@@ -297,8 +302,14 @@ private <T extends IFeatureComplexItemValueHandler & IBoundInstanceModel> void w
 
       IBoundInstanceFlag jsonKey = handler.getItemJsonKey(parentItem);
       if (jsonKey != null) {
+        Object keyValue = jsonKey.getValue(parentItem);
+        if (keyValue == null) {
+          throw new IOException(String.format("Null value for json-key for definition '%s'",
+              jsonKey.getContainingDefinition().toCoordinates()));
+        }
+
         // the field will be the JSON key value
-        String key = jsonKey.getJavaTypeAdapter().asString(ObjectUtils.requireNonNull(jsonKey.getValue(parentItem)));
+        String key = jsonKey.getJavaTypeAdapter().asString(keyValue);
         writer.writeFieldName(key);
 
         // next the value will be a start object
@@ -323,8 +334,14 @@ private <T extends IFeatureComplexItemValueHandler & IBoundInstanceModelGroupedN
       IBoundInstanceModelChoiceGroup choiceGroup = handler.getParentContainer();
       IBoundInstanceFlag jsonKey = choiceGroup.getItemJsonKey(parentItem);
       if (jsonKey != null) {
+        Object keyValue = jsonKey.getValue(parentItem);
+        if (keyValue == null) {
+          throw new IOException(String.format("Null value for json-key for definition '%s'",
+              jsonKey.getContainingDefinition().toCoordinates()));
+        }
+
         // the field will be the JSON key value
-        String key = jsonKey.getJavaTypeAdapter().asString(ObjectUtils.requireNonNull(jsonKey.getValue(parentItem)));
+        String key = jsonKey.getJavaTypeAdapter().asString(keyValue);
         writer.writeFieldName(key);
 
         // next the value will be a start object

diff --git a/...c/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/BindingModule.java b/...c/main/java/gov/nist/secauto/metaschema/databind/model/metaschema/impl/BindingModule.java
@@ -183,12 +183,20 @@ public URI getLocation() {
 
   @Override
   public MarkupLine getName() {
-    return ObjectUtils.requireNonNull(getBinding().getSchemaName());
+    MarkupLine retval = getBinding().getSchemaName();
+    if (retval == null) {
+      throw new IllegalStateException(String.format("The schema name is NULL for module '%s'.", getLocation()));
+    }
+    return retval;
   }
 
   @Override
   public String getVersion() {
-    return ObjectUtils.requireNonNull(getBinding().getSchemaVersion());
+    String retval = getBinding().getSchemaVersion();
+    if (retval == null) {
+      throw new IllegalStateException(String.format("The schema version is NULL for module '%s'.", getLocation()));
+    }
+    return retval;
   }
 
   @Override
@@ -198,17 +206,30 @@ public MarkupMultiline getRemarks() {
 
   @Override
   public String getShortName() {
-    return ObjectUtils.requireNonNull(getBinding().getShortName());
+    String retval = getBinding().getShortName();
+    if (retval == null) {
+      throw new IllegalStateException(String.format("The schema short name is NULL for module '%s'.", getLocation()));
+    }
+    return retval;
   }
 
   @Override
   public URI getXmlNamespace() {
-    return ObjectUtils.requireNonNull(getBinding().getNamespace());
+    URI retval = getBinding().getNamespace();
+    if (retval == null) {
+      throw new IllegalStateException(
+          String.format("The XML schema namespace is NULL for module '%s'.", getLocation()));
+    }
+    return retval;
   }
 
   @Override
   public URI getJsonBaseUri() {
-    return ObjectUtils.requireNonNull(getBinding().getJsonBaseUri());
+    URI retval = getBinding().getJsonBaseUri();
+    if (retval == null) {
+      throw new IllegalStateException(String.format("The JSON schema URI is NULL for module '%s'.", getLocation()));
+    }
+    return retval;
   }
 
   @NonNull

diff --git a/...c/test/java/gov/nist/secauto/metaschema/databind/model/AbstractBoundModelTestSupport.java b/...c/test/java/gov/nist/secauto/metaschema/databind/model/AbstractBoundModelTestSupport.java
@@ -76,7 +76,12 @@ protected IBindingContext getBindingContext() {
 
   @NonNull
   protected IBoundDefinitionModel registerClassBinding(@NonNull Class<?> clazz) {
-    return ObjectUtils.requireNonNull(getBindingContext().getBoundDefinitionForClass(clazz));
+    IBoundDefinitionModel definition = getBindingContext().getBoundDefinitionForClass(clazz);
+    if (definition == null) {
+      throw new IllegalArgumentException(String.format("Unable to find bound definition for class '%s'.",
+          clazz.getName()));
+    }
+    return definition;
   }
 
   @NonNull

diff --git a/...va/gov/nist/secauto/metaschema/schemagen/json/impl/AbstractModelDefinitionJsonSchema.java b/...va/gov/nist/secauto/metaschema/schemagen/json/impl/AbstractModelDefinitionJsonSchema.java
@@ -71,7 +71,13 @@ protected AbstractModelDefinitionJsonSchema(
 
     // determine the flag instances to generate
     if (jsonKeyFlagName != null) {
-      IFlagInstance jsonKeyFlag = ObjectUtils.requireNonNull(definition.getFlagInstanceByName(jsonKeyFlagName));
+      IFlagInstance jsonKeyFlag = definition.getFlagInstanceByName(jsonKeyFlagName);
+      if (jsonKeyFlag == null) {
+        throw new IllegalArgumentException(
+            String.format("The referenced json-key flag-name '%s' does not exist on definition '%s'.",
+                jsonKeyFlagName,
+                definition.getName()));
+      }
       flagStream = flagStream.filter(instance -> instance != jsonKeyFlag);
     }