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

GEOM LDD schematron rules are not executing as expected <kernel_type> validation #797

Closed
mace-space opened this issue Jul 10, 2024 · 6 comments · Fixed by #799 or #807
Closed

GEOM LDD schematron rules are not executing as expected <kernel_type> validation #797

mace-space opened this issue Jul 10, 2024 · 6 comments · Fixed by #799 or #807
Assignees
Labels

Comments

@mace-space
Copy link

mace-space commented Jul 10, 2024

Checked for duplicates

Yes - I've already checked

🐛 Describe the bug

Validate is passing labels that contain an incorrect kernel_type. E.g. this label:

                    <geom:SPICE_Kernel_Identification>
                        <geom:kernel_type>BPC</geom:kernel_type>
                        <geom:spice_kernel_file_name>earth_720101_031229.bpc</geom:spice_kernel_file_name>
                    </geom:SPICE_Kernel_Identification>
                    <geom:SPICE_Kernel_Identification>

However, the Geometry dictionary uses pds:kernel_type not geom:kernel_type – there is no definition of kernel_type within the Geom dictionary, and the SPICE_Kernel_Identification class includes pds.kernel_type as a member. The change log says that this change from kernel_type to pds:kernel_type was made back in 2015.

The schematron states:

  <sch:pattern>
    <sch:rule context="geom:SPICE_Kernel_Identification/pds:kernel_type">
      <sch:assert test=". = ('CK', 'DBK', 'DSK', 'EK', 'FK', 'IK', 'LSK', 'MK', 'PCK', 'SCLK', 'SPK')">
        The attribute pds:kernel_type must be equal to one of the following values 'CK', 'DBK', 'DSK', 'EK', 'FK', 'IK', 'LSK', 'MK', 'PCK', 'SCLK', 'SPK'.</sch:assert>
    </sch:rule>
  </sch:pattern>

Isn't Validate incorrect to allow the call to geom:kernel_type? It’s not a valid member of the class so it should trigger a structural error (not a schematron error).

Alternatively, if it was reasonable for Validate to interpret geom:kernel_type as if it were pds:kernel_type, then it should have rejected "BPC" as a value because it is not part of the enumerated list.

🕵️ Expected behavior

I expected Validate to flag an error for <geom:kernel_type>BPC</geom:kernel_type>

📜 To Reproduce

validate-3.5.0-SNAPSHOT/bin/validate <path/to/bundle> -R pds4.bundle -v 2 -r validate_report.log

🖥 Environment Info

  • Operating System: MacOS 10.15.7

📚 Version of Software Used

  • Version 3.5.0-SNAPSHOT
    Release Date: 2024-03-06 21:32:16

🩺 Test Data / Additional context

Example label linked above (see bundleset)

🦄 Related requirements

No response

⚙️ Engineering Details

It looks like the GEOM schema expects geom:SPICE_Kernel_Identification/geom:kernel_type (as expected), but the schematron generation does not correctly translate the pds:kernel_type type into the geom: namespace.

Changing the schematron xpath from pds:kernel_type to geom:kernel_type, like it should be, modifying the XML example above in to a local version of the schematron, and it now throws an error for a bad value like it should be doing.

image

It looks like this may be specific to when an attribute is re-used from the core namespace:

        <DD_Association>
            <identifier_reference>pds.kernel_type</identifier_reference>
            <reference_type>attribute_of</reference_type>
            <minimum_occurrences>0</minimum_occurrences>
            <maximum_occurrences>1</maximum_occurrences>
        </DD_Association>

The issue identified is pds.kernel_type is not exposed by the core namespace. LDDTool should be raising an error, but instead, mistakenly puts the disconnected references in the schema/schematron, causing the bug noted above.

🎉 Integration & Test

No response

@mace-space
Copy link
Author

When I try validating labels using pds:kernel_type (instead of geom:kernel_type), they fail:

SXXP0003   Error reported by XML parser: The prefix "pds" for element "pds:kernel_type" is
  not bound.`

@jordanpadams
Copy link
Member

@mace-space We are investigating this right now, but Steve is on vacation. At least initially, I believe this is a major bug with LDDTool, and the schematron is actually incorrect. I believe the schematron should actually be:

  <sch:pattern>
    <sch:rule context="geom:SPICE_Kernel_Identification/geom:kernel_type">
      <sch:assert test=". = ('CK', 'DBK', 'DSK', 'EK', 'FK', 'IK', 'LSK', 'MK', 'PCK', 'SCLK', 'SPK')">
        The attribute pds:kernel_type must be equal to one of the following values 'CK', 'DBK', 'DSK', 'EK', 'FK', 'IK', 'LSK', 'MK', 'PCK', 'SCLK', 'SPK'.</sch:assert>
    </sch:rule>
  </sch:pattern>

Will let you know when we make a determination there.

@jordanpadams jordanpadams transferred this issue from NASA-PDS/validate Jul 18, 2024
@github-project-automation github-project-automation bot moved this to Release Backlog in B15.0 Jul 18, 2024
@github-project-automation github-project-automation bot moved this from Release Backlog to 🏁 Done in B15.0 Aug 7, 2024
@github-project-automation github-project-automation bot moved this from ToDo to 🏁 Done in EN Portfolio Backlog Aug 7, 2024
@jordanpadams jordanpadams changed the title Validate does not catch incorrect <kernel_type>s GEOM LDD schermatron rules are not executing as expected <kernel_type> validation Aug 7, 2024
@jordanpadams jordanpadams changed the title GEOM LDD schermatron rules are not executing as expected <kernel_type> validation GEOM LDD schematron rules are not executing as expected <kernel_type> validation Aug 7, 2024
@jordanpadams jordanpadams reopened this Aug 7, 2024
@github-project-automation github-project-automation bot moved this from 🏁 Done to In Progress in EN Portfolio Backlog Aug 7, 2024
@github-project-automation github-project-automation bot moved this from 🏁 Done to In Progress in B15.0 Aug 7, 2024
@jordanpadams
Copy link
Member

@jshughes actually, looking at the example schematron and schema you produced on #799 (https://github.com/user-attachments/files/16385007/KernelType.zip), this is still not correct. We will take this discussion offline.

@github-project-automation github-project-automation bot moved this from In Progress to 🏁 Done in EN Portfolio Backlog Aug 23, 2024
@github-project-automation github-project-automation bot moved this from In Progress to 🏁 Done in B15.0 Aug 23, 2024
@rchenatjpl
Copy link
Contributor

rchenatjpl commented Oct 7, 2024

@jshughes: Is this expected:
% lddtool -pl -V1M00 PDS4_GEOM_IngestLDD.xml
produces a good .sch (and .xsd) with rule

    <sch:rule context="geom:SPICE_Kernel_Identification/pds:kernel_type">
      <sch:assert test=". = ('CK', 'DBK', 'DSK', 'EK', 'FK', ...)">
        <title>geom:SPICE_Kernel_Identification/pds:kernel_type/pds:kernel_type</title>
        The attribute geom:SPICE_Kernel_Identification/pds:kernel_type must be ...</sch:assert>
    </sch:rule>

but if I specify -V1N00 or don't specify -V at all, the .sch has two such rules:

    <sch:rule context="geom:SPICE_Kernel_Identification/pds:kernel_type">
      <sch:assert test=". = ('CK', 'DBK', 'DSK', 'EK', 'FK', ...)">
        <title>geom:SPICE_Kernel_Identification/pds:kernel_type/pds:kernel_type</title>
        The attribute geom:SPICE_Kernel_Identification/pds:kernel_type must be ...</sch:assert>
    </sch:rule>
    <sch:rule context="geom:SPICE_Kernel_Identification/geom:kernel_type">
      <sch:assert test=". = ('CK', 'DBK', 'DSK', 'EK', 'FK', ...)">
        <title>geom:SPICE_Kernel_Identification/geom:kernel_type/kernel_type</title>
        The attribute geom:SPICE_Kernel_Identification/geom:kernel_type must be ...</sch:assert>
    </sch:rule>

That's bad, right?

@jshughes
Copy link
Collaborator

jshughes commented Oct 8, 2024

This is correct for IM Version 1N00 and onward. The two rules have slightly different contexts, geom:SPICE_Kernel_Identification/pds:kernel_type vs geom:SPICE_Kernel_Identification/geom:kernel_type. The use of pds:kernel_type is new and more correct, however geom:kernel_type needs to remain since the Geometry LDD did not properly "expose" kernel_type. Whether and how the Geometry LDD is fixed is now a DDWG issue to be resolved.

@rchenatjpl
Copy link
Contributor

ah ok thanks

jordanpadams added a commit that referenced this issue Oct 31, 2024
Issues with this tests when running after other. Tracked here: #822

Tests still fail but this is now expected
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 🏁 Done
Status: 🏁 Done
4 participants