[backend] feat(chaining): step CRUD (#4824)#5243
[backend] feat(chaining): step CRUD (#4824)#5243camrrx wants to merge 8 commits intorelease/currentfrom
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release/current #5243 +/- ##
=====================================================
+ Coverage 60.25% 60.40% +0.14%
- Complexity 5248 5304 +56
=====================================================
Files 1053 1059 +6
Lines 32148 32365 +217
Branches 2411 2428 +17
=====================================================
+ Hits 19371 19550 +179
- Misses 11696 11718 +22
- Partials 1081 1097 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Implements initial CRUD APIs for the new chaining engine’s step templates and condition trees (events), including the underlying model refactor to support a condition↔step join table and updated DTOs/tests/front-end types.
Changes:
- Add
/api/chaining/steps,/api/chaining/conditions, and aggregated/api/chainingendpoints with new DTOs/mappers. - Refactor
Condition↔Stepassociation via newConditionStepjoin entity + Flyway migration to evolveconditionsschema. - Update chaining services and tests to use
StepInputandkey_typesemantics.
Reviewed changes
Copilot reviewed 30 out of 31 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| openaev-model/src/main/java/io/openaev/database/repository/ConditionRepository.java | Adds join-table based query for conditions linked to a step and root-condition query by workflow. |
| openaev-model/src/main/java/io/openaev/database/model/Step.java | Adds join-table relation to ConditionStep and helper accessor for linked conditions. |
| openaev-model/src/main/java/io/openaev/database/model/ConditionStep.java | Introduces join entity for condition↔step links (with is_root). |
| openaev-model/src/main/java/io/openaev/database/model/Condition.java | Refactors condition fields (workflow/name/desc/key_type), adds join-table links + backward-compatible accessors. |
| openaev-front/src/utils/api-types.d.ts | Updates generated API types for steps/events/conditions payloads & outputs. |
| openaev-api/src/test/java/io/openaev/utils/fixtures/ConditionFixture.java | Updates fixture to use keyType instead of legacy key. |
| openaev-api/src/test/java/io/openaev/service/chaining/StepServiceTest.java | Migrates tests to StepInput and adds CRUD-focused unit tests. |
| openaev-api/src/test/java/io/openaev/service/chaining/StepServiceIntegrationTest.java | Migrates integration tests to StepInput and keyType. |
| openaev-api/src/test/java/io/openaev/service/chaining/ConditionServiceTest.java | Adds tests for linking/unlinking conditions↔steps and condition tree create/update helpers. |
| openaev-api/src/test/java/io/openaev/database/model/ConditionRepositoryTest.java | Updates assertions to use getKeyType(). |
| openaev-api/src/test/java/io/openaev/api/chaining/StepApiTest.java | Adds controller unit tests for Step API. |
| openaev-api/src/test/java/io/openaev/api/chaining/InjectExecutionStepTest.java | Updates helper conversion method and mapper-condition key usage (keyType). |
| openaev-api/src/test/java/io/openaev/api/chaining/ConditionApiTest.java | Adds controller unit tests for Condition API. |
| openaev-api/src/main/java/io/openaev/service/chaining/StepService.java | Adds step-template CRUD methods and updates condition template building to use new mapper/link model. |
| openaev-api/src/main/java/io/openaev/service/chaining/ConditionService.java | Adds condition-tree CRUD + linking/unlinking logic and refactors condition evaluation helpers. |
| openaev-api/src/main/java/io/openaev/migration/V4_79__Update_conditions_for_condition_tree.java | Migrates schema to support condition trees + join table and new condition columns. |
| openaev-api/src/main/java/io/openaev/api/chaining/dto/StepsCreateInput.java | Removes old step creation DTO in favor of StepInput. |
| openaev-api/src/main/java/io/openaev/api/chaining/dto/StepOutput.java | Adds output DTO for step template CRUD. |
| openaev-api/src/main/java/io/openaev/api/chaining/dto/StepMapper.java | Adds mapper from Step entity to StepOutput. |
| openaev-api/src/main/java/io/openaev/api/chaining/dto/StepInput.java | Adds input DTO for step template create/update, including condition linking. |
| openaev-api/src/main/java/io/openaev/api/chaining/dto/EventOutput.java | Adds output DTO representing an “event” (root condition tree). |
| openaev-api/src/main/java/io/openaev/api/chaining/dto/EventMapper.java | Adds mapper between condition trees and event DTOs, plus input→entity mapping helpers. |
| openaev-api/src/main/java/io/openaev/api/chaining/dto/EventInput.java | Adds input DTO for event/condition-tree create/update. |
| openaev-api/src/main/java/io/openaev/api/chaining/dto/ConditionOutput.java | Adds nested condition DTO for event output. |
| openaev-api/src/main/java/io/openaev/api/chaining/dto/ConditionCreateInput.java | Renames input field key → key_type and reshuffles temp-parent fields. |
| openaev-api/src/main/java/io/openaev/api/chaining/dto/ChainingOutput.java | Adds aggregated output DTO for conditions + steps. |
| openaev-api/src/main/java/io/openaev/api/chaining/StepApi.java | Introduces REST controller for step template CRUD. |
| openaev-api/src/main/java/io/openaev/api/chaining/InjectExecutionStep.java | Updates ActionStep implementation to accept StepInput and new key field. |
| openaev-api/src/main/java/io/openaev/api/chaining/ConditionApi.java | Introduces REST controller for condition tree CRUD (“events”). |
| openaev-api/src/main/java/io/openaev/api/chaining/ChainingApi.java | Adds aggregated read endpoint returning all steps + condition trees. |
| openaev-api/src/main/java/io/openaev/api/chaining/ActionStep.java | Updates ActionStep interface to use StepInput. |
openaev-api/src/main/java/io/openaev/api/chaining/ChainingApi.java
Outdated
Show resolved
Hide resolved
openaev-api/src/main/java/io/openaev/service/chaining/ConditionService.java
Outdated
Show resolved
Hide resolved
openaev-api/src/main/java/io/openaev/api/chaining/dto/ConditionOutput.java
Outdated
Show resolved
Hide resolved
| public void deleteConditionTree(String conditionRootId) { | ||
| conditionRepository.deleteById(conditionRootId); | ||
| } |
There was a problem hiding this comment.
deleteConditionTree() calls conditionRepository.deleteById(). If the ID doesn't exist, Spring Data typically throws EmptyResultDataAccessException, which isn't handled by RestBehavior and will likely surface as a 500 instead of the documented 404. Consider checking existence first or translating the exception to EntityNotFoundException.
openaev-api/src/main/java/io/openaev/service/chaining/ConditionService.java
Outdated
Show resolved
Hide resolved
| List<Step> steps = stepRepository.findAllById(stepIds); | ||
| if (steps.size() != stepIds.size()) { | ||
| List<String> found = steps.stream().map(Step::getId).toList(); | ||
| List<String> missing = stepIds.stream().filter(id -> !found.contains(id)).toList(); | ||
| throw new EntityNotFoundException("Steps not found: " + missing); |
There was a problem hiding this comment.
linkStepsToRoot() compares steps.size() with stepIds.size() to detect missing steps. If stepIds contains duplicates, this will incorrectly report missing steps even when all IDs exist. De-duplicate stepIds before querying/validating (e.g., via LinkedHashSet) and compute missing IDs against that de-duplicated list.
| @Query( | ||
| """ | ||
| SELECT c | ||
| FROM Condition c | ||
| JOIN c.conditionSteps cs |
There was a problem hiding this comment.
This repository method is now implemented via a custom @Query joining conditionSteps, but the name findAllByStep_Id still reads like a derived query on a (now removed) "step" property. Renaming it (e.g., findAllLinkedToStepId) would reduce confusion and avoid accidental future misuse.
openaev-model/src/main/java/io/openaev/database/model/Condition.java
Outdated
Show resolved
Hide resolved
| import io.hypersistence.utils.hibernate.type.json.JsonType; | ||
| import io.swagger.v3.oas.annotations.media.Schema; | ||
| import jakarta.persistence.*; | ||
| import jakarta.persistence.CascadeType; |
There was a problem hiding this comment.
Step.java imports jakarta.persistence.* and also explicitly imports jakarta.persistence.CascadeType, which is redundant. Consider removing the explicit CascadeType import to keep imports clean and avoid confusion about what's actually used.
| import jakarta.persistence.CascadeType; |
openaev-api/src/main/java/io/openaev/api/chaining/dto/ConditionOutput.java
Outdated
Show resolved
Hide resolved
openaev-api/src/main/java/io/openaev/api/chaining/dto/ConditionOutput.java
Outdated
Show resolved
Hide resolved
| if (ConditionType.MAPPER.equals(condition.getType())) { | ||
|
|
||
| Map<String, Object> input = new HashMap<>(); | ||
| input.put("key", condition.getKey()); |
There was a problem hiding this comment.
I kept the key, I just renamed it to keyType
openaev-api/src/main/java/io/openaev/service/chaining/ConditionService.java
Outdated
Show resolved
Hide resolved
| private String value; | ||
| @Column(name = "condition_key_type") | ||
| @Schema(description = "Key type") | ||
| private String keyType; |
There was a problem hiding this comment.
Add an Enum related to each type of output available
| @JsonProperty("key") | ||
| private String key; | ||
| @JsonProperty("key_type") | ||
| private String keyType; |
There was a problem hiding this comment.
Add an Enum related to each type of output available
| * condition. | ||
| */ | ||
| @JsonProperty("step_from") | ||
| String stepFrom; |
There was a problem hiding this comment.
Why is this stepFrom needed here? Isn’t the one in the Condition sufficient?
| @ApiResponses({ | ||
| @ApiResponse(responseCode = "200", description = "Chaining data retrieved successfully") | ||
| }) | ||
| @AccessControl(actionPerformed = Action.READ, resourceType = ResourceType.SIMULATION_OR_SCENARIO) |
There was a problem hiding this comment.
As it is, it is not going to work with RBAC, but I will create a task to implement whats is needed for the RBAC on these APIs
openaev-api/src/main/java/io/openaev/api/chaining/dto/ConditionOutput.java
Outdated
Show resolved
Hide resolved
| log.error( | ||
| String.format( | ||
| "Unexpected error while creating a new Steps Workflow: %s", e.getMessage())); | ||
| return null; |
There was a problem hiding this comment.
Why are we returning null here ?
| private String id; | ||
|
|
||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "condition_id", nullable = false) |
There was a problem hiding this comment.
We don't need cascade here?
openaev-model/src/main/java/io/openaev/database/model/Condition.java
Outdated
Show resolved
Hide resolved
openaev-model/src/main/java/io/openaev/database/model/Condition.java
Outdated
Show resolved
Hide resolved
90db769 to
3c90921
Compare
# Conflicts: # openaev-api/src/main/java/io/openaev/service/chaining/ConditionService.java
0ceec25 to
1d14acc
Compare
Proposed changes
Testing Instructions
use the previously created step id as step_from
Related issues
Checklist
Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...