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

Schema converter has code comment that omits schema name and version #62

Closed
galtm opened this issue Aug 15, 2023 · 4 comments
Closed

Schema converter has code comment that omits schema name and version #62

galtm opened this issue Aug 15, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@galtm
Copy link
Collaborator

galtm commented Aug 15, 2023

Describe the bug

I think the src/converter-gen/xml-to-json/produce-xml-converter.xsl is looking for metadata in the wrong place, so the schema name and version will be omitted from the comment in the generated XSLT file.

The line that says

<xsl:comment expand-text="true"> METASCHEMA: { schema-name}{ schema-version ! (' (version ' || . || ')' ) } in namespace "{ $source-namespace }"</xsl:comment>

should have metadata/ inserted in two spots:

<xsl:comment expand-text="true"> METASCHEMA: { metadata/schema-name}{ metadata/schema-version ! (' (version ' || . || ')' ) } in namespace "{ $source-namespace }"</xsl:comment>

Who is the bug affecting?

Anyone who looks at the code for the XML-to-JSON converter that gets generated using the XSLT pipeline. The mistake is in a code comment, so usage of the converter is not affected.

What is affected by this bug?

The code comment is less informative than it's supposed to be. For example:

<!-- METASCHEMA:  in namespace "..."-->

instead of

<!-- METASCHEMA: OSCAL Profile Model (version 1.0.4) in namespace "..."-->

When does this occur?

Anytime the XML-JSON or JSON-to-XML converter generator is used, I think.

How do we replicate the issue?

{What are the steps to reproduce the behavior?

  1. Copy a file like models_metaschema.xml from the metaschema repo to the metaschema-xslt/src/converter-gen directory in this repo.
  2. Execute a transformation like the following, substituting your own path to Saxon.
java -cp "...path to Saxon-HE jar file" net.sf.saxon.Transform -s:models_metaschema.xml -xsl:nist-metaschema-MAKE-XML-TO-JSON-CONVERTER.xsl -o:output.xsl

Line 83 in output.xsl is missing information.

<!-- METASCHEMA:  in namespace "http://csrc.nist.gov/ns/metaschema/everything"-->

Expected behavior (i.e. solution)

That comment line should indicate the name and version of the metaschema.

Other Comments

I am working on unit tests in this area of the code, so I can include the fix in an eventual pull request for tests. If you want me to do that, let me know whether I should

a) Wait until I am done with a batch of tests or

b) Make a pull request for this fix as soon as I have a minimal test file that confirms the bug fix

@galtm galtm added the bug Something isn't working label Aug 15, 2023
@wendellpiez
Copy link
Collaborator

I think this is a touchup with immediate positive impact; indeed it falls into the 'sooner the better' category.

@galtm assuming a clear front on your side could you expedite this correction in a separate (standalone) PR against develop, which @aj-stein-nist and @nikitawootten-nist could integrate readily? That would be so awesome.

This sounds like option b), which I like very much. Thank you!

@galtm
Copy link
Collaborator Author

galtm commented Aug 18, 2023

Sure, I'll do that. Yes, it's option b).

galtm added a commit to galtm/metaschema-xslt that referenced this issue Aug 22, 2023
@galtm
Copy link
Collaborator Author

galtm commented Aug 22, 2023

separate (standalone) PR against develop, which @aj-stein-nist and @nikitawootten-nist could integrate readily?

Created pull request #64, putting the test near the XSLT (which was one option mentioned in discussion #63).

@nikitawootten-nist
Copy link
Collaborator

Fixed via #64

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants