Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not throw exception when importing a bad enum #120

Merged
merged 2 commits into from
Mar 20, 2018

Conversation

palemieux
Copy link
Contributor

Closes #119

@thomasheritage
Copy link
Contributor

In this code, when creating dictionaries from Registers, the only "Enumeration" Types that are included in the output dictionaries are those with an "approved" BaseType. All others are simply omitted from the output dictionary. This is regardless of whether the Type has any Facets or not.

This means that dictionaries are created in which some PropertyDefinitions reference non-existent TypeDefinitions. When regxmllib is used to generate schemas from these dictionaries an exception is thrown. I've not tested what happens if you try to process an MXF file using these dictionaries (although presumably that's OK as that issue would have shown-up in the regxmllib test suite).

Either:

  1. "Invalid" dictionaries should not be generated. To achieve this any references to the affected Types would need to be removed (can you leave "Type" unspecified for affected Properties?)
  2. regxmllib should be able to handle "invalid" dictionaries.

(Note: perhaps "incomplete" would be more accurate here than "invalid"?)

This situation affects even currently "published" Registers. This is because they contain some Enumeration Types with a BaseType of "ISO7" (which is a string). These were not found earlier because none of these Types specify any Facets and therefore they have never caused any problems.

On a related note: this suggests that conversion of dictionaries to schemas is not included in the regxmllib test suite -- perhaps it should be added?

@palemieux
Copy link
Contributor Author

regxmllib should be able to handle "invalid" dictionaries.

Well, let's figure out how to actually support ISO7 enumerations, and regxmllib will support them.

@palemieux
Copy link
Contributor Author

@thomasheritage For enumeration, ST 2001-1 states:

Elements: specifies the name, value and description associated with each enumerated value. Each value is a positive integer value. Each name and value is unique within the enumeration type definition. The description is optional and may be omitted.

Why couldn't ISO7 enumerations be turned into byte enumerations?

@thomasheritage
Copy link
Contributor

To clarify:

  1. As this code stands, dictionaries are produced from the "published" Registers which cause regxmllib to raise an Exception when trying to convert these dictionaries to XML Schemas:
Exception in thread "main" com.sandflow.smpte.regxml.XMLSchemaBuilder$RuleException: Type UL does not resolve at Element urn:smpte:ul:060e2b34.01010103.02300501.02000000
        at com.sandflow.smpte.regxml.XMLSchemaBuilder.applyRule5(XMLSchemaBuilder.java:416)
        at com.sandflow.smpte.regxml.XMLSchemaBuilder.fromDictionary(XMLSchemaBuilder.java:291)
        at com.sandflow.smpte.tools.GenerateDictionaryXMLSchema.main(GenerateDictionaryXMLSchema.java:173)
  1. To resolve the above, I suggest that either regxmllib should be tolerant of unresolved Type references within dictionaries, or dictionaries should be "pruned" during generation such that no unresolved Type references exist.

  2. To guard against this happening in the future I suggest adding XML Schema generation to the regxmllib test suite.

@palemieux
Copy link
Contributor Author

palemieux commented Mar 19, 2018

@thomasheritage Thanks for the detailed report. This is a bug. Will fix.

Add Metadictionay XSD generation tests
@palemieux
Copy link
Contributor Author

@thomasheritage Please review updated pull request. Elements referencing undefined types are now pruned. XSD generation tests have been added.

@thomasheritage
Copy link
Contributor

I've tested this. All looks good. This should now be ready for merging.

@palemieux palemieux merged commit ff4e990 into master Mar 20, 2018
@palemieux palemieux deleted the issue-119-not-fail-for-bad-enums branch March 29, 2018 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants