Skip to content

Commit

Permalink
Put back "whole schema" validation to get
Browse files Browse the repository at this point in the history
UPA detection back.

DAFFODIL-2965
  • Loading branch information
mbeckerle committed Dec 20, 2024
1 parent 6db21af commit e067061
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,37 @@ final class DFDLSchemaFile(
private lazy val errHandler = new DFDLSchemaFileLoadErrorHandler(schemaFileLocation)
private lazy val loader = new DaffodilXMLLoader(errHandler)

lazy val isValidAsCompleteDFDLSchema: Boolean = LV('isValidAsCompleteDFDLSchema) {
try {
loader.validateAsCompleteDFDLSchema(schemaSource)
} catch {
case e: org.xml.sax.SAXParseException =>
errHandler.error(e) // accumulate with error handler.
} finally {
errHandler.handleLoadErrors(this)
}
true
}.value

lazy val iiXMLSchemaDocument = LV('iiXMLSchemaDocument) {
val res = makeXMLSchemaDocument(seenBefore, Some(this))
val res = makeXMLSchemaDocument(seenBefore, Some(this))
if (res.isDFDLSchema && sset.shouldValidateDFDLSchemas) {
//
// We validate DFDL schemas, only if validation is requested.
// Some things, tests generally, want to turn this validation off.
//
try {
loader.validateAsIndividualDFDLSchemaFile(schemaSource)
} catch {
// validateAsIndividualDFDLSchemaFile doesn't always capture all exceptions in the
// loader's error handler. Even for fatal errors it sometimes
// just throws them.
case e: org.xml.sax.SAXParseException =>
errHandler.error(e) // accumulate with error handler.
} finally {
errHandler.handleLoadErrors(this)
}
}
res
}.value

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,21 +171,33 @@ final class SchemaSet private (
lazy val schemaFileList = schemas.map(s => s.uriString)

private lazy val isValid: Boolean = {
//
// We use keepGoing here, because we want to gather a validation error,
// suppress further propagation of it, and return false.
//
val isV = OOLAG.keepGoing(false) {
val files = allSchemaFiles
val fileValids = files.map {
_.isValid
if (!shouldValidateDFDLSchemas) true // pretend it's valid though for some specific tests it may not be
else {
//
// We use keepGoing here, because we want to gather a validation error,
// suppress further propagation of it, and return false.
//
val isEachFileIndividuallyValid = OOLAG.keepGoing(false) {
val files = allSchemaFiles
val fileValids = files.map {
_.isValid
}
val res = fileValids.length > 0 && fileValids.fold(true) {
_ && _
}
res
}
val res = fileValids.length > 0 && fileValids.fold(true) {
_ && _
val isEntireSchemaValidAsAnXSD: Boolean = OOLAG.keepGoing(false) {
this.root.xmlSchemaDocument.schemaFile
.map { primaryDfdlSchemaFile =>
primaryDfdlSchemaFile.isValidAsCompleteDFDLSchema
}
.getOrElse(true)
}

val res = isEachFileIndividuallyValid && isEntireSchemaValidAsAnXSD
res
}
isV
}

lazy val validationDiagnostics = {
Expand Down Expand Up @@ -369,7 +381,7 @@ final class SchemaSet private (
* Or, you can leave the root unspecified, and this method will determine it from the
* first element declaration of the first schema file.
*/
lazy val root: Root = {
lazy val root: Root = LV('root) {
val re: GlobalElementDecl =
optPFRootSpec match {
case Some(rs) =>
Expand All @@ -393,7 +405,7 @@ final class SchemaSet private (
case _ => Assert.invariantFailed("illegal combination of root element specifications")
}
re.asRoot
}
}.value

/**
* Retrieve schema by namespace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,9 @@ class ProcessorFactory private[japi] (private var pf: SProcessorFactory)
*/
def onPath(path: String) = {
val dp = pf.onPath(path).asInstanceOf[SDataProcessor]
new DataProcessor(dp)
val res = new DataProcessor(dp)
res.isError // ensure all errors have been detected before we return the DP
res
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1252,7 +1252,7 @@ public void testJavaAPI27() throws IOException {
Throwable cause = e.getCause();
assertTrue(cause.toString().contains("Must call isError"));
assertTrue(cause.getCause().toString().contains("Schema Definition Error"));
assertTrue(cause.getCause().toString().contains("Cannot resolve the name 'tns:nonExistent'"));
assertTrue(cause.getCause().toString().contains("tns:nonExistent"));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -542,11 +542,42 @@ class DaffodilXMLLoader(val errorHandler: org.xml.sax.ErrorHandler)
sf
}

/**
* checks that a DFDL schema file, viewed as an XML document is valid
* relative to the XML Schema for DFDL Schemas.
*
* For example this ensures that you can only have minOccurs and maxOccurs
* on local element declarations and element refs, not on sequence/choice/group.
*/
def validateAsIndividualDFDLSchemaFile(source: DaffodilSchemaSource): Unit = {
// first we load it, with validation explicitly against the
// schema for DFDL Schemas.
try {
load(source, Some(XMLUtils.schemaForDFDLSchemas), addPositionAttributes = true)
} catch {
// fatal errors are thrown.
// validation errors are never fatal.
case e: SAXParseException => {
// Capturing this would be redundant.
// It will already have been passed to the errorHandler.fatalError
// method.
// So it is explicitly ok to just rethrow this exception.
// we don't want to record it again, but we do want to stop with a
// fatal error because the schema was invalid. Daffodil assumes the
// schema is valid all over its code base.
throw e
}
}
}

/**
* This loads the DFDL schema as an XML Schema. This will
* check many more things (ex: UPA) about the DFDL schema other than
* just whether it validates against the XML Schema for DFDL schemas.
*
* This should only be called once, for the entire schema, not per file of
* the schema.
*
* Unfortunately, we don't have control over how Xerces loads up these schemas
* (other than the resolver anyway), so we can't really redirect the way
* it issues error messages so that it properly lays blame at say, the schema fragments
Expand All @@ -562,13 +593,10 @@ class DaffodilXMLLoader(val errorHandler: org.xml.sax.ErrorHandler)
* on the XMLUtils.setSecureDefaults, so we don't need to
* further check that here.
*/
def validateAsDFDLSchema(source: DaffodilSchemaSource): Unit = {
// first we load it, with validation explicitly against the
// schema for DFDL Schemas.
def validateAsCompleteDFDLSchema(source: DaffodilSchemaSource): Boolean = {
try {
load(source, Some(XMLUtils.schemaForDFDLSchemas), addPositionAttributes = true)
//
// Then we validate explicitly so Xerces can check things
// We validate explicitly so Xerces can check things
// such as for UPA violations
//
val inputSource = source.newInputSource()
Expand All @@ -584,6 +612,7 @@ class DaffodilXMLLoader(val errorHandler: org.xml.sax.ErrorHandler)
} finally {
inputSource.getByteStream().close()
}
true
} catch {
// fatal errors are thrown.
// validation errors are never fatal.
Expand All @@ -598,7 +627,6 @@ class DaffodilXMLLoader(val errorHandler: org.xml.sax.ErrorHandler)
throw e
}
}

}

// $COVERAGE-OFF$ These three functions should only be used if someone calls one of the
Expand All @@ -618,14 +646,15 @@ class DaffodilXMLLoader(val errorHandler: org.xml.sax.ErrorHandler)
/**
* Obtain and initialize parser which validates the schema is defined.
*/
private def parserFromURI(optSchemaURI: Option[URI]): SAXParser =
private def parserFromURI(optSchemaURI: Option[URI]): SAXParser = {
if (optSchemaURI.isEmpty) noSchemaParser
else {
val f = parserFactory()
val schema = schemaFromURI(optSchemaURI.get)
f.setSchema(schema)
parserFromFactory(f)
}
}

private def schemaFromURI(schemaURI: URI): Schema = {
val sf = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI)
Expand All @@ -635,7 +664,7 @@ class DaffodilXMLLoader(val errorHandler: org.xml.sax.ErrorHandler)
schema
}

private def parserFactory(): SAXParserFactory = {
private def parserFactory() = {
val f = DaffodilSAXParserFactory()
f.setNamespaceAware(true)
f.setFeature(XMLUtils.SAX_NAMESPACE_PREFIXES_FEATURE, true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1234,7 +1234,7 @@ class TestScalaAPI {
cause.getCause.toString.contains("Schema Definition Error")
)
assertTrue(
cause.getCause.toString.contains("Cannot resolve the name 'tns:nonExistent'")
cause.getCause.toString.contains("tns:nonExistent")
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,8 @@
<tdml:document/>
<tdml:errors>
<tdml:error>Schema Definition Error</tdml:error>
<tdml:error>Unable to resolve</tdml:error>
<tdml:error>this/does/not/exist/schema.dfdl.xsd</tdml:error>
<tdml:error>Schema context: Location in</tdml:error>
<tdml:error>Include Location</tdml:error>
</tdml:errors>
</tdml:parserTestCase>

Expand Down

0 comments on commit e067061

Please sign in to comment.