Profile resolver: Test merge phase, plus minor XSLT enhancements #1207

galtm · 2022-04-17T16:11:59Z

Committer Notes

This pull request tests the Merge phase of the XSLT profile resolver. It also updates the XSLT as listed below.

XSLT Changes

Move some code from oscal-profile-resolve-select.xsl to new file, select-or-custom-merge.xsl, and share it between Selection phase and Merge phase.
Add error checking for multiple structuring directives.
Finish the incomplete support for insert-controls/@order.
Fix bug in support for @with-parent-controls in Selection phase.

XSpec Changes

In select.xspec, add test for order of controls matching imported document, not profile document. Also, add test for @with-parent-controls bug fix.
Create merge-combine.xspec for testing combine-elements and mode="o:combine-elements" templates.
Organize tests among merge.xspec, merge-combine.xspec, merge-as-is.xspec, and merge-custom.xspec.
Integrate back-matter testing into template-level tests in merge.xspec and merge-as-is.xspec, and delete merge-back-matter.xspec.

All Submissions:

Have you selected the correct base branch per Contributing guidance?
Have you set "Allow edits and access to secrets by maintainers
"?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?
Have you squashed any non-relevant commits and commit messages? [instructions]
Do all automated CI/CD checks pass?

Changes to Core Features:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your core changes, as applicable?
Have you included examples of how to use your new feature(s)?
Have you updated all OSCAL website and readme documentation affected by the changes you made? Changes to the OSCAL website can be made in the docs/content directory of your branch.

galtm · 2022-04-17T16:15:44Z

src/utils/util/resolver-pipeline/oscal-profile-resolve-merge.xsl

+        If there is also a merge/custom directive, we apply the
+        higher-priority template instead of this one. -->
+    <xsl:template match="catalog[merge/as-is=$true-content]" priority="12" as="element(catalog)">
+        <xsl:call-template name="detect-multiple-structuring-directives"/>


Adding detection here and in custom merge template because the spec says, "If more than one appears, processors MUST generate an error and cease processing."

galtm · 2022-04-17T16:21:15Z

src/utils/util/resolver-pipeline/oscal-profile-resolve-merge.xsl

-                <xsl:for-each-group select="current-group()" group-by="(@opr:id,@id,generate-id())[1]">
+                <xsl:for-each-group select="back-matter/* | selection/back-matter/*" group-by="(@opr:id,@uuid,generate-id())[1]">


Change in group-by: Current schema says the attribute on resource is @uuid rather than @id.

The other changes on this line relate to use of xsl:where-populated on line 109, which I just thought was more intuitive than <xsl:for-each-group group-by="true()" ...> on line 82 of the old revision. (The GitHub diff is a little hard to read because of the new detect-multiple-structuring-directives template in the middle.)

galtm · 2022-04-17T16:22:36Z

src/utils/util/resolver-pipeline/oscal-profile-resolve-merge.xsl

    </xsl:template>

    <xsl:template match="selection">
        <xsl:apply-templates select="param   | .//group/param"/>
        <xsl:apply-templates select="control | .//group/control"/>
    </xsl:template>

-    <xsl:key name="control-by-id" match="control" use="@id"/>


No longer needed because of included code shared with Selection phase

galtm · 2022-04-17T16:23:43Z

src/utils/util/resolver-pipeline/oscal-profile-resolve-merge.xsl

    <xsl:template match="insert-controls" mode="o:custom-merge">
-        <xsl:variable name="inserted-controls">
+        <xsl:variable name="inserted-controls" as="element()*">


Here, a sequence data type rather than a document node is important so xsl:perform-sort can sort the elements.

galtm · 2022-04-17T16:24:08Z

src/utils/util/resolver-pipeline/oscal-profile-resolve-merge.xsl

-    <xsl:template match="include-controls" mode="o:custom-merge">
-        <xsl:variable name="match-patterns" select="matching/@pattern"/>
-        <xsl:call-template name="combine-elements">
-            <xsl:with-param name="who" select="key('control-by-id', with-id),
-                /*/selection//control[some $p in ($match-patterns) satisfies (matches(@id,o:glob-as-regex(string($p))))]"/>
-        </xsl:call-template>
-    </xsl:template>


No longer needed because of included code shared with Selection phase

galtm · 2022-04-17T16:27:36Z

src/utils/util/resolver-pipeline/oscal-profile-resolve-merge.xsl

        </xsl:call-template>
    </xsl:template>

-    <xsl:include href="oscal-profile-resolve-functions.xsl"/>


Included from select-or-custom-merge.xsl, so it doesn't need to be included from here, too.

galtm · 2022-04-17T16:30:21Z

src/utils/util/resolver-pipeline/select-or-custom-merge.xsl

+           <xsl:otherwise>
+               <!-- Visit child controls in case they are selected using
+                   with-parent-controls="no". -->
+               <xsl:apply-templates mode="#current" select="control"/>
+           </xsl:otherwise>


This wasn't in the original Selection phase code from which this file was created. Adding it makes with-parent-controls="no" work correctly in both the Selection and Merge phases.

galtm · 2022-04-17T16:38:14Z

src/utils/util/resolver-pipeline/testing/3_merged/merge.xspec

+        <x:scenario label="as-is element appears but content is not true">
+            <x:variable name="ov:as-is-content">0</x:variable>
+            <x:like label="SHARED: as-is element with placeholder for content"/>
+        </x:scenario>
+        <x:scenario label="as-is element appears but has no content">
+            <x:variable name="ov:as-is-content"/>
+            <x:like label="SHARED: as-is element with placeholder for content"/>
+        </x:scenario>


FYI: x:variable and x:like make a good pair if you find yourself doing a lot of copy-paste-modify operations in an XSpec file. See also x:expand-text on line 292, which helps with parameterizing the shared content.

XSLT - Move some code from oscal-profile-resolve-select.xsl to new file, select-or-custom-merge.xsl, and share it between Selection phase and Merge phase. - Add error checking for multiple structuring directives. - Finish the incomplete support for `insert-controls/@order`. - Fix bug in support for `@with-parent-controls` in Selection phase. XSpec - In select.xspec, add test for order of controls matching imported document, not profile document. Also, add test for `@with-parent-controls` bug fix. - Create merge-combine.xspec for testing combine-elements and mode="o:combine-elements" templates. - Organize tests among merge.xspec, merge-combine.xspec, merge-as-is.xspec, and merge-custom.xspec. - Integrate back-matter testing into template-level tests in merge.xspec and merge-as-is.xspec, and delete merge-back-matter.xspec.

wendellpiez

Fantastic progress thanks @galtm!

wendellpiez · 2022-04-20T15:20:08Z

Wanting this PR to repair lapses exposed in unit testing! 🥇

wendellpiez · 2022-04-20T15:20:08Z

Wanting this PR to repair lapses exposed in unit testing! 🥇

wendellpiez · 2022-04-21T13:05:58Z

Oops - I meant to say (or hint) I am finding errors in my unit testing in another branch, which are addressed by corrections in this PR. Way too cryptic!

david-waltermire

LGTM. Thanks!

XSLT - Move some code from oscal-profile-resolve-select.xsl to new file, select-or-custom-merge.xsl, and share it between Selection phase and Merge phase. - Add error checking for multiple structuring directives. - Finish the incomplete support for `insert-controls/@order`. - Fix bug in support for `@with-parent-controls` in Selection phase. XSpec - In select.xspec, add test for order of controls matching imported document, not profile document. Also, add test for `@with-parent-controls` bug fix. - Create merge-combine.xspec for testing combine-elements and mode="o:combine-elements" templates. - Organize tests among merge.xspec, merge-combine.xspec, merge-as-is.xspec, and merge-custom.xspec. - Integrate back-matter testing into template-level tests in merge.xspec and merge-as-is.xspec, and delete merge-back-matter.xspec.

galtm commented Apr 17, 2022

View reviewed changes

wendellpiez approved these changes Apr 18, 2022

View reviewed changes

wendellpiez mentioned this pull request Apr 26, 2022

Profile support for selection 'with-parent-controls' #1218

Open

3 tasks

david-waltermire linked an issue Apr 28, 2022 that may be closed by this pull request

[EPIC] Update XSLT profile resolver to current specification #1076

Open

david-waltermire removed a link to an issue Apr 28, 2022

[EPIC] Update XSLT profile resolver to current specification #1076

Open

wendellpiez mentioned this pull request Apr 29, 2022

[EPIC] Update XSLT profile resolver to current specification #1076

Open

david-waltermire approved these changes May 4, 2022

View reviewed changes

david-waltermire merged commit 6587e35 into usnistgov:develop May 4, 2022

galtm deleted the profiler-tests-3 branch May 7, 2022 01:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Profile resolver: Test merge phase, plus minor XSLT enhancements #1207

Profile resolver: Test merge phase, plus minor XSLT enhancements #1207

galtm commented Apr 17, 2022

galtm Apr 17, 2022

galtm Apr 17, 2022

galtm Apr 17, 2022

galtm Apr 17, 2022

galtm Apr 17, 2022

galtm Apr 17, 2022

galtm Apr 17, 2022

galtm Apr 17, 2022

wendellpiez left a comment

wendellpiez commented Apr 20, 2022

wendellpiez commented Apr 20, 2022

wendellpiez commented Apr 21, 2022

david-waltermire left a comment

		<xsl:for-each-group select="current-group()" group-by="(@opr:id,@id,generate-id())[1]">
		<xsl:for-each-group select="back-matter/* \| selection/back-matter/*" group-by="(@opr:id,@uuid,generate-id())[1]">

Profile resolver: Test merge phase, plus minor XSLT enhancements #1207

Profile resolver: Test merge phase, plus minor XSLT enhancements #1207

Conversation

galtm commented Apr 17, 2022

Committer Notes

All Submissions:

Changes to Core Features:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wendellpiez left a comment

Choose a reason for hiding this comment

wendellpiez commented Apr 20, 2022

wendellpiez commented Apr 20, 2022

wendellpiez commented Apr 21, 2022

david-waltermire left a comment

Choose a reason for hiding this comment