fix: update for OSCAL 1.2.0 model compatibility #223
Conversation
- Update OSCAL submodule to version 1.2.0 - Use ProfileMatching instead of Matching for profile control selection - Add metaschema bindings to disambiguate matching assembly class names - Convert URI to String for remove.getByNs() in ProfileResolver
📝 WalkthroughWalkthroughThe PR updates type references from Matching to ProfileMatching across profile selection logic, adds Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring attention:
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
oscal(1 hunks)src/main/java/gov/nist/secauto/oscal/lib/model/control/profile/AbstractProfileSelectControlById.java(2 hunks)src/main/java/gov/nist/secauto/oscal/lib/model/control/profile/IProfileSelectControlById.java(2 hunks)src/main/java/gov/nist/secauto/oscal/lib/profile/resolver/ProfileResolver.java(1 hunks)src/main/java/gov/nist/secauto/oscal/lib/profile/resolver/alter/RemoveVisitor.java(1 hunks)src/main/java/gov/nist/secauto/oscal/lib/profile/resolver/selection/DefaultControlSelectionFilter.java(3 hunks)src/main/metaschema-bindings/oscal-metaschema-bindings.xml(2 hunks)src/test/java/gov/nist/secauto/oscal/lib/profile/resolver/selection/DefaultControlSelectionFilterTest.java(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Website
- GitHub Check: Code
🔇 Additional comments (6)
oscal (1)
1-1: CI correctly fetches submodules, but verify the OSCAL commit hash.
The GitHub Actions workflows in.github/workflows/build.ymlandrelease.ymlboth usesubmodules: recursivein their checkout steps, so CI will correctly initialize submodules. However, please confirm that commit8064bf7f09648505c57c5ca54a1ac409c98f92a0corresponds to OSCAL 1.2.0—web searches on the OSCAL release pages did not return a match for this commit hash, so it may be an unlabeled or development commit rather than a release tag.src/test/java/gov/nist/secauto/oscal/lib/profile/resolver/selection/DefaultControlSelectionFilterTest.java (1)
11-13: Test migration toProfileMatchingis consistent and low-risk.Also applies to: 38-44
src/main/java/gov/nist/secauto/oscal/lib/model/control/profile/AbstractProfileSelectControlById.java (1)
8-10: Builder now emitsProfileMatchingas intended for OSCAL 1.2.0 compatibility.Also applies to: 61-67
src/main/java/gov/nist/secauto/oscal/lib/profile/resolver/selection/DefaultControlSelectionFilter.java (1)
8-10: Selection logic updated cleanly toProfileMatchingwith no functional drift.Also applies to: 92-95, 150-158
src/main/metaschema-bindings/oscal-metaschema-bindings.xml (1)
34-45: These class name bindings are correct.MappedControlMatchingandProfileMatchingare assembly names defined in external metaschema files (oscal_mapping-common_metaschema.xmland profile metaschema) that will be generated as Java classes during the metaschema code generation phase. The unqualified class names with<use-package-name>gov.nist.secauto.oscal.lib.model</use-package-name>follow the standard metaschema binding pattern and will correctly generate the classes in the specified package. No changes needed.src/main/java/gov/nist/secauto/oscal/lib/model/control/profile/IProfileSelectControlById.java (1)
8-19: No action needed. All usages ofIProfileSelectControlById.getMatching()in the codebase already expect theList<ProfileMatching>return type, including test mocks and theDefaultControlSelectionFilterconsumer. There are no unupdated implementers or callers.
src/main/java/gov/nist/secauto/oscal/lib/profile/resolver/alter/RemoveVisitor.java
Outdated
Show resolved
Hide resolved
src/main/java/gov/nist/secauto/oscal/lib/profile/resolver/ProfileResolver.java
Show resolved
Hide resolved
Addresses PR review feedback to remove the unused import that was accidentally added during OSCAL 1.2.0 compatibility changes.
- Fix ModifierOrder violations by placing @nonnull annotations on separate lines before modifiers in IControlFilter, IControlSelectionFilter, and IIdentifierMapper - Add @FunctionalInterface annotations to IReferencePolicy and IControlSelectionFilter to address ImplicitFunctionalInterface warnings - Update parent POM version from 9-SNAPSHOT to 9
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/java/gov/nist/secauto/oscal/lib/profile/resolver/selection/IControlSelectionFilter.java (1)
20-46: HardenmatchIds/applynull-contracts (or document them).
TodaymatchIds(null)will NPE atArrays.stream(identifiers), andapply(null)will NPE atcontrol.getId(). If nulls are invalid inputs, consider making that explicit by annotating parameters as@NonNull(includingapply(@NonNull IControl control)), or defensively handling nulls in the factory.public interface IControlSelectionFilter extends Function<IControl, Pair<Boolean, Boolean>> { @@ - static IControlSelectionFilter matchIds(@NonNull String... identifiers) { + static IControlSelectionFilter matchIds(@NonNull String... identifiers) { return new IControlSelectionFilter() { private final Set<String> keys = Arrays.stream(identifiers).collect(Collectors.toUnmodifiableSet()); @@ - public Pair<Boolean, Boolean> apply(IControl control) { + public Pair<Boolean, Boolean> apply(@NonNull IControl control) { return ObjectUtils.notNull(Pair.of(keys.contains(control.getId()), false)); } }; } @@ - Pair<Boolean, Boolean> apply(IControl control); + Pair<Boolean, Boolean> apply(@NonNull IControl control); }src/main/java/gov/nist/secauto/oscal/lib/profile/resolver/selection/IControlFilter.java (1)
22-62: Good: explicit@NonNullreturn contracts on the anonymous implementations.
Small consistency nit: consider also adding@NonNulltoIControlFilter.Filter.match(...)’s return to match the interface contract and avoid tool noise.@@ - public Pair<Boolean, Boolean> match(@NonNull IControl control, boolean defaultMatch) { + @Override + @NonNull + public Pair<Boolean, Boolean> match(@NonNull IControl control, boolean defaultMatch) { @NonNull Pair<Boolean, Boolean> result = getInclusionFilter().apply(control); @@ return result; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
pom.xml(1 hunks)src/main/java/gov/nist/secauto/oscal/lib/profile/resolver/policy/IReferencePolicy.java(1 hunks)src/main/java/gov/nist/secauto/oscal/lib/profile/resolver/selection/IControlFilter.java(2 hunks)src/main/java/gov/nist/secauto/oscal/lib/profile/resolver/selection/IControlSelectionFilter.java(2 hunks)src/main/java/gov/nist/secauto/oscal/lib/profile/resolver/support/IIdentifierMapper.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-13T15:28:58.145Z
Learnt from: david-waltermire
Repo: metaschema-framework/liboscal-java PR: 222
File: .github/workflows/build.yml:30-32
Timestamp: 2025-12-13T15:28:58.145Z
Learning: In the liboscal-java repository, nightly builds are managed independently using GitHub Actions schedule triggers (cron), not via repository_dispatch from metaschema-java. Each repo in the metaschema-framework manages its own nightly builds.
Applied to files:
pom.xml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Code
- GitHub Check: Website
🔇 Additional comments (2)
src/main/java/gov/nist/secauto/oscal/lib/profile/resolver/support/IIdentifierMapper.java (1)
34-38: Annotation move is fine; keep return-contract style consistent across overrides.
This aligns the anonymous override with the “method-level@NonNull” style used elsewhere in this PR; just make sure the team sticks to one approach (method vs return-type type-use) for SpotBugs consistency.src/main/java/gov/nist/secauto/oscal/lib/profile/resolver/policy/IReferencePolicy.java (1)
13-14:@FunctionalInterfaceis appropriate here.
Guards the contract (single abstract method) and matches the PR’s functional-style direction.
david-waltermire
merged commit dec578f
into
metaschema-framework:develop
* fix: update for OSCAL 1.2.0 model compatibility - Update OSCAL submodule to version 1.2.0 - Use ProfileMatching instead of Matching for profile control selection - Add metaschema bindings to disambiguate matching assembly class names - Convert URI to String for remove.getByNs() in ProfileResolver * fix: remove unused java.net.URI import Addresses PR review feedback to remove the unused import that was accidentally added during OSCAL 1.2.0 compatibility changes. * fix: resolve PMD priority 1 and 2 violations - Fix ModifierOrder violations by placing @nonnull annotations on separate lines before modifiers in IControlFilter, IControlSelectionFilter, and IIdentifierMapper - Add @FunctionalInterface annotations to IReferencePolicy and IControlSelectionFilter to address ImplicitFunctionalInterface warnings - Update parent POM version from 9-SNAPSHOT to 9
1 participant
Summary
ProfileMatchinginstead ofMatchingfor profile control selection (disambiguates from other matching assemblies)matchingassembly to distinct class namesURItoStringforremove.getByNs()inProfileResolverTest Plan
Summary by CodeRabbit
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.