Skip to content

SED-4838 Plan Editor: Normalized Subplan Referencing #697

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

Draft
cmisev wants to merge 3 commits into
base: SED-4811-implement-a-dedicated-ui-to-execute-and-monitor-agentic-test-generation
Choose a base branch
from
Draft

SED-4838 Plan Editor: Normalized Subplan Referencing #697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
895b5ea
SED-4838 initial commit of referencing plan by name
cmisev Aug 21, 2026
25c5967
Merge branch 'SED-4811-implement-a-dedicated-ui-to-execute-and-monito…
cmisev Aug 21, 2026
9f6d400
SED-4838 references update
cmisev Aug 21, 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
24 changes: 11 additions & 13 deletions step-automation-packages/step-automation-packages-collections/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,18 @@
<artifactId>step-automation-packages-yaml</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>

@david-stephan david-stephan Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure why we need these dependencies as compile scope now, not seeing related code changes.

<groupId>ch.exense.step</groupId>
<artifactId>step-functions-plugins-jmeter-def</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>ch.exense.step</groupId>
<artifactId>step-functions-plugins-node-def</artifactId>
<version>${project.version}</version>
</dependency>

<!-- Test dependencies -->
<!-- Test dependencies -->
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
Expand All @@ -35,18 +45,6 @@
<version>${project.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>ch.exense.step</groupId>
<artifactId>step-functions-plugins-jmeter-def</artifactId>
<version>${project.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>ch.exense.step</groupId>
<artifactId>step-functions-plugins-node-def</artifactId>
<version>${project.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>ch.exense.step</groupId>
<artifactId>step-automation-packages-controller</artifactId>
Expand Down
112 changes: 75 additions & 37 deletions ...aml/src/main/java/step/automation/packages/yaml/AutomationPackageYamlFragmentManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import step.automation.packages.StagingAutomationPackageContext;
import step.automation.packages.mappers.interfaces.BusinessObjectToYamlMapper;
import step.automation.packages.mappers.interfaces.BusinessObjectToYamlMapping;
import step.automation.packages.mappers.interfaces.ReferenceHandlingObjectMapper;
import step.automation.packages.mappers.interfaces.YamlToBusinessObjectMapper;
import step.automation.packages.mappers.interfaces.YamlToBusinessObjectMapping;
import step.automation.packages.yaml.model.AutomationPackageDescriptorYaml;
Expand Down Expand Up @@ -83,70 +84,79 @@ public enum NewObjectFragmentMode {
protected Properties properties = new Properties();
public final AutomationPackageFragmentYaml descriptorYaml;

private final Map<Class<?>, BusinessObjectToYamlMapper<?, ?>> businessObjectToYamlMappers;
private final Map<Class<?>, BusinessObjectToYamlMapper<?, ?>> mappers = new HashMap<>();

public AutomationPackageYamlFragmentManager(ResourcePathMatchingResolver resourcePathMatchingResolver, AutomationPackageDescriptorYaml descriptorYaml, Set<AutomationPackageFragmentYaml> fragments, AutomationPackageDescriptorReader descriptorReader, StagingAutomationPackageContext stagingContext) {
this.resourcePathMatchingResolver = resourcePathMatchingResolver;
this.descriptorYaml = descriptorYaml;

apRoot = descriptorYaml.getFragmentPath().getParent();

Map<Class<?>, Object> injectables = new HashMap<>();
Map<Class<?>, Object> singletons = new HashMap<>();

injectables.put(YamlPlanReader.class, descriptorReader.getPlanReader());
injectables.put(StagingAutomationPackageContext.class, stagingContext);
singletons.put(YamlPlanReader.class, descriptorReader.getPlanReader());
singletons.put(StagingAutomationPackageContext.class, stagingContext);

businessObjectToYamlMappers = createBusinessObjectToYamlMappers(injectables);
Collection<YamlToBusinessObjectMapper<?, ?>> yamlToBusinessObjectMappers = createYamlToBusinessObjectMappers(injectables);
createBusinessObjectToYamlMappers(singletons);
Collection<YamlToBusinessObjectMapper<?, ?>> yamlObjectMappers = createYamlToBusinessObjectMappers(singletons);

initializeMaps(descriptorYaml, yamlToBusinessObjectMappers);
initializeMaps(descriptorYaml, yamlObjectMappers);

fragments.stream()
.filter(f -> f != descriptorYaml)
.forEach(f -> initializeMaps(f, yamlToBusinessObjectMappers));
.forEach(f -> initializeMaps(f, yamlObjectMappers));

yamlObjectMappers.forEach(m -> {
if (m instanceof ReferenceHandlingObjectMapper referencingMapper) {
referencingMapper.setReferences();
}
});
}

private Map<Class<?>, BusinessObjectToYamlMapper<?, ?>> createBusinessObjectToYamlMappers(Map<Class<?>, Object> injectables) {
var mappers = new HashMap<Class<?>, BusinessObjectToYamlMapper<?, ?>>();
private void createBusinessObjectToYamlMappers(Map<Class<?>, Object> singletons) {

for (Class<?> annotatedClass : CachedAnnotationScanner.getClassesWithAnnotation(BusinessObjectToYamlMapping.class)) {
BusinessObjectToYamlMapping annotation = annotatedClass.getAnnotation(BusinessObjectToYamlMapping.class);
mappers.put(annotation.sourceClass(), instantiateWithInjectables(BusinessObjectToYamlMapping.class, annotatedClass, injectables));
mappers.put(annotation.sourceClass(), (BusinessObjectToYamlMapper<?, ?>) constructSingleton(annotatedClass, singletons));
}

return mappers;
}

private Collection<YamlToBusinessObjectMapper<?, ?>> createYamlToBusinessObjectMappers(Map<Class<?>, Object> injectables) {
List<YamlToBusinessObjectMapper<?, ?>> list = new ArrayList<>();

private Collection<YamlToBusinessObjectMapper<?, ?>> createYamlToBusinessObjectMappers(Map<Class<?>, Object> singletons) {
List<YamlToBusinessObjectMapper<?, ?>> list = new ArrayList<>();
for (Class<?> annotatedClass : CachedAnnotationScanner.getClassesWithAnnotation(YamlToBusinessObjectMapping.class)) {
list.add(instantiateWithInjectables(YamlToBusinessObjectMapping.class, annotatedClass, injectables));
list.add((YamlToBusinessObjectMapper<?, ?>) constructSingleton(annotatedClass, singletons));
}

return list;
}


private <T> T constructSingleton(Class<T> annotatedClass, Map<Class<?>, Object> singletons) {
return (T) constructSingleton(annotatedClass, singletons, annotatedClass);
}

// Instantiates a class found by scanning annotations; The class must have exactly one constructor
// whose arguments can be found in the injectables map.
@SuppressWarnings("unchecked")
private <T> T instantiateWithInjectables(Class<?> annotationType, Class<?> annotatedClass, Map<Class<?>, Object> injectables) {
var constructors = annotatedClass.getConstructors();
if (constructors.length != 1) {
throw new IllegalStateException("Expected exactly one constructor for @" + annotationType.getSimpleName() + "-annotated class "
+ annotatedClass.getName() + ", but found " + constructors.length);
}

var constructor = constructors[0];
try {
var parameters = Arrays.stream(constructor.getParameterTypes())
.map(injectables::get)
.toArray();

return (T) constructor.newInstance(parameters);
} catch (Exception e) {
throw new RuntimeException(e);
}
private <T, RT> T constructSingleton(Class<T> singletonClass, Map<Class<?>, Object> singletons, Class<RT> rootClass) {
return (T) singletons.computeIfAbsent(singletonClass, clazz -> {
var constructors = singletonClass.getConstructors();
if (constructors.length != 1) {
throw new IllegalStateException("Expected exactly one constructor for singleton class "
+ singletonClass.getName() + ", but found " + constructors.length);
}
var constructor = constructors[0];
try {
return constructor.newInstance(Arrays.stream(constructor.getParameterTypes()).map(dependentSignletonClass -> {
if (dependentSignletonClass == rootClass) {
throw new IllegalArgumentException("Circular singleton dependency while trying to instantiate " + rootClass.getName());
}
return constructSingleton(dependentSignletonClass, singletons, rootClass);
}).toArray());
} catch (Exception e) {
throw new RuntimeException(e);
}
});
}
Comment on lines +134 to 160

@gemini-code-assist gemini-code-assist Bot Aug 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

critical

Using Map.computeIfAbsent recursively on a HashMap (via constructSingleton) will modify the map's structural state (modCount) during the computation, which throws a ConcurrentModificationException at runtime in Java 9+. Additionally, the circular dependency check only compares against the rootClass, which can lead to a StackOverflowError for indirect circular dependencies (e.g., A -> B -> C -> B).

This suggestion refactors the singleton construction to avoid recursive computeIfAbsent calls and uses a LinkedHashSet construction stack to robustly detect any circular dependencies.

    private <T> T constructSingleton(Class<T> annotatedClass, Map<Class<?>, Object> singletons) {
        return (T) constructSingleton(annotatedClass, singletons, new java.util.LinkedHashSet<>());
    }

    // Instantiates a class found by scanning annotations; The class must have exactly one constructor
    // whose arguments can be found in the injectables map.
    @SuppressWarnings("unchecked")
    private <T> T constructSingleton(Class<T> singletonClass, Map<Class<?>, Object> singletons, java.util.Set<Class<?>> constructionStack) {
        if (singletons.containsKey(singletonClass)) {
            return (T) singletons.get(singletonClass);
        }
        if (!constructionStack.add(singletonClass)) {
            throw new IllegalArgumentException("Circular singleton dependency detected: " + constructionStack + " -> " + singletonClass.getName());
        }
        try {
            var constructors = singletonClass.getConstructors();
            if (constructors.length != 1) {
                throw new IllegalStateException("Expected exactly one constructor for singleton class "
                    + singletonClass.getName() + ", but found " + constructors.length);
            }
            var constructor = constructors[0];
            Object[] parameters = Arrays.stream(constructor.getParameterTypes())
                .map(dependentSingletonClass -> constructSingleton(dependentSingletonClass, singletons, constructionStack))
                .toArray();
            T instance = (T) constructor.newInstance(parameters);
            singletons.put(singletonClass, instance);
            return instance;
        } catch (Exception e) {
            throw new RuntimeException(e);
        } finally {
            constructionStack.remove(singletonClass);
        }
    }


private void initializeMaps(AutomationPackageFragmentYaml fragment, Collection<YamlToBusinessObjectMapper<?, ?>> yamlObjectMappers) {
Expand All @@ -167,9 +177,27 @@ public <BO extends AbstractOrganizableObject, T> Iterable<BO> getBusinessObjects
}

@SuppressWarnings("unchecked")
public synchronized <BO extends AbstractOrganizableObject, YO extends PatchableYamlModel> BO save(BO object) {
public <BO extends AbstractOrganizableObject, YO extends PatchableYamlModel> BO save(BO object) {
BusinessObjectToYamlMapper<BO, YO> mapper = (BusinessObjectToYamlMapper<BO, YO>) mappers.get(object.getClass());

if (mapper instanceof ReferenceHandlingObjectMapper referenceHandlingObjectMapper) {
referenceHandlingObjectMapper.getReferrers(object).forEach(this::saveSingle);
BO newObject = saveSingle(mapper, object);
referenceHandlingObjectMapper.updateReferences(newObject);
return newObject;
} else {
return saveSingle(object);
}
}

@SuppressWarnings("unchecked")
private <BO extends AbstractOrganizableObject, YO extends PatchableYamlModel> BO saveSingle(BO object) {
BusinessObjectToYamlMapper<BO, YO> mapper = (BusinessObjectToYamlMapper<BO, YO>) mappers.get(object.getClass());
return saveSingle(mapper, object);
}

private synchronized <BO extends AbstractOrganizableObject, YO extends PatchableYamlModel> BO saveSingle(BusinessObjectToYamlMapper<BO, YO> mapper, BO object) {

BusinessObjectToYamlMapper<BO, YO> mapper = (BusinessObjectToYamlMapper<BO, YO>) businessObjectToYamlMappers.get(object.getClass());
if (mapper == null) {
throw new AutomationPackageUpdateException("No BusinessObjectToYamlMapper registered for class: " + object.getClass().getName());
}
Expand All @@ -193,10 +221,20 @@ public synchronized <BO extends AbstractOrganizableObject, YO extends PatchableY

@SuppressWarnings("unchecked")
public synchronized <BO extends AbstractOrganizableObject> void remove(BO object) {
BusinessObjectToYamlMapper<BO, ?> mapper = (BusinessObjectToYamlMapper<BO, ?>) businessObjectToYamlMappers.get(object.getClass());
BusinessObjectToYamlMapper<BO, ?> mapper = (BusinessObjectToYamlMapper<BO, ?>) mappers.get(object.getClass());
if (mapper == null) {
throw new AutomationPackageUpdateException("No BusinessObjectToYamlMapper registered for class: " + object.getClass().getName());
}

if (mapper instanceof ReferenceHandlingObjectMapper referencingMapper) {
Collection<AbstractOrganizableObject> referrers = referencingMapper.getReferrers(object);
if (!referrers.isEmpty()) {
String referrersList = referrers.stream().map(r -> r.getAttribute(AbstractOrganizableObject.NAME)).collect(Collectors.joining(", "));
throw new AutomationPackageUpdateException(object.getAttribute(AbstractOrganizableObject.NAME) + " is referenced by " + referrersList + ", please remove the references before deleting.");
} else {
referencingMapper.removeReferences(object);
}
}
AutomationPackageFragmentYaml fragment = fragmentMap.get(object);
removeFragmentEntity(fragment, fragment.getListForYamlObject(mapper.getCollectionName()), object);
}
Expand Down
112 changes: 99 additions & 13 deletions ...omation-packages-yaml/src/main/java/step/automation/packages/yaml/mappers/PlanMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,28 @@
package step.automation.packages.yaml.mappers;

@david-stephan david-stephan Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

copyright should remain at the top of the file, that also create unrequired diff


import step.artefacts.CallPlan;
import step.artefacts.automation.YamlCallPlan;
import step.automation.packages.mappers.interfaces.BusinessObjectToYamlMapper;
import step.automation.packages.mappers.interfaces.BusinessObjectToYamlMapping;
import step.automation.packages.mappers.interfaces.ReferenceHandlingObjectMapper;
import step.automation.packages.mappers.interfaces.YamlToBusinessObjectMapper;
import step.automation.packages.mappers.interfaces.YamlToBusinessObjectMapping;
import step.core.accessors.AbstractOrganizableObject;
import step.core.artefacts.AbstractArtefact;
import step.core.dynamicbeans.DynamicValue;
import step.core.plans.Plan;
import step.core.yaml.model.NamedYamlArtefact;
import step.plans.parser.yaml.YamlPlan;
import step.plans.parser.yaml.YamlPlanReader;

import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

/*******************************************************************************
* Copyright (C) 2026, exense GmbH
*
Expand All @@ -16,41 +41,102 @@
* You should have received a copy of the GNU Affero General Public License
* along with STEP. If not, see <http://www.gnu.org/licenses/>.
******************************************************************************/
package step.automation.packages.yaml.mappers;

import step.automation.packages.mappers.interfaces.BusinessObjectToYamlMapper;
import step.automation.packages.mappers.interfaces.BusinessObjectToYamlMapping;
import step.automation.packages.mappers.interfaces.YamlToBusinessObjectMapper;
import step.automation.packages.mappers.interfaces.YamlToBusinessObjectMapping;
import step.core.plans.Plan;
import step.plans.parser.yaml.YamlPlan;
import step.plans.parser.yaml.YamlPlanReader;


@BusinessObjectToYamlMapping(sourceClass = Plan.class)
@YamlToBusinessObjectMapping
public class PlanMapper implements BusinessObjectToYamlMapper<Plan, YamlPlan>,
YamlToBusinessObjectMapper<YamlPlan, Plan> {
YamlToBusinessObjectMapper<YamlPlan, Plan>,
ReferenceHandlingObjectMapper {

private final YamlPlanReader planReader;

private final Map<String, AbstractOrganizableObject> idToObjectMap = new HashMap<>();
private final Map<String, Set<AbstractOrganizableObject>> idToReferencingObjectMap = new HashMap<>();

public PlanMapper(YamlPlanReader planReader) {
this.planReader = planReader;
}

@Override
public YamlPlan toYamlObject(Plan plan) {
return planReader.planToYamlPlan(plan);
YamlPlan yamlPlan = planReader.planToYamlPlan(plan);
setNameReferences(yamlPlan.getRoot());
return yamlPlan;
}

private void setNameReferences(NamedYamlArtefact node) {
node.getYamlArtefact().getChildren().forEach(this::setNameReferences);

if (node.getYamlArtefact() instanceof YamlCallPlan yamlCallPlan) {
AbstractOrganizableObject plan = idToObjectMap.get(yamlCallPlan.getPlanId());
if (plan != null) {
yamlCallPlan.setPlan(plan.getAttribute(AbstractOrganizableObject.NAME));
yamlCallPlan.setPlanId(null);
}
}
}

@Override
public Plan toBusinessObject(YamlPlan yamlPlan) {
return planReader.yamlPlanToPlan(yamlPlan);
Plan plan = planReader.yamlPlanToPlan(yamlPlan);
idToObjectMap.put(plan.getId().toString(), plan);
return plan;
}

@Override
public String getCollectionName() {
return YamlPlan.PLANS_ENTITY_NAME;
}

@Override
public void updateReferences(AbstractOrganizableObject object) {
idToReferencingObjectMap.clear();
setReferences();
}

@Override
public void setReferences() {
Map<String, String> nameToIdMap = idToObjectMap.values().stream()
.collect(Collectors.toMap(
p -> p.getAttribute(AbstractOrganizableObject.NAME),
p -> p.getId().toString()));
idToObjectMap.values().forEach(p -> {
if (p instanceof Plan plan) {
setReferences(nameToIdMap, plan, plan.getRoot());
}
});
}

private void setReferences(Map<String, String> nameToIdMap, Plan plan, AbstractArtefact node) {
node.getChildren().forEach(a -> setReferences(nameToIdMap, plan, a));

if (node instanceof CallPlan callPlan) {
String planName = callPlan.getPlan();
String planId = callPlan.getPlanId();
if ((planName != null && !planName.trim().isEmpty()) || planId != null) {
String referencedPlanId = nameToIdMap.getOrDefault(planName, planId);
if (referencedPlanId != null) {
idToReferencingObjectMap.computeIfAbsent(referencedPlanId, p -> new HashSet<>()).add(plan);
callPlan.setPlanId(referencedPlanId);
callPlan.setPlan(null);
} else {
callPlan.setSelectionAttributes(new DynamicValue<>("{\"name\": {\"value\": \"" + callPlan.getPlan() + "\", \"dynamic\": false}}"));
callPlan.setPlan(null);
}
}
}
}

@Override
public Collection<AbstractOrganizableObject> getReferrers(AbstractOrganizableObject plan) {
idToObjectMap.put(plan.getId().toString(), plan);
return idToReferencingObjectMap.getOrDefault(plan.getId().toString(), Collections.emptySet());
}

@Override
public void removeReferences(AbstractOrganizableObject object) {
idToObjectMap.remove(object.getId().toString());
idToReferencingObjectMap.clear();
setReferences();
}
}
3 changes: 3 additions & 0 deletions ...main/resources/step/automation/packages/yaml/step-automation-package-schema-os-1.3.0.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,9 @@
"selectionAttributes": {
"$ref": "#/$defs/DynamicKeywordInputsDef"
},
"plan": {

@david-stephan david-stephan Aug 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we do change the schema to support an alias, we should decide if we want to go with a minor bump 1.2.1. Also I think there are some constant related to that version that should be udpated, I did not see the change in the PR.

"type": "string"
},
"planId": {
"type": "string"
}
Expand Down
Loading