Profile resolver error handling #1549

galtm · 2022-11-13T22:53:30Z

Committer Notes

This PR enhances the XSLT profile resolver so that (a) if there is a fatal error, the resolver reports one such error and does not produce a final output, and (b) if there are no fatal errors, the resolver reports all nonfatal error and warning messages when producing the final output document.

Separate before/after from starting/ending in modify phase because they are no longer synonyms
Enhance message-handler.xsl for pipeline usage and testability
Adapt XSLT to message handler returning PI
Enable saving intermediate XML files for debugging purposes
Add sample document for nonfatal errors/warnings

Included bug fixes for bugs I noticed during this work:

Enable param to have insertions before/after AND have param set
Fix merge phase bug where descendant controls weren't being flattened

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?

By submitting a pull request, you are agreeing to provide this contribution under the CC0 1.0 Universal public domain dedication.

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.~~

Testing Done

While qualifying these changes, I ran commands like the following in a Windows shell, from the resolver-pipeline directory.

java -cp C:\SaxonHE10-8J\saxon-he-10.8.jar net.sf.saxon.Transform -t -s:..\..\..\specifications\profile-resolution\profile-resolution-examples\broken_profile.xml -xsl:oscal-profile-RESOLVE.xsl -o:myoutput.xml

The command window output shows the error message as in the following example.

Terminating Error: Document not acquired: missing_profile.xml resolved as file:/C:/github/oscal/OSCAL/src/specifications/profile-resolution/profile-resolution-examples/missing_profile.xml (as OSCAL XML)
Error at xsl:message on line 150 column 60 of oscal-profile-RESOLVE.xsl:
  XTMM9000  Processing terminated by xsl:message at line 150 in oscal-profile-RESOLVE.xsl
     invoked by xsl:for-each at file:/C:/github/oscal/OSCAL/src/utils/util/resolver-pipeline/oscal-profile-RESOLVE.xsl#149
  In template rule with match="opr:terminate-if-severe-errors" on line 144 of oscal-profile-RESOLVE.xsl
     invoked by xsl:apply-templates at file:/C:/github/oscal/OSCAL/src/utils/util/resolver-pipeline/oscal-profile-RESOLVE.xsl#81
     invoked by xsl:iterate at file:/C:/github/oscal/OSCAL/src/utils/util/resolver-pipeline/oscal-profile-RESOLVE.xsl#77
  In template rule with match="/" on line 69 of oscal-profile-RESOLVE.xsl
Processing terminated by xsl:message at line 150 in oscal-profile-RESOLVE.xsl

In another command-line example with nonfatal errors and warnings, the command window output includes the messages near the end:

Warning: Found 2 elements of type &lt;title&gt;. Only the last will be present in result.
Error: Element of type &lt;back-matter&gt; is not valid inside &lt;control&gt; and will not be present in result.
Warning: set-parameter with param-id="a3_prm1" does not match any param id.
Execution time: 644.6529ms
Memory used: 30Mb

Behavior when using Oxygen transformation scenario: Messages appear in the Messages pane. (In the case of fatal error messages, this pane is underneath the Transformation problems pane.)

galtm

Some annotations to make large diffs more manageable

src/utils/util/resolver-pipeline/message-handler.xsl

src/utils/util/resolver-pipeline/oscal-profile-RESOLVE.xsl

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

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

galtm · 2022-11-13T23:04:15Z

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

-    <xsl:template match="catalog[merge/as-is=$true-content]" priority="12" as="element(catalog)">
+    <!-- Template return value is element(catalog) possibly preceded by a
+        processing instruction from detect-multiple-structuring-directives. -->
+    <xsl:template match="catalog[merge/as-is=$true-content]" priority="12" as="item()+">


Example of the kind of data type change required by Item 3 in PR description.

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

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

...ifications/profile-resolution/profile-resolution-examples/nonfatal-error-warning_profile.xml

galtm · 2022-11-13T23:13:19Z

@david-waltermire-nist , @wendellpiez : I'm a little concerned about potential conflicts between this PR and #1424 (which I haven't looked at in detail, but I see some of the same files). If I can help make file merges go more smoothly, let me know.

wendellpiez

Thanks again for all the tremendous work here, it's looking nice!

aj-stein-nist · 2023-01-12T14:34:09Z

Hi @galtm, thanks for submitting yet another excellent PR. We had to rebase develop and that leads to the "This branch has conflicts that must be resolved" conflicts below. I can help you with that, please let me know if and when you need that help. Thanks.

galtm · 2023-01-12T14:51:58Z

Hi, @aj-stein-nist . Thanks for notifying me and offering to help. The files listed are files that I haven't touched, which should make it easier. I'll try to look into this today or tomorrow and I'll keep you posted.

galtm · 2023-01-13T00:09:34Z

@aj-stein-nist : I resolved the merge conflicts. If anything doesn't look right to you, let me know. Thanks.

@wendellpiez : When I did the force-push, GitHub "dismissed" your earlier code review.

david-waltermire · 2023-01-19T20:39:58Z

@david-waltermire-nist , @wendellpiez : I'm a little concerned about potential conflicts between this PR and #1424 (which I haven't looked at in detail, but I see some of the same files). If I can help make file merges go more smoothly, let me know.

We should figure out how to move forward with this given your concerns.

Would it make sense to move this PR against #1424?

galtm · 2023-01-19T21:30:48Z

Would it make sense to move this PR against #1424?

Would that mean that this PR can't be merged until #1424 is merged? #1424 is a draft and has been open for about 5 months. To keep things moving, can you merge this change onto develop and rebase #1424 on top?

If I could understand the essence of what is changing with #1424 I could look into whether there are any potential merge conflicts with this PR that might be hard to resolve. However, I don't know how to make sense of the 56 commits that probably resulted from rebasing the develop branch. Which commits should I be looking at in that list?

david-waltermire · 2023-01-23T20:49:38Z

Would it make sense to move this PR against #1424?

Would that mean that this PR can't be merged until #1424 is merged? #1424 is a draft and has been open for about 5 months. To keep things moving, can you merge this change onto develop and rebase #1424 on top?

If I could understand the essence of what is changing with #1424 I could look into whether there are any potential merge conflicts with this PR that might be hard to resolve. However, I don't know how to make sense of the 56 commits that probably resulted from rebasing the develop branch. Which commits should I be looking at in that list?

I think your suggestion of merging this now and rebasing #1424 on top is a good path.

wendellpiez

Beautiful work here, and much interesting progress.

galtm · 2023-02-04T15:14:40Z

2/4/23: Pushed 2 commits to fix minor issues I noticed while working on something unrelated.

galtm · 2023-03-10T01:47:00Z

3/9/23: Manually resolved merge conflicts now that #1685 has been merged into develop branch.

galtm · 2023-08-31T14:04:39Z

src/utils/util/resolver-pipeline/testing/4_modified/modify.xspec

+            </x:expect>
+            <x:expect label="The target control does not appear" test="empty($x:result//*[@id='a1-stmt'])"/>
+        </x:scenario>
+        <x:scenario label="Explicit binding to a param that needs to be set *and* have new params inserted">


@wendellpiez : Does this test scenario cover the situation in issue #1859?

I don't think so ... what is being described is wanting to add a new prop to an existing param, but not being able to use alter. Apparently alter is a no-op against params. set-param is the available workaround (it can be used to add new content). But the spec does not say that you can't target param elements using alter/add, so that should also work.

Thanks. You're right, this scenario isn't the same as what the issue is about. But I think the XSLT in this PR fixes the issue. If I create a similar test scenario with

<alter control-id="a1"> <add position="starting" by-id="a1_prm1"> <prop ns="https://fedramp.gov/ns/oscal" name="param-label" value="AC-1(a)"/> </add> </alter>

then I can see that the new prop gets added at the beginning of the existing param, like this:

<param id="a1_prm1"> <prop ns="https://fedramp.gov/ns/oscal" name="param-label" value="AC-1(a)"/> <label>A1 Parameter 1</label> <constraint>at least every 3 years</constraint> </param>

Should I add the new test scenario?

Sure! I guess I was not looking closely enough, or not looking at the current code, to see that this should actually work.

If it actually works, so much the better, we only need to be able to show that it does.

I pushed commit 5d3b9b3 to this PR branch, to provide a test of the situation in #1859. I also tweaked the label of the other test scenario to distinguish the two scenarios from each other more.

aj-stein-nist · 2023-09-28T04:07:00Z

Well, this is frustrating, I have a backup of this branch before rebase, but I am unable to push to your fork @galtm after "aligning" it to develop (with --strategy=ours to save time) but that did not work.

@galtm, can you check out my branch and push it here to reopen? Shorthand instructions below. Thanks. I was trying to spare you the effort of rebasing this yourself.

# Just for alignment on what remotes I mean
git add remote me git@github.com:galtm/OSCAL.git
git add remote aj git@github.com:aj-stein-nist/OSCAL.git
git fetch all
git checkout --track aj/profiler-error-handling-backup
git push me profiler-error-handling-backup:profiler-error-handling -f

Then you should be able to continue on this PR by ID and receive the credit you deserve. Apologies.

galtm · 2023-09-28T16:28:13Z

@aj-stein-nist : I tried to follow your shorthand instructions, but I'm not sure if things worked. Here's the output I got from the push:

$ git push origin profiler-error-handling-backup:profiler-error-handling -f
Enumerating objects: 192, done.
Counting objects: 100% (157/157), done.
Delta compression using up to 8 threads
Compressing objects: 100% (81/81), done.
Writing objects: 100% (112/112), 53.52 KiB | 3.57 MiB/s, done.
Total 112 (delta 84), reused 44 (delta 26), pack-reused 0
remote: Resolving deltas: 100% (84/84), completed with 33 local objects.
To https://github.com/galtm/OSCAL.git
 + 0825b9fc4...5d3b9b302 profiler-error-handling-backup -> profiler-error-handling (forced update)

where origin is as follows (the git@github.com:... syntax didn't work for me when I tried to fetch).

origin  https://github.com/galtm/OSCAL.git (fetch)
origin  https://github.com/galtm/OSCAL.git (push)

Despite the lack of error messages, this PR page still says 0 commits. I thought it would add commits to this PR but they'd be aligned with where develop stands now. Did I misunderstand the result of those commands or what I'm supposed to do next?

It may be simpler for me to sync my develop branch, create a fresh branch from that, do a file-merge (outside Git) of the profile resolver file contents based on the files in a backup branch, and try again to push to the branch of this PR. I started doing that but still need to check the results. I'll be busy for a few hours, but I will come back to this in the late afternoon.

aj-stein-nist · 2023-09-28T16:58:40Z

Well now I am confused because it does look like you "put it back" to the way it was from my backup if I simulate what a PR would like, so now I am scratching my head.

develop...galtm:OSCAL:profiler-error-handling

aj-stein-nist · 2023-09-28T17:00:25Z

It seems now I can reopen?

aj-stein-nist · 2023-09-28T17:03:26Z

Well ti seems even though I have reopened it and commented it seems to ignore the state of the branch on your end even though it looks just as before (155 commits off of develop and the same series of changes I worked through last night). I guess I will try and reopen a new PR, @galtm. I sincerely apologize this mishap is frustrating and embarrassing on my end!

aj-stein-nist · 2023-09-28T17:09:37Z

Apologize, I guess we move #1943 in favor of this PR, I cannot even reopen it. Apologies and sincere thanks, @galtm.

galtm · 2023-09-28T19:51:33Z

I'm confused, too, because I thought rebasing against develop would mean that the list of commits and changed files would look like my work in isolation rather than the long list that I see in #1943.

I also realized that the code needs a few tweaks because the profiler resolver code is less deep in the directory hierarchy than it used to be.

I created #1944 with what I think is the right code, on top of the develop branch. Does it look right from your perspective?

aj-stein-nist · 2023-09-28T20:02:38Z

I'm confused, too, because I thought rebasing against develop would mean that the list of commits and changed files would look like my work in isolation rather than the long list that I see in #1943.

I also realized that the code needs a few tweaks because the profiler resolver code is less deep in the directory hierarchy than it used to be.

I created #1944 with what I think is the right code, on top of the develop branch. Does it look right from your perspective?

I am looking at in a few minutes, we just wrapped our sprint review, thanks again @galtm.

galtm requested a review from david-waltermire as a code owner November 13, 2022 22:53

galtm commented Nov 13, 2022

View reviewed changes

david-waltermire requested a review from wendellpiez December 7, 2022 05:33

wendellpiez previously approved these changes Dec 7, 2022

View reviewed changes

galtm mentioned this pull request Dec 30, 2022

Profiler resolver support for importing another profile #1596

Merged

7 tasks

aj-stein-nist force-pushed the develop branch from eff9ce3 to dd6486a Compare January 10, 2023 18:39

galtm dismissed wendellpiez’s stale review via dcfdcdb January 13, 2023 00:01

galtm force-pushed the profiler-error-handling branch from e00cbaf to dcfdcdb Compare January 13, 2023 00:01

david-waltermire added the Profile Resolution Issues relating to profile resolution. label Jan 23, 2023

david-waltermire previously approved these changes Jan 23, 2023

View reviewed changes

david-waltermire requested a review from wendellpiez January 23, 2023 20:53

wendellpiez previously approved these changes Jan 23, 2023

View reviewed changes

aj-stein-nist assigned galtm Feb 3, 2023

galtm dismissed stale reviews from wendellpiez and david-waltermire via 31b54f0 February 4, 2023 14:57

galtm mentioned this pull request Feb 7, 2023

Profile resolver compatibility with Saxon 11 (on top of other unmerged PRs) #1639

Closed

9 tasks

galtm mentioned this pull request Mar 2, 2023

Profile resolver: Compatibility with Saxon 10 and 11 #1685

Merged

7 tasks

galtm mentioned this pull request Apr 13, 2023

Provide a warning when matching against a group #1746

Open

4 tasks

aj-stein-nist force-pushed the develop branch from 7dea8c0 to 465a1d0 Compare June 29, 2023 21:24

aj-stein-nist requested a review from iMichaela as a code owner June 29, 2023 21:24

aj-stein-nist force-pushed the develop branch from 8a57d69 to e5d24f9 Compare July 11, 2023 12:40

aj-stein-nist self-requested a review as a code owner July 11, 2023 12:40

galtm commented Aug 31, 2023

View reviewed changes

Arminta-Jenkins-NIST force-pushed the develop branch from fb85a27 to 6661368 Compare September 12, 2023 19:22

galtm mentioned this pull request Sep 14, 2023

Profile resolver not applying alterations to parameters #1859

Open

aj-stein-nist changed the title ~~Profile resolver error handling~~ [DONOTMERGE] Profile resolver error handling Sep 27, 2023

aj-stein-nist force-pushed the profiler-error-handling branch from 5d3b9b3 to 30e81c9 Compare September 28, 2023 03:50

aj-stein-nist closed this Sep 28, 2023

aj-stein-nist force-pushed the profiler-error-handling branch from 30e81c9 to 0825b9f Compare September 28, 2023 04:01

aj-stein-nist mentioned this pull request Sep 28, 2023

[WIP] Profiler error handling #1940

Closed

9 tasks

aj-stein-nist changed the title ~~[DONOTMERGE] Profile resolver error handling~~ Profile resolver error handling Sep 28, 2023

aj-stein-nist mentioned this pull request Sep 28, 2023

Profiler error handling #1943

Closed

9 tasks

galtm added a commit to galtm/OSCAL that referenced this pull request Sep 28, 2023

Recreate profiler-error-handling work from usnistgov#1549

18d52fa

galtm mentioned this pull request Sep 28, 2023

Profiler error handling #1944

Merged

7 tasks

aj-stein-nist pushed a commit that referenced this pull request Sep 29, 2023

Recreate profiler-error-handling work from #1549

1b5ee69

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Profile resolver error handling #1549

Profile resolver error handling #1549

galtm commented Nov 13, 2022 •

edited by aj-stein-nist

Loading

galtm left a comment

galtm Nov 13, 2022

galtm commented Nov 13, 2022

wendellpiez left a comment

aj-stein-nist commented Jan 12, 2023

galtm commented Jan 12, 2023

galtm commented Jan 13, 2023

david-waltermire commented Jan 19, 2023

galtm commented Jan 19, 2023

david-waltermire commented Jan 23, 2023

wendellpiez left a comment

galtm commented Feb 4, 2023

galtm commented Mar 10, 2023

galtm Aug 31, 2023

wendellpiez Aug 31, 2023

galtm Aug 31, 2023

wendellpiez Sep 1, 2023

galtm Sep 14, 2023

aj-stein-nist commented Sep 28, 2023

galtm commented Sep 28, 2023

aj-stein-nist commented Sep 28, 2023

aj-stein-nist commented Sep 28, 2023

aj-stein-nist commented Sep 28, 2023 •

edited

Loading

aj-stein-nist commented Sep 28, 2023

galtm commented Sep 28, 2023

aj-stein-nist commented Sep 28, 2023

Profile resolver error handling #1549

Profile resolver error handling #1549

Conversation

galtm commented Nov 13, 2022 • edited by aj-stein-nist Loading

Committer Notes

All Submissions:

Changes to Core Features:

Testing Done

galtm left a comment

Choose a reason for hiding this comment

galtm Nov 13, 2022

Choose a reason for hiding this comment

galtm commented Nov 13, 2022

wendellpiez left a comment

Choose a reason for hiding this comment

aj-stein-nist commented Jan 12, 2023

galtm commented Jan 12, 2023

galtm commented Jan 13, 2023

david-waltermire commented Jan 19, 2023

galtm commented Jan 19, 2023

david-waltermire commented Jan 23, 2023

wendellpiez left a comment

Choose a reason for hiding this comment

galtm commented Feb 4, 2023

galtm commented Mar 10, 2023

galtm Aug 31, 2023

Choose a reason for hiding this comment

wendellpiez Aug 31, 2023

Choose a reason for hiding this comment

galtm Aug 31, 2023

Choose a reason for hiding this comment

wendellpiez Sep 1, 2023

Choose a reason for hiding this comment

galtm Sep 14, 2023

Choose a reason for hiding this comment

aj-stein-nist commented Sep 28, 2023

galtm commented Sep 28, 2023

aj-stein-nist commented Sep 28, 2023

aj-stein-nist commented Sep 28, 2023

aj-stein-nist commented Sep 28, 2023 • edited Loading

aj-stein-nist commented Sep 28, 2023

galtm commented Sep 28, 2023

aj-stein-nist commented Sep 28, 2023

galtm commented Nov 13, 2022 •

edited by aj-stein-nist

Loading

aj-stein-nist commented Sep 28, 2023 •

edited

Loading