Skip to content

Commit

Permalink
Add stricter validation checks, optionally error instead of silently …
Browse files Browse the repository at this point in the history
…skipping if duplicate elements exist in a library document. Validate that the types used in the document have been registered as types in the document
  • Loading branch information
Lee Kerley authored and ld-kerley committed Mar 7, 2024
1 parent 0514302 commit e9b720e
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 18 deletions.
30 changes: 24 additions & 6 deletions python/Scripts/mxvalidate.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,43 @@ def main():
parser = argparse.ArgumentParser(description="Verify that the given file is a valid MaterialX document.")
parser.add_argument("--resolve", dest="resolve", action="store_true", help="Resolve inheritance and string substitutions.")
parser.add_argument("--verbose", dest="verbose", action="store_true", help="Print summary of elements found in the document.")
parser.add_argument("--stdlib", dest="stdlib", action="store_true", help="Import standard MaterialX libraries into the document.")
grp = parser.add_mutually_exclusive_group()
grp.add_argument("--stdlib", dest="stdlib", action="store_true", help="Import standard MaterialX libraries into the document.")
grp.add_argument("--path", dest="searchPath", action="store", help="")
parser.add_argument("--library", dest="libraryFolders", action="append", help="")
parser.add_argument("--errorOnDuplicates", dest="errorOnDuplicates", action="store_true", help="")
parser.add_argument(dest="inputFilename", help="Filename of the input document.")
opts = parser.parse_args()

print(opts)

doc = mx.createDocument()
try:
mx.readFromXmlFile(doc, opts.inputFilename)
except mx.ExceptionFileMissing as err:
print(err)
sys.exit(0)

if opts.stdlib:
stdlib = mx.createDocument()
if opts.stdlib or opts.searchPath:
libDoc = mx.createDocument()
try:
mx.loadLibraries(mx.getDefaultDataLibraryFolders(), mx.getDefaultDataSearchPath(), stdlib)
except Exception as err:
if opts.stdlib:
dataSearchPath = mx.getDefaultDataSearchPath()
else:
dataSearchPath = mx.FileSearchPath(opts.searchPath, ";")

if opts.libraryFolders:
dataLibaryFolders = opt.libraryFolders
else:
dataLibaryFolders = mx.getDefaultDataLibraryFolders()

print(f"Loading Libraries\nSearch Path: {dataSearchPath.asString()}\nFolders : {dataLibaryFolders}")

mx.loadLibraries(dataLibaryFolders, dataSearchPath, libDoc, errorOnDuplicates=opts.errorOnDuplicates)
except err:
print(err)
sys.exit(0)
doc.importLibrary(stdlib)
doc.importLibrary(libDoc, errorOnDuplicates=opts.errorOnDuplicates)

(valid, message) = doc.validate()
if (valid):
Expand Down
4 changes: 3 additions & 1 deletion source/MaterialXCore/Document.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ NodeDefPtr Document::addNodeDefFromGraph(const NodeGraphPtr nodeGraph, const str
return nodeDef;
}

void Document::importLibrary(const ConstDocumentPtr& library)
void Document::importLibrary(const ConstDocumentPtr& library, bool errorOnDuplicates)
{
if (!library)
{
Expand All @@ -286,6 +286,8 @@ void Document::importLibrary(const ConstDocumentPtr& library)
ConstElementPtr previous = getChild(childName);
if (previous)
{
if (errorOnDuplicates)
throw Exception("Trying to import a child that already exists " + child->getName());
continue;
}

Expand Down
3 changes: 2 additions & 1 deletion source/MaterialXCore/Document.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ class MX_CORE_API Document : public GraphElement
/// The contents of the library document are copied into this one, and
/// are assigned the source URI of the library.
/// @param library The library document to be imported.
void importLibrary(const ConstDocumentPtr& library);
/// @param errorOnDuplicates If true then an Exception will be raised if elements have duplicate names
void importLibrary(const ConstDocumentPtr& library, bool errorOnDuplicates=false);

/// Get a list of source URI's referenced by the document
StringSet getReferencedSourceUris() const;
Expand Down
25 changes: 25 additions & 0 deletions source/MaterialXCore/Element.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,31 @@ TypeDefPtr TypedElement::getTypeDef() const
return resolveNameReference<TypeDef>(getType());
}

bool TypedElement::validate(string* message) const
{
bool res = true;

if (hasType())
{
// check that the type specified is declared as a type in the document
const auto& typeStr = getType();
bool isValidType = getDocument()->getTypeDef(typeStr) != nullptr;

if (!isValidType) {
// there is one special type "multioutput" that can be used on node
// elements when they have more than one output port.
if (typeStr == "multioutput" && isA<Node>())
{
isValidType = true;
}
}

validateRequire(isValidType, res, message, "Element type is not defined '"+getType()+"'");
}

return Element::validate(message) && res;
}

//
// ValueElement methods
//
Expand Down
8 changes: 8 additions & 0 deletions source/MaterialXCore/Element.h
Original file line number Diff line number Diff line change
Expand Up @@ -890,6 +890,14 @@ class MX_CORE_API TypedElement : public Element
TypeDefPtr getTypeDef() const;

/// @}
/// @name Validation
/// @{

/// Validate that the given element is
/// consistent with the MaterialX specification.
bool validate(string* message = nullptr) const override;

/// @}

public:
static const string TYPE_ATTRIBUTE;
Expand Down
11 changes: 6 additions & 5 deletions source/MaterialXFormat/Util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,18 +73,19 @@ void loadDocuments(const FilePath& rootPath, const FileSearchPath& searchPath, c
}
}

void loadLibrary(const FilePath& file, DocumentPtr doc, const FileSearchPath& searchPath, const XmlReadOptions* readOptions)
void loadLibrary(const FilePath& file, DocumentPtr doc, const FileSearchPath& searchPath, const XmlReadOptions* readOptions, bool errorOnDuplicates)
{
DocumentPtr libDoc = createDocument();
readFromXmlFile(libDoc, file, searchPath, readOptions);
doc->importLibrary(libDoc);
doc->importLibrary(libDoc, errorOnDuplicates);
}

StringSet loadLibraries(const FilePathVec& libraryFolders,
const FileSearchPath& searchPath,
DocumentPtr doc,
const StringSet& excludeFiles,
const XmlReadOptions* readOptions)
const XmlReadOptions* readOptions,
bool errorOnDuplicates)
{
// Append environment path to the specified search path.
FileSearchPath librarySearchPath = searchPath;
Expand All @@ -105,7 +106,7 @@ StringSet loadLibraries(const FilePathVec& libraryFolders,
const FilePath& file = path / filename;
if (loadedLibraries.count(file) == 0)
{
loadLibrary(file, doc, searchPath, readOptions);
loadLibrary(file, doc, searchPath, readOptions, errorOnDuplicates);
loadedLibraries.insert(file.asString());
}
}
Expand All @@ -128,7 +129,7 @@ StringSet loadLibraries(const FilePathVec& libraryFolders,
const FilePath& file = path / filename;
if (loadedLibraries.count(file) == 0)
{
loadLibrary(file, doc, searchPath, readOptions);
loadLibrary(file, doc, searchPath, readOptions, errorOnDuplicates);
loadedLibraries.insert(file.asString());
}
}
Expand Down
6 changes: 4 additions & 2 deletions source/MaterialXFormat/Util.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,17 @@ MX_FORMAT_API void loadDocuments(const FilePath& rootPath,
MX_FORMAT_API void loadLibrary(const FilePath& file,
DocumentPtr doc,
const FileSearchPath& searchPath = FileSearchPath(),
const XmlReadOptions* readOptions = nullptr);
const XmlReadOptions* readOptions = nullptr,
bool errorOnDuplicates=false);

/// Load all MaterialX files within the given library folders into a document,
/// using the given search path to locate the folders on the file system.
MX_FORMAT_API StringSet loadLibraries(const FilePathVec& libraryFolders,
const FileSearchPath& searchPath,
DocumentPtr doc,
const StringSet& excludeFiles = StringSet(),
const XmlReadOptions* readOptions = nullptr);
const XmlReadOptions* readOptions = nullptr,
bool errorOnDuplicates=false);

/// Flatten all filenames in the given document, applying string resolvers at the
/// scope of each element and removing all fileprefix attributes.
Expand Down
3 changes: 2 additions & 1 deletion source/PyMaterialX/PyMaterialXCore/PyDocument.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ void bindPyDocument(py::module& mod)
py::class_<mx::Document, mx::DocumentPtr, mx::GraphElement>(mod, "Document")
.def("initialize", &mx::Document::initialize)
.def("copy", &mx::Document::copy)
.def("importLibrary", &mx::Document::importLibrary)
.def("importLibrary", &mx::Document::importLibrary,
py::arg("library"), py::arg("errorOnDuplicates")=false)
.def("getReferencedSourceUris", &mx::Document::getReferencedSourceUris)
.def("addNodeGraph", &mx::Document::addNodeGraph,
py::arg("name") = mx::EMPTY_STRING)
Expand Down
4 changes: 2 additions & 2 deletions source/PyMaterialX/PyMaterialXFormat/PyUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ void bindPyUtil(py::module& mod)
py::arg("rootPath"), py::arg("searchPath"), py::arg("skipFiles"), py::arg("includeFiles"), py::arg("documents"), py::arg("documentsPaths"),
py::arg("readOptions") = (mx::XmlReadOptions*) nullptr, py::arg("errors") = (mx::StringVec*) nullptr);
mod.def("loadLibrary", &mx::loadLibrary,
py::arg("file"), py::arg("doc"), py::arg("searchPath") = mx::FileSearchPath(), py::arg("readOptions") = (mx::XmlReadOptions*) nullptr);
py::arg("file"), py::arg("doc"), py::arg("searchPath") = mx::FileSearchPath(), py::arg("readOptions") = (mx::XmlReadOptions*) nullptr, py::arg("errorOnDuplicates") = false);
mod.def("loadLibraries", &mx::loadLibraries,
py::arg("libraryFolders"), py::arg("searchPath"), py::arg("doc"), py::arg("excludeFiles") = mx::StringSet(), py::arg("readOptions") = (mx::XmlReadOptions*) nullptr);
py::arg("libraryFolders"), py::arg("searchPath"), py::arg("doc"), py::arg("excludeFiles") = mx::StringSet(), py::arg("readOptions") = (mx::XmlReadOptions*) nullptr, py::arg("errorOnDuplicates") = false);
mod.def("flattenFilenames", &mx::flattenFilenames,
py::arg("doc"), py::arg("searchPath") = mx::FileSearchPath(), py::arg("customResolver") = (mx::StringResolverPtr) nullptr);
mod.def("getSourceSearchPath", &mx::getSourceSearchPath);
Expand Down

0 comments on commit e9b720e

Please sign in to comment.