Skip to content

Commit

Permalink
Improved error handling to better report errors such as in #221.
Browse files Browse the repository at this point in the history 
Changed expected behavior in unit test to provide null values in JSON and empty values in XML, which is consistent with current behavior.
Improved error handling for data type conformance issues. Related to GSA/fedramp-automation#823.
  • Loading branch information
@david-waltermire
david-waltermire committed Oct 30, 2024
1 parent a42d838 commit 70f0436
Show file tree
Hide file tree
Showing 10 changed files with 181 additions and 55 deletions.
6 changes: 3 additions & 3 deletions core/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@
</distributionManagement>

<scm>
<url>${scm.url}/tree/develop/core</url>
<tag>HEAD</tag>
</scm>
<url>${scm.url}/tree/develop/core</url>
<tag>HEAD</tag>
</scm>

<dependencies>
<dependency>
Expand Down
35 changes: 27 additions & 8 deletions core/src/main/java/gov/nist/secauto/metaschema/core/datatype/AbstractDataTypeAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@

import gov.nist.secauto.metaschema.core.metapath.function.InvalidValueForCastFunctionException;
import gov.nist.secauto.metaschema.core.metapath.item.atomic.IAnyAtomicItem;
import gov.nist.secauto.metaschema.core.model.util.JsonUtil;
import gov.nist.secauto.metaschema.core.model.util.XmlEventUtil;
import gov.nist.secauto.metaschema.core.util.ObjectUtils;

import org.codehaus.stax2.XMLEventReader2;
import org.codehaus.stax2.XMLStreamWriter2;
Expand Down Expand Up @@ -42,7 +44,7 @@ public abstract class AbstractDataTypeAdapter<TYPE, ITEM_TYPE extends IAnyAtomic
/**
* The default JSON property name for a Metaschema field value.
*/
public static final String DEFAULT_JSON_FIELD_NAME = "STRVALUE";
public static final String DEFAULT_JSON_FIELD_VALUE_NAME = "STRVALUE";

@NonNull
private final Class<TYPE> clazz;
Expand Down Expand Up @@ -75,7 +77,7 @@ public boolean canHandleQName(QName nextQName) {

@Override
public String getDefaultJsonValueKey() {
return DEFAULT_JSON_FIELD_NAME;
return DEFAULT_JSON_FIELD_VALUE_NAME;
}

@Override
Expand All @@ -91,8 +93,8 @@ public boolean isXmlMixed() {
@Override
public TYPE parse(XMLEventReader2 eventReader) throws IOException {
StringBuilder builder = new StringBuilder();
XMLEvent nextEvent;
try {
XMLEvent nextEvent;
while (!(nextEvent = eventReader.peek()).isEndElement()) {
if (!nextEvent.isCharacters()) {
throw new IOException(String.format("Invalid content '%s' at %s", XmlEventUtil.toString(nextEvent),
Expand All @@ -105,9 +107,17 @@ public TYPE parse(XMLEventReader2 eventReader) throws IOException {
}

// trim leading and trailing whitespace
@SuppressWarnings("null")
@NonNull String value = builder.toString().trim();
return parse(value);
String value = ObjectUtils.notNull(builder.toString().trim());
try {
return parse(value);
} catch (IllegalArgumentException ex) {
throw new IOException(
String.format("Malformed data '%s'%s. %s",
value,
XmlEventUtil.generateLocationMessage(nextEvent),
ex.getLocalizedMessage()),
ex);
}
} catch (XMLStreamException ex) {
throw new IOException(ex);
}
Expand All @@ -121,11 +131,20 @@ public TYPE parse(XMLEventReader2 eventReader) throws IOException {
public TYPE parse(JsonParser parser) throws IOException {
String value = parser.getValueAsString();
if (value == null) {
throw new IOException("Unable to parse field value as text");
throw new IOException("Unable to null value as text");
}
// skip over value
parser.nextToken();
return parse(value);
try {
return parse(value);
} catch (IllegalArgumentException ex) {
throw new IOException(
String.format("Malformed data '%s'%s. %s",
value,
JsonUtil.generateLocationMessage(parser),
ex.getLocalizedMessage()),
ex);
}
}

@SuppressWarnings("null")
Expand Down
2 changes: 2 additions & 0 deletions core/src/main/java/gov/nist/secauto/metaschema/core/datatype/IDataTypeAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,8 @@ default boolean isAtomic() {
* @return a supplier that will provide new instances of the parsed data
* @throws IOException
* if an error occurs while parsing
* @throws IllegalArgumentException
* if the provided value is invalid based on the data type
* @see #parse(String)
*/
@NonNull
Expand Down
15 changes: 12 additions & 3 deletions core/src/main/java/gov/nist/secauto/metaschema/core/model/AbstractAssemblyInstance.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

import gov.nist.secauto.metaschema.core.util.ObjectUtils;

import javax.xml.namespace.QName;

import edu.umd.cs.findbugs.annotations.NonNull;

/**
Expand Down Expand Up @@ -42,10 +44,17 @@ protected AbstractAssemblyInstance(@NonNull PARENT parent) {

@Override
public DEFINITION getDefinition() {
QName qname = getReferencedDefinitionQName();
// this should always be not null
return ObjectUtils.asType(ObjectUtils.requireNonNull(
getContainingModule()
.getScopedAssemblyDefinitionByName(getReferencedDefinitionQName())));
IAssemblyDefinition definition = getContainingModule().getScopedAssemblyDefinitionByName(qname);
if (definition == null) {
throw new IllegalStateException(
String.format("Unable to resolve assembly reference '%s' in definition '%s' in module '%s'",
qname,
getParentContainer().getOwningDefinition().getName(),
getContainingModule().getShortName()));
}
return ObjectUtils.asType(definition);
}

/**
Expand Down
15 changes: 12 additions & 3 deletions core/src/main/java/gov/nist/secauto/metaschema/core/model/AbstractFieldInstance.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

import gov.nist.secauto.metaschema.core.util.ObjectUtils;

import javax.xml.namespace.QName;

import edu.umd.cs.findbugs.annotations.NonNull;

/**
Expand Down Expand Up @@ -42,10 +44,17 @@ protected AbstractFieldInstance(@NonNull PARENT parent) {

@Override
public DEFINITION getDefinition() {
QName qname = getReferencedDefinitionQName();
// this should always be not null
return ObjectUtils.asType(ObjectUtils.requireNonNull(
getContainingModule()
.getScopedFieldDefinitionByName(getReferencedDefinitionQName())));
IFieldDefinition definition = getContainingModule().getScopedFieldDefinitionByName(qname);
if (definition == null) {
throw new IllegalStateException(
String.format("Unable to resolve field reference '%s' in definition '%s' in module '%s'",
qname,
getParentContainer().getOwningDefinition().getName(),
getContainingModule().getShortName()));
}
return ObjectUtils.asType(definition);
}

/**
Expand Down
15 changes: 12 additions & 3 deletions core/src/main/java/gov/nist/secauto/metaschema/core/model/AbstractFlagInstance.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

import gov.nist.secauto.metaschema.core.util.ObjectUtils;

import javax.xml.namespace.QName;

import edu.umd.cs.findbugs.annotations.NonNull;

/**
Expand Down Expand Up @@ -38,10 +40,17 @@ protected AbstractFlagInstance(@NonNull PARENT parent) {

@Override
public DEFINITION getDefinition() {
QName qname = getReferencedDefinitionQName();
// this should always be not null
return ObjectUtils.asType(ObjectUtils.requireNonNull(
getContainingModule()
.getScopedFlagDefinitionByName(getReferencedDefinitionQName())));
IFlagDefinition definition = getContainingModule().getScopedFlagDefinitionByName(qname);
if (definition == null) {
throw new IllegalStateException(
String.format("Unable to resolve field reference '%s' in definition '%s' in module '%s'",
qname,
getContainingDefinition().getName(),
getContainingModule().getShortName()));
}
return ObjectUtils.asType(definition);
}

@Override
Expand Down
49 changes: 38 additions & 11 deletions ...bind/src/main/java/gov/nist/secauto/metaschema/databind/io/json/MetaschemaJsonReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,17 @@ public IBoundObject readItemField(IBoundObject parentItem, IBoundDefinitionModel
@Override
public Object readItemFieldValue(IBoundObject parentItem, IBoundFieldValue fieldValue) throws IOException {
// read the field value's value
return readScalarItem(fieldValue);
return checkMissingFieldValue(readScalarItem(fieldValue));
}

@NonNull
private Object checkMissingFieldValue(Object value) throws IOException {
if (value == null && LOGGER.isWarnEnabled()) {
LOGGER.atWarn().log("Missing property value{}",
JsonUtil.generateLocationMessage(getReader()));
}
// TODO: change nullness annotations to be @Nullable
return ObjectUtils.notNull(value);
}

@Override
Expand Down Expand Up @@ -491,8 +501,17 @@ public void accept(

// the field will be the JSON key
String key = ObjectUtils.notNull(parser.currentName());
Object value = jsonKey.getDefinition().getJavaTypeAdapter().parse(key);
jsonKey.setValue(parent, ObjectUtils.notNull(value.toString()));
try {
Object value = jsonKey.getDefinition().getJavaTypeAdapter().parse(key);
jsonKey.setValue(parent, ObjectUtils.notNull(value.toString()));
} catch (IllegalArgumentException ex) {
throw new IOException(
String.format("Malformed data '%s'%s. %s",
key,
JsonUtil.generateLocationMessage(parser),
ex.getLocalizedMessage()),
ex);
}

// skip to the next token
parser.nextToken();
Expand Down Expand Up @@ -626,14 +645,14 @@ private final class JsomValueKeyProblemHandler implements IJsonProblemHandler {
@NonNull
private final IJsonProblemHandler delegate;
@NonNull
private final IBoundInstanceFlag jsonValueKyeFlag;
private final IBoundInstanceFlag jsonValueKeyFlag;
private boolean foundJsonValueKey; // false

private JsomValueKeyProblemHandler(
@NonNull IJsonProblemHandler delegate,
@NonNull IBoundInstanceFlag jsonValueKyeFlag) {
@NonNull IBoundInstanceFlag jsonValueKeyFlag) {
this.delegate = delegate;
this.jsonValueKyeFlag = jsonValueKyeFlag;
this.jsonValueKeyFlag = jsonValueKeyFlag;
}

@Override
Expand All @@ -656,9 +675,17 @@ public boolean handleUnknownProperty(
} else {
// handle JSON value key
String key = ObjectUtils.notNull(parser.currentName());
Object keyValue = jsonValueKyeFlag.getJavaTypeAdapter().parse(key);
jsonValueKyeFlag.setValue(ObjectUtils.notNull(parentItem), keyValue);

try {
Object keyValue = jsonValueKeyFlag.getJavaTypeAdapter().parse(key);
jsonValueKeyFlag.setValue(ObjectUtils.notNull(parentItem), keyValue);
} catch (IllegalArgumentException ex) {
throw new IOException(
String.format("Malformed data '%s'%s. %s",
key,
JsonUtil.generateLocationMessage(parser),
ex.getLocalizedMessage()),
ex);
}
// advance past the field name
JsonUtil.assertAndAdvance(parser, JsonToken.FIELD_NAME);

Expand All @@ -672,7 +699,6 @@ public boolean handleUnknownProperty(
}
return retval;
}

}

private class ModelInstanceReadHandler<ITEM>
Expand Down Expand Up @@ -719,7 +745,8 @@ public Map<String, ITEM> readMap() throws IOException {

IBoundInstanceModel<?> instance = getCollectionInfo().getInstance();

@SuppressWarnings("PMD.UseConcurrentHashMap") Map<String, ITEM> items = new LinkedHashMap<>();
@SuppressWarnings("PMD.UseConcurrentHashMap")
Map<String, ITEM> items = new LinkedHashMap<>();

// A map value is always wrapped in a START_OBJECT, since fields are used for
// the keys
Expand Down
41 changes: 33 additions & 8 deletions databind/src/main/java/gov/nist/secauto/metaschema/databind/io/xml/MetaschemaXmlReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import gov.nist.secauto.metaschema.databind.model.info.IItemReadHandler;
import gov.nist.secauto.metaschema.databind.model.info.IModelInstanceCollectionInfo;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.codehaus.stax2.XMLEventReader2;

import java.io.IOException;
Expand All @@ -56,6 +58,7 @@

public class MetaschemaXmlReader
implements IXmlParsingContext {
private static final Logger LOGGER = LogManager.getLogger(MetaschemaXmlReader.class);
@NonNull
private final XMLEventReader2 reader;
@NonNull
Expand Down Expand Up @@ -184,11 +187,20 @@ protected void readFlagInstances(
XmlEventUtil.generateLocationMessage(attribute)));
}
} else {
// get the attribute value
Object value = instance.getDefinition().getJavaTypeAdapter().parse(ObjectUtils.notNull(attribute.getValue()));
// apply the value to the parentObject
instance.setValue(targetObject, value);
flagInstanceMap.remove(qname);
try {
// get the attribute value
Object value = instance.getDefinition().getJavaTypeAdapter().parse(ObjectUtils.notNull(attribute.getValue()));
// apply the value to the parentObject
instance.setValue(targetObject, value);
flagInstanceMap.remove(qname);
} catch (IllegalArgumentException ex) {
throw new IOException(
String.format("Malformed data '%s'%s. %s",
attribute.getValue(),
XmlEventUtil.generateLocationMessage(start),
ex.getLocalizedMessage()),
ex);
}
}
}

Expand Down Expand Up @@ -453,6 +465,7 @@ private <DEF extends IBoundDefinitionModelComplex> IBoundObject readDefinitionEl
public Object readItemFlag(
IBoundObject parent,
IBoundInstanceFlag flag) throws IOException {
// should never be called
throw new UnsupportedOperationException("handled by readFlagInstances()");
}

Expand Down Expand Up @@ -481,7 +494,7 @@ public Object readItemField(
XmlEventUtil.requireStartElement(getReader(), wrapper);
}

Object retval = readScalarItem(instance);
Object retval = checkMissingFieldValue(readScalarItem(instance));

if (wrapper != null) {
XmlEventUtil.skipWhitespace(getReader());
Expand Down Expand Up @@ -534,7 +547,19 @@ public IBoundObject readItemField(
public Object readItemFieldValue(
IBoundObject parent,
IBoundFieldValue fieldValue) throws IOException {
return readScalarItem(fieldValue);
return checkMissingFieldValue(readScalarItem(fieldValue));
}

@SuppressWarnings("null")
@NonNull
private Object checkMissingFieldValue(Object value) throws IOException {
if (value == null && LOGGER.isWarnEnabled()) {
StartElement start = getStartElement();
LOGGER.atWarn().log("Missing property value{}",
XmlEventUtil.generateLocationMessage(start));
}
// TODO: change nullness annotations to be @Nullable
return value;
}

private void handleAssemblyDefinitionBody(
Expand Down Expand Up @@ -578,7 +603,7 @@ public IBoundObject readItemAssembly(
this::handleAssemblyDefinitionBody);
}

@NonNull
@Nullable
private Object readScalarItem(@NonNull IFeatureScalarItemValueHandler handler)
throws IOException {
return handler.getJavaTypeAdapter().parse(getReader());
Expand Down
Loading

0 comments on commit 70f0436

Please sign in to comment.