NASA-PDS / pds4-information-model

The software tools and data necessary for generating the Information Model including PDS4 ontology, data, and information model.
https://nasa-pds.github.io/pds4-information-model/
Other
11 stars 7 forks source link

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

Closed mace-space closed 3 months ago

mace-space commented 4 months ago

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

πŸ“š Version of Software Used

🩺 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 commented 4 months ago

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 commented 4 months ago

@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 commented 3 months ago

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

rchenatjpl commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

ah ok thanks