Skip to content

refactor: introduce IControlSelection/IControlMatching common interfaces #281

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.

Sign up for GitHub

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

Merged
david-waltermire merged 5 commits into from
Apr 23, 2026
Merged

refactor: introduce IControlSelection/IControlMatching common interfaces #281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a865bf4
refactor: introduce IControlSelection/IControlMatching common interfaces
david-waltermire Apr 23, 2026
de7ec52
refactor: rename shared/assessment select-control-by-id classes
david-waltermire Apr 23, 2026
ef644ff
refactor: move control-common select types up from profile package
david-waltermire Apr 23, 2026
43f5507
refactor: rename AbstractProfileMatching to AbstractControlMatching
david-waltermire Apr 23, 2026
315aa28
fix(pmd): add protected constructor and suppress implicit functional …
david-waltermire Apr 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions ...ile/AbstractProfileSelectControlById.java → ...stractControlCommonSelectControlById.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,19 @@
* SPDX-License-Identifier: CC0-1.0
*/

package dev.metaschema.oscal.lib.model.control.profile;
package dev.metaschema.oscal.lib.model.control;

import java.util.Collection;
import java.util.LinkedList;
import java.util.List;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import dev.metaschema.oscal.lib.model.ControlCommonSelectControlById;
import dev.metaschema.oscal.lib.model.ProfileMatching;
import dev.metaschema.oscal.lib.model.ProfileSelectControlById;
import edu.umd.cs.findbugs.annotations.NonNull;

public abstract class AbstractProfileSelectControlById implements IProfileSelectControlById {
public abstract class AbstractControlCommonSelectControlById implements IControlCommonSelectControlById {
// TODO: move implementation from profile resolver selection code here

@NonNull
Expand Down Expand Up @@ -53,8 +53,8 @@ public Builder matching(@NonNull Pattern pattern) {
}

@NonNull
public ProfileSelectControlById build() {
ProfileSelectControlById retval = new ProfileSelectControlById();
public ControlCommonSelectControlById build() {
ControlCommonSelectControlById retval = new ControlCommonSelectControlById();
retval.setWithChildControls(withChildControls ? "yes" : "no");
retval.setWithIds(withIds);
retval.setMatching(matching.stream()
Expand Down
12 changes: 12 additions & 0 deletions src/main/java/dev/metaschema/oscal/lib/model/control/AbstractControlMatching.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* SPDX-FileCopyrightText: none
* SPDX-License-Identifier: CC0-1.0
*/

package dev.metaschema.oscal.lib.model.control;

public abstract class AbstractControlMatching implements IControlMatching {
protected AbstractControlMatching() {
// base class for generated matching bindings
}
}
9 changes: 3 additions & 6 deletions ...ol/profile/IProfileSelectControlById.java → ...trol/IControlCommonSelectControlById.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,15 @@
* SPDX-License-Identifier: CC0-1.0
*/

package dev.metaschema.oscal.lib.model.control.profile;
package dev.metaschema.oscal.lib.model.control;

import java.util.List;

import dev.metaschema.oscal.lib.model.ProfileMatching;

public interface IProfileSelectControlById {

String getWithChildControls();

List<String> getWithIds();
public interface IControlCommonSelectControlById extends IControlSelection {

@Override
List<ProfileMatching> getMatching();

}
13 changes: 13 additions & 0 deletions src/main/java/dev/metaschema/oscal/lib/model/control/IControlMatching.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* SPDX-FileCopyrightText: none
* SPDX-License-Identifier: CC0-1.0
*/

package dev.metaschema.oscal.lib.model.control;

@SuppressWarnings("PMD.ImplicitFunctionalInterface")
public interface IControlMatching {

String getPattern();

}
18 changes: 18 additions & 0 deletions src/main/java/dev/metaschema/oscal/lib/model/control/IControlSelection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* SPDX-FileCopyrightText: none
* SPDX-License-Identifier: CC0-1.0
*/

package dev.metaschema.oscal.lib.model.control;

import java.util.List;

public interface IControlSelection {

String getWithChildControls();

List<String> getWithIds();

List<? extends IControlMatching> getMatching();

}
6 changes: 3 additions & 3 deletions ...va/dev/metaschema/oscal/lib/profile/resolver/selection/DefaultControlSelectionFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@

import dev.metaschema.core.util.ObjectUtils;
import dev.metaschema.oscal.lib.model.ProfileMatching;
import dev.metaschema.oscal.lib.model.control.IControlCommonSelectControlById;
import dev.metaschema.oscal.lib.model.control.catalog.IControl;
import dev.metaschema.oscal.lib.model.control.profile.IProfileSelectControlById;
import dev.metaschema.oscal.lib.profile.resolver.ProfileResolutionEvaluationException;
import edu.umd.cs.findbugs.annotations.NonNull;

Expand All @@ -37,7 +37,7 @@ public class DefaultControlSelectionFilter implements IControlSelectionFilter {
* a list of select criteria
*/
@SuppressWarnings("null")
public DefaultControlSelectionFilter(@NonNull List<? extends IProfileSelectControlById> selections) {
public DefaultControlSelectionFilter(@NonNull List<? extends IControlCommonSelectControlById> selections) {
this.selections = selections.stream()
// ignore null entries
.filter(Objects::nonNull)
Expand Down Expand Up @@ -132,7 +132,7 @@ private static class Selection {
private final Set<String> identifiers;
private final List<Pattern> patterns;

public Selection(IProfileSelectControlById selection) {
public Selection(IControlCommonSelectControlById selection) {
// process with-child-controls
// default is "no"
this.withChildControls = "yes".equals(selection.getWithChildControls());
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/dev/metaschema/oscal/lib/profile/resolver/selection/IControlFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
import dev.metaschema.core.util.ObjectUtils;
import dev.metaschema.oscal.lib.model.IncludeAll;
import dev.metaschema.oscal.lib.model.ProfileImport;
import dev.metaschema.oscal.lib.model.control.IControlCommonSelectControlById;
import dev.metaschema.oscal.lib.model.control.catalog.IControl;
import dev.metaschema.oscal.lib.model.control.profile.IProfileSelectControlById;
import edu.umd.cs.findbugs.annotations.NonNull;

public interface IControlFilter {
Expand Down Expand Up @@ -127,7 +127,7 @@ public Filter(@NonNull ProfileImport profileImport) {
IncludeAll includeAll = profileImport.getIncludeAll();

if (includeAll == null) {
List<? extends IProfileSelectControlById> includeSelections = profileImport.getIncludeControls();
List<? extends IControlCommonSelectControlById> includeSelections = profileImport.getIncludeControls();
if (includeSelections.isEmpty()) {
this.inclusionFilter = IControlSelectionFilter.NONE_MATCH;
} else {
Expand All @@ -137,7 +137,7 @@ public Filter(@NonNull ProfileImport profileImport) {
this.inclusionFilter = IControlSelectionFilter.ALL_MATCH;
}

List<? extends IProfileSelectControlById> excludeSelections = profileImport.getExcludeControls();
List<? extends IControlCommonSelectControlById> excludeSelections = profileImport.getExcludeControls();
if (excludeSelections.isEmpty()) {
this.exclusionFilter = IControlSelectionFilter.NONE_MATCH;
} else {
Expand Down
13 changes: 11 additions & 2 deletions src/main/metaschema-bindings/oscal-metaschema-bindings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,14 @@
</define-assembly-binding>
<define-assembly-binding name="select-control-by-id">
<java>
<use-class-name>ProfileSelectControlById</use-class-name>
<extend-base-class>dev.metaschema.oscal.lib.model.control.profile.AbstractProfileSelectControlById</extend-base-class>
<use-class-name>ControlCommonSelectControlById</use-class-name>
<extend-base-class>dev.metaschema.oscal.lib.model.control.AbstractControlCommonSelectControlById</extend-base-class>
</java>
</define-assembly-binding>
<define-assembly-binding name="matching">
<java>
<use-class-name>ProfileMatching</use-class-name>
<extend-base-class>dev.metaschema.oscal.lib.model.control.AbstractControlMatching</extend-base-class>
</java>
</define-assembly-binding>
</metaschema-binding>
Expand Down Expand Up @@ -158,6 +159,14 @@
</java>
</define-assembly-binding>
</metaschema-binding>
<metaschema-binding
href="../../../oscal/src/metaschema/oscal_assessment-common_metaschema.xml">
<define-assembly-binding name="select-control-by-id">
<java>
<use-class-name>AssessmentCommonSelectControlById</use-class-name>
</java>
</define-assembly-binding>
</metaschema-binding>
Comment on lines +162 to +169

@coderabbitai coderabbitai Bot Apr 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there's an AbstractAssessmentCommonSelectControlById or similar base class defined
fd -t f -e java -i "selectcontrolbyid" src/main/java | xargs -I {} grep -l "Assessment" {} 2>/dev/null

# Check the OSCAL assessment-common metaschema structure to understand the select-control-by-id definition
rg -n "select-control-by-id" --glob "*.xml" oscal/src/metaschema/ 2>/dev/null | head -20

Repository: metaschema-framework/liboscal-java

Length of output: 60

🏁 Script executed:

# First, let's check the structure of the repository
git ls-files | grep -E "(metaschema-bindings|IControlSelection)" | head -20

Repository: metaschema-framework/liboscal-java

Length of output: 401

🏁 Script executed:

# Let's read the actual file mentioned in the review to understand the context
wc -l src/main/metaschema-bindings/oscal-metaschema-bindings.xml

Repository: metaschema-framework/liboscal-java

Length of output: 140

🏁 Script executed:

# Read the relevant section and surrounding context
sed -n '150,180p' src/main/metaschema-bindings/oscal-metaschema-bindings.xml

Repository: metaschema-framework/liboscal-java

Length of output: 1330

🏁 Script executed:

# Search for IControlSelection interface
fd -t f -name "*.java" | xargs grep -l "interface IControlSelection" 2>/dev/null

Repository: metaschema-framework/liboscal-java

Length of output: 310

🏁 Script executed:

# Search for control-common binding to compare
grep -n "control-common" src/main/metaschema-bindings/oscal-metaschema-bindings.xml | head -10

Repository: metaschema-framework/liboscal-java

Length of output: 156

🏁 Script executed:

# Let's see the control-common binding definition
sed -n '47,120p' src/main/metaschema-bindings/oscal-metaschema-bindings.xml

Repository: metaschema-framework/liboscal-java

Length of output: 2976

🏁 Script executed:

# Read the IControlSelection interface
cat src/main/java/dev/metaschema/oscal/lib/model/control/IControlSelection.java

Repository: metaschema-framework/liboscal-java

Length of output: 381

🏁 Script executed:

# Search for other assessment-related interfaces that might be relevant
fd -t f -name "*.java" -path "*/assessment*" | xargs grep -l "interface.*Assessment.*Select" 2>/dev/null

Repository: metaschema-framework/liboscal-java

Length of output: 310

🏁 Script executed:

# Look for any existing AssessmentCommonSelectControlById references
rg "AssessmentCommonSelectControlById" --type java

Repository: metaschema-framework/liboscal-java

Length of output: 60

🏁 Script executed:

# Check if there's an AbstractControlCommonSelectControlById class
fd -t f -name "*SelectControlById*" src/main/java

Repository: metaschema-framework/liboscal-java

Length of output: 308

🏁 Script executed:

# Find AbstractControlCommonSelectControlById
fd -t f -name "*AbstractControlCommonSelectControlById*" src/main/java

Repository: metaschema-framework/liboscal-java

Length of output: 308

🏁 Script executed:

# Search for the class definition
rg "class AbstractControlCommonSelectControlById" --type java -A 5

Repository: metaschema-framework/liboscal-java

Length of output: 912

🏁 Script executed:

# Check if it implements IControlSelection
rg "AbstractControlCommonSelectControlById" --type java -A 10 | head -30

Repository: metaschema-framework/liboscal-java

Length of output: 3470

🏁 Script executed:

# Look for any assessment module base classes that might be models
fd -t f -path "*/assessment*" -name "Abstract*.java" src/main/java

Repository: metaschema-framework/liboscal-java

Length of output: 246

🏁 Script executed:

# Search for the actual generated or expected class pattern in assessment module
rg "AssessmentCommon" --type java | head -20

Repository: metaschema-framework/liboscal-java

Length of output: 60

🏁 Script executed:

# Find IControlCommonSelectControlById interface
rg "interface IControlCommonSelectControlById" --type-list

Repository: metaschema-framework/liboscal-java

Length of output: 467

🏁 Script executed:

# Use correct search for the interface
rg "interface IControlCommonSelectControlById"

Repository: metaschema-framework/liboscal-java

Length of output: 244

🏁 Script executed:

# Examine the interface
cat src/main/java/dev/metaschema/oscal/lib/model/control/IControlCommonSelectControlById.java

Repository: metaschema-framework/liboscal-java

Length of output: 414

🏁 Script executed:

# Check if AbstractControlCommonSelectControlById has any relationship with IControlSelection
rg "IControlSelection" src/main/java/dev/metaschema/oscal/lib/model/control/AbstractControlCommonSelectControlById.java

Repository: metaschema-framework/liboscal-java

Length of output: 60

🏁 Script executed:

# Look for what IControlCommonSelectControlById extends or implements
rg -A 3 "interface IControlCommonSelectControlById"

Repository: metaschema-framework/liboscal-java

Length of output: 566

🏁 Script executed:

# Check if there are any assessment-related selection classes
rg "assessment.*select" -i src/main/metaschema-bindings/oscal-metaschema-bindings.xml

Repository: metaschema-framework/liboscal-java

Length of output: 148

🏁 Script executed:

# Check if AbstractAssessmentCommonSelectControlById exists
find src/main/java -name "*AssessmentCommon*" -type f

Repository: metaschema-framework/liboscal-java

Length of output: 60

🏁 Script executed:

# Search for any assessment module select-control references in code
rg "assessment.*SelectControlById" -i

Repository: metaschema-framework/liboscal-java

Length of output: 207

🏁 Script executed:

# Check what other assessment-common bindings look like in the file
rg "oscal_assessment-common" src/main/metaschema-bindings/oscal-metaschema-bindings.xml -A 8

Repository: metaschema-framework/liboscal-java

Length of output: 454

🏁 Script executed:

# Look at the full assessment module bindings pattern
grep -A 6 "oscal_assessment" src/main/metaschema-bindings/oscal-metaschema-bindings.xml | head -60

Repository: metaschema-framework/liboscal-java

Length of output: 962

Add extend-base-class to AssessmentCommonSelectControlById to implement IControlSelection.

The select-control-by-id binding in oscal_assessment-common_metaschema.xml lacks an extend-base-class, unlike its counterpart in oscal_control-common_metaschema.xml. The control-common binding extends AbstractControlCommonSelectControlById, which implements IControlCommonSelectControlById (which extends IControlSelection), enabling polymorphic usage via the module-agnostic interface. Without a corresponding base class for the assessment module, AssessmentCommonSelectControlById won't implement IControlSelection, breaking consistency with PR objectives for module-agnostic interfaces.

Consider creating AbstractAssessmentCommonSelectControlById as the extend-base-class for this binding to maintain interface consistency across modules.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/main/metaschema-bindings/oscal-metaschema-bindings.xml` around lines 162
- 169, The select-control-by-id metaschema binding must extend a base class so
the generated AssessmentCommonSelectControlById implements the module-agnostic
IControlSelection; update the <define-assembly-binding
name="select-control-by-id"> for the assessment metaschema to include an
extend-base-class that references AbstractAssessmentCommonSelectControlById
(mirroring the control-common pattern where
AbstractControlCommonSelectControlById implements
IControlCommonSelectControlById which extends IControlSelection), ensuring
AssessmentCommonSelectControlById implements the expected interface and
preserves polymorphic usage across modules.

<metaschema-binding
href="../../../oscal/src/metaschema/oscal_assessment-plan_metaschema.xml">
<define-assembly-binding name="assessment-plan">
Expand Down
6 changes: 3 additions & 3 deletions ...ev/metaschema/oscal/lib/profile/resolver/selection/DefaultControlSelectionFilterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@

import dev.metaschema.core.util.ObjectUtils;
import dev.metaschema.oscal.lib.model.ProfileMatching;
import dev.metaschema.oscal.lib.model.control.IControlCommonSelectControlById;
import dev.metaschema.oscal.lib.model.control.catalog.IControl;
import dev.metaschema.oscal.lib.model.control.profile.IProfileSelectControlById;
import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;

Expand All @@ -35,13 +35,13 @@ class DefaultControlSelectionFilterTest {
};

@Mock
private IProfileSelectControlById selectControlByIdA;
private IControlCommonSelectControlById selectControlByIdA;
@Mock
private ProfileMatching matchingA;
@Mock
private ProfileMatching matchingB;
@Mock
private IProfileSelectControlById selectControlByIdB;
private IControlCommonSelectControlById selectControlByIdB;

@SuppressFBWarnings("RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT")
@NonNull
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/dev/metaschema/oscal/lib/profile/resolver/selection/ImportTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
import dev.metaschema.oscal.lib.model.IncludeAll;
import dev.metaschema.oscal.lib.model.Profile;
import dev.metaschema.oscal.lib.model.ProfileImport;
import dev.metaschema.oscal.lib.model.control.AbstractControlCommonSelectControlById;
import dev.metaschema.oscal.lib.model.control.catalog.AbstractControl;
import dev.metaschema.oscal.lib.model.control.profile.AbstractProfileSelectControlById;
import dev.metaschema.oscal.lib.profile.resolver.ProfileResolutionException;
import edu.umd.cs.findbugs.annotations.NonNull;

Expand Down Expand Up @@ -59,7 +59,7 @@ void test() throws ProfileResolutionException {
ProfileImport profileImport = new ProfileImport();
profileImport.setIncludeAll(new IncludeAll());
profileImport.setExcludeControls(Collections.singletonList(
AbstractProfileSelectControlById.builder()
AbstractControlCommonSelectControlById.builder()
.withId("control1")
.build()));
profileImport.setHref(cwd);
Expand Down
Loading