From 73070cb1655635a2b6b364b7a0e67a506a79656b Mon Sep 17 00:00:00 2001 From: Antonio Perez Dieppa Date: Thu, 14 May 2026 07:44:38 +0100 Subject: [PATCH 1/6] refactor: CommunityPlanner filter the elegible stages from PipelineRun --- .../community/CommunityExecutionPlanner.java | 26 +++++-- .../CommunityExecutionPlannerTest.java | 69 +++++++++++++++++++ 2 files changed, 91 insertions(+), 4 deletions(-) diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlanner.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlanner.java index 207a3121c..4be59c981 100644 --- a/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlanner.java +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlanner.java @@ -32,6 +32,7 @@ import io.flamingock.internal.core.pipeline.execution.ExecutableStage; import io.flamingock.internal.core.pipeline.loaded.stage.AbstractLoadedStage; import io.flamingock.internal.core.pipeline.run.PipelineRun; +import io.flamingock.internal.core.pipeline.run.StageRun; import io.flamingock.internal.core.external.store.audit.community.CommunityAuditReader; import io.flamingock.internal.util.id.RunnerId; import io.flamingock.internal.util.TimeService; @@ -115,15 +116,18 @@ public CommunityExecutionPlanner(RunnerId instanceId, *

Error Handling: If any exception occurs after acquiring the lock, the lock is released * in the catch block to prevent lock leaks.

* - * @param pipelineRun the in-flight run aggregate; this implementation reads the loaded - * stages from it ({@code pipelineRun.getLoadedStages()}) and does not - * yet consult per-stage state (deferred to a later phase) + * @param pipelineRun the in-flight run aggregate; this implementation considers only the + * stages still eligible for planning — i.e., it excludes stages whose + * state has reached a terminal failed shape + * ({@link io.flamingock.internal.common.core.response.data.StageState.Failed} + * or its subclass {@code BlockedForMI}). See + * {@link #stagesEligibleForPlanning(PipelineRun)}. * @return ExecutionPlan containing either stages to execute (with lock held) or CONTINUE (no lock) * @throws LockException if unable to acquire the distributed lock within the configured timeout */ @Override public ExecutionPlan getNextExecution(PipelineRun pipelineRun) throws LockException { - List loadedStages = pipelineRun.getLoadedStages(); + List loadedStages = stagesEligibleForPlanning(pipelineRun); Map initialSnapshot = auditReader.getAuditSnapshotByChangeId(); logger.debug("Pulled initial remote state:\n{}", initialSnapshot); @@ -176,6 +180,20 @@ public ExecutionPlan getNextExecution(PipelineRun pipelineRun) throws LockExcept } } + /** + * Returns the loaded stages this planner considers eligible to plan in the current iteration. + * + *

Today's policy: exclude any stage already in a terminal failed shape + * ({@code StageState.Failed} or its subclass {@code BlockedForMI}). Future evolution may + * narrow this (e.g., retry stages that failed with a recoverable cause). + */ + private static List stagesEligibleForPlanning(PipelineRun pipelineRun) { + return pipelineRun.getStageRuns().stream() + .filter(stageRun -> !stageRun.getState().isFailed()) + .map(StageRun::getLoadedStage) + .collect(Collectors.toList()); + } + private Lock acquireLock() { return CommunityLock.getLock( configuration.getLockAcquiredForMillis(), diff --git a/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlannerTest.java b/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlannerTest.java index 41b092a3c..e9440c4cd 100644 --- a/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlannerTest.java +++ b/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlannerTest.java @@ -19,6 +19,8 @@ import io.flamingock.internal.common.core.audit.AuditTxType; import io.flamingock.api.RecoveryStrategy; import io.flamingock.internal.common.core.change.RecoveryDescriptor; +import io.flamingock.internal.common.core.recovery.ManualInterventionRequiredException; +import io.flamingock.internal.common.core.recovery.RecoveryIssue; import io.flamingock.internal.core.configuration.core.CoreConfigurable; import io.flamingock.internal.core.external.store.audit.community.CommunityAuditReader; import io.flamingock.internal.core.external.store.lock.LockAcquisition; @@ -102,6 +104,73 @@ void shouldAcquireLockAndReturnExecutionPlanWhenNoManualIntervention() { verify(lockService).upsert(any(), any(), anyLong()); } + @Test + @DisplayName("Should skip stages already marked Failed in the run and plan only the remaining stages") + void shouldSkipFailedStagesAndPlanRemaining() { + AbstractLoadedChange change1 = mockLoadedChange("change-1"); + AbstractLoadedChange change2 = mockLoadedChange("change-2"); + AbstractLoadedStage stage1 = mockStage("stage-1", change1); + AbstractLoadedStage stage2 = mockStage("stage-2", change2); + + PipelineRun pipelineRun = PipelineRun.of(java.util.Arrays.asList(stage1, stage2)); + pipelineRun.markStageFailed("stage-1", new RuntimeException("boom")); + + when(auditReader.getAuditSnapshotByChangeId()).thenReturn(Collections.emptyMap()); + when(lockService.upsert(any(), any(), anyLong())) + .thenReturn(new LockAcquisition(RunnerId.fromString("test-runner"), 60000L)); + + ExecutionPlan plan = planner.getNextExecution(pipelineRun); + + assertFalse(plan.isAborted()); + assertTrue(plan.isExecutionRequired()); + // stage-1 was Failed → filtered before action-building. stage-2 was eligible → planned. + verify(stage1, never()).applyActions(any()); + verify(stage2, atLeastOnce()).applyActions(any()); + } + + @Test + @DisplayName("Should skip stages in BlockedForMI (subtype of Failed) and plan only the remaining stages") + void shouldSkipBlockedForMIStagesAndPlanRemaining() { + AbstractLoadedChange change1 = mockLoadedChange("change-1"); + AbstractLoadedChange change2 = mockLoadedChange("change-2"); + AbstractLoadedStage stage1 = mockStage("stage-1", change1); + AbstractLoadedStage stage2 = mockStage("stage-2", change2); + + PipelineRun pipelineRun = PipelineRun.of(java.util.Arrays.asList(stage1, stage2)); + ManualInterventionRequiredException miEx = new ManualInterventionRequiredException( + Collections.singletonList(new RecoveryIssue("change-1")), "stage-1"); + pipelineRun.markStagesBlockedFromMI(miEx); + + when(auditReader.getAuditSnapshotByChangeId()).thenReturn(Collections.emptyMap()); + when(lockService.upsert(any(), any(), anyLong())) + .thenReturn(new LockAcquisition(RunnerId.fromString("test-runner"), 60000L)); + + ExecutionPlan plan = planner.getNextExecution(pipelineRun); + + assertFalse(plan.isAborted()); + assertTrue(plan.isExecutionRequired()); + verify(stage1, never()).applyActions(any()); + verify(stage2, atLeastOnce()).applyActions(any()); + } + + @Test + @DisplayName("Should return CONTINUE without acquiring lock when every remaining stage is already failed") + void shouldReturnContinueWhenAllRemainingStagesAreFailed() { + AbstractLoadedChange change = mockLoadedChange("change-1"); + AbstractLoadedStage stage = mockStage("stage-1", change); + + PipelineRun pipelineRun = PipelineRun.of(Collections.singletonList(stage)); + pipelineRun.markStageFailed("stage-1", new RuntimeException("boom")); + + when(auditReader.getAuditSnapshotByChangeId()).thenReturn(Collections.emptyMap()); + + ExecutionPlan plan = planner.getNextExecution(pipelineRun); + + assertFalse(plan.isAborted()); + assertFalse(plan.isExecutionRequired()); + verify(lockService, never()).upsert(any(), any(), anyLong()); + } + @Test @DisplayName("Should return CONTINUE without lock when all changes are already applied") void shouldReturnContinueWithoutLockWhenAllChangesApplied() { From aea4cc87194d17f757a1419c3cf42ede28a393e0 Mon Sep 17 00:00:00 2001 From: Antonio Perez Dieppa Date: Thu, 14 May 2026 10:21:05 +0100 Subject: [PATCH 2/6] refactor: Cloud API mapping from PipelineRun --- .../cloud/api/request/StageRequest.java | 24 ++++++- .../cloud/api/vo/CloudStageStatus.java | 31 ++++++++ .../io/flamingock/cloud/CloudApiMapper.java | 21 ++++++ .../planner/CloudExecutionPlanMapper.java | 16 +++-- .../cloud/planner/CloudExecutionPlanner.java | 6 +- .../flamingock/cloud/CloudApiMapperTest.java | 70 +++++++++++++++++++ .../planner/CloudExecutionPlanMapperTest.java | 47 ++++++++++++- .../community/CommunityExecutionPlanner.java | 3 +- .../common/test/cloud/MockRunnerServer.java | 6 +- .../cloud/deprecated/MockRunnerServerOld.java | 6 +- 10 files changed, 214 insertions(+), 16 deletions(-) create mode 100644 cloud/flamingock-cloud-api/src/main/java/io/flamingock/cloud/api/vo/CloudStageStatus.java create mode 100644 cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/CloudApiMapperTest.java diff --git a/cloud/flamingock-cloud-api/src/main/java/io/flamingock/cloud/api/request/StageRequest.java b/cloud/flamingock-cloud-api/src/main/java/io/flamingock/cloud/api/request/StageRequest.java index 62647538c..7387b9ae7 100644 --- a/cloud/flamingock-cloud-api/src/main/java/io/flamingock/cloud/api/request/StageRequest.java +++ b/cloud/flamingock-cloud-api/src/main/java/io/flamingock/cloud/api/request/StageRequest.java @@ -15,6 +15,8 @@ */ package io.flamingock.cloud.api.request; +import io.flamingock.cloud.api.vo.CloudStageStatus; + import java.util.List; public class StageRequest { @@ -22,14 +24,25 @@ public class StageRequest { private int order; + /** + * Per-stage status reported by the client. Nullable for back-compat with older clients; + * servers must treat {@code null} as {@link CloudStageStatus#NOT_STARTED}. + */ + private CloudStageStatus status; + private List changes; public StageRequest() { } public StageRequest(String name, int order, List changes) { + this(name, order, null, changes); + } + + public StageRequest(String name, int order, CloudStageStatus status, List changes) { this.name = name; this.order = order; + this.status = status; this.changes = changes; } @@ -41,6 +54,10 @@ public int getOrder() { return order; } + public CloudStageStatus getStatus() { + return status; + } + public List getChanges() { return changes; } @@ -53,6 +70,10 @@ public void setOrder(int order) { this.order = order; } + public void setStatus(CloudStageStatus status) { + this.status = status; + } + public void setChanges(List changes) { this.changes = changes; } @@ -64,11 +85,12 @@ public boolean equals(Object o) { StageRequest that = (StageRequest) o; return order == that.order && java.util.Objects.equals(name, that.name) + && status == that.status && java.util.Objects.equals(changes, that.changes); } @Override public int hashCode() { - return java.util.Objects.hash(name, order, changes); + return java.util.Objects.hash(name, order, status, changes); } } diff --git a/cloud/flamingock-cloud-api/src/main/java/io/flamingock/cloud/api/vo/CloudStageStatus.java b/cloud/flamingock-cloud-api/src/main/java/io/flamingock/cloud/api/vo/CloudStageStatus.java new file mode 100644 index 000000000..0106cbfcf --- /dev/null +++ b/cloud/flamingock-cloud-api/src/main/java/io/flamingock/cloud/api/vo/CloudStageStatus.java @@ -0,0 +1,31 @@ +/* + * Copyright 2026 Flamingock (https://www.flamingock.io) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.flamingock.cloud.api.vo; + +/** + * Wire-level per-stage status sent from the client to the cloud planner. + * + *

Mirrors the internal {@code StageState} hierarchy in shape; the cloud server uses this to + * decide what to do with each stage on the next iteration (e.g., skip stages already failed + * or blocked, route MI cases). Client-side mapping lives in {@code CloudApiMapper.toCloud(StageState)}. + */ +public enum CloudStageStatus { + NOT_STARTED, + STARTED, + COMPLETED, + FAILED, + BLOCKED_MANUAL_INTERVENTION +} diff --git a/cloud/flamingock-cloud/src/main/java/io/flamingock/cloud/CloudApiMapper.java b/cloud/flamingock-cloud/src/main/java/io/flamingock/cloud/CloudApiMapper.java index 9c81bae30..1c7aaf9ec 100644 --- a/cloud/flamingock-cloud/src/main/java/io/flamingock/cloud/CloudApiMapper.java +++ b/cloud/flamingock-cloud/src/main/java/io/flamingock/cloud/CloudApiMapper.java @@ -17,10 +17,12 @@ import io.flamingock.cloud.api.vo.CloudAuditStatus; import io.flamingock.cloud.api.vo.CloudChangeType; +import io.flamingock.cloud.api.vo.CloudStageStatus; import io.flamingock.cloud.api.vo.CloudTargetSystemAuditMarkType; import io.flamingock.cloud.api.vo.CloudTxStrategy; import io.flamingock.internal.common.core.audit.AuditEntry; import io.flamingock.internal.common.core.audit.AuditTxType; +import io.flamingock.internal.common.core.response.data.StageState; import io.flamingock.internal.common.core.targets.TargetSystemAuditMarkType; public final class CloudApiMapper { @@ -44,4 +46,23 @@ public static CloudChangeType toCloud(AuditEntry.ChangeType changeType) { return CloudChangeType.valueOf(changeType.name()); } + /** + * Maps the internal {@link StageState} hierarchy to the wire enum {@link CloudStageStatus}. + * + *

Returns {@code null} for {@code NOT_STARTED} (or a null state) — the canonical wire + * shape for "not started" is field absence/null, matching back-compat semantics with older + * clients that don't populate the field. The server treats {@code null} as {@code NOT_STARTED}. + * + *

Order is important: {@code BlockedForMI} extends {@code Failed}, so the + * blocked-for-MI check must come before the generic failed check. + */ + public static CloudStageStatus toCloud(StageState state) { + if (state == null || state.isNotStarted()) return null; + if (state.isBlockedForManualIntervention()) return CloudStageStatus.BLOCKED_MANUAL_INTERVENTION; + if (state.isFailed()) return CloudStageStatus.FAILED; + if (state.isCompleted()) return CloudStageStatus.COMPLETED; + if (state.isStarted()) return CloudStageStatus.STARTED; + throw new IllegalStateException("Unknown StageState: " + state); + } + } diff --git a/cloud/flamingock-cloud/src/main/java/io/flamingock/cloud/planner/CloudExecutionPlanMapper.java b/cloud/flamingock-cloud/src/main/java/io/flamingock/cloud/planner/CloudExecutionPlanMapper.java index d8a5899a6..739338cf2 100644 --- a/cloud/flamingock-cloud/src/main/java/io/flamingock/cloud/planner/CloudExecutionPlanMapper.java +++ b/cloud/flamingock-cloud/src/main/java/io/flamingock/cloud/planner/CloudExecutionPlanMapper.java @@ -24,8 +24,11 @@ import io.flamingock.cloud.api.response.StageResponse; import io.flamingock.cloud.api.response.ChangeResponse; import io.flamingock.cloud.api.vo.CloudChangeAction; +import io.flamingock.cloud.api.vo.CloudStageStatus; import io.flamingock.cloud.api.vo.CloudTargetSystemAuditMarkType; import io.flamingock.cloud.CloudApiMapper; +import io.flamingock.internal.core.pipeline.run.PipelineRun; +import io.flamingock.internal.core.pipeline.run.StageRun; import io.flamingock.internal.common.core.targets.TargetSystemAuditMarkType; import io.flamingock.cloud.lock.CloudLockService; import io.flamingock.internal.core.configuration.core.CoreConfigurable; @@ -51,19 +54,22 @@ public final class CloudExecutionPlanMapper { - public static ExecutionPlanRequest toRequest(List loadedStages, + public static ExecutionPlanRequest toRequest(PipelineRun pipelineRun, long lockAcquiredForMillis, Map ongoingStatusesMap) { - List requestStages = new ArrayList<>(loadedStages.size()); - for (int i = 0; i < loadedStages.size(); i++) { - AbstractLoadedStage currentStage = loadedStages.get(i); + List stageRuns = pipelineRun.getStageRuns(); + List requestStages = new ArrayList<>(stageRuns.size()); + for (int i = 0; i < stageRuns.size(); i++) { + StageRun stageRun = stageRuns.get(i); + AbstractLoadedStage currentStage = stageRun.getLoadedStage(); List stageChanges = currentStage .getChanges() .stream() .map(descriptor -> CloudExecutionPlanMapper.mapToChangeRequest(descriptor, ongoingStatusesMap)) .collect(Collectors.toList()); - requestStages.add(new StageRequest(currentStage.getName(), i, stageChanges)); + CloudStageStatus status = CloudApiMapper.toCloud(stageRun.getState()); + requestStages.add(new StageRequest(currentStage.getName(), i, status, stageChanges)); } return new ExecutionPlanRequest(lockAcquiredForMillis, requestStages); diff --git a/cloud/flamingock-cloud/src/main/java/io/flamingock/cloud/planner/CloudExecutionPlanner.java b/cloud/flamingock-cloud/src/main/java/io/flamingock/cloud/planner/CloudExecutionPlanner.java index 3126c8f59..15277e280 100644 --- a/cloud/flamingock-cloud/src/main/java/io/flamingock/cloud/planner/CloudExecutionPlanner.java +++ b/cloud/flamingock-cloud/src/main/java/io/flamingock/cloud/planner/CloudExecutionPlanner.java @@ -94,7 +94,7 @@ public ExecutionPlan getNextExecution(PipelineRun pipelineRun) throws LockExcept do { try { logger.info("Requesting cloud execution plan - elapsed[{}ms]", counterPerGuid.getElapsed()); - ExecutionPlanResponse response = createExecution(loadedStages, snapshot.getMarks(), lastOwnerGuid, counterPerGuid.getElapsed()); + ExecutionPlanResponse response = createExecution(pipelineRun, snapshot.getMarks(), lastOwnerGuid, counterPerGuid.getElapsed()); logger.info("Obtained cloud execution plan: {}", response.getAction()); //TODO should check if it has the lock? @@ -148,7 +148,7 @@ public ExecutionPlan getNextExecution(PipelineRun pipelineRun) throws LockExcept } while (true); } - private ExecutionPlanResponse createExecution(List loadedStages, + private ExecutionPlanResponse createExecution(PipelineRun pipelineRun, Collection auditMarks, String lastAcquisitionId, long elapsedMillis) { @@ -158,7 +158,7 @@ private ExecutionPlanResponse createExecution(List loadedSt .collect(Collectors.toMap(TargetSystemAuditMark::getChangeId, TargetSystemAuditMark::getOperation)); ExecutionPlanRequest requestBody = CloudExecutionPlanMapper.toRequest( - loadedStages, + pipelineRun, coreConfiguration.getLockAcquiredForMillis(), auditMarksMap); diff --git a/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/CloudApiMapperTest.java b/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/CloudApiMapperTest.java new file mode 100644 index 000000000..2f20ae12a --- /dev/null +++ b/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/CloudApiMapperTest.java @@ -0,0 +1,70 @@ +/* + * Copyright 2026 Flamingock (https://www.flamingock.io) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.flamingock.cloud; + +import io.flamingock.cloud.api.vo.CloudStageStatus; +import io.flamingock.internal.common.core.recovery.RecoveryIssue; +import io.flamingock.internal.common.core.response.data.ErrorInfo; +import io.flamingock.internal.common.core.response.data.StageState; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.util.Collections; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +class CloudApiMapperTest { + + @Test + @DisplayName("toCloud(null) returns null (wire shape for NOT_STARTED is field absent)") + void nullReturnsNull() { + assertNull(CloudApiMapper.toCloud((StageState) null)); + } + + @Test + @DisplayName("toCloud(NOT_STARTED) returns null (wire shape for NOT_STARTED is field absent)") + void notStartedReturnsNull() { + assertNull(CloudApiMapper.toCloud(StageState.NOT_STARTED)); + } + + @Test + @DisplayName("toCloud(STARTED) maps to STARTED") + void startedMaps() { + assertEquals(CloudStageStatus.STARTED, CloudApiMapper.toCloud(StageState.STARTED)); + } + + @Test + @DisplayName("toCloud(COMPLETED) maps to COMPLETED") + void completedMaps() { + assertEquals(CloudStageStatus.COMPLETED, CloudApiMapper.toCloud(StageState.COMPLETED)); + } + + @Test + @DisplayName("toCloud(Failed) maps to FAILED") + void failedMaps() { + StageState failed = StageState.failed(new ErrorInfo("RuntimeException", "boom", Collections.emptyList(), "stage-1")); + assertEquals(CloudStageStatus.FAILED, CloudApiMapper.toCloud(failed)); + } + + @Test + @DisplayName("toCloud(BlockedForMI) maps to BLOCKED_MANUAL_INTERVENTION — not FAILED — even though BlockedForMI extends Failed") + void blockedForMIMapsBeforeFailed() { + StageState blocked = StageState.blockedManualIntervention( + "stage-1", Collections.singletonList(new RecoveryIssue("change-1"))); + assertEquals(CloudStageStatus.BLOCKED_MANUAL_INTERVENTION, CloudApiMapper.toCloud(blocked)); + } +} diff --git a/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlanMapperTest.java b/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlanMapperTest.java index 05bb2a157..68205cefe 100644 --- a/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlanMapperTest.java +++ b/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlanMapperTest.java @@ -38,8 +38,14 @@ import io.flamingock.cloud.api.request.ExecutionPlanRequest; import io.flamingock.cloud.api.request.ChangeRequest; +import io.flamingock.cloud.api.request.StageRequest; +import io.flamingock.cloud.api.vo.CloudStageStatus; import io.flamingock.cloud.api.vo.CloudTargetSystemAuditMarkType; +import io.flamingock.internal.common.core.recovery.RecoveryIssue; +import io.flamingock.internal.common.core.response.data.StageResult; +import io.flamingock.internal.common.core.response.data.StageState; import io.flamingock.internal.common.core.targets.TargetSystemAuditMarkType; +import io.flamingock.internal.core.pipeline.run.PipelineRun; import java.util.Arrays; import java.util.HashMap; @@ -202,7 +208,8 @@ void shouldMapOngoingStatusFromAuditMarksToChangeRequests() { HashMap ongoingStatusesMap = new HashMap<>(); ongoingStatusesMap.put(change1.getId(), TargetSystemAuditMarkType.APPLIED); - ExecutionPlanRequest request = CloudExecutionPlanMapper.toRequest(loadedStages, 60000L, ongoingStatusesMap); + ExecutionPlanRequest request = CloudExecutionPlanMapper.toRequest( + PipelineRun.of(loadedStages), 60000L, ongoingStatusesMap); Map marksByChangeId = request.getClientSubmission().getStages().get(0).getChanges().stream() .collect(Collectors.toMap(ChangeRequest::getId, ChangeRequest::getOngoingStatus)); @@ -211,6 +218,44 @@ void shouldMapOngoingStatusFromAuditMarksToChangeRequests() { assertEquals(CloudTargetSystemAuditMarkType.NONE, marksByChangeId.get(change2.getId())); } + @Test + @DisplayName("Should map per-stage status from PipelineRun into StageRequest.status") + void shouldMapPerStageStatusFromPipelineRun() { + // change2 lives ONLY in stage-blocked so the MI exception resolves to that stage + // (markStagesBlockedFromMI maps recovery issues to the first stage whose changes match). + AbstractLoadedStage notStartedStage = buildStage("stage-not-started", change1); + AbstractLoadedStage completedStage = buildStage("stage-completed", change1); + AbstractLoadedStage failedStage = buildStage("stage-failed", change1); + AbstractLoadedStage blockedStage = buildStage("stage-blocked", change2); + + PipelineRun pipelineRun = PipelineRun.of(Arrays.asList( + notStartedStage, completedStage, failedStage, blockedStage)); + + // Mark states explicitly. + pipelineRun.markStageCompleted("stage-completed", + StageResult.builder().stageId("stage-completed").stageName("stage-completed") + .state(StageState.COMPLETED).build()); + pipelineRun.markStageFailed("stage-failed", new RuntimeException("boom")); + ManualInterventionRequiredException miEx = new ManualInterventionRequiredException( + Collections.singletonList(new RecoveryIssue(change2.getId())), "stage-blocked"); + pipelineRun.markStagesBlockedFromMI(miEx); + + ExecutionPlanRequest request = CloudExecutionPlanMapper.toRequest( + pipelineRun, 60000L, Collections.emptyMap()); + + // Plain loop because Collectors.toMap rejects null values; NOT_STARTED maps to null. + Map statusByName = new HashMap<>(); + for (StageRequest stageRequest : request.getClientSubmission().getStages()) { + statusByName.put(stageRequest.getName(), stageRequest.getStatus()); + } + + // NOT_STARTED is encoded as null on the wire (back-compat: missing field == not started). + assertNull(statusByName.get("stage-not-started")); + assertEquals(CloudStageStatus.COMPLETED, statusByName.get("stage-completed")); + assertEquals(CloudStageStatus.FAILED, statusByName.get("stage-failed")); + assertEquals(CloudStageStatus.BLOCKED_MANUAL_INTERVENTION, statusByName.get("stage-blocked")); + } + private static DefaultLoadedStage buildStage(String name, AbstractLoadedChange... changes) { return new DefaultLoadedStage(name, StageType.DEFAULT, Arrays.asList(changes)); } diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlanner.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlanner.java index 4be59c981..65a38f854 100644 --- a/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlanner.java +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlanner.java @@ -119,8 +119,7 @@ public CommunityExecutionPlanner(RunnerId instanceId, * @param pipelineRun the in-flight run aggregate; this implementation considers only the * stages still eligible for planning — i.e., it excludes stages whose * state has reached a terminal failed shape - * ({@link io.flamingock.internal.common.core.response.data.StageState.Failed} - * or its subclass {@code BlockedForMI}). See + * ({@code StageState.Failed} or its subclass {@code BlockedForMI}). See * {@link #stagesEligibleForPlanning(PipelineRun)}. * @return ExecutionPlan containing either stages to execute (with lock held) or CONTINUE (no lock) * @throws LockException if unable to acquire the distributed lock within the configured timeout diff --git a/utils/test-util/src/main/java/io/flamingock/common/test/cloud/MockRunnerServer.java b/utils/test-util/src/main/java/io/flamingock/common/test/cloud/MockRunnerServer.java index 733333f14..098352cd1 100644 --- a/utils/test-util/src/main/java/io/flamingock/common/test/cloud/MockRunnerServer.java +++ b/utils/test-util/src/main/java/io/flamingock/common/test/cloud/MockRunnerServer.java @@ -236,7 +236,9 @@ private void mockExecutionEndpoint() { String requestJson = toJson(executionPlanBuilder.getRequest(requestResponse)); - wireMockServer.stubFor(post(urlPathEqualTo(executionUrl)).withRequestBody(equalToJson(requestJson)) + // ignoreArrayOrder=true, ignoreExtraElements=true — tolerate forward-compatible + // additions to the wire (e.g. StageRequest.status) without breaking existing stubs. + wireMockServer.stubFor(post(urlPathEqualTo(executionUrl)).withRequestBody(equalToJson(requestJson, true, true)) .willReturn(aResponse().withStatus(201).withHeader("Content-Type", "application/json").withBody(toJson(response)))); // verificableRequests.add(postRequestedFor(urlEqualTo(executionUrl)) @@ -262,7 +264,7 @@ private void mockExecutionEndpoint() { .withHeader("Content-Type", "application/json") .withBody(toJson(executionPlanBuilder.getResponse(requestResponse))); - wireMockServer.stubFor(scenarioMappingBuilder.withRequestBody(equalToJson(requestJson)).willReturn(responseDefBuilder)); + wireMockServer.stubFor(scenarioMappingBuilder.withRequestBody(equalToJson(requestJson, true, true)).willReturn(responseDefBuilder)); // verificableRequests.add(postRequestedFor(urlEqualTo(executionUrl)) // .withHeader("Content-Type", equalTo("application/json")) diff --git a/utils/test-util/src/main/java/io/flamingock/common/test/cloud/deprecated/MockRunnerServerOld.java b/utils/test-util/src/main/java/io/flamingock/common/test/cloud/deprecated/MockRunnerServerOld.java index 8bfb17e24..8afaba6b1 100644 --- a/utils/test-util/src/main/java/io/flamingock/common/test/cloud/deprecated/MockRunnerServerOld.java +++ b/utils/test-util/src/main/java/io/flamingock/common/test/cloud/deprecated/MockRunnerServerOld.java @@ -315,7 +315,9 @@ private void mockExecutionEndpoint() { ExecutionPlanResponse response = getExecutionPlanResponse(0); ExecutionPlanRequest request = getExecutionPlanRequest(0); - wireMockServer.stubFor(post(urlPathEqualTo(executionUrl)).withRequestBody(equalToJson(toJson(request))).willReturn(aResponse().withStatus(201).withHeader("Content-Type", "application/json").withBody(toJson(response)))); + // ignoreArrayOrder=true, ignoreExtraElements=true — tolerate forward-compatible additions + // to the wire (e.g. StageRequest.status) without breaking pre-existing stubs. + wireMockServer.stubFor(post(urlPathEqualTo(executionUrl)).withRequestBody(equalToJson(toJson(request), true, true)).willReturn(aResponse().withStatus(201).withHeader("Content-Type", "application/json").withBody(toJson(response)))); } else { String scenarioName = "Execution-plan-request"; @@ -331,7 +333,7 @@ private void mockExecutionEndpoint() { scenarioMappingBuilder.willSetStateTo("execution-state-" + (i + 1)); } String json = toJson(request); - wireMockServer.stubFor(scenarioMappingBuilder.withRequestBody(equalToJson(json)).willReturn(aResponse().withStatus(201).withHeader("Content-Type", "application/json").withBody(toJson(response)))); + wireMockServer.stubFor(scenarioMappingBuilder.withRequestBody(equalToJson(json, true, true)).willReturn(aResponse().withStatus(201).withHeader("Content-Type", "application/json").withBody(toJson(response)))); } } From b7f56118b58f8733e24340b0c02456b09caafa68 Mon Sep 17 00:00:00 2001 From: Antonio Perez Dieppa Date: Thu, 14 May 2026 13:01:47 +0100 Subject: [PATCH 3/6] refactor: plugin COmpilerArgsCOnfigurator --- .../internal/CompilerArgsConfigurator.kt | 24 +++++++++---------- .../internal/CompilerArgsConfiguratorTest.kt | 9 ++++--- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/internal/CompilerArgsConfigurator.kt b/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/internal/CompilerArgsConfigurator.kt index 3385bd83b..e944b9150 100644 --- a/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/internal/CompilerArgsConfigurator.kt +++ b/flamingock-gradle-plugin/src/main/kotlin/io/flamingock/gradle/internal/CompilerArgsConfigurator.kt @@ -106,9 +106,12 @@ internal object CompilerArgsConfigurator { } /** - * Reflectively call `KaptExtension.arguments(Action)` - * to add `flamingock.sources` / `flamingock.resources`. KAPT's `arg(name: Any, vararg - * values: Any)` takes a vararg, so the reflected signature is `arg(Object, Object[])`. + * Reflectively call `KaptExtension.arguments(action: KaptArguments.() -> Unit)` to add + * `flamingock.sources` / `flamingock.resources`. The DSL is a Kotlin lambda receiver, + * which compiles to a `Function1` parameter — NOT a Gradle `Action`. + * KAPT also exposes a sibling `arguments(Closure)` overload for Groovy; we deliberately + * skip that one. KAPT's `arg(name: Any, vararg values: Any)` takes a vararg, so the + * reflected signature is `arg(Object, Object[])`. * * Visible to the same module so tests can invoke the dispatch path directly without * needing the real Kotlin/KAPT plugin classpath. @@ -119,17 +122,14 @@ internal object CompilerArgsConfigurator { val kaptExt = project.extensions.findByName("kapt") ?: return val argumentsMethod = kaptExt::class.java.methods.firstOrNull { - it.name == "arguments" && it.parameterCount == 1 + it.name == "arguments" + && it.parameterCount == 1 + && Function1::class.java.isAssignableFrom(it.parameterTypes[0]) } ?: return - // Plain SAM impl rather than the kotlin-dsl `Action { ... }` form because the - // DSL form makes the parameter the implicit `this` and the receiver type Any tends - // to confuse Kotlin's SAM inference here. - val action = object : Action { - override fun execute(argsObj: Any) { - invokeArg(argsObj, SOURCES_OPTION, sourcesArg) - if (resourcesArg != null) invokeArg(argsObj, RESOURCES_OPTION, resourcesArg) - } + val action: (Any) -> Unit = { argsObj -> + invokeArg(argsObj, SOURCES_OPTION, sourcesArg) + if (resourcesArg != null) invokeArg(argsObj, RESOURCES_OPTION, resourcesArg) } argumentsMethod.invoke(kaptExt, action) } diff --git a/flamingock-gradle-plugin/src/test/kotlin/io/flamingock/gradle/internal/CompilerArgsConfiguratorTest.kt b/flamingock-gradle-plugin/src/test/kotlin/io/flamingock/gradle/internal/CompilerArgsConfiguratorTest.kt index 5c64decf8..7af07b3bc 100644 --- a/flamingock-gradle-plugin/src/test/kotlin/io/flamingock/gradle/internal/CompilerArgsConfiguratorTest.kt +++ b/flamingock-gradle-plugin/src/test/kotlin/io/flamingock/gradle/internal/CompilerArgsConfiguratorTest.kt @@ -221,12 +221,15 @@ class CompilerArgsConfiguratorTest { /** * Mirrors the shape of `KaptExtension` enough for our reflective lookup: an - * `arguments(Action)` method. + * `arguments(action: StubKaptArguments.() -> Unit)` method. The Kotlin lambda receiver + * compiles to a `Function1` parameter, mirroring the real + * `KaptExtension.arguments` JVM signature so the production reflective dispatch is + * exercised the same way as it would be against the real Kotlin plugin classpath. */ open class StubKaptExtension { val arguments = StubKaptArguments() - fun arguments(action: org.gradle.api.Action) { - action.execute(arguments) + fun arguments(action: StubKaptArguments.() -> Unit) { + arguments.action() } } From 2e786c5c821f8b8a73862f2f6672e708f3367592 Mon Sep 17 00:00:00 2001 From: Antonio Perez Dieppa Date: Fri, 15 May 2026 11:21:04 +0100 Subject: [PATCH 4/6] refactor: first phase execute error message --- CLAUDE.md | 22 +++ .../planner/CloudExecutionPlanMapperTest.java | 15 +- .../planner/CloudExecutionPlannerTest.java | 7 +- .../core/cloud/CloudAuditPersistenceTest.java | 11 +- .../response/data/ExecuteResponseData.java | 20 ++- .../AbstractPipelineTraverseOperation.java | 150 +++++++++--------- .../operation/ExecuteOperationException.java | 113 +++++++++++++ .../core/operation/OperationException.java | 75 ++------- .../PipelineExecuteOperationException.java | 34 ++++ .../StagedExecuteOperationException.java | 54 +++++++ .../pipeline/execution/ExecutableStage.java | 20 +++ .../core/pipeline/run/PipelineRun.java | 43 +---- .../internal/core/plan/ExecutionPlan.java | 41 +---- .../community/CommunityExecutionPlanner.java | 13 -- ...AbstractPipelineTraverseOperationTest.java | 43 +++-- .../operation/ExecuteApplyOperationTest.java | 25 ++- .../operation/ValidateApplyOperationTest.java | 10 +- .../core/pipeline/run/PipelineRunTest.java | 52 ++---- .../run/PipelineRunToResponseTest.java | 7 +- .../internal/core/plan/ExecutionPlanTest.java | 23 +-- .../CommunityExecutionPlannerTest.java | 18 ++- .../FlamingockTestSupportIntegrationTest.java | 14 +- docs/ERROR_REPORTING_PROPOSAL.md | 147 +++++++++++++++++ .../core/e2e/CoreStrategiesE2ETest.java | 15 +- .../flamingock/core/e2e/RecoveryE2ETest.java | 64 ++++---- .../couchbase/CouchbaseImporterTest.java | 26 ++- .../dynamodb/DynamoDBImporterTest.java | 26 ++- .../mongock/mongodb/MongoDBImporterTest.java | 26 ++- 28 files changed, 726 insertions(+), 388 deletions(-) create mode 100644 core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/ExecuteOperationException.java create mode 100644 core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/PipelineExecuteOperationException.java create mode 100644 core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/StagedExecuteOperationException.java create mode 100644 docs/ERROR_REPORTING_PROPOSAL.md diff --git a/CLAUDE.md b/CLAUDE.md index 68fecf4d2..8167b97ad 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -204,6 +204,28 @@ This is a multi-module Gradle project using Kotlin DSL. - Test resources in `src/test/resources/flamingock/pipeline.yaml` - Each module has isolated test suite +### Validation Policy (when changing code) + +Two tiers — use them in order: + +**Tier 1 — Incremental validation (during iteration):** +- Run targeted module tests for fast feedback: `./gradlew :core:flamingock-core:test`, `./gradlew :legacy:mongock-importer-mongodb:test`, etc. +- Use `--tests "fully.qualified.ClassName"` to narrow further when iterating on a specific test. +- Cheap, quick, good for confirming a focused change compiles and the obvious cases pass. + +**Tier 2 — Final validation (before declaring a task done):** +- Run `./gradlew clean build` from the repo root. This is the **authoritative, definitive check**. +- It surfaces failures that targeted/incremental runs miss: + - Cross-module integration regressions. + - Stale Gradle / annotation-processor caches masking compile errors. + - License-header drift (`spotlessCheck` runs as part of `build`). + - Test-resource generation issues (annotation processors regenerating pipeline metadata). + - Module-dependency ordering bugs. +- A passing per-module test run is **not sufficient** to claim a task complete. Per-module runs use cached artifacts and may not exercise the full graph. +- Do not skip this step on the grounds that "the affected module passed in isolation." If you reach the end of a task without a clean full build, say so explicitly rather than implying success. + +If `clean build` is too slow to run on every minor iteration, that's expected — Tier 1 is for iteration. But the **final hand-off** must include a clean build. + ### Java Version - Target Java 8 compatibility - Kotlin stdlib used in build scripts only diff --git a/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlanMapperTest.java b/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlanMapperTest.java index 68205cefe..2b7c58643 100644 --- a/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlanMapperTest.java +++ b/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlanMapperTest.java @@ -164,17 +164,18 @@ void shouldFilterStagesNotInResponse() { } @Test - @DisplayName("Should throw ManualInterventionRequiredException when validate() is called on plan with MANUAL_INTERVENTION") - void shouldThrowManualInterventionOnValidate() { + @DisplayName("Should throw ManualInterventionRequiredException when ExecutableStage.validate() is called with a MANUAL_INTERVENTION change") + void shouldThrowManualInterventionOnStageValidate() { List loadedStages = Arrays.asList(buildStage("stage-1", change1)); ExecutionPlanResponse response = buildResponse( buildStageResponse("stage-1", 0, changeResponse(change1.getId(), CloudChangeAction.MANUAL_INTERVENTION)) ); List stages = CloudExecutionPlanMapper.getExecutableStages(response, loadedStages); - ExecutionPlan plan = ExecutionPlan.newExecution("exec-1", null, stages); - assertThrows(ManualInterventionRequiredException.class, plan::validate); + // MI is now a per-stage concern: ExecutableStage.validate() throws so the operation lambda + // can demote the stage to BlockedForMI without affecting the rest of the pipeline. + assertThrows(ManualInterventionRequiredException.class, stages.get(0)::validate); } @Test @@ -236,9 +237,9 @@ void shouldMapPerStageStatusFromPipelineRun() { StageResult.builder().stageId("stage-completed").stageName("stage-completed") .state(StageState.COMPLETED).build()); pipelineRun.markStageFailed("stage-failed", new RuntimeException("boom")); - ManualInterventionRequiredException miEx = new ManualInterventionRequiredException( - Collections.singletonList(new RecoveryIssue(change2.getId())), "stage-blocked"); - pipelineRun.markStagesBlockedFromMI(miEx); + pipelineRun.markStageBlockedFromMI( + "stage-blocked", + Collections.singletonList(new RecoveryIssue(change2.getId()))); ExecutionPlanRequest request = CloudExecutionPlanMapper.toRequest( pipelineRun, 60000L, Collections.emptyMap()); diff --git a/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlannerTest.java b/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlannerTest.java index 4c03dfef6..617d0fc83 100644 --- a/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlannerTest.java +++ b/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlannerTest.java @@ -26,6 +26,7 @@ import io.flamingock.cloud.api.vo.CloudTargetSystemAuditMarkType; import io.flamingock.cloud.lock.CloudLockService; import io.flamingock.cloud.planner.client.ExecutionPlannerClient; +import io.flamingock.internal.common.core.error.FlamingockException; import io.flamingock.internal.common.core.recovery.ManualInterventionRequiredException; import io.flamingock.internal.common.core.targets.TargetSystemAuditMarkType; import io.flamingock.internal.core.change.loaded.AbstractLoadedChange; @@ -113,8 +114,12 @@ void shouldReturnAbortPlanWhenServerReturnsAbort() { ExecutionPlan plan = planner.getNextExecution(PipelineRun.of(stages)); + // ExecutionPlan.validate() now only signals the abort itself; MI is a per-stage concern + // enforced by ExecutableStage.validate() inside the operation lambda. assertTrue(plan.isAborted()); - assertThrows(ManualInterventionRequiredException.class, plan::validate); + assertThrows(FlamingockException.class, plan::validate); + assertThrows(ManualInterventionRequiredException.class, + () -> plan.getExecutableStages().get(0).validate()); } @Test diff --git a/cloud/flamingock-cloud/src/test/java/io/flamingock/core/cloud/CloudAuditPersistenceTest.java b/cloud/flamingock-cloud/src/test/java/io/flamingock/core/cloud/CloudAuditPersistenceTest.java index e5bc496e6..f7bbfd939 100644 --- a/cloud/flamingock-cloud/src/test/java/io/flamingock/core/cloud/CloudAuditPersistenceTest.java +++ b/cloud/flamingock-cloud/src/test/java/io/flamingock/core/cloud/CloudAuditPersistenceTest.java @@ -28,6 +28,7 @@ import io.flamingock.internal.core.builder.FlamingockFactory; import io.flamingock.cloud.api.vo.CloudAuditStatus; import io.flamingock.internal.core.external.store.lock.LockException; +import io.flamingock.internal.core.operation.PipelineExecuteOperationException; import io.flamingock.internal.core.builder.runner.Runner; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; @@ -202,9 +203,13 @@ void shouldWaitAndEventuallyAbort() { //THEN - Assertions.assertTrue(exception instanceof LockException); - Assertions.assertTrue(exception.getMessage().startsWith("Quit trying to acquire the lock after")); - Assertions.assertTrue(exception.getMessage().endsWith("[ Maximum waiting millis reached: 375 ]")); + // LockException is now wrapped in PipelineExecuteOperationException so the response data + // travels with the failure; the original LockException is preserved as cause. + Assertions.assertTrue(exception instanceof PipelineExecuteOperationException); + Throwable cause = exception.getCause(); + Assertions.assertTrue(cause instanceof LockException, "Expected LockException as cause, got: " + cause); + Assertions.assertTrue(cause.getMessage().startsWith("Quit trying to acquire the lock after")); + Assertions.assertTrue(cause.getMessage().endsWith("[ Maximum waiting millis reached: 375 ]")); } diff --git a/core/flamingock-core-commons/src/main/java/io/flamingock/internal/common/core/response/data/ExecuteResponseData.java b/core/flamingock-core-commons/src/main/java/io/flamingock/internal/common/core/response/data/ExecuteResponseData.java index 48335580b..2bf9273c5 100644 --- a/core/flamingock-core-commons/src/main/java/io/flamingock/internal/common/core/response/data/ExecuteResponseData.java +++ b/core/flamingock-core-commons/src/main/java/io/flamingock/internal/common/core/response/data/ExecuteResponseData.java @@ -44,7 +44,13 @@ public class ExecuteResponseData { // Per-stage breakdown private List stages; - // Error information (if failed) + /** + * @deprecated Per-stage error info is the authoritative carrier (see {@link StageResult} / + * {@code StageState.getErrorInfo()}). The top-level field is kept temporarily to preserve the + * JSON contract with the CLI; it is populated by {@code PipelineRun.toResponse()} from the + * first failed stage's {@link ErrorInfo} and will be removed in a follow-up. + */ + @Deprecated private ErrorInfo error; public ExecuteResponseData() { @@ -163,10 +169,18 @@ public void setStages(List stages) { this.stages = stages; } + /** + * @deprecated See {@link #error}. + */ + @Deprecated public ErrorInfo getError() { return error; } + /** + * @deprecated See {@link #error}. + */ + @Deprecated public void setError(ErrorInfo error) { this.error = error; } @@ -266,6 +280,10 @@ public Builder addStage(StageResult stage) { return this; } + /** + * @deprecated See {@link ExecuteResponseData#error}. + */ + @Deprecated public Builder error(ErrorInfo error) { this.error = error; return this; diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperation.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperation.java index c9a600f92..899ff4aa8 100644 --- a/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperation.java +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperation.java @@ -36,6 +36,7 @@ import io.flamingock.internal.core.pipeline.loaded.LoadedPipeline; import io.flamingock.internal.core.pipeline.loaded.stage.AbstractLoadedStage; import io.flamingock.internal.core.pipeline.run.PipelineRun; +import io.flamingock.internal.core.pipeline.run.StageRun; import io.flamingock.internal.core.plan.ExecutionPlan; import io.flamingock.internal.core.plan.ExecutionPlanner; import io.flamingock.internal.core.external.store.lock.Lock; @@ -48,7 +49,15 @@ import java.util.List; /** - * Common execution flow for Apply and Validate operations + * Common execution flow for Apply and Validate operations. + * + *

Stages are independent: a stage failure (Failed or BlockedForMI) does NOT abort the + * pipeline. The do-while keeps asking the planner for the next stage; failed stages are + * excluded from subsequent iterations by the planner. Only pipeline-wide errors + * ({@link LockException}, {@link PendingChangesException}, or unexpected throwables from + * outside a stage) break the loop and produce a {@link PipelineExecuteOperationException}. + * When all stages have been visited and at least one failed, the operation throws + * {@link StagedExecuteOperationException}. Otherwise it returns the response data. */ public abstract class AbstractPipelineTraverseOperation implements Operation { @@ -88,17 +97,11 @@ public AbstractPipelineTraverseOperation(RunnerId runnerId, @Override public ExecuteResult execute(ExecuteArgs args) { - ExecuteResponseData result; try { - result = this.execute(args.getPipeline()); - } catch (OperationException operationException) { - throw operationException; - } catch (Throwable throwable) { - throw processAndGetFlamingockException(throwable); + return this.execute(args.getPipeline()); } finally { this.finalizer.run(); } - return new ExecuteResult(result); } private static List validateAndGetExecutableStages(LoadedPipeline pipeline) { @@ -111,7 +114,7 @@ private static List validateAndGetExecutableStages(LoadedPi return stages; } - private ExecuteResponseData execute(LoadedPipeline pipeline) throws FlamingockException { + private ExecuteResult execute(LoadedPipeline pipeline) { List allStages = validateAndGetExecutableStages(pipeline); int stageCount = allStages.size(); long changeCount = allStages.stream() @@ -124,86 +127,91 @@ private ExecuteResponseData execute(LoadedPipeline pipeline) throws FlamingockEx PipelineRun pipelineRun = PipelineRun.of(pipeline); pipelineRun.start(); + Throwable pipelineLevelError = null; + boolean throwPipelineLevelError = true; + do { validateAndGetExecutableStages(pipeline); try (ExecutionPlan execution = executionPlanner.getNextExecution(pipelineRun)) { - // Validate execution plan for manual intervention requirements - // This centralized validation ensures both community and cloud paths are validated execution.validate(); - if (execution.isExecutionRequired()) { - if (validateOnlyMode()) { - throw new PendingChangesException(); - } - execution.applyOnEach((executionId, lock, executableStage) -> - runStage(executionId, lock, executableStage, pipelineRun)); - } else { + if (!execution.isExecutionRequired()) { break; } + + if (validateOnlyMode()) { + throw new PendingChangesException(); + } + + execution.applyOnEach((executionId, lock, executableStage) -> + runStage(executionId, lock, executableStage, pipelineRun)); } catch (LockException exception) { pipelineRun.markPipelineFailed(exception); - eventPublisher.publish(new StageFailedEvent(exception)); - eventPublisher.publish(new PipelineFailedEvent(exception)); + pipelineLevelError = exception; if (throwExceptionIfCannotObtainLock) { logger.debug("Required process lock not acquired - ABORTING OPERATION", exception); - throw exception; } else { logger.warn("Process lock not acquired but throwExceptionIfCannotObtainLock=false - CONTINUING WITHOUT LOCK", exception); + throwPipelineLevelError = false; } break; - } catch (StageExecutionException e) { - // Defensive: runStage normally records the failure into pipelineRun before - // rethrowing. If for some reason this exception arrives here without that - // having happened (e.g. thrown by something other than runStage), make sure - // the failure is reflected in the response. - String stageName = e.getResult() != null ? e.getResult().getStageName() : null; - if (stageName != null) { - io.flamingock.internal.core.pipeline.run.StageRun stageRun = pipelineRun.getStageRun(stageName); - if (stageRun != null && !stageRun.getState().isFailed()) { - pipelineRun.markStageFailed(stageName, e); - } - } - pipelineRun.stop(); - throw OperationException.fromExisting(e.getCause(), pipelineRun.toResponse()); - } catch (ManualInterventionRequiredException miEx) { - // Per-stage attribution: each RecoveryIssue is mapped to its owning stage and that - // stage's state becomes BLOCKED_MANUAL_INTERVENTION with the subset of issues - // affecting it. The blocked stages carry their own ErrorInfo, so no pipeline-level - // mark is needed. - pipelineRun.markStagesBlockedFromMI(miEx); - throw miEx; - } catch (FlamingockException e) { - // Validate-time exceptions (aborted FlamingockException, PendingChangesException) - // and any FlamingockException that bubbles up from runStage's generic-throwable - // handler. Stage-level failures (if any) have already been recorded; pipeline-level - // marking is idempotent (first-wins) and toResponse() keeps the stage error as primary. - pipelineRun.markPipelineFailed(e); - throw e; - } + } catch (Throwable exception) { + pipelineRun.markPipelineFailed(exception); + pipelineLevelError = exception; + break; + }//FlamingockException } while (true); pipelineRun.stop(); ExecuteResponseData result = pipelineRun.toResponse(); + if (pipelineLevelError != null) { + logger.debug("Error executing the process. ABORTED OPERATION", pipelineLevelError); + eventPublisher.publish(new PipelineFailedEvent(toException(pipelineLevelError))); + if (throwPipelineLevelError) { + throw ExecuteOperationException.fromExisting(pipelineLevelError, result); + } + return new ExecuteResult(result); + } + + if (hasAnyFailedStage(pipelineRun)) { + logger.info("Flamingock execution finished with stage failures [duration={}ms applied={} skipped={} failed={}]", + result.getTotalDurationMs(), result.getAppliedChanges(), result.getSkippedChanges(), result.getFailedChanges()); + StagedExecuteOperationException stagedException = new StagedExecuteOperationException(result); + eventPublisher.publish(new PipelineFailedEvent(stagedException)); + throw stagedException; + } + logger.info("Flamingock execution completed [duration={}ms applied={} skipped={}]", result.getTotalDurationMs(), result.getAppliedChanges(), result.getSkippedChanges()); eventPublisher.publish(new PipelineCompletedEvent()); - return result; + return new ExecuteResult(result); } private void runStage(String executionId, Lock lock, ExecutableStage executableStage, PipelineRun pipelineRun) { + String stageName = executableStage.getName(); + try { + executableStage.validate(); + } catch (ManualInterventionRequiredException miException) { + logger.warn("ABORTED STAGE '{}' - Manual intervention required for changes: [{}]", + stageName, miException.getConflictingSummary()); + pipelineRun.markStageStarted(stageName); + eventPublisher.publish(new StageStartedEvent()); + pipelineRun.markStageBlockedFromMI(stageName, miException.getConflictingChanges()); + eventPublisher.publish(new StageFailedEvent(miException)); + return; + } + try { startStage(executionId, lock, executableStage, pipelineRun); } catch (StageExecutionException exception) { - pipelineRun.markStageFailed(executableStage.getName(), exception); + pipelineRun.markStageFailed(stageName, exception); eventPublisher.publish(new StageFailedEvent(exception)); - eventPublisher.publish(new PipelineFailedEvent(exception)); - throw exception; } catch (Throwable generalException) { - pipelineRun.markStageFailed(executableStage.getName(), generalException); - throw processAndGetFlamingockException(generalException); + pipelineRun.markStageFailed(stageName, generalException); + eventPublisher.publish(new StageFailedEvent(toException(generalException))); } } @@ -218,28 +226,16 @@ private void startStage(String executionId, Lock lock, ExecutableStage executabl eventPublisher.publish(new StageCompletedEvent(executionOutput)); } -private FlamingockException processAndGetFlamingockException(Throwable exception) throws FlamingockException { - FlamingockException flamingockException; - if (exception instanceof OperationException) { - OperationException pipelineException = (OperationException) exception; - if (pipelineException.getCause() instanceof FlamingockException) { - flamingockException = (FlamingockException) pipelineException.getCause(); - } else { - flamingockException = (OperationException) exception; + private static boolean hasAnyFailedStage(PipelineRun pipelineRun) { + for (StageRun stageRun : pipelineRun.getStageRuns()) { + if (stageRun.getState().isFailed()) { + return true; } - } else if (exception instanceof FlamingockException) { - flamingockException = (FlamingockException) exception; - } else { - flamingockException = new FlamingockException(exception); - } - if (flamingockException instanceof ManualInterventionRequiredException) { - ManualInterventionRequiredException miException = (ManualInterventionRequiredException) flamingockException; - logger.error("ABORTED OPERATION - Manual intervention required for changes: [{}]", miException.getConflictingSummary()); - } else { - logger.debug("Error executing the process. ABORTED OPERATION", exception); } - eventPublisher.publish(new StageFailedEvent(flamingockException)); - eventPublisher.publish(new PipelineFailedEvent(flamingockException)); - return flamingockException; + return false; + } + + private static Exception toException(Throwable throwable) { + return throwable instanceof Exception ? (Exception) throwable : new FlamingockException(throwable); } } diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/ExecuteOperationException.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/ExecuteOperationException.java new file mode 100644 index 000000000..22ae86e20 --- /dev/null +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/ExecuteOperationException.java @@ -0,0 +1,113 @@ +/* + * Copyright 2023 Flamingock (https://www.flamingock.io) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.flamingock.internal.core.operation; + +import io.flamingock.internal.common.core.error.FlamingockException; +import io.flamingock.internal.common.core.response.data.ExecuteResponseData; +import io.flamingock.internal.util.ThrowableUtil; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.UndeclaredThrowableException; +import java.util.concurrent.CompletionException; +import java.util.concurrent.ExecutionException; + +/** + * Parent of execute-operation failures. Always carries the {@link ExecuteResponseData} produced + * during the run so callers (the runner, the CLI) can render per-stage results even on failure. + * + *

Two concrete subclasses: + *

    + *
  • {@link StagedExecuteOperationException} — one or more stages ended in a failed state but + * the iteration completed (stage-scoped failure).
    • + *
  • {@link PipelineExecuteOperationException} — a pipeline-wide error broke the iteration + * (e.g. {@code LockException}); carries the originating cause.
    • + *
+ */ +public abstract class ExecuteOperationException extends OperationException { + + private static final int MAX_UNWRAP_DEPTH = 32; + + /** + * Builds a {@link PipelineExecuteOperationException} from a raw throwable, unwrapping common + * reflective/async wrappers ({@link InvocationTargetException}, {@link UndeclaredThrowableException}, + * {@link ExecutionException}, {@link CompletionException}) and the redundant outermost + * {@link FlamingockException} layer if it only carries a cause. + */ + public static PipelineExecuteOperationException fromExisting(Throwable exception, ExecuteResponseData result) { + Throwable root = unwrapKnownWrappers(exception); + + if (root instanceof FlamingockException) { + Throwable cause = root.getCause(); + if (cause != null) { + return new PipelineExecuteOperationException(cause, result); + } + // Leaf FlamingockException (no further cause): preserve the throwable itself as cause + // so callers can read the original type/message. + return new PipelineExecuteOperationException(ThrowableUtil.messageOf(root), root, result); + } + + return new PipelineExecuteOperationException(root, result); + } + + private static Throwable unwrapKnownWrappers(Throwable t) { + if (t == null) return new NullPointerException("Throwable is null"); + + Throwable cur = t; + int guard = 0; + + while (guard++ < MAX_UNWRAP_DEPTH) { + if (cur instanceof InvocationTargetException) { + Throwable next = ((InvocationTargetException) cur).getTargetException(); + if (next != null && next != cur) { cur = next; continue; } + break; + } + if (cur instanceof UndeclaredThrowableException) { + Throwable next = ((UndeclaredThrowableException) cur).getUndeclaredThrowable(); + if (next != null && next != cur) { cur = next; continue; } + break; + } + if (cur instanceof ExecutionException + || cur instanceof CompletionException) { + Throwable next = cur.getCause(); + if (next != null && next != cur) { cur = next; continue; } + break; + } + break; + } + return cur; + } + + private final ExecuteResponseData result; + + protected ExecuteOperationException(String message, ExecuteResponseData result) { + super(message); + this.result = result; + } + + protected ExecuteOperationException(Throwable cause, ExecuteResponseData result) { + super(cause); + this.result = result; + } + + protected ExecuteOperationException(String message, Throwable cause, ExecuteResponseData result) { + super(message, cause); + this.result = result; + } + + public ExecuteResponseData getResult() { + return result; + } +} diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/OperationException.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/OperationException.java index ca0c45c9f..32104e0ec 100644 --- a/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/OperationException.java +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/OperationException.java @@ -16,76 +16,23 @@ package io.flamingock.internal.core.operation; import io.flamingock.internal.common.core.error.FlamingockException; -import io.flamingock.internal.common.core.response.data.ExecuteResponseData; -import io.flamingock.internal.util.ThrowableUtil; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.UndeclaredThrowableException; -import java.util.concurrent.CompletionException; -import java.util.concurrent.ExecutionException; - -public class OperationException extends FlamingockException { - - private static final int MAX_UNWRAP_DEPTH = 32; - - public static OperationException fromExisting(Throwable exception, ExecuteResponseData result) { - Throwable root = unwrapKnownWrappers(exception); - - if (root instanceof FlamingockException) { - Throwable cause = root.getCause(); - if (cause != null) { - return new OperationException(cause, result); - } - OperationException oe = new OperationException(ThrowableUtil.messageOf(root), result); - oe.setStackTrace(root.getStackTrace()); - return oe; - } - - return new OperationException(root, result); - } - - - private static Throwable unwrapKnownWrappers(Throwable t) { - if (t == null) return new NullPointerException("Throwable is null"); - - Throwable cur = t; - int guard = 0; - - while (guard++ < MAX_UNWRAP_DEPTH) { - if (cur instanceof InvocationTargetException) { - Throwable next = ((InvocationTargetException) cur).getTargetException(); - if (next != null && next != cur) { cur = next; continue; } - break; - } - if (cur instanceof UndeclaredThrowableException) { - Throwable next = ((UndeclaredThrowableException) cur).getUndeclaredThrowable(); - if (next != null && next != cur) { cur = next; continue; } - break; - } - if (cur instanceof ExecutionException - || cur instanceof CompletionException) { - Throwable next = cur.getCause(); - if (next != null && next != cur) { cur = next; continue; } - break; - } - break; - } - return cur; - } - - private final ExecuteResponseData result; +/** + * Parent of every operation-thrown exception. Concrete operations throw their own subclass + * (e.g. {@link ExecuteOperationException} for execute). Higher layers can catch + * {@code OperationException} generically when they don't need to differentiate. + */ +public abstract class OperationException extends FlamingockException { - private OperationException(String message, ExecuteResponseData result) { + protected OperationException(String message) { super(message); - this.result = result; } - private OperationException(Throwable throwable, ExecuteResponseData result) { - super(throwable); - this.result = result; + protected OperationException(Throwable cause) { + super(cause); } - public ExecuteResponseData getResult() { - return result; + protected OperationException(String message, Throwable cause) { + super(message, cause); } } diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/PipelineExecuteOperationException.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/PipelineExecuteOperationException.java new file mode 100644 index 000000000..f3fc8135d --- /dev/null +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/PipelineExecuteOperationException.java @@ -0,0 +1,34 @@ +/* + * Copyright 2023 Flamingock (https://www.flamingock.io) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.flamingock.internal.core.operation; + +import io.flamingock.internal.common.core.response.data.ExecuteResponseData; + +/** + * Thrown when a pipeline-wide error breaks iteration (e.g. {@code LockException}, planner abort, + * unexpected throwable). Always carries the response data assembled so far plus the originating + * cause. + */ +public class PipelineExecuteOperationException extends ExecuteOperationException { + + public PipelineExecuteOperationException(Throwable cause, ExecuteResponseData result) { + super(cause, result); + } + + public PipelineExecuteOperationException(String message, Throwable cause, ExecuteResponseData result) { + super(message, cause, result); + } +} diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/StagedExecuteOperationException.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/StagedExecuteOperationException.java new file mode 100644 index 000000000..2a49f0613 --- /dev/null +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/StagedExecuteOperationException.java @@ -0,0 +1,54 @@ +/* + * Copyright 2023 Flamingock (https://www.flamingock.io) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.flamingock.internal.core.operation; + +import io.flamingock.internal.common.core.response.data.ExecuteResponseData; +import io.flamingock.internal.common.core.response.data.StageResult; + +import java.util.stream.Collectors; + +/** + * Thrown when one or more stages ended in a failed state ({@code Failed} or + * {@code BlockedForMI}) but the pipeline iteration completed. The per-stage error details live + * in {@code getResult().getStages()}. + * + *

The exception message is a single-line, log-aggregator-friendly summary built from the + * carried {@link ExecuteResponseData}: failed stage count + names, change counts, and run + * duration. Multi-line / per-stage detail rendering is deferred to a future {@code toString()} + * override and a default event listener (see {@code docs/ERROR_REPORTING_PROPOSAL.md}). + */ +public class StagedExecuteOperationException extends ExecuteOperationException { + + public StagedExecuteOperationException(ExecuteResponseData result) { + super(buildMessage(result), result); + } + + private static String buildMessage(ExecuteResponseData result) { + String failedStageNames = result.getStages().stream() + .filter(s -> s.getState().isFailed()) + .map(StageResult::getStageName) + .collect(Collectors.joining(", ")); + return String.format( + "Flamingock execution failed: %d of %d stage(s) failed [%s]; changes applied=%d, failed=%d, skipped=%d; duration=%dms", + result.getFailedStages(), + result.getTotalStages(), + failedStageNames, + result.getAppliedChanges(), + result.getFailedChanges(), + result.getSkippedChanges(), + result.getTotalDurationMs()); + } +} diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/execution/ExecutableStage.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/execution/ExecutableStage.java index dd3060640..b44f510fc 100644 --- a/core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/execution/ExecutableStage.java +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/execution/ExecutableStage.java @@ -17,8 +17,12 @@ import io.flamingock.internal.common.core.pipeline.StageDescriptor; import io.flamingock.internal.common.core.change.ChangeDescriptor; +import io.flamingock.internal.common.core.recovery.ManualInterventionRequiredException; +import io.flamingock.internal.common.core.recovery.RecoveryIssue; +import io.flamingock.internal.common.core.recovery.action.ChangeAction; import io.flamingock.internal.core.change.executable.ExecutableChange; +import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Objects; @@ -54,5 +58,21 @@ public boolean isExecutionRequired() { .anyMatch(executableChange -> !executableChange.isAlreadyApplied()); } + /** + * Validates this stage before execution. Throws {@link ManualInterventionRequiredException} + * if any change has {@link ChangeAction#MANUAL_INTERVENTION} so the caller can demote the + * stage to a {@code BlockedForMI} state without affecting other stages. + */ + public void validate() { + List recoveryIssues = new ArrayList<>(); + for (ExecutableChange change : changes) { + if (change != null && change.getAction() == ChangeAction.MANUAL_INTERVENTION) { + recoveryIssues.add(new RecoveryIssue(change.getId())); + } + } + if (!recoveryIssues.isEmpty()) { + throw new ManualInterventionRequiredException(recoveryIssues, name); + } + } } diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/run/PipelineRun.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/run/PipelineRun.java index e161c5a9b..b275bf617 100644 --- a/core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/run/PipelineRun.java +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/run/PipelineRun.java @@ -15,7 +15,6 @@ */ package io.flamingock.internal.core.pipeline.run; -import io.flamingock.internal.common.core.recovery.ManualInterventionRequiredException; import io.flamingock.internal.common.core.recovery.RecoveryIssue; import io.flamingock.internal.common.core.response.data.ChangeResult; import io.flamingock.internal.common.core.response.data.ChangeStatus; @@ -24,7 +23,6 @@ import io.flamingock.internal.common.core.response.data.ExecutionStatus; import io.flamingock.internal.common.core.response.data.StageResult; import io.flamingock.internal.common.core.response.data.StageState; -import io.flamingock.internal.core.change.loaded.AbstractLoadedChange; import io.flamingock.internal.core.pipeline.execution.StageExecutionException; import io.flamingock.internal.core.pipeline.loaded.LoadedPipeline; import io.flamingock.internal.core.pipeline.loaded.stage.AbstractLoadedStage; @@ -131,40 +129,15 @@ public void markStageFailed(String stageName, Throwable cause) { } /** - * Attributes a {@link ManualInterventionRequiredException} to its owning stages. Each - * {@link RecoveryIssue} is resolved to the stage that owns the change, and that stage's - * state becomes {@link StageState#blockedManualIntervention(String, List)} with the subset - * of issues that affect it. - * - * @throws IllegalStateException if any issue's changeId can't be resolved to a loaded - * stage (signals a planner/loaded-stage inconsistency). + * Marks a single stage as blocked for manual intervention with its attributed recovery + * issues. Per-stage attribution: each stage owns its own MI state, so a single stage's + * BlockedForMI does not affect the rest of the pipeline. */ - public void markStagesBlockedFromMI(ManualInterventionRequiredException exception) { - Map> issuesByStage = new LinkedHashMap<>(); - for (RecoveryIssue issue : exception.getConflictingChanges()) { - String stageName = findStageForChange(issue.getChangeId()); - issuesByStage.computeIfAbsent(stageName, k -> new ArrayList<>()).add(issue); - } - for (Map.Entry> entry : issuesByStage.entrySet()) { - StageRun run = lookupOrThrow(entry.getKey()); - run.setResult(StageResult.builder(run.getResult()) - .state(StageState.blockedManualIntervention(entry.getKey(), entry.getValue())) - .build()); - } - } - - private String findStageForChange(String changeId) { - for (StageRun stageRun : stageRuns) { - for (AbstractLoadedChange change : stageRun.getLoadedStage().getChanges()) { - if (changeId.equals(change.getId())) { - return stageRun.getName(); - } - } - } - throw new IllegalStateException( - "Cannot attribute manual-intervention issue: changeId '" + changeId - + "' is not part of any loaded stage in this run. " - + "This indicates a planner/loaded-stage inconsistency."); + public void markStageBlockedFromMI(String stageName, List issues) { + StageRun run = lookupOrThrow(stageName); + run.setResult(StageResult.builder(run.getResult()) + .state(StageState.blockedManualIntervention(stageName, issues)) + .build()); } /** diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/ExecutionPlan.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/ExecutionPlan.java index f6b1ffcb8..ab6b5bd1a 100644 --- a/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/ExecutionPlan.java +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/ExecutionPlan.java @@ -18,14 +18,9 @@ import io.flamingock.internal.util.TriConsumer; import io.flamingock.internal.core.external.store.lock.Lock; import io.flamingock.internal.core.pipeline.execution.ExecutableStage; -import io.flamingock.internal.core.change.executable.ExecutableChange; import io.flamingock.internal.common.core.error.FlamingockException; -import io.flamingock.internal.common.core.recovery.action.ChangeAction; -import io.flamingock.internal.common.core.recovery.ManualInterventionRequiredException; -import io.flamingock.internal.common.core.recovery.RecoveryIssue; import java.util.List; -import java.util.ArrayList; public class ExecutionPlan implements AutoCloseable { @@ -86,40 +81,14 @@ public void applyOnEach(TriConsumer consumer) { } /** - * Validates the execution plan. - *

- * Checks two conditions: - *

    - *
  1. If any changes require manual intervention, throws {@link ManualInterventionRequiredException}
    2. - *
  2. If the plan is aborted (even without MI changes), throws {@link FlamingockException} - * — the execution planner decided to abort for reasons beyond individual change state
    3. - *
+ * Validates the execution plan. If the planner decided to abort the run for reasons beyond + * individual change state, throws {@link FlamingockException}. * - * @throws ManualInterventionRequiredException if any changes require manual intervention - * @throws FlamingockException if the plan is aborted without specific MI changes + *

Manual-intervention validation is no longer performed here: it is per-stage and lives in + * {@code ExecutableStage.validate()}, called inside the operation lambda so a single stage's + * MI state never aborts the rest of the pipeline. */ public void validate() { - List recoveryIssues = new ArrayList<>(); - String firstStageName = "unknown"; - boolean hasStages = false; - - for (ExecutableStage stage : executableStages) { - if (!hasStages) { - firstStageName = stage.getName(); - hasStages = true; - } - - for (ExecutableChange change : stage.getChanges()) { - if (change.getAction() == ChangeAction.MANUAL_INTERVENTION) { - recoveryIssues.add(new RecoveryIssue(change.getId())); - } - } - } - - if (!recoveryIssues.isEmpty()) { - throw new ManualInterventionRequiredException(recoveryIssues, firstStageName); - } - if (aborted) { throw new FlamingockException("Execution aborted by the execution planner"); } diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlanner.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlanner.java index 65a38f854..90dae0092 100644 --- a/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlanner.java +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlanner.java @@ -17,7 +17,6 @@ import io.flamingock.internal.common.core.audit.AuditEntry; -import io.flamingock.internal.common.core.recovery.action.ChangeAction; import io.flamingock.internal.common.core.recovery.action.ChangeActionMap; import io.flamingock.internal.core.plan.ExecutionId; import io.flamingock.internal.core.external.store.lock.community.CommunityLock; @@ -132,10 +131,6 @@ public ExecutionPlan getNextExecution(PipelineRun pipelineRun) throws LockExcept List initialStages = buildExecutableStages(loadedStages, initialSnapshot); - if (hasManualInterventionChanges(initialStages)) { - return ExecutionPlan.ABORT(initialStages); - } - if (!hasExecutableStages(initialStages)) { return ExecutionPlan.CONTINUE(initialStages); } @@ -147,8 +142,6 @@ public ExecutionPlan getNextExecution(PipelineRun pipelineRun) throws LockExcept List validatedStages = buildExecutableStages(loadedStages, validatedSnapshot); - //TODO add hasManualInterventionChanges here too, after lock acquisition - Optional nextStageOpt = getFirstExecutableStage(validatedStages); if (!nextStageOpt.isPresent()) { @@ -227,12 +220,6 @@ private List buildExecutableStages( .collect(Collectors.toList()); } - private boolean hasManualInterventionChanges(List stages) { - return stages.stream() - .flatMap(stage -> stage.getChanges().stream()) - .anyMatch(change -> change.getAction() == ChangeAction.MANUAL_INTERVENTION); - } - /** * Checks if any stage requires execution. * diff --git a/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperationTest.java b/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperationTest.java index 464a39934..e40b5a903 100644 --- a/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperationTest.java +++ b/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperationTest.java @@ -15,8 +15,9 @@ */ package io.flamingock.internal.core.operation; -import io.flamingock.internal.common.core.recovery.ManualInterventionRequiredException; +import io.flamingock.internal.common.core.error.FlamingockException; import io.flamingock.internal.common.core.recovery.action.ChangeAction; +import io.flamingock.internal.common.core.response.data.StageResult; import io.flamingock.internal.core.change.executable.ExecutableChange; import io.flamingock.internal.core.change.loaded.AbstractLoadedChange; import io.flamingock.internal.core.event.EventPublisher; @@ -43,27 +44,33 @@ class AbstractPipelineTraverseOperationTest { @Test - @DisplayName("Should throw ManualInterventionRequiredException when planner returns ABORT with MI changes") - void shouldThrowManualInterventionWhenPlannerReturnsAbort() { + @DisplayName("MI changes per stage should mark the stage BLOCKED_FOR_MI and throw StagedExecuteOperationException") + void miChangesPerStageShouldBlockStageAndThrowStagedException() { ExecutableChange miChange = mockChange("change-1", ChangeAction.MANUAL_INTERVENTION); ExecutableStage stage = new ExecutableStage("stage-1", Collections.singletonList(miChange)); - ExecutionPlan abortPlan = ExecutionPlan.ABORT(Collections.singletonList(stage)); + ExecutionPlan plan = ExecutionPlan.newExecution("exec-1", null, Collections.singletonList(stage)); + ExecutionPlan continuePlan = ExecutionPlan.CONTINUE(Collections.emptyList()); ExecutionPlanner planner = mock(ExecutionPlanner.class); - when(planner.getNextExecution(any(PipelineRun.class))).thenReturn(abortPlan); + when(planner.getNextExecution(any(PipelineRun.class))).thenReturn(plan, continuePlan); LoadedPipeline pipeline = mockPipeline("change-1"); - ExecuteApplyOperation operation = buildOperation(planner); + StageExecutor stageExecutor = mock(StageExecutor.class); + ExecuteApplyOperation operation = buildOperation(planner, stageExecutor); - ManualInterventionRequiredException ex = assertThrows( - ManualInterventionRequiredException.class, + StagedExecuteOperationException ex = assertThrows( + StagedExecuteOperationException.class, () -> operation.execute(new ExecuteArgs(pipeline))); - assertTrue(ex.getConflictingSummary().contains("change-1")); + + StageResult stageResult = ex.getResult().getStages().get(0); + assertEquals("stage-1", stageResult.getStageName()); + assertTrue(stageResult.getState().isBlockedForManualIntervention()); + verify(stageExecutor, never()).executeStage(any(), any(), any()); } @Test - @DisplayName("Should throw FlamingockException and not execute changes when plan is ABORT without MI changes") - void shouldNotExecuteChangesWhenPlanIsAbort() { + @DisplayName("ABORT plan without MI should throw PipelineExecuteOperationException with abort cause") + void abortPlanShouldThrowPipelineExecuteOperationException() { ExecutableChange change = mockChange("change-1", ChangeAction.APPLY); ExecutableStage stage = new ExecutableStage("stage-1", Collections.singletonList(change)); ExecutionPlan abortPlan = ExecutionPlan.ABORT(Collections.singletonList(stage)); @@ -75,15 +82,19 @@ void shouldNotExecuteChangesWhenPlanIsAbort() { LoadedPipeline pipeline = mockPipeline(); ExecuteApplyOperation operation = buildOperation(planner, stageExecutor); - assertThrows(io.flamingock.internal.common.core.error.FlamingockException.class, + ExecuteOperationException ex = assertThrows( + PipelineExecuteOperationException.class, () -> operation.execute(new ExecuteArgs(pipeline))); + assertNotNull(ex.getResult()); + // The abort is signalled as a generic FlamingockException carrying the planner message. + assertTrue(ex.getMessage() != null && ex.getMessage().contains("Execution aborted") + || (ex.getCause() instanceof FlamingockException + && ex.getCause().getMessage().contains("Execution aborted")), + "Expected the planner abort message to be propagated, got: " + ex.getMessage() + + " / cause=" + ex.getCause()); verify(stageExecutor, never()).executeStage(any(), any(), any()); } - private static ExecuteApplyOperation buildOperation(ExecutionPlanner planner) { - return buildOperation(planner, mock(StageExecutor.class)); - } - private static ExecuteApplyOperation buildOperation(ExecutionPlanner planner, StageExecutor stageExecutor) { return new ExecuteApplyOperation( RunnerId.fromString("test"), diff --git a/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/ExecuteApplyOperationTest.java b/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/ExecuteApplyOperationTest.java index aea58b6c8..566dba663 100644 --- a/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/ExecuteApplyOperationTest.java +++ b/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/ExecuteApplyOperationTest.java @@ -24,6 +24,7 @@ import io.flamingock.internal.core.operation.execute.ExecuteApplyOperation; import io.flamingock.internal.core.operation.execute.ExecuteArgs; import io.flamingock.internal.core.operation.execute.ExecuteResult; +import io.flamingock.internal.core.pipeline.execution.ExecutableStage; import io.flamingock.internal.core.pipeline.execution.OrphanExecutionContext; import io.flamingock.internal.core.pipeline.execution.StageExecutionException; import io.flamingock.internal.core.pipeline.execution.StageExecutor; @@ -31,6 +32,7 @@ import io.flamingock.internal.core.pipeline.loaded.stage.AbstractLoadedStage; import io.flamingock.internal.core.plan.ExecutionPlan; import io.flamingock.internal.core.plan.ExecutionPlanner; +import io.flamingock.internal.util.TriConsumer; import io.flamingock.internal.core.change.loaded.AbstractLoadedChange; import io.flamingock.internal.util.id.RunnerId; import org.junit.jupiter.api.BeforeEach; @@ -141,7 +143,7 @@ void shouldReturnResultWithCorrectCounts() throws Exception { } @Test - @DisplayName("Should throw OperationException on stage failure") + @DisplayName("Should throw StagedExecuteOperationException on stage failure and continue past it") void shouldThrowOperationExceptionOnStageFailure() throws Exception { // Given RuntimeException originalException = new RuntimeException("Change failed"); @@ -152,18 +154,25 @@ void shouldThrowOperationExceptionOnStageFailure() throws Exception { "change-001" ); - ExecutionPlan executionPlan = mockExecutionRequiredPlanWithException(stageException); + ExecutableStage executableStage = mock(ExecutableStage.class); + when(executableStage.getName()).thenReturn("stage-1"); + doNothing().when(executableStage).validate(); + + when(stageExecutor.executeStage(any(), any(), any())).thenThrow(stageException); + + ExecutionPlan failingPlan = mockPlanInvokingConsumerWith(executableStage); + ExecutionPlan continuePlan = mockNoExecutionRequiredPlan(); when(pipeline.getSystemStage()).thenReturn(java.util.Optional.empty()); when(pipeline.getStages()).thenReturn(Collections.singletonList(loadedStage)); when(loadedStage.getName()).thenReturn("stage-1"); when(loadedStage.getChanges()).thenReturn(Collections.singletonList(loadedChange)); - when(executionPlanner.getNextExecution(any())).thenReturn(executionPlan); + when(executionPlanner.getNextExecution(any())).thenReturn(failingPlan, continuePlan); ExecuteArgs args = new ExecuteArgs(pipeline); // When / Then - OperationException thrown = assertThrows(OperationException.class, () -> operation.execute(args)); + StagedExecuteOperationException thrown = assertThrows(StagedExecuteOperationException.class, () -> operation.execute(args)); assertNotNull(thrown.getResult()); assertEquals(ExecutionStatus.FAILED, thrown.getResult().getStatus()); assertEquals(1, thrown.getResult().getFailedStages()); @@ -218,13 +227,17 @@ private ExecutionPlan mockNoExecutionRequiredPlan() { return plan; } - private ExecutionPlan mockExecutionRequiredPlanWithException(StageExecutionException exception) { + private ExecutionPlan mockPlanInvokingConsumerWith(ExecutableStage executableStage) { ExecutionPlan plan = mock(ExecutionPlan.class); when(plan.isExecutionRequired()).thenReturn(true); doNothing().when(plan).validate(); doNothing().when(plan).close(); doAnswer(invocation -> { - throw exception; + @SuppressWarnings("unchecked") + TriConsumer consumer = + invocation.getArgument(0); + consumer.accept("exec-1", null, executableStage); + return null; }).when(plan).applyOnEach(any()); return plan; } diff --git a/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/ValidateApplyOperationTest.java b/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/ValidateApplyOperationTest.java index b64b78cdc..6574471f7 100644 --- a/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/ValidateApplyOperationTest.java +++ b/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/ValidateApplyOperationTest.java @@ -46,6 +46,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; @@ -146,10 +147,15 @@ void shouldThrowPendingChangesExceptionWhenPendingChangesExist() throws Exceptio ExecuteArgs args = new ExecuteArgs(pipeline); // When / Then - PendingChangesException thrown = assertThrows( - PendingChangesException.class, + // PendingChangesException is now wrapped as a pipeline-wide failure carrying response data, + // so callers see PipelineExecuteOperationException with the PendingChangesException as cause. + PipelineExecuteOperationException thrown = assertThrows( + PipelineExecuteOperationException.class, () -> operation.execute(args) ); + assertTrue(thrown.getCause() instanceof PendingChangesException, + "Expected PendingChangesException as cause, got: " + thrown.getCause()); + assertNotNull(thrown.getResult()); } // ─────────────────────────── Helpers ─────────────────────────── diff --git a/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunTest.java b/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunTest.java index 6c0fb3c81..6b473075d 100644 --- a/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunTest.java +++ b/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunTest.java @@ -15,7 +15,6 @@ */ package io.flamingock.internal.core.pipeline.run; -import io.flamingock.internal.common.core.recovery.ManualInterventionRequiredException; import io.flamingock.internal.common.core.recovery.RecoveryIssue; import io.flamingock.internal.common.core.response.data.ErrorInfo; import io.flamingock.internal.common.core.response.data.StageResult; @@ -143,53 +142,34 @@ void getPipelineErrorIsEmptyUntilMarked() { } @Test - void markStagesBlockedFromMIGroupsIssuesByOwningStage() { - AbstractLoadedChange alphaChange1 = mockChange("alpha-c1"); - AbstractLoadedChange alphaChange2 = mockChange("alpha-c2"); - AbstractLoadedChange betaChange = mockChange("beta-c1"); + void markStageBlockedFromMISetsBlockedForMIState() { + AbstractLoadedChange alphaChange = mockChange("alpha-c1"); + AbstractLoadedStage alpha = mockStageWithChanges("alpha", alphaChange); + AbstractLoadedStage beta = mockStageWithChanges("beta"); - AbstractLoadedStage alpha = mockStageWithChanges("alpha", alphaChange1, alphaChange2); - AbstractLoadedStage beta = mockStageWithChanges("beta", betaChange); - AbstractLoadedStage gamma = mockStageWithChanges("gamma"); // no MI'd changes + PipelineRun pipelineRun = PipelineRun.of(Arrays.asList(alpha, beta)); - PipelineRun pipelineRun = PipelineRun.of(Arrays.asList(alpha, beta, gamma)); - - List issues = Arrays.asList( - new RecoveryIssue("alpha-c1"), - new RecoveryIssue("alpha-c2"), - new RecoveryIssue("beta-c1")); - ManualInterventionRequiredException miEx = new ManualInterventionRequiredException(issues, "alpha"); - - pipelineRun.markStagesBlockedFromMI(miEx); + pipelineRun.markStageBlockedFromMI( + "alpha", + Arrays.asList(new RecoveryIssue("alpha-c1"))); StageState alphaState = pipelineRun.getStageRun("alpha").getState(); assertTrue(alphaState.isBlockedForManualIntervention()); - assertEquals(2, alphaState.getRecoveryIssues().size()); - - StageState betaState = pipelineRun.getStageRun("beta").getState(); - assertTrue(betaState.isBlockedForManualIntervention()); - assertEquals(1, betaState.getRecoveryIssues().size()); - assertEquals("beta-c1", betaState.getRecoveryIssues().get(0).getChangeId()); + assertEquals(1, alphaState.getRecoveryIssues().size()); - // No MI'd changes for gamma → stays NOT_STARTED. - assertSame(StageState.NOT_STARTED, pipelineRun.getStageRun("gamma").getState()); + // Other stages are unaffected by a single stage's MI block — independent stage state. + assertSame(StageState.NOT_STARTED, pipelineRun.getStageRun("beta").getState()); } @Test - void markStagesBlockedFromMIThrowsForUnknownChange() { + void markStageBlockedFromMIThrowsForUnknownStage() { AbstractLoadedStage alpha = mockStageWithChanges("alpha", mockChange("alpha-c1")); PipelineRun pipelineRun = PipelineRun.of(java.util.Collections.singletonList(alpha)); - ManualInterventionRequiredException miEx = new ManualInterventionRequiredException( - Arrays.asList(new RecoveryIssue("ghost-change")), "alpha"); - - IllegalStateException ex = assertThrows( - IllegalStateException.class, - () -> pipelineRun.markStagesBlockedFromMI(miEx)); - assertTrue(ex.getMessage().contains("ghost-change")); - - // No partial state — the alpha stage stays NOT_STARTED. - assertSame(StageState.NOT_STARTED, pipelineRun.getStageRun("alpha").getState()); + assertThrows(IllegalArgumentException.class, + () -> pipelineRun.markStageBlockedFromMI( + "ghost-stage", + Arrays.asList(new RecoveryIssue("alpha-c1")))); } @Test diff --git a/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunToResponseTest.java b/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunToResponseTest.java index 26d7e2f71..ce1eceeee 100644 --- a/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunToResponseTest.java +++ b/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunToResponseTest.java @@ -15,7 +15,6 @@ */ package io.flamingock.internal.core.pipeline.run; -import io.flamingock.internal.common.core.recovery.ManualInterventionRequiredException; import io.flamingock.internal.common.core.recovery.RecoveryIssue; import io.flamingock.internal.common.core.response.data.ChangeResult; import io.flamingock.internal.common.core.response.data.ChangeStatus; @@ -167,9 +166,9 @@ void blockedForMIStageIsTrackedInResponseWithBlockedState() { PipelineRun pipelineRun = PipelineRun.of(java.util.Collections.singletonList(alpha)); pipelineRun.start(); - ManualInterventionRequiredException miEx = new ManualInterventionRequiredException( - java.util.Arrays.asList(new RecoveryIssue("c1")), "alpha"); - pipelineRun.markStagesBlockedFromMI(miEx); + pipelineRun.markStageBlockedFromMI( + "alpha", + java.util.Arrays.asList(new RecoveryIssue("c1"))); pipelineRun.stop(); ExecuteResponseData response = pipelineRun.toResponse(); diff --git a/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/ExecutionPlanTest.java b/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/ExecutionPlanTest.java index 05941d3f6..3109b5993 100644 --- a/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/ExecutionPlanTest.java +++ b/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/ExecutionPlanTest.java @@ -16,7 +16,6 @@ package io.flamingock.internal.core.plan; import io.flamingock.internal.common.core.error.FlamingockException; -import io.flamingock.internal.common.core.recovery.ManualInterventionRequiredException; import io.flamingock.internal.common.core.recovery.action.ChangeAction; import io.flamingock.internal.core.change.executable.ExecutableChange; import io.flamingock.internal.core.pipeline.execution.ExecutableStage; @@ -36,7 +35,7 @@ class ExecutionPlanTest { @DisplayName("ABORT plan should not require execution") void abortPlanShouldNotRequireExecution() { ExecutionPlan plan = ExecutionPlan.ABORT(Collections.singletonList( - stageWith(mockChange("change-1", ChangeAction.MANUAL_INTERVENTION)) + stageWith(mockChange("change-1", ChangeAction.APPLY)) )); assertFalse(plan.isExecutionRequired()); } @@ -49,27 +48,13 @@ void abortPlanShouldBeAborted() { } @Test - @DisplayName("ABORT plan with MANUAL_INTERVENTION changes should throw on validate") - void abortPlanShouldThrowOnValidateWhenManualInterventionChanges() { - ExecutionPlan plan = ExecutionPlan.ABORT(Collections.singletonList( - stageWith( - mockChange("change-1", ChangeAction.APPLY), - mockChange("change-2", ChangeAction.MANUAL_INTERVENTION) - ) - )); - ManualInterventionRequiredException ex = assertThrows( - ManualInterventionRequiredException.class, plan::validate); - assertTrue(ex.getConflictingSummary().contains("change-2")); - } - - @Test - @DisplayName("ABORT plan without MANUAL_INTERVENTION changes should throw FlamingockException on validate") - void abortPlanShouldThrowFlamingockExceptionWhenNoManualInterventionChanges() { + @DisplayName("ABORT plan should throw generic FlamingockException on validate") + void abortPlanShouldThrowFlamingockExceptionOnValidate() { ExecutionPlan plan = ExecutionPlan.ABORT(Collections.singletonList( stageWith(mockChange("change-1", ChangeAction.APPLY)) )); FlamingockException ex = assertThrows(FlamingockException.class, plan::validate); - assertFalse(ex instanceof ManualInterventionRequiredException); + assertEquals("Execution aborted by the execution planner", ex.getMessage()); } @Test diff --git a/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlannerTest.java b/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlannerTest.java index e9440c4cd..5f8fbabf7 100644 --- a/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlannerTest.java +++ b/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlannerTest.java @@ -19,7 +19,6 @@ import io.flamingock.internal.common.core.audit.AuditTxType; import io.flamingock.api.RecoveryStrategy; import io.flamingock.internal.common.core.change.RecoveryDescriptor; -import io.flamingock.internal.common.core.recovery.ManualInterventionRequiredException; import io.flamingock.internal.common.core.recovery.RecoveryIssue; import io.flamingock.internal.core.configuration.core.CoreConfigurable; import io.flamingock.internal.core.external.store.audit.community.CommunityAuditReader; @@ -72,19 +71,22 @@ void setup() { } @Test - @DisplayName("Should return ABORT without acquiring lock when manual intervention is required") - void shouldReturnAbortWithoutAcquiringLockWhenManualInterventionRequired() { + @DisplayName("Should acquire lock and return execution plan even when changes require manual intervention (MI is now a per-stage concern)") + void shouldProceedEvenWhenManualInterventionRequired() { AbstractLoadedChange change = mockLoadedChange("change-1"); AbstractLoadedStage stage = mockStage("stage-1", change); Map snapshot = new HashMap<>(); snapshot.put("change-1", buildAuditEntry("change-1", AuditEntry.Status.FAILED, AuditTxType.NON_TX)); when(auditReader.getAuditSnapshotByChangeId()).thenReturn(snapshot); + when(lockService.upsert(any(), any(), anyLong())) + .thenReturn(new LockAcquisition(RunnerId.fromString("test-runner"), 60000L)); ExecutionPlan plan = planner.getNextExecution(PipelineRun.of(Collections.singletonList(stage))); - assertTrue(plan.isAborted()); - verify(lockService, never()).upsert(any(), any(), anyLong()); + // The planner no longer aborts based on MI; the operation lambda gates MI per stage. + assertFalse(plan.isAborted()); + verify(lockService).upsert(any(), any(), anyLong()); } @Test @@ -137,9 +139,9 @@ void shouldSkipBlockedForMIStagesAndPlanRemaining() { AbstractLoadedStage stage2 = mockStage("stage-2", change2); PipelineRun pipelineRun = PipelineRun.of(java.util.Arrays.asList(stage1, stage2)); - ManualInterventionRequiredException miEx = new ManualInterventionRequiredException( - Collections.singletonList(new RecoveryIssue("change-1")), "stage-1"); - pipelineRun.markStagesBlockedFromMI(miEx); + pipelineRun.markStageBlockedFromMI( + "stage-1", + Collections.singletonList(new RecoveryIssue("change-1"))); when(auditReader.getAuditSnapshotByChangeId()).thenReturn(Collections.emptyMap()); when(lockService.upsert(any(), any(), anyLong())) diff --git a/core/flamingock-test-support/src/test/java/io/flamingock/support/integration/FlamingockTestSupportIntegrationTest.java b/core/flamingock-test-support/src/test/java/io/flamingock/support/integration/FlamingockTestSupportIntegrationTest.java index 7dce259c3..abfb0cebf 100644 --- a/core/flamingock-test-support/src/test/java/io/flamingock/support/integration/FlamingockTestSupportIntegrationTest.java +++ b/core/flamingock-test-support/src/test/java/io/flamingock/support/integration/FlamingockTestSupportIntegrationTest.java @@ -19,6 +19,7 @@ import io.flamingock.common.test.pipeline.PipelineTestHelper; import io.flamingock.internal.common.core.metadata.MetadataLoader; import io.flamingock.internal.core.operation.OperationException; +import io.flamingock.internal.core.operation.StagedExecuteOperationException; import io.flamingock.support.FlamingockTestSupport; import io.flamingock.support.inmemory.InMemoryFlamingockBuilder; import io.flamingock.support.integration.changes.*; @@ -159,8 +160,17 @@ void shouldVerifyDependencyInjectionInRollbackForNonTransactionalChanges() { FlamingockTestSupport .givenBuilder(InMemoryFlamingockBuilder.create().addTargetSystem(targetSystem)) .whenRun() - .thenExpectException(OperationException.class, ex -> { - assertTrue(ex.getMessage().contains("Intentional failure")); + .thenExpectException(StagedExecuteOperationException.class, ex -> { + // The per-stage error info carries the original throwable message; the + // exception itself only signals the staged failure. + StagedExecuteOperationException stagedEx = (StagedExecuteOperationException) ex; + boolean foundIntentional = stagedEx.getResult().getStages().stream() + .filter(s -> s.getState().isFailed()) + .map(s -> s.getState().getErrorInfo().orElse(null)) + .filter(java.util.Objects::nonNull) + .anyMatch(info -> info.getMessage() != null + && info.getMessage().contains("Intentional failure")); + assertTrue(foundIntentional, "Expected a stage error mentioning 'Intentional failure'"); assertTrue(counter.isExecuted(), "Counter should be executed"); assertTrue(counter.isRollbacked(), "Counter should be rolled back"); }) diff --git a/docs/ERROR_REPORTING_PROPOSAL.md b/docs/ERROR_REPORTING_PROPOSAL.md new file mode 100644 index 000000000..cbacf6353 --- /dev/null +++ b/docs/ERROR_REPORTING_PROPOSAL.md @@ -0,0 +1,147 @@ +# Error reporting proposal — rich execution report via event listener + +**Status:** partially implemented. +- **Step 1 (single-line `getMessage()` summary) — DONE** on `StagedExecuteOperationException`. The message now reads e.g. `"Flamingock execution failed: 1 of 3 stage(s) failed [flamingock-system-stage]; changes applied=3, failed=1, skipped=0; duration=312ms"`. The message-building helper is inlined as a `private static String buildMessage(ExecuteResponseData)` inside the exception class — natural seed to extract into a reusable `ExecutionReportFormatter.summary(...)` when step 2 lands. +- **Step 2 (`toString()` override with rich multi-line report) — DEFERRED.** +- **Step 3 (default event listener writing via SLF4J + builder opt-out) — DEFERRED.** + +**Related code:** `core/flamingock-core/.../operation/AbstractPipelineTraverseOperation.java`, `ExecuteOperationException` / `StagedExecuteOperationException` / `PipelineExecuteOperationException`, `core/flamingock-core-commons/.../response/data/ExecuteResponseData.java`, `core/flamingock-core/.../event/`. + +## Context + +After Phase 3 of removing fail-fast made stage execution independent, the typed exception hierarchy (`ExecuteOperationException` and its `Staged…` / `Pipeline…` subclasses) carries `ExecuteResponseData` through to callers. The information is there, but the *default rendering* is poor. + +Today, when the runner fails, the log shows: + +``` +io.flamingock.internal.core.operation.StagedExecuteOperationException: One or more stages failed during execution + at io.flamingock.internal.core.operation.AbstractPipelineTraverseOperation.execute(...) + [stack frames…] +``` + +A developer reading this gets only the class name and a bland one-liner. The structured per-stage failure data (status counts, duration, per-stage error info) is in `getResult()` but invisible unless the consumer explicitly extracts it. + +We want a richer default output without hurting enterprise log pipelines. + +## The trade-off space + +Three positions exist, each with documented precedent: + +1. **Terse `getMessage()` + structured getter** — pattern used by `jakarta.validation.ConstraintViolationException`. The exception is a clean signal; callers iterate `getConstraintViolations()` themselves. Aggregator-friendly. Costs: developers don't see the rich info on `e.printStackTrace()` / default `logger.error(..., e)`. Worst onboarding experience. + +2. **Rich `getMessage()`** — pattern used by Spring's `BindException` / `MethodArgumentNotValidException`. `getMessage()` builds a multi-line summary of all field errors. Developers see everything automatically. Costs: log-aggregator fragmentation in any team whose Splunk / Datadog / Filebeat lacks a multi-line pattern. Spring users in enterprise have learned to deal with this; it's a known wart. + +3. **`toString()` override + terse `getMessage()`** — less common but technically clean. JDK's `printStackTrace()` uses `toString()` for the headline (so IDEs / dev terminals show the rich report). Logback's `%ex` and most JSON encoders use `getMessage()` (so log aggregators see the one-liner). Costs: subtle invariant; future maintainers may not understand why `toString()` differs from `getMessage()`; some consumers do `logger.error("X: " + ex)` (concatenation) which calls `toString()` and lands the rich content in log fields. + +Spring and Hibernate Validator disagree on which is "right". Both are widely deployed. + +## Proposed approach — beyond Spring/Hibernate + +Separate the *signal* (exception) from the *rendering* (event listener): + +- `getMessage()` stays a **single line** with the count + failed stage names. Example: `"Flamingock execution failed: 1 stage(s) failed [flamingock-system-stage]"`. Enterprise-aggregator friendly. +- `toString()` returns the **rich multi-line report** as a safety net for ad-hoc usage (`System.err.println(ex)`, debuggers, REPL). Not the primary path. +- Default rendering happens **via an event listener** registered by the builder. The listener subscribes to `PipelineFailedEvent` / `PipelineCompletedEvent` and writes the rich report via SLF4J under a dedicated logger name (e.g., `FK-Report`). Two layers of control: a builder flag for full-stop disable, plus standard SLF4J config (``) for fine-grained silencing without rebuild. + +This gives: +- Default users see a rich, useful report out of the box (better than Spring/Hibernate). +- Enterprises silence it via standard logging configuration — no library code change needed. +- The same data is available to *any* consumer (CLI, dashboards, Cloud Edition, custom Slack/metrics listeners) through the existing event system. No format is hardcoded; everything is composable. +- The exception remains a clean typed signal carrying structured data. + +## Design decisions to lock down + +### Event payloads carry structured data directly + +`PipelineFailedEvent` and `PipelineCompletedEvent` should carry `ExecuteResponseData` as a typed field — *not* require listeners to downcast a `Throwable` to extract it. Events become self-describing. Per-stage events (`StageFailedEvent`, `StageCompletedEvent`) carry `StageResult` for symmetry. + +### Default listener writes via SLF4J + +A dedicated logger name (e.g., `FK-Report`) at: +- `INFO` for `PipelineCompletedEvent`. +- `ERROR` for `PipelineFailedEvent`. + +Two layers of control: +1. Builder flag `enableDefaultExecutionReport(boolean)` — default `true`. Full opt-out. +2. SLF4J config — `` silences without rebuild. + +### Consolidate today's ad-hoc summary logs into the listener + +Today `AbstractPipelineTraverseOperation` inlines: +- `logger.info("Flamingock execution completed [duration=Xms applied=N skipped=M]")` +- `logger.info("Flamingock execution finished with stage failures [duration=Xms applied=N skipped=M failed=K]")` + +These should move into the default listener so there is a single source of truth for "what gets logged about a run." Otherwise we'll double-log once the listener exists. + +### Formatter as a reusable utility + +A static `ExecutionReportFormatter` (in `flamingock-core-commons`, next to `ExecuteResponseData`) produces the multi-line report from response data. Used by: +- The default listener (rendering for log output). +- `ExecuteOperationException.toString()` (rendering for the safety-net path). +- Any third-party consumer who wants the same canonical rendering. + +### Report shape (proposed) + +``` + + +Summary: + Status: FAILED + Duration: 312ms (2026-05-15T08:37:56.224 → 2026-05-15T08:37:56.536) + Stages: 1 failed, 2 completed (3 total) + Changes: 3 applied, 0 skipped, 1 failed (4 total) + +Failed stages: + - flamingock-system-stage [InvalidConfigurationException] + Invalid value for internal.mongock.import.skip: invalid_value (expected "true" or "false" or empty) + Changes: migration-mongock-to-flamingock-community +``` + +For `BlockedForMI` stages: distinct label (`BLOCKED — manual intervention required`) plus the change IDs from `recoveryIssues` instead of `errorInfo.changeIds`. The CLI's existing `ExecutionResultFormatter` stays separate (it's for terminal output with decorative banners; this report is for log streams). + +### Defensive listener + +The default listener must **never throw**. If formatting fails, fall back to a one-line error log and continue. A misbehaving renderer should never mask the real underlying error or break event propagation. + +## Risks and footguns + +- **Test output noise.** Every test that triggers a failure will surface a rich report in test logs. Test infrastructure (`InternalInMemoryTestKit`, etc.) may want to disable the default listener by default for unit tests to keep output clean. +- **Plugin event publishers.** Some integration plugins (e.g. Spring Boot) contribute their own event publishers. Our default listener registers against the *core* publisher. Worth verifying no duplicate firing if a plugin re-publishes core events. +- **Order of operations on failure.** With the listener writing via SLF4J synchronously, report log lines appear *before* the consumer's "Flamingock failed" log line in their own catch block. Time-ordered in any sink — not a bug, but worth being explicit about so it's not surprising. +- **Double-rendering if consumers add their own listener.** Listeners are additive by default. A consumer who registers a custom listener AND leaves the default enabled gets both outputs. Accept additive behavior; document the opt-out flag. +- **`toString()` ≠ `getMessage()`** is a subtle invariant. A future maintainer might "fix" them to match. Add a code comment near the override explaining why they differ and pointing at this doc. +- **Plain-text legacy log pipelines.** Even with `getMessage()` terse, the default listener still writes multi-line via SLF4J. Enterprises with unconfigured multi-line patterns will still see fragmentation. The SLF4J-level opt-out (`level="OFF"` on the `FK-Report` logger) addresses this without disabling the whole listener system. +- **`PipelineFailedEvent` with neither stages nor cause.** Defensive case (shouldn't happen but). Listener handles gracefully — print whatever's available, never throw. + +## Why this is better than Spring / Hibernate + +Spring puts everything into `getMessage()` and accepts the aggregator pain — that's a one-size-fits-all decision. Hibernate Validator puts nothing into the message and accepts the developer-onboarding pain — also one-size-fits-all. Neither library decouples the data from its rendering. + +By routing default rendering through the event system: +- The exception stays a clean typed signal (Hibernate's strength). +- Developers see rich output by default (Spring's strength). +- Enterprises silence the rich path via standard logging config (neither framework offers this cleanly). +- Third parties compose new output formats (Slack, metrics, JSON, anything) without library code changes (neither framework offers this). + +The listener pattern *replaces* the inline `logger.info(...)` calls in the operation, so we don't end up with two reporting paths. + +## Implementation scope (when picked up) + +Rough sketch — not a plan, just sizing: + +1. Update event payloads — `PipelineFailedEvent` / `PipelineCompletedEvent` carry `ExecuteResponseData`; `StageFailedEvent` / `StageCompletedEvent` carry `StageResult`. +2. Add `ExecutionReportFormatter` in `flamingock-core-commons`. +3. Override `toString()` on `ExecuteOperationException` to call the formatter. +4. Keep `getMessage()` terse — include failed stage names so log-line triage works. +5. Add `DefaultExecutionReportListener` in `flamingock-core/.../event/listener/`. Listens to `PipelineFailedEvent` / `PipelineCompletedEvent`. Writes via SLF4J under `FK-Report`. Defensive (never throws). +6. Register the listener by default in `AbstractFlamingockBuilder.build()` unless disabled. +7. Add `enableDefaultExecutionReport(boolean)` to the builder (default `true`). +8. Remove the inline summary `logger.info(...)` calls from `AbstractPipelineTraverseOperation` (replaced by the listener). +9. Test-kit defaults — consider disabling the default listener in test kits for clean test output. +10. Tests — formatter rendering snapshot; listener fires-once and writes-expected-content; builder flag opt-out works; defensive case (listener doesn't throw on bad data). + +## Open questions + +- Should `PipelineCompletedEvent` always get a full report, or only a one-line summary when there are no failures? (Symmetry argument: same listener for both, just different log levels; rendering can be conditional.) +- Should the formatter be open for extension (themes, locales) or stay closed and reused as-is by anyone who wants different formatting (they write their own listener)? Leaning closed — the canonical rendering is a contract; bespoke formats live in bespoke listeners. +- Default behaviour in test kits: opt-out by default, or opt-in via test flag? Likely opt-out by default to keep CI logs clean. diff --git a/e2e/core-e2e/src/test/java/io/flamingock/core/e2e/CoreStrategiesE2ETest.java b/e2e/core-e2e/src/test/java/io/flamingock/core/e2e/CoreStrategiesE2ETest.java index d1bb8fc61..6ef5cb547 100644 --- a/e2e/core-e2e/src/test/java/io/flamingock/core/e2e/CoreStrategiesE2ETest.java +++ b/e2e/core-e2e/src/test/java/io/flamingock/core/e2e/CoreStrategiesE2ETest.java @@ -24,7 +24,10 @@ import io.flamingock.internal.common.core.metadata.MetadataLoader; import io.flamingock.internal.common.core.audit.AuditEntry; import io.flamingock.internal.common.core.audit.AuditTxType; +import io.flamingock.internal.common.core.response.data.ErrorInfo; +import io.flamingock.internal.common.core.response.data.StageResult; import io.flamingock.internal.core.operation.OperationException; +import io.flamingock.internal.core.operation.StagedExecuteOperationException; import io.flamingock.targetsystem.nontransactional.NonTransactionalTargetSystem; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -244,7 +247,7 @@ void testDependencyInjectionInRollbackForNonTxChange() { ) ); - OperationException exception = assertThrows(OperationException.class, () -> { + StagedExecuteOperationException exception = assertThrows(StagedExecuteOperationException.class, () -> { testKit.createBuilder() .addTargetSystem(targetSystem) .build() @@ -252,7 +255,15 @@ void testDependencyInjectionInRollbackForNonTxChange() { }); assertNotNull(exception); - assertTrue(exception.getMessage().contains("Intentional failure")); + StageResult failedStage = exception.getResult().getStages().stream() + .filter(s -> s.getState().isFailed()) + .findFirst() + .orElseThrow(() -> new AssertionError("Expected at least one failed stage in the response")); + ErrorInfo errorInfo = failedStage.getState().getErrorInfo() + .orElseThrow(() -> new AssertionError("Failed stage should carry an ErrorInfo")); + assertNotNull(errorInfo.getMessage()); + assertTrue(errorInfo.getMessage().contains("Intentional failure"), + "Expected per-stage error to contain 'Intentional failure', got: " + errorInfo.getMessage()); } assertTrue(counter.isExecuted(), "Counter.executed should be true after execution"); diff --git a/e2e/core-e2e/src/test/java/io/flamingock/core/e2e/RecoveryE2ETest.java b/e2e/core-e2e/src/test/java/io/flamingock/core/e2e/RecoveryE2ETest.java index 669ae3891..5ec045fee 100644 --- a/e2e/core-e2e/src/test/java/io/flamingock/core/e2e/RecoveryE2ETest.java +++ b/e2e/core-e2e/src/test/java/io/flamingock/core/e2e/RecoveryE2ETest.java @@ -28,8 +28,9 @@ import io.flamingock.internal.common.core.audit.AuditEntry; import io.flamingock.internal.common.core.audit.AuditTxType; import io.flamingock.targetsystem.nontransactional.NonTransactionalTargetSystem; -import io.flamingock.internal.common.core.recovery.ManualInterventionRequiredException; import io.flamingock.internal.common.core.recovery.RecoveryIssue; +import io.flamingock.internal.common.core.response.data.StageResult; +import io.flamingock.internal.core.operation.StagedExecuteOperationException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -37,6 +38,7 @@ import org.mockito.Mockito; import java.util.Collections; +import java.util.List; import static io.flamingock.core.kit.audit.AuditEntryExpectation.APPLIED; import static io.flamingock.core.kit.audit.AuditEntryExpectation.FAILED; @@ -251,8 +253,8 @@ void testRollbackFailedNonTxRequiresManualIntervention() { ) ); - ManualInterventionRequiredException exception = assertThrows( - ManualInterventionRequiredException.class, + StagedExecuteOperationException exception = assertThrows( + StagedExecuteOperationException.class, () -> { testKit.createBuilder() .addTargetSystem(new NonTransactionalTargetSystem("kafka")) @@ -262,21 +264,16 @@ void testRollbackFailedNonTxRequiresManualIntervention() { } ); - // Then - Verify exception details - assertNotNull(exception.getConflictingChanges()); - assertEquals(1, exception.getConflictingChanges().size()); - - RecoveryIssue recoveryIssue = exception.getConflictingChanges().get(0); - assertEquals(changeId, recoveryIssue.getChangeId()); - - // Verify exception message contains expected recovery guidance - String message = exception.getMessage(); - assertTrue(message.contains("MANUAL INTERVENTION REQUIRED"), - "Exception should indicate manual intervention is required"); - assertTrue(message.contains(changeId), - "Exception message should mention the specific change ID"); - assertTrue(message.contains("flamingock mark"), - "Exception should provide CLI command guidance"); + // Then - Verify the affected stage is BlockedForMI with the expected RecoveryIssue. + StageResult blockedStage = exception.getResult().getStages().stream() + .filter(s -> s.getState().isBlockedForManualIntervention()) + .findFirst() + .orElseThrow(() -> new AssertionError("Expected a BlockedForMI stage in the response")); + + List recoveryIssues = blockedStage.getState().getRecoveryIssues(); + assertNotNull(recoveryIssues); + assertEquals(1, recoveryIssues.size()); + assertEquals(changeId, recoveryIssues.get(0).getChangeId()); } // Then - Verify audit log remains unchanged (only the pre-inserted ROLLBACK_FAILED entry) @@ -506,8 +503,8 @@ private void testForManualInterventionException(String changeId, ) ); - ManualInterventionRequiredException exception = assertThrows( - ManualInterventionRequiredException.class, + StagedExecuteOperationException exception = assertThrows( + StagedExecuteOperationException.class, () -> { testKit.createBuilder() .addTargetSystem(new NonTransactionalTargetSystem("keycloak")) @@ -517,21 +514,18 @@ private void testForManualInterventionException(String changeId, } ); - // Then - Verify exception details - assertNotNull(exception.getConflictingChanges()); - assertEquals(1, exception.getConflictingChanges().size()); - - RecoveryIssue recoveryIssue = exception.getConflictingChanges().get(0); - assertEquals(changeId, recoveryIssue.getChangeId()); - - // Verify exception message contains expected recovery guidance - String message = exception.getMessage(); - assertTrue(message.contains("MANUAL INTERVENTION REQUIRED"), - "Exception should indicate manual intervention is required"); - assertTrue(message.contains(changeId), - "Exception message should mention the specific change ID"); - assertTrue(message.contains("flamingock mark"), - "Exception should provide CLI command guidance"); + // Then - Verify the affected stage is BlockedForMI with the expected RecoveryIssue. + // MI is now a per-stage state (no longer a thrown ManualInterventionRequiredException) + // so callers inspect the per-stage data carried by the response. + StageResult blockedStage = exception.getResult().getStages().stream() + .filter(s -> s.getState().isBlockedForManualIntervention()) + .findFirst() + .orElseThrow(() -> new AssertionError("Expected a BlockedForMI stage in the response")); + + List recoveryIssues = blockedStage.getState().getRecoveryIssues(); + assertNotNull(recoveryIssues); + assertEquals(1, recoveryIssues.size()); + assertEquals(changeId, recoveryIssues.get(0).getChangeId()); } // Then - Verify audit log shows expected sequence (usually just the pre-existing entry) diff --git a/legacy/mongock-importer-couchbase/src/test/java/io/flamingock/importer/mongock/couchbase/CouchbaseImporterTest.java b/legacy/mongock-importer-couchbase/src/test/java/io/flamingock/importer/mongock/couchbase/CouchbaseImporterTest.java index 312d4b62c..7cf0dfa1b 100644 --- a/legacy/mongock-importer-couchbase/src/test/java/io/flamingock/importer/mongock/couchbase/CouchbaseImporterTest.java +++ b/legacy/mongock-importer-couchbase/src/test/java/io/flamingock/importer/mongock/couchbase/CouchbaseImporterTest.java @@ -28,10 +28,11 @@ import io.flamingock.couchbase.kit.CouchbaseTestKit; import io.flamingock.internal.common.core.audit.AuditEntry; import io.flamingock.store.couchbase.CouchbaseAuditStore; -import io.flamingock.internal.common.core.error.FlamingockException; +import io.flamingock.internal.common.core.response.data.ErrorInfo; import io.flamingock.internal.common.couchbase.CouchbaseCollectionHelper; import io.flamingock.internal.core.builder.FlamingockFactory; import io.flamingock.internal.core.builder.runner.Runner; +import io.flamingock.internal.core.operation.StagedExecuteOperationException; import io.flamingock.internal.util.constants.CommunityPersistenceConstants; import io.flamingock.support.mongock.annotations.MongockSupport; import io.flamingock.targetsystem.couchbase.CouchbaseTargetSystem; @@ -215,8 +216,9 @@ void GIVEN_mongockAuditHistoryEmptyAndNoFailIfEmptyOriginValueProvided_WHEN_migr .addTargetSystem(targetSystem) .build(); - FlamingockException ex = assertThrows(FlamingockException.class, flamingock::run); - assertEquals("No audit entries found when importing from 'couchbase-target-system'.", ex.getMessage()); + StagedExecuteOperationException ex = assertThrows(StagedExecuteOperationException.class, flamingock::run); + assertEquals("No audit entries found when importing from 'couchbase-target-system'.", + firstFailedStageErrorMessage(ex)); } @@ -233,8 +235,9 @@ void GIVEN_mongockAuditHistoryEmptyAndFailIfEmptyOriginEnabled_WHEN_migratingToF .setProperty(MONGOCK_IMPORT_EMPTY_ORIGIN_ALLOWED_PROPERTY_KEY, Boolean.FALSE.toString()) .build(); - FlamingockException ex = assertThrows(FlamingockException.class, flamingock::run); - assertEquals("No audit entries found when importing from 'couchbase-target-system'.", ex.getMessage()); + StagedExecuteOperationException ex = assertThrows(StagedExecuteOperationException.class, flamingock::run); + assertEquals("No audit entries found when importing from 'couchbase-target-system'.", + firstFailedStageErrorMessage(ex)); } @@ -333,9 +336,18 @@ void GIVEN_skipImportFlagWithInvalidValue_WHEN_migratingToFlamingockCommunity_TH .setProperty(MONGOCK_IMPORT_SKIP_PROPERTY_KEY, SKIP_IMPORT_VALUE) // only allows empty / true / false .build(); - FlamingockException ex = assertThrows(FlamingockException.class, flamingock::run); + StagedExecuteOperationException ex = assertThrows(StagedExecuteOperationException.class, flamingock::run); assertEquals("Invalid value for " + MONGOCK_IMPORT_SKIP_PROPERTY_KEY + ": " + SKIP_IMPORT_VALUE - + " (expected \"true\" or \"false\" or empty)", ex.getMessage()); + + " (expected \"true\" or \"false\" or empty)", firstFailedStageErrorMessage(ex)); + } + + private static String firstFailedStageErrorMessage(StagedExecuteOperationException ex) { + return ex.getResult().getStages().stream() + .filter(s -> s.getState().isFailed()) + .findFirst() + .flatMap(s -> s.getState().getErrorInfo()) + .map(ErrorInfo::getMessage) + .orElseThrow(() -> new AssertionError("Expected a failed stage with ErrorInfo")); } @Test diff --git a/legacy/mongock-importer-dynamodb/src/test/java/io/flamingock/importer/mongock/dynamodb/DynamoDBImporterTest.java b/legacy/mongock-importer-dynamodb/src/test/java/io/flamingock/importer/mongock/dynamodb/DynamoDBImporterTest.java index 08388f080..9cd64eeba 100644 --- a/legacy/mongock-importer-dynamodb/src/test/java/io/flamingock/importer/mongock/dynamodb/DynamoDBImporterTest.java +++ b/legacy/mongock-importer-dynamodb/src/test/java/io/flamingock/importer/mongock/dynamodb/DynamoDBImporterTest.java @@ -22,7 +22,8 @@ import io.flamingock.core.kit.audit.AuditTestHelper; import io.flamingock.dynamodb.kit.DynamoDBTableFactory; import io.flamingock.dynamodb.kit.DynamoDBTestKit; -import io.flamingock.internal.common.core.error.FlamingockException; +import io.flamingock.internal.common.core.response.data.ErrorInfo; +import io.flamingock.internal.core.operation.StagedExecuteOperationException; import io.flamingock.internal.core.builder.runner.Runner; import io.flamingock.support.mongock.annotations.MongockSupport; import io.flamingock.targetsystem.dynamodb.DynamoDBTargetSystem; @@ -216,8 +217,9 @@ void GIVEN_mongockAuditHistoryEmptyAndNoFailIfEmptyOriginValueProvided_WHEN_migr .addTargetSystem(dynamodbTargetSystem) .build(); - FlamingockException ex = assertThrows(FlamingockException.class, flamingock::run); - assertEquals("No audit entries found when importing from 'dynamodb-target-system'.", ex.getMessage()); + StagedExecuteOperationException ex = assertThrows(StagedExecuteOperationException.class, flamingock::run); + assertEquals("No audit entries found when importing from 'dynamodb-target-system'.", + firstFailedStageErrorMessage(ex)); } @@ -236,8 +238,9 @@ void GIVEN_mongockAuditHistoryEmptyAndFailIfEmptyOriginEnabled_WHEN_migratingToF .setProperty(MONGOCK_IMPORT_EMPTY_ORIGIN_ALLOWED_PROPERTY_KEY, Boolean.FALSE.toString()) .build(); - FlamingockException ex = assertThrows(FlamingockException.class, flamingock::run); - assertEquals("No audit entries found when importing from 'dynamodb-target-system'.", ex.getMessage()); + StagedExecuteOperationException ex = assertThrows(StagedExecuteOperationException.class, flamingock::run); + assertEquals("No audit entries found when importing from 'dynamodb-target-system'.", + firstFailedStageErrorMessage(ex)); } @Test @@ -363,12 +366,21 @@ void GIVEN_skipImportFlagWithInvalidValue_WHEN_migratingToFlamingockCommunity_TH .setProperty(MONGOCK_IMPORT_SKIP_PROPERTY_KEY, SKIP_IMPORT_VALUE) // only allows empty / true / false .build(); - FlamingockException ex = assertThrows(FlamingockException.class, flamingock::run); + StagedExecuteOperationException ex = assertThrows(StagedExecuteOperationException.class, flamingock::run); assertEquals("Invalid value for " + MONGOCK_IMPORT_SKIP_PROPERTY_KEY + ": " + SKIP_IMPORT_VALUE - + " (expected \"true\" or \"false\" or empty)", ex.getMessage()); + + " (expected \"true\" or \"false\" or empty)", firstFailedStageErrorMessage(ex)); } + private static String firstFailedStageErrorMessage(StagedExecuteOperationException ex) { + return ex.getResult().getStages().stream() + .filter(s -> s.getState().isFailed()) + .findFirst() + .flatMap(s -> s.getState().getErrorInfo()) + .map(ErrorInfo::getMessage) + .orElseThrow(() -> new AssertionError("Expected a failed stage with ErrorInfo")); + } + @Test @DisplayName("GIVEN all Mongock changeUnits already executed " + "AND skip import flag enabled " + diff --git a/legacy/mongock-importer-mongodb/src/test/java/io/flamingock/importer/mongock/mongodb/MongoDBImporterTest.java b/legacy/mongock-importer-mongodb/src/test/java/io/flamingock/importer/mongock/mongodb/MongoDBImporterTest.java index d369afecf..58f56447a 100644 --- a/legacy/mongock-importer-mongodb/src/test/java/io/flamingock/importer/mongock/mongodb/MongoDBImporterTest.java +++ b/legacy/mongock-importer-mongodb/src/test/java/io/flamingock/importer/mongock/mongodb/MongoDBImporterTest.java @@ -25,7 +25,8 @@ import io.flamingock.store.mongodb.sync.MongoDBSyncAuditStore; import io.flamingock.core.kit.TestKit; import io.flamingock.core.kit.audit.AuditTestHelper; -import io.flamingock.internal.common.core.error.FlamingockException; +import io.flamingock.internal.common.core.response.data.ErrorInfo; +import io.flamingock.internal.core.operation.StagedExecuteOperationException; import io.flamingock.internal.core.builder.runner.Runner; import io.flamingock.mongodb.kit.MongoDBSyncTestKit; import io.flamingock.support.mongock.annotations.MongockSupport; @@ -228,8 +229,9 @@ void GIVEN_mongockAuditHistoryEmptyAndNoFailIfEmptyOriginValueProvided_WHEN_migr .addTargetSystem(mongodbTargetSystem) .build(); - FlamingockException ex = assertThrows(FlamingockException.class, flamingock::run); - assertEquals("No audit entries found when importing from 'mongodb-target-system'.", ex.getMessage()); + StagedExecuteOperationException ex = assertThrows(StagedExecuteOperationException.class, flamingock::run); + assertEquals("No audit entries found when importing from 'mongodb-target-system'.", + firstFailedStageErrorMessage(ex)); } @@ -248,8 +250,9 @@ void GIVEN_mongockAuditHistoryEmptyAndFailIfEmptyOriginEnabled_WHEN_migratingToF .setProperty(MONGOCK_IMPORT_EMPTY_ORIGIN_ALLOWED_PROPERTY_KEY, Boolean.FALSE.toString()) .build(); - FlamingockException ex = assertThrows(FlamingockException.class, flamingock::run); - assertEquals("No audit entries found when importing from 'mongodb-target-system'.", ex.getMessage()); + StagedExecuteOperationException ex = assertThrows(StagedExecuteOperationException.class, flamingock::run); + assertEquals("No audit entries found when importing from 'mongodb-target-system'.", + firstFailedStageErrorMessage(ex)); } @@ -385,9 +388,18 @@ void GIVEN_skipImportFlagWithInvalidValue_WHEN_migratingToFlamingockCommunity_TH .setProperty(MONGOCK_IMPORT_SKIP_PROPERTY_KEY, SKIP_IMPORT_VALUE) // only allows empty / true / false .build(); - FlamingockException ex = assertThrows(FlamingockException.class, flamingock::run); + StagedExecuteOperationException ex = assertThrows(StagedExecuteOperationException.class, flamingock::run); assertEquals("Invalid value for " + MONGOCK_IMPORT_SKIP_PROPERTY_KEY + ": " + SKIP_IMPORT_VALUE - + " (expected \"true\" or \"false\" or empty)", ex.getMessage()); + + " (expected \"true\" or \"false\" or empty)", firstFailedStageErrorMessage(ex)); + } + + private static String firstFailedStageErrorMessage(StagedExecuteOperationException ex) { + return ex.getResult().getStages().stream() + .filter(s -> s.getState().isFailed()) + .findFirst() + .flatMap(s -> s.getState().getErrorInfo()) + .map(ErrorInfo::getMessage) + .orElseThrow(() -> new AssertionError("Expected a failed stage with ErrorInfo")); } From 26554fe757753173c6a389ae3fac27da6e9dc552 Mon Sep 17 00:00:00 2001 From: Antonio Perez Dieppa Date: Fri, 15 May 2026 13:29:09 +0100 Subject: [PATCH 5/6] refactor: add StageRunBlock --- .../AbstractPipelineTraverseOperation.java | 30 +--- .../core/pipeline/run/PipelineRun.java | 73 +++++++- .../core/pipeline/run/StageRunBlock.java | 98 +++++++++++ .../core/pipeline/run/PipelineRunTest.java | 65 +++++++ .../core/pipeline/run/StageRunBlockTest.java | 159 ++++++++++++++++++ 5 files changed, 400 insertions(+), 25 deletions(-) create mode 100644 core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/run/StageRunBlock.java create mode 100644 core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/StageRunBlockTest.java diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperation.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperation.java index 899ff4aa8..0b2493a01 100644 --- a/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperation.java +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperation.java @@ -34,7 +34,6 @@ import io.flamingock.internal.core.pipeline.execution.StageExecutionException; import io.flamingock.internal.core.pipeline.execution.StageExecutor; import io.flamingock.internal.core.pipeline.loaded.LoadedPipeline; -import io.flamingock.internal.core.pipeline.loaded.stage.AbstractLoadedStage; import io.flamingock.internal.core.pipeline.run.PipelineRun; import io.flamingock.internal.core.pipeline.run.StageRun; import io.flamingock.internal.core.plan.ExecutionPlan; @@ -45,8 +44,6 @@ import io.flamingock.internal.util.log.FlamingockLoggerFactory; import org.slf4j.Logger; -import java.util.ArrayList; -import java.util.List; /** * Common execution flow for Apply and Validate operations. @@ -104,34 +101,25 @@ public ExecuteResult execute(ExecuteArgs args) { } } - private static List validateAndGetExecutableStages(LoadedPipeline pipeline) { - pipeline.validate(); - List stages = new ArrayList<>(); - if (pipeline.getSystemStage().isPresent()) { - stages.add(pipeline.getSystemStage().get()); - } - stages.addAll(pipeline.getStages()); - return stages; - } - private ExecuteResult execute(LoadedPipeline pipeline) { - List allStages = validateAndGetExecutableStages(pipeline); - int stageCount = allStages.size(); - long changeCount = allStages.stream() - .mapToLong(stage -> stage.getChanges().size()) - .sum(); - logger.info("Flamingock execution started [stages={} changes={}]", stageCount, changeCount); + // Single transform point: PipelineRun.of(pipeline) validates static structure, partitions + // stages by type into StageRunBlocks (SYSTEM -> LEGACY -> DEFAULT, sparse), and becomes the + // single source of truth for the rest of this method. LoadedPipeline is not referenced again. + PipelineRun pipelineRun = PipelineRun.of(pipeline); + + logger.info( + "Flamingock execution started [stages={} changes={}]", + pipelineRun.getStageCount(), + pipelineRun.getTotalChangeCount()); eventPublisher.publish(new PipelineStartedEvent()); - PipelineRun pipelineRun = PipelineRun.of(pipeline); pipelineRun.start(); Throwable pipelineLevelError = null; boolean throwPipelineLevelError = true; do { - validateAndGetExecutableStages(pipeline); try (ExecutionPlan execution = executionPlanner.getNextExecution(pipelineRun)) { execution.validate(); diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/run/PipelineRun.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/run/PipelineRun.java index b275bf617..89e6e05ef 100644 --- a/core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/run/PipelineRun.java +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/run/PipelineRun.java @@ -15,6 +15,7 @@ */ package io.flamingock.internal.core.pipeline.run; +import io.flamingock.api.StageType; import io.flamingock.internal.common.core.recovery.RecoveryIssue; import io.flamingock.internal.common.core.response.data.ChangeResult; import io.flamingock.internal.common.core.response.data.ChangeStatus; @@ -30,6 +31,7 @@ import java.time.Duration; import java.time.Instant; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; @@ -39,7 +41,18 @@ public class PipelineRun { + /** + * Block-dependency ordering: every stage in an earlier block must succeed before the next + * block may execute. SYSTEM is the foundation, LEGACY runs second, DEFAULT (user) runs last. + */ + private static final List BLOCK_ORDER = + Arrays.asList(StageType.SYSTEM, StageType.LEGACY, StageType.DEFAULT); + public static PipelineRun of(LoadedPipeline pipeline) { + // Static-structure validation runs as part of construction. Builder-time validation in + // AbstractChangeRunnerBuilder still fails fast on malformed pipelines; this call covers + // the runtime entry point so callers don't have to validate separately. + pipeline.validate(); List stages = new ArrayList<>(); pipeline.getSystemStage().ifPresent(stages::add); stages.addAll(pipeline.getStages()); @@ -47,21 +60,49 @@ public static PipelineRun of(LoadedPipeline pipeline) { } public static PipelineRun of(List stages) { - List runs = new ArrayList<>(); + // Partition by StageType in dependency order, skipping empty types (sparse). The resulting + // flat list is the concatenation of blocks in canonical order — a single source of truth + // for stage ordering across the rest of the runtime. + Map> byType = new LinkedHashMap<>(); + for (StageType type : BLOCK_ORDER) { + byType.put(type, new ArrayList<>()); + } for (AbstractLoadedStage stage : stages) { - runs.add(new StageRun(stage)); + byType.get(resolveType(stage)).add(new StageRun(stage)); } - return new PipelineRun(runs); + + List blocks = new ArrayList<>(); + List flat = new ArrayList<>(); + for (StageType type : BLOCK_ORDER) { + List blockRuns = byType.get(type); + if (!blockRuns.isEmpty()) { + blocks.add(new StageRunBlock(type, blockRuns)); + flat.addAll(blockRuns); + } + } + return new PipelineRun(flat, blocks); + } + + /** + * Defensive null-handling: production stages always carry a {@link StageType}; legacy test + * mocks may not stub {@code getType()} and yield null. Treat null as DEFAULT (the most common + * type, what a normal user stage would be). + */ + private static StageType resolveType(AbstractLoadedStage stage) { + StageType type = stage.getType(); + return type != null ? type : StageType.DEFAULT; } private final List stageRuns; + private final List stageBlocks; private final Map byName; private Instant startedAt; private Instant stoppedAt; private ErrorInfo pipelineError; - private PipelineRun(List stageRuns) { + private PipelineRun(List stageRuns, List stageBlocks) { this.stageRuns = Collections.unmodifiableList(stageRuns); + this.stageBlocks = Collections.unmodifiableList(stageBlocks); Map index = new LinkedHashMap<>(); for (StageRun run : stageRuns) { index.put(run.getName(), run); @@ -77,6 +118,30 @@ public StageRun getStageRun(String name) { return byName.get(name); } + /** + * Stages grouped into typed blocks in dependency order. Empty blocks are omitted (sparse): if + * a pipeline has no SYSTEM stages, no SYSTEM block is returned. Consumers treat the list as + * an ordered execution dependency — every stage in {@code blocks[i]} must complete + * successfully before any stage in {@code blocks[i+1]} may run. + */ + public List getStageBlocks() { + return stageBlocks; + } + + /** Number of stages in this run, across all blocks. */ + public int getStageCount() { + return stageRuns.size(); + } + + /** Total number of changes across all stages. */ + public long getTotalChangeCount() { + long count = 0; + for (StageRun stageRun : stageRuns) { + count += stageRun.getLoadedStage().getChanges().size(); + } + return count; + } + public List getLoadedStages() { return stageRuns.stream() .map(StageRun::getLoadedStage) diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/run/StageRunBlock.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/run/StageRunBlock.java new file mode 100644 index 000000000..22f78c8f7 --- /dev/null +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/run/StageRunBlock.java @@ -0,0 +1,98 @@ +/* + * Copyright 2026 Flamingock (https://www.flamingock.io) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.flamingock.internal.core.pipeline.run; + +import io.flamingock.api.StageType; + +import java.util.Collections; +import java.util.List; + +/** + * Typed window over a group of {@link StageRun}s sharing the same {@link StageType}. Blocks are + * the structural unit of execution dependency: blocks are returned by + * {@link PipelineRun#getStageBlocks()} in dependency order, and a later block depends on the + * successful completion of every prior block. + * + *

This class is intentionally a thin abstraction. It exposes only: + *

    + *
  • The block's {@link StageType}.
    • + *
  • The {@link StageRun}s belonging to it (shared references with {@link PipelineRun}).
    • + *
  • Predicates over the block's collective state: {@link #isTerminal()}, + * {@link #isSuccessful()}, {@link #hasFailures()}.
    • + *
+ * + *

It does NOT decide which stage is "next to execute" — that is the planner's responsibility. + * Consumers wanting the next eligible stage iterate {@link #getStageRuns()} and apply their own + * filter. + */ +public final class StageRunBlock { + + private final StageType type; + private final List stageRuns; + + StageRunBlock(StageType type, List stageRuns) { + this.type = type; + this.stageRuns = Collections.unmodifiableList(stageRuns); + } + + public StageType getType() { + return type; + } + + public List getStageRuns() { + return stageRuns; + } + + /** + * Every stage in this block has reached a terminal state (Completed / Failed / BlockedForMI). + * Vacuous true for an empty block. + */ + public boolean isTerminal() { + for (StageRun stageRun : stageRuns) { + if (!isTerminalState(stageRun)) { + return false; + } + } + return true; + } + + /** + * Every stage in this block ended in {@code Completed}. Vacuous true for an empty block. + * Precondition for the next block to safely begin under the future dependency-gating planner. + */ + public boolean isSuccessful() { + for (StageRun stageRun : stageRuns) { + if (!stageRun.getState().isCompleted()) { + return false; + } + } + return true; + } + + /** At least one stage in this block is {@code Failed} or {@code BlockedForMI}. */ + public boolean hasFailures() { + for (StageRun stageRun : stageRuns) { + if (stageRun.getState().isFailed()) { + return true; + } + } + return false; + } + + private static boolean isTerminalState(StageRun stageRun) { + return stageRun.getState().isCompleted() || stageRun.getState().isFailed(); + } +} diff --git a/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunTest.java b/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunTest.java index 6b473075d..7c8729c4d 100644 --- a/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunTest.java +++ b/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunTest.java @@ -185,12 +185,77 @@ void getLoadedStagesReturnsTheUnderlyingStagesInOrder() { assertSame(b, loaded.get(1)); } + @Test + void getStageBlocksReturnsBlocksInSystemThenLegacyThenDefaultOrder() { + AbstractLoadedStage userStage = mockStage("user", io.flamingock.api.StageType.DEFAULT); + AbstractLoadedStage systemStage = mockStage("system", io.flamingock.api.StageType.SYSTEM); + AbstractLoadedStage legacyStage = mockStage("legacy", io.flamingock.api.StageType.LEGACY); + + // Input order is intentionally NOT dependency order to verify partitioning re-orders. + PipelineRun pipelineRun = PipelineRun.of(Arrays.asList(userStage, systemStage, legacyStage)); + + List blocks = pipelineRun.getStageBlocks(); + assertEquals(3, blocks.size()); + assertSame(io.flamingock.api.StageType.SYSTEM, blocks.get(0).getType()); + assertEquals("system", blocks.get(0).getStageRuns().get(0).getName()); + assertSame(io.flamingock.api.StageType.LEGACY, blocks.get(1).getType()); + assertEquals("legacy", blocks.get(1).getStageRuns().get(0).getName()); + assertSame(io.flamingock.api.StageType.DEFAULT, blocks.get(2).getType()); + assertEquals("user", blocks.get(2).getStageRuns().get(0).getName()); + } + + @Test + void getStageBlocksOmitsEmptyTypes() { + AbstractLoadedStage user1 = mockStage("user1", io.flamingock.api.StageType.DEFAULT); + AbstractLoadedStage user2 = mockStage("user2", io.flamingock.api.StageType.DEFAULT); + + PipelineRun pipelineRun = PipelineRun.of(Arrays.asList(user1, user2)); + + List blocks = pipelineRun.getStageBlocks(); + assertEquals(1, blocks.size()); + assertSame(io.flamingock.api.StageType.DEFAULT, blocks.get(0).getType()); + assertEquals(2, blocks.get(0).getStageRuns().size()); + } + + @Test + void flatStageRunsListIsConcatenationOfBlocksInOrder() { + AbstractLoadedStage userStage = mockStage("user", io.flamingock.api.StageType.DEFAULT); + AbstractLoadedStage systemStage = mockStage("system", io.flamingock.api.StageType.SYSTEM); + + PipelineRun pipelineRun = PipelineRun.of(Arrays.asList(userStage, systemStage)); + + // Flat list is canonicalised: SYSTEM block first, then DEFAULT block, regardless of input + // order. This is the new single source of truth. + List flat = pipelineRun.getStageRuns(); + assertEquals(2, flat.size()); + assertEquals("system", flat.get(0).getName()); + assertEquals("user", flat.get(1).getName()); + } + + @Test + void getTotalChangeCountSumsAcrossAllStages() { + AbstractLoadedStage alpha = mockStageWithChanges("alpha", mockChange("c1"), mockChange("c2")); + AbstractLoadedStage beta = mockStageWithChanges("beta", mockChange("c3")); + + PipelineRun pipelineRun = PipelineRun.of(Arrays.asList(alpha, beta)); + + assertEquals(2, pipelineRun.getStageCount()); + assertEquals(3L, pipelineRun.getTotalChangeCount()); + } + private static AbstractLoadedStage mockStage(String name) { AbstractLoadedStage stage = mock(AbstractLoadedStage.class); when(stage.getName()).thenReturn(name); return stage; } + private static AbstractLoadedStage mockStage(String name, io.flamingock.api.StageType type) { + AbstractLoadedStage stage = mock(AbstractLoadedStage.class); + when(stage.getName()).thenReturn(name); + when(stage.getType()).thenReturn(type); + return stage; + } + private static AbstractLoadedStage mockStageWithChanges(String name, AbstractLoadedChange... changes) { AbstractLoadedStage stage = mock(AbstractLoadedStage.class); when(stage.getName()).thenReturn(name); diff --git a/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/StageRunBlockTest.java b/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/StageRunBlockTest.java new file mode 100644 index 000000000..855cd17f5 --- /dev/null +++ b/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/StageRunBlockTest.java @@ -0,0 +1,159 @@ +/* + * Copyright 2026 Flamingock (https://www.flamingock.io) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.flamingock.internal.core.pipeline.run; + +import io.flamingock.api.StageType; +import io.flamingock.internal.common.core.response.data.ErrorInfo; +import io.flamingock.internal.common.core.response.data.StageResult; +import io.flamingock.internal.common.core.response.data.StageState; +import io.flamingock.internal.core.pipeline.loaded.stage.AbstractLoadedStage; +import org.junit.jupiter.api.Test; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class StageRunBlockTest { + + @Test + void exposesTypeAndStageRuns() { + StageRun a = newStageRun("alpha"); + StageRun b = newStageRun("beta"); + StageRunBlock block = new StageRunBlock(StageType.DEFAULT, Arrays.asList(a, b)); + + assertSame(StageType.DEFAULT, block.getType()); + assertEquals(2, block.getStageRuns().size()); + assertSame(a, block.getStageRuns().get(0)); + assertSame(b, block.getStageRuns().get(1)); + } + + @Test + void isTerminalTrueWhenAllStagesAreCompletedOrFailed() { + StageRun a = newStageRun("alpha", StageState.COMPLETED); + StageRun b = newStageRun("beta", StageState.failed(errorInfo("boom"))); + StageRunBlock block = new StageRunBlock(StageType.DEFAULT, Arrays.asList(a, b)); + + assertTrue(block.isTerminal()); + } + + @Test + void isTerminalFalseIfAnyStageNotStartedOrStarted() { + StageRun a = newStageRun("alpha", StageState.COMPLETED); + StageRun b = newStageRun("beta", StageState.STARTED); + StageRunBlock block = new StageRunBlock(StageType.DEFAULT, Arrays.asList(a, b)); + + assertFalse(block.isTerminal()); + } + + @Test + void isSuccessfulTrueOnlyWhenEveryStageCompleted() { + StageRun a = newStageRun("alpha", StageState.COMPLETED); + StageRun b = newStageRun("beta", StageState.COMPLETED); + StageRunBlock block = new StageRunBlock(StageType.DEFAULT, Arrays.asList(a, b)); + + assertTrue(block.isSuccessful()); + } + + @Test + void isSuccessfulFalseIfAnyStageNotCompleted() { + StageRun a = newStageRun("alpha", StageState.COMPLETED); + StageRun b = newStageRun("beta", StageState.failed(errorInfo("boom"))); + StageRunBlock block = new StageRunBlock(StageType.DEFAULT, Arrays.asList(a, b)); + + assertFalse(block.isSuccessful()); + } + + @Test + void hasFailuresTrueWhenAnyStageFailed() { + StageRun a = newStageRun("alpha", StageState.COMPLETED); + StageRun b = newStageRun("beta", StageState.failed(errorInfo("boom"))); + StageRunBlock block = new StageRunBlock(StageType.DEFAULT, Arrays.asList(a, b)); + + assertTrue(block.hasFailures()); + } + + @Test + void hasFailuresTrueForBlockedForMIBecauseItIsAFailedSubtype() { + StageRun a = newStageRun( + "alpha", + StageState.blockedManualIntervention("alpha", Collections.emptyList())); + StageRunBlock block = new StageRunBlock(StageType.LEGACY, Collections.singletonList(a)); + + assertTrue(block.hasFailures()); + assertTrue(block.isTerminal()); + assertFalse(block.isSuccessful()); + } + + @Test + void hasFailuresFalseWhenAllStagesSuccessfulOrPending() { + StageRun a = newStageRun("alpha", StageState.COMPLETED); + StageRun b = newStageRun("beta", StageState.STARTED); + StageRunBlock block = new StageRunBlock(StageType.DEFAULT, Arrays.asList(a, b)); + + assertFalse(block.hasFailures()); + } + + @Test + void emptyBlockPredicatesAreVacuouslyTrueForTerminalAndSuccessful() { + StageRunBlock block = new StageRunBlock(StageType.SYSTEM, Collections.emptyList()); + + assertTrue(block.isTerminal()); + assertTrue(block.isSuccessful()); + assertFalse(block.hasFailures()); + } + + @Test + void getStageRunsReturnsAnUnmodifiableList() { + StageRun a = newStageRun("alpha"); + StageRunBlock block = new StageRunBlock(StageType.DEFAULT, Collections.singletonList(a)); + + List stageRuns = block.getStageRuns(); + try { + stageRuns.add(newStageRun("beta")); + // If we reach here, the list is mutable — fail the test. + org.junit.jupiter.api.Assertions.fail("Expected getStageRuns() to return an unmodifiable list"); + } catch (UnsupportedOperationException expected) { + // expected + } + } + + // --- helpers --- + + private static StageRun newStageRun(String name) { + AbstractLoadedStage loaded = mock(AbstractLoadedStage.class); + when(loaded.getName()).thenReturn(name); + return new StageRun(loaded); + } + + private static StageRun newStageRun(String name, StageState state) { + StageRun stageRun = newStageRun(name); + stageRun.setResult(StageResult.builder(stageRun.getResult()) + .state(state) + .build()); + return stageRun; + } + + private static ErrorInfo errorInfo(String message) { + return ErrorInfo.fromThrowable(new RuntimeException(message), Collections.emptyList(), null); + } +} From 7f1ee19a4872ee5c89c23be0eeca3d24c972f58b Mon Sep 17 00:00:00 2001 From: Antonio Perez Dieppa Date: Fri, 15 May 2026 20:20:36 +0100 Subject: [PATCH 6/6] refactor: add StageRunBlock to PipelineRun --- .../cloud/planner/CloudExecutionPlanner.java | 7 +- .../planner/CloudExecutionPlanMapperTest.java | 8 +- .../planner/CloudExecutionPlannerTest.java | 15 +- .../response/data/ExecuteResponseData.java | 36 --- .../AbstractPipelineTraverseOperation.java | 15 +- .../core/pipeline/run/PipelineRun.java | 32 +-- .../internal/core/plan/ExecutionPlan.java | 42 ++-- .../community/CommunityExecutionPlanner.java | 138 ++++++++++-- ...AbstractPipelineTraverseOperationTest.java | 212 ++++++++++++++++-- .../operation/ExecuteApplyOperationTest.java | 2 - .../operation/ValidateApplyOperationTest.java | 2 - .../core/pipeline/run/PipelineRunTest.java | 22 -- .../run/PipelineRunToResponseTest.java | 56 +---- .../internal/core/plan/ExecutionPlanTest.java | 22 +- .../CommunityExecutionPlannerTest.java | 119 +++++++++- 15 files changed, 487 insertions(+), 241 deletions(-) diff --git a/cloud/flamingock-cloud/src/main/java/io/flamingock/cloud/planner/CloudExecutionPlanner.java b/cloud/flamingock-cloud/src/main/java/io/flamingock/cloud/planner/CloudExecutionPlanner.java index 15277e280..1f61d1dd8 100644 --- a/cloud/flamingock-cloud/src/main/java/io/flamingock/cloud/planner/CloudExecutionPlanner.java +++ b/cloud/flamingock-cloud/src/main/java/io/flamingock/cloud/planner/CloudExecutionPlanner.java @@ -33,7 +33,6 @@ import io.flamingock.internal.core.plan.ExecutionPlan; import io.flamingock.internal.core.plan.ExecutionPlanner; import io.flamingock.internal.core.external.store.lock.LockException; -import io.flamingock.internal.core.pipeline.execution.ExecutableStage; import io.flamingock.internal.core.pipeline.loaded.stage.AbstractLoadedStage; import io.flamingock.internal.core.pipeline.run.PipelineRun; import io.flamingock.internal.util.log.FlamingockLoggerFactory; @@ -103,8 +102,7 @@ public ExecutionPlan getNextExecution(PipelineRun pipelineRun) throws LockExcept } if (response.isContinue()) { - List executableStages = CloudExecutionPlanMapper.getExecutableStages(response, loadedStages); - return ExecutionPlan.CONTINUE(executableStages); + return ExecutionPlan.CONTINUE(); } else if (response.isExecute()) { Lock lock = CloudLock.initialiseLocal(response.getLock(), coreConfiguration, runnerId, lockService, timeService); @@ -132,8 +130,7 @@ public ExecutionPlan getNextExecution(PipelineRun pipelineRun) throws LockExcept ); } else if (response.isAbort()) { - List stages = CloudExecutionPlanMapper.getExecutableStages(response, loadedStages); - return ExecutionPlan.ABORT(stages); + return ExecutionPlan.ABORT(); } else { throw new RuntimeException("Unrecognized action from response. Not within(CONTINUE, EXECUTE, AWAIT, ABORT)"); diff --git a/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlanMapperTest.java b/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlanMapperTest.java index 2b7c58643..5595a4cdf 100644 --- a/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlanMapperTest.java +++ b/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlanMapperTest.java @@ -29,7 +29,6 @@ import io.flamingock.internal.core.pipeline.execution.ExecutableStage; import io.flamingock.internal.core.pipeline.loaded.stage.AbstractLoadedStage; import io.flamingock.internal.core.pipeline.loaded.stage.DefaultLoadedStage; -import io.flamingock.internal.core.plan.ExecutionPlan; import io.flamingock.core.cloud.changes._001__CloudChange1; import io.flamingock.core.cloud.changes._002__CloudChange2; import org.junit.jupiter.api.BeforeAll; @@ -190,11 +189,10 @@ void shouldBuildAbortPlanFromAbortResponse() { ); List result = CloudExecutionPlanMapper.getExecutableStages(response, loadedStages); - ExecutionPlan plan = ExecutionPlan.ABORT(result); - - assertTrue(plan.isAborted()); - assertFalse(plan.isExecutionRequired()); + // CloudExecutionPlanMapper.getExecutableStages preserves change actions on the mapped + // ExecutableStage regardless of the response action; verify that mapping directly. + // ExecutionPlan.ABORT() no longer carries stages — that's a planner-control-flow concern. Map actions = result.get(0).getChanges().stream() .collect(Collectors.toMap(ExecutableChange::getId, ExecutableChange::getAction)); assertEquals(ChangeAction.MANUAL_INTERVENTION, actions.get(change1.getId())); diff --git a/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlannerTest.java b/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlannerTest.java index 617d0fc83..bbe3cbb6b 100644 --- a/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlannerTest.java +++ b/cloud/flamingock-cloud/src/test/java/io/flamingock/cloud/planner/CloudExecutionPlannerTest.java @@ -26,8 +26,6 @@ import io.flamingock.cloud.api.vo.CloudTargetSystemAuditMarkType; import io.flamingock.cloud.lock.CloudLockService; import io.flamingock.cloud.planner.client.ExecutionPlannerClient; -import io.flamingock.internal.common.core.error.FlamingockException; -import io.flamingock.internal.common.core.recovery.ManualInterventionRequiredException; import io.flamingock.internal.common.core.targets.TargetSystemAuditMarkType; import io.flamingock.internal.core.change.loaded.AbstractLoadedChange; import io.flamingock.internal.core.change.loaded.LoadedChangeBuilder; @@ -98,7 +96,7 @@ private CloudExecutionPlanner buildPlanner(List auditMa } @Test - @DisplayName("Should return ABORT plan when server returns ABORT with MANUAL_INTERVENTION changes") + @DisplayName("Should return ABORT plan when server returns ABORT (regardless of change actions)") void shouldReturnAbortPlanWhenServerReturnsAbort() { CloudExecutionPlanner planner = buildPlanner(Collections.emptyList()); @@ -114,16 +112,14 @@ void shouldReturnAbortPlanWhenServerReturnsAbort() { ExecutionPlan plan = planner.getNextExecution(PipelineRun.of(stages)); - // ExecutionPlan.validate() now only signals the abort itself; MI is a per-stage concern - // enforced by ExecutableStage.validate() inside the operation lambda. + // ABORT is a control-flow signal — the operation reads isAborted() and breaks the loop. + // No exception flows from ExecutionPlan itself. MI per-stage is tested separately on + // ExecutableStage.validate() (see core's ExecutableStageTest / AbstractPipelineTraverseOperationTest). assertTrue(plan.isAborted()); - assertThrows(FlamingockException.class, plan::validate); - assertThrows(ManualInterventionRequiredException.class, - () -> plan.getExecutableStages().get(0).validate()); } @Test - @DisplayName("Should return ABORT plan that throws FlamingockException when server returns ABORT but no MI changes") + @DisplayName("Should return ABORT plan when server returns ABORT (no MI changes)") void shouldReturnAbortPlanWhenServerReturnsAbortWithNoMIChanges() { CloudExecutionPlanner planner = buildPlanner(Collections.emptyList()); @@ -140,7 +136,6 @@ void shouldReturnAbortPlanWhenServerReturnsAbortWithNoMIChanges() { ExecutionPlan plan = planner.getNextExecution(PipelineRun.of(stages)); assertTrue(plan.isAborted()); - assertThrows(io.flamingock.internal.common.core.error.FlamingockException.class, plan::validate); } @Test diff --git a/core/flamingock-core-commons/src/main/java/io/flamingock/internal/common/core/response/data/ExecuteResponseData.java b/core/flamingock-core-commons/src/main/java/io/flamingock/internal/common/core/response/data/ExecuteResponseData.java index 2bf9273c5..c29ebe045 100644 --- a/core/flamingock-core-commons/src/main/java/io/flamingock/internal/common/core/response/data/ExecuteResponseData.java +++ b/core/flamingock-core-commons/src/main/java/io/flamingock/internal/common/core/response/data/ExecuteResponseData.java @@ -44,15 +44,6 @@ public class ExecuteResponseData { // Per-stage breakdown private List stages; - /** - * @deprecated Per-stage error info is the authoritative carrier (see {@link StageResult} / - * {@code StageState.getErrorInfo()}). The top-level field is kept temporarily to preserve the - * JSON contract with the CLI; it is populated by {@code PipelineRun.toResponse()} from the - * first failed stage's {@link ErrorInfo} and will be removed in a follow-up. - */ - @Deprecated - private ErrorInfo error; - public ExecuteResponseData() { this.stages = new ArrayList<>(); } @@ -70,7 +61,6 @@ private ExecuteResponseData(Builder builder) { this.skippedChanges = builder.skippedChanges; this.failedChanges = builder.failedChanges; this.stages = builder.stages != null ? builder.stages : new ArrayList<>(); - this.error = builder.error; } public ExecutionStatus getStatus() { @@ -169,22 +159,6 @@ public void setStages(List stages) { this.stages = stages; } - /** - * @deprecated See {@link #error}. - */ - @Deprecated - public ErrorInfo getError() { - return error; - } - - /** - * @deprecated See {@link #error}. - */ - @Deprecated - public void setError(ErrorInfo error) { - this.error = error; - } - public boolean isSuccess() { return status == ExecutionStatus.SUCCESS || status == ExecutionStatus.NO_CHANGES; } @@ -210,7 +184,6 @@ public static class Builder { private int skippedChanges; private int failedChanges; private List stages = new ArrayList<>(); - private ErrorInfo error; public Builder status(ExecutionStatus status) { this.status = status; @@ -280,15 +253,6 @@ public Builder addStage(StageResult stage) { return this; } - /** - * @deprecated See {@link ExecuteResponseData#error}. - */ - @Deprecated - public Builder error(ErrorInfo error) { - this.error = error; - return this; - } - public ExecuteResponseData build() { return new ExecuteResponseData(this); } diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperation.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperation.java index 0b2493a01..5e5e58e50 100644 --- a/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperation.java +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperation.java @@ -19,6 +19,7 @@ import io.flamingock.internal.common.core.recovery.ManualInterventionRequiredException; import io.flamingock.internal.common.core.error.PendingChangesException; import io.flamingock.internal.common.core.response.data.ExecuteResponseData; +import io.flamingock.internal.common.core.response.data.ExecutionStatus; import io.flamingock.internal.core.event.EventPublisher; import io.flamingock.internal.core.event.model.impl.PipelineCompletedEvent; import io.flamingock.internal.core.event.model.impl.PipelineFailedEvent; @@ -121,7 +122,10 @@ private ExecuteResult execute(LoadedPipeline pipeline) { do { try (ExecutionPlan execution = executionPlanner.getNextExecution(pipelineRun)) { - execution.validate(); + if (execution.isAborted()) { + logger.info("Pipeline execution aborted by the planner — earlier block has failures or another structural reason"); + break; + } if (!execution.isExecutionRequired()) { break; @@ -134,7 +138,6 @@ private ExecuteResult execute(LoadedPipeline pipeline) { execution.applyOnEach((executionId, lock, executableStage) -> runStage(executionId, lock, executableStage, pipelineRun)); } catch (LockException exception) { - pipelineRun.markPipelineFailed(exception); pipelineLevelError = exception; if (throwExceptionIfCannotObtainLock) { logger.debug("Required process lock not acquired - ABORTING OPERATION", exception); @@ -143,17 +146,19 @@ private ExecuteResult execute(LoadedPipeline pipeline) { throwPipelineLevelError = false; } break; - } catch (Throwable exception) { - pipelineRun.markPipelineFailed(exception); + } catch (Throwable exception) { pipelineLevelError = exception; break; - }//FlamingockException + } } while (true); pipelineRun.stop(); ExecuteResponseData result = pipelineRun.toResponse(); if (pipelineLevelError != null) { + // toResponse() derives status from per-stage state; override to FAILED so the + // response reflects the pipeline-wide error carried in pipelineLevelError. + result.setStatus(ExecutionStatus.FAILED); logger.debug("Error executing the process. ABORTED OPERATION", pipelineLevelError); eventPublisher.publish(new PipelineFailedEvent(toException(pipelineLevelError))); if (throwPipelineLevelError) { diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/run/PipelineRun.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/run/PipelineRun.java index 89e6e05ef..ae31c1f95 100644 --- a/core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/run/PipelineRun.java +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/pipeline/run/PipelineRun.java @@ -36,7 +36,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.stream.Collectors; public class PipelineRun { @@ -98,7 +97,6 @@ private static StageType resolveType(AbstractLoadedStage stage) { private final Map byName; private Instant startedAt; private Instant stoppedAt; - private ErrorInfo pipelineError; private PipelineRun(List stageRuns, List stageBlocks) { this.stageRuns = Collections.unmodifiableList(stageRuns); @@ -205,19 +203,6 @@ public void markStageBlockedFromMI(String stageName, List issues) .build()); } - /** - * Pipeline-wide failure marker. Idempotent: first call wins. - */ - public void markPipelineFailed(Throwable cause) { - if (this.pipelineError == null) { - this.pipelineError = ErrorInfo.fromThrowable(cause, Collections.emptyList(), null); - } - } - - public Optional getPipelineError() { - return Optional.ofNullable(pipelineError); - } - private StageRun lookupOrThrow(String stageName) { StageRun run = byName.get(stageName); if (run == null) { @@ -226,6 +211,12 @@ private StageRun lookupOrThrow(String stageName) { return run; } + /** + * Builds the response data view derived purely from per-stage state. Status is FAILED iff any + * stage failed, else SUCCESS. Pipeline-wide errors (e.g., LockException) that aren't reflected + * in any stage state are signalled by the operation post-loop via + * {@code ExecuteResponseData.setStatus(FAILED)} after {@code toResponse()} returns. + */ public ExecuteResponseData toResponse() { List stages = new ArrayList<>(); int totalStages = 0; @@ -235,7 +226,6 @@ public ExecuteResponseData toResponse() { int appliedChanges = 0; int skippedChanges = 0; int failedChanges = 0; - ErrorInfo error = null; boolean anyFailed = false; for (StageRun stageRun : stageRuns) { @@ -249,9 +239,6 @@ public ExecuteResponseData toResponse() { } else if (state.isFailed()) { failedStages++; anyFailed = true; - if (error == null) { - error = state.getErrorInfo().orElse(null); - } } if (stageResult.getChanges() != null) { @@ -268,11 +255,7 @@ public ExecuteResponseData toResponse() { } } - if (error == null) { - error = this.pipelineError; - } - boolean pipelineFailed = anyFailed || this.pipelineError != null; - ExecutionStatus status = pipelineFailed ? ExecutionStatus.FAILED : ExecutionStatus.SUCCESS; + ExecutionStatus status = anyFailed ? ExecutionStatus.FAILED : ExecutionStatus.SUCCESS; long durationMs = (startedAt != null && stoppedAt != null) ? Duration.between(startedAt, stoppedAt).toMillis() : 0L; @@ -290,7 +273,6 @@ public ExecuteResponseData toResponse() { .skippedChanges(skippedChanges) .failedChanges(failedChanges) .stages(stages) - .error(error) .build(); } } diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/ExecutionPlan.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/ExecutionPlan.java index ab6b5bd1a..e270cf7fb 100644 --- a/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/ExecutionPlan.java +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/ExecutionPlan.java @@ -18,25 +18,41 @@ import io.flamingock.internal.util.TriConsumer; import io.flamingock.internal.core.external.store.lock.Lock; import io.flamingock.internal.core.pipeline.execution.ExecutableStage; -import io.flamingock.internal.common.core.error.FlamingockException; +import java.util.Collections; import java.util.List; +/** + * One iteration's planner verdict. + * + *

    + *
  • {@link #newExecution(String, Lock, List)} — carries a list of stages to execute under the + * given lock. {@link #isExecutionRequired()} reflects whether any of those stages still has + * work pending.
    • + *
  • {@link #CONTINUE()} — pipeline finished. Successfully. Nothing else to do.
    • + *
  • {@link #ABORT()} — stop early. Something went wrong (e.g., an earlier block failed and + * its dependents cannot proceed). The operation reads {@link #isAborted()} to break out of + * the run loop.
    • + *
+ * + *

{@code CONTINUE} and {@code ABORT} carry no stages — the run-loop only needs the verdict. + * The pipeline-level state lives in {@code PipelineRun}; block-aware queries should read + * {@code PipelineRun.getStageBlocks()} directly. + */ public class ExecutionPlan implements AutoCloseable { - public static ExecutionPlan newExecution(String executionId, Lock lock, List stages) { return new ExecutionPlan(executionId, lock, stages); } - public static ExecutionPlan CONTINUE(List stages) { - return new ExecutionPlan(false, stages); + public static ExecutionPlan CONTINUE() { + return new ExecutionPlan(false, Collections.emptyList()); } - public static ExecutionPlan ABORT(List stages) { - return new ExecutionPlan(true, stages); + public static ExecutionPlan ABORT() { + return new ExecutionPlan(true, Collections.emptyList()); } private final String executionId; @@ -80,20 +96,6 @@ public void applyOnEach(TriConsumer consumer) { } } - /** - * Validates the execution plan. If the planner decided to abort the run for reasons beyond - * individual change state, throws {@link FlamingockException}. - * - *

Manual-intervention validation is no longer performed here: it is per-stage and lives in - * {@code ExecutableStage.validate()}, called inside the operation lambda so a single stage's - * MI state never aborts the rest of the pipeline. - */ - public void validate() { - if (aborted) { - throw new FlamingockException("Execution aborted by the execution planner"); - } - } - @Override public void close() { if (lock != null) { diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlanner.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlanner.java index 90dae0092..263b24a87 100644 --- a/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlanner.java +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlanner.java @@ -18,6 +18,7 @@ import io.flamingock.internal.common.core.audit.AuditEntry; import io.flamingock.internal.common.core.recovery.action.ChangeActionMap; +import io.flamingock.internal.common.core.response.data.StageResult; import io.flamingock.internal.core.plan.ExecutionId; import io.flamingock.internal.core.external.store.lock.community.CommunityLock; import io.flamingock.internal.core.external.store.lock.community.CommunityLockService; @@ -32,6 +33,7 @@ import io.flamingock.internal.core.pipeline.loaded.stage.AbstractLoadedStage; import io.flamingock.internal.core.pipeline.run.PipelineRun; import io.flamingock.internal.core.pipeline.run.StageRun; +import io.flamingock.internal.core.pipeline.run.StageRunBlock; import io.flamingock.internal.core.external.store.audit.community.CommunityAuditReader; import io.flamingock.internal.util.id.RunnerId; import io.flamingock.internal.util.TimeService; @@ -44,6 +46,8 @@ import java.util.Optional; import java.util.stream.Collectors; +import static io.flamingock.internal.common.core.response.data.StageState.COMPLETED; + public class CommunityExecutionPlanner extends ExecutionPlanner { private static final Logger logger = FlamingockLoggerFactory.getLogger("LocalExecution"); @@ -115,26 +119,55 @@ public CommunityExecutionPlanner(RunnerId instanceId, *

Error Handling: If any exception occurs after acquiring the lock, the lock is released * in the catch block to prevent lock leaks.

* - * @param pipelineRun the in-flight run aggregate; this implementation considers only the - * stages still eligible for planning — i.e., it excludes stages whose - * state has reached a terminal failed shape - * ({@code StageState.Failed} or its subclass {@code BlockedForMI}). See - * {@link #stagesEligibleForPlanning(PipelineRun)}. - * @return ExecutionPlan containing either stages to execute (with lock held) or CONTINUE (no lock) + * @param pipelineRun the in-flight run aggregate. The planner walks + * {@link PipelineRun#getStageBlocks()} in dependency order and plans from + * the first non-terminal block. Blocks already terminal+successful are + * skipped; if a terminal block has failures, the planner returns + * {@code ABORT} so the operation stops before later-dependent blocks run. + * @return ExecutionPlan: {@code newExecution} with a single stage when there's work, + * {@code CONTINUE} when the pipeline is fully done, or {@code ABORT} when an earlier + * block has failures and downstream work must not proceed. * @throws LockException if unable to acquire the distributed lock within the configured timeout */ @Override public ExecutionPlan getNextExecution(PipelineRun pipelineRun) throws LockException { - List loadedStages = stagesEligibleForPlanning(pipelineRun); - Map initialSnapshot = auditReader.getAuditSnapshotByChangeId(); - logger.debug("Pulled initial remote state:\n{}", initialSnapshot); + // Inner loop: skip past blocks whose stages have no work to do per audit (e.g., legacy + // stages whose changes were already applied in a previous run). Such blocks are not + // marked Completed by the runtime — there's no executor invocation — so we mark them + // here, then re-run block selection to advance to the next block. The loop terminates + // either when we find work (return newExecution) or when block selection yields + // CONTINUE / ABORT. + while (true) { + BlockSelection selection = selectActiveBlock(pipelineRun); + if (selection.isAborted()) { + return ExecutionPlan.ABORT(); + } + if (!selection.getActiveBlock().isPresent()) { + return ExecutionPlan.CONTINUE(); + } - List initialStages = buildExecutableStages(loadedStages, initialSnapshot); + StageRunBlock activeBlock = selection.getActiveBlock().get(); + List loadedStages = eligibleLoadedStagesFor(activeBlock); + Map initialSnapshot = auditReader.getAuditSnapshotByChangeId(); + logger.debug("Pulled initial remote state:\n{}", initialSnapshot); - if (!hasExecutableStages(initialStages)) { - return ExecutionPlan.CONTINUE(initialStages); + List initialStages = buildExecutableStages(loadedStages, initialSnapshot); + + if (!hasExecutableStages(initialStages)) { + // Optimistic view says no work in the active block. The block's stages are + // effectively done per audit — mark them Completed (synthetically) and advance to + // the next block. Trust the optimistic view; the two-phase dance below is only + // needed when we actually have work to do under the lock. + markBlockSyntheticallyCompleted(pipelineRun, activeBlock); + continue; + } + + return planWorkUnderLock(loadedStages, initialStages); } + } + private ExecutionPlan planWorkUnderLock(List loadedStages, + List initialStages) throws LockException { Lock lock = acquireLock(); try { @@ -151,7 +184,7 @@ public ExecutionPlan getNextExecution(PipelineRun pipelineRun) throws LockExcept "Releasing lock and continuing." ); lock.release(); - return ExecutionPlan.CONTINUE(validatedStages); + return ExecutionPlan.CONTINUE(); } logPlanChanges(initialStages, validatedStages); @@ -173,19 +206,84 @@ public ExecutionPlan getNextExecution(PipelineRun pipelineRun) throws LockExcept } /** - * Returns the loaded stages this planner considers eligible to plan in the current iteration. - * - *

Today's policy: exclude any stage already in a terminal failed shape - * ({@code StageState.Failed} or its subclass {@code BlockedForMI}). Future evolution may - * narrow this (e.g., retry stages that failed with a recoverable cause). + * Walks {@link PipelineRun#getStageBlocks()} in dependency order and decides what to do: + *

    + *
  • First non-terminal block → that's the active block. Plan from it.
    • + *
  • Block is terminal + has failures → ABORT (downstream blocks must not run).
    • + *
  • Block is terminal + successful → skip and check the next block.
    • + *
  • All blocks terminal + successful → pipeline is fully done, return ALL_DONE.
    • + *
*/ - private static List stagesEligibleForPlanning(PipelineRun pipelineRun) { - return pipelineRun.getStageRuns().stream() + private static BlockSelection selectActiveBlock(PipelineRun pipelineRun) { + for (StageRunBlock block : pipelineRun.getStageBlocks()) { + if (!block.isTerminal()) { + return BlockSelection.proceed(block); + } + if (block.hasFailures()) { + return BlockSelection.ABORT; + } + // terminal + successful → move to next block + } + return BlockSelection.ALL_DONE; + } + + /** + * Eligible loaded stages within the active block: every stage in the block whose runtime + * state is not yet a terminal failed shape ({@code Failed} / {@code BlockedForMI}). Within + * a non-terminal block these are the stages still candidates for execution. + */ + private static List eligibleLoadedStagesFor(StageRunBlock activeBlock) { + return activeBlock.getStageRuns().stream() .filter(stageRun -> !stageRun.getState().isFailed()) .map(StageRun::getLoadedStage) .collect(Collectors.toList()); } + /** + * Marks every non-failed, non-completed stage in the block as Completed. Used when the + * planner detects that the active block has no executable work per audit — its stages are + * already done from the runtime's perspective, so we reflect that in {@link PipelineRun} and + * advance to the next block. + */ + private static void markBlockSyntheticallyCompleted(PipelineRun pipelineRun, StageRunBlock block) { + for (StageRun stageRun : block.getStageRuns()) { + if (stageRun.getState().isFailed() || stageRun.getState().isCompleted()) { + continue; + } + StageResult completed = StageResult.builder(stageRun.getResult()).state(COMPLETED).build(); + pipelineRun.markStageCompleted(stageRun.getName(), completed); + } + } + + /** + * Compact three-state value: ABORT (singleton), ALL_DONE (singleton), or PROCEED with an + * active block. Avoids leaking the "decision shape" outside the planner. + */ + private static final class BlockSelection { + private static final BlockSelection ABORT = new BlockSelection(null, true); + private static final BlockSelection ALL_DONE = new BlockSelection(null, false); + + static BlockSelection proceed(StageRunBlock block) { + return new BlockSelection(block, false); + } + + private final StageRunBlock activeBlock; + private final boolean aborted; + + private BlockSelection(StageRunBlock activeBlock, boolean aborted) { + this.activeBlock = activeBlock; + this.aborted = aborted; + } + + public Optional getActiveBlock() { + return Optional.ofNullable(activeBlock); + } + + public boolean isAborted() { + return aborted; + } + } + private Lock acquireLock() { return CommunityLock.getLock( configuration.getLockAcquiredForMillis(), diff --git a/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperationTest.java b/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperationTest.java index e40b5a903..66c93a9d2 100644 --- a/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperationTest.java +++ b/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/AbstractPipelineTraverseOperationTest.java @@ -15,27 +15,36 @@ */ package io.flamingock.internal.core.operation; -import io.flamingock.internal.common.core.error.FlamingockException; +import io.flamingock.api.StageType; import io.flamingock.internal.common.core.recovery.action.ChangeAction; +import io.flamingock.internal.common.core.response.data.ExecuteResponseData; +import io.flamingock.internal.common.core.response.data.ExecutionStatus; import io.flamingock.internal.common.core.response.data.StageResult; +import io.flamingock.internal.common.core.response.data.StageState; import io.flamingock.internal.core.change.executable.ExecutableChange; import io.flamingock.internal.core.change.loaded.AbstractLoadedChange; import io.flamingock.internal.core.event.EventPublisher; import io.flamingock.internal.core.operation.execute.ExecuteApplyOperation; import io.flamingock.internal.core.operation.execute.ExecuteArgs; +import io.flamingock.internal.core.operation.execute.ExecuteResult; import io.flamingock.internal.core.pipeline.execution.ExecutableStage; import io.flamingock.internal.core.pipeline.execution.OrphanExecutionContext; +import io.flamingock.internal.core.pipeline.execution.StageExecutionException; import io.flamingock.internal.core.pipeline.execution.StageExecutor; import io.flamingock.internal.core.pipeline.loaded.LoadedPipeline; import io.flamingock.internal.core.pipeline.loaded.stage.AbstractLoadedStage; import io.flamingock.internal.core.pipeline.run.PipelineRun; import io.flamingock.internal.core.plan.ExecutionPlan; import io.flamingock.internal.core.plan.ExecutionPlanner; +import io.flamingock.internal.util.TriConsumer; import io.flamingock.internal.util.id.RunnerId; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import java.util.Arrays; import java.util.Collections; +import java.util.List; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.any; @@ -49,7 +58,7 @@ void miChangesPerStageShouldBlockStageAndThrowStagedException() { ExecutableChange miChange = mockChange("change-1", ChangeAction.MANUAL_INTERVENTION); ExecutableStage stage = new ExecutableStage("stage-1", Collections.singletonList(miChange)); ExecutionPlan plan = ExecutionPlan.newExecution("exec-1", null, Collections.singletonList(stage)); - ExecutionPlan continuePlan = ExecutionPlan.CONTINUE(Collections.emptyList()); + ExecutionPlan continuePlan = ExecutionPlan.CONTINUE(); ExecutionPlanner planner = mock(ExecutionPlanner.class); when(planner.getNextExecution(any(PipelineRun.class))).thenReturn(plan, continuePlan); @@ -69,11 +78,14 @@ void miChangesPerStageShouldBlockStageAndThrowStagedException() { } @Test - @DisplayName("ABORT plan without MI should throw PipelineExecuteOperationException with abort cause") - void abortPlanShouldThrowPipelineExecuteOperationException() { - ExecutableChange change = mockChange("change-1", ChangeAction.APPLY); - ExecutableStage stage = new ExecutableStage("stage-1", Collections.singletonList(change)); - ExecutionPlan abortPlan = ExecutionPlan.ABORT(Collections.singletonList(stage)); + @DisplayName("ABORT plan without pre-existing stage failures should exit the loop cleanly without throwing") + void abortPlanWithoutStageFailuresExitsCleanly() { + // Under the new ABORT/CONTINUE semantics, ABORT is a control-flow signal — the operation + // breaks the loop and returns the result. In production, ABORT is only returned by the + // planner when stage failures are already recorded in PipelineRun; the realistic path is + // covered by miChangesPerStageShouldBlockStageAndThrowStagedException. This test + // documents the synthetic "ABORT with no failures" case: no exception, no work done. + ExecutionPlan abortPlan = ExecutionPlan.ABORT(); ExecutionPlanner planner = mock(ExecutionPlanner.class); when(planner.getNextExecution(any(PipelineRun.class))).thenReturn(abortPlan); @@ -82,19 +94,185 @@ void abortPlanShouldThrowPipelineExecuteOperationException() { LoadedPipeline pipeline = mockPipeline(); ExecuteApplyOperation operation = buildOperation(planner, stageExecutor); - ExecuteOperationException ex = assertThrows( - PipelineExecuteOperationException.class, - () -> operation.execute(new ExecuteArgs(pipeline))); - assertNotNull(ex.getResult()); - // The abort is signalled as a generic FlamingockException carrying the planner message. - assertTrue(ex.getMessage() != null && ex.getMessage().contains("Execution aborted") - || (ex.getCause() instanceof FlamingockException - && ex.getCause().getMessage().contains("Execution aborted")), - "Expected the planner abort message to be propagated, got: " + ex.getMessage() - + " / cause=" + ex.getCause()); + operation.execute(new ExecuteArgs(pipeline)); // returns normally — no exception verify(stageExecutor, never()).executeStage(any(), any(), any()); } + @Test + @DisplayName("Should execute multiple blocks in dependency order (happy path: SYSTEM -> LEGACY -> DEFAULT, all succeed)") + void shouldExecuteMultipleBlocksInDependencyOrderHappyPath() { + // Three executable stages, one per block. The mocked planner returns a newExecution plan + // for each in dependency order, then CONTINUE() when there's no more work. The operation + // drives the loop; we verify the executor was invoked once per stage in the right order + // and the response reflects a clean success. + ExecutableStage systemExec = mockExecutableStage("flamingock-system-stage"); + ExecutableStage legacyExec = mockExecutableStage("flamingock-legacy-stage"); + ExecutableStage defaultExec = mockExecutableStage("changes"); + + StageExecutor stageExecutor = mock(StageExecutor.class); + when(stageExecutor.executeStage(any(), any(), any())).thenAnswer(invocation -> { + ExecutableStage es = invocation.getArgument(0); + return successOutput(es.getName()); + }); + + ExecutionPlan planSystem = mockPlanInvokingConsumerWith(systemExec); + ExecutionPlan planLegacy = mockPlanInvokingConsumerWith(legacyExec); + ExecutionPlan planDefault = mockPlanInvokingConsumerWith(defaultExec); + ExecutionPlan donePlan = mockContinuePlan(); + + ExecutionPlanner planner = mock(ExecutionPlanner.class); + when(planner.getNextExecution(any(PipelineRun.class))) + .thenReturn(planSystem, planLegacy, planDefault, donePlan); + + LoadedPipeline pipeline = mockMultiBlockPipeline(); + ExecuteApplyOperation operation = buildOperation(planner, stageExecutor); + + ExecuteResult result = operation.execute(new ExecuteArgs(pipeline)); + + // Response status is SUCCESS and all three stages ran. + ExecuteResponseData response = result.getData(); + assertEquals(ExecutionStatus.SUCCESS, response.getStatus()); + assertEquals(3, response.getCompletedStages()); + assertEquals(0, response.getFailedStages()); + + // Verify execution order: SYSTEM first, LEGACY second, DEFAULT third. + ArgumentCaptor captor = ArgumentCaptor.forClass(ExecutableStage.class); + verify(stageExecutor, times(3)).executeStage(captor.capture(), any(), any()); + List executed = captor.getAllValues(); + assertEquals("flamingock-system-stage", executed.get(0).getName()); + assertEquals("flamingock-legacy-stage", executed.get(1).getName()); + assertEquals("changes", executed.get(2).getName()); + } + + @Test + @DisplayName("Should throw StagedExecuteOperationException with downstream block stages NOT_STARTED when an earlier block fails") + void shouldThrowStagedExceptionWithDownstreamStagesNotStartedWhenEarlierBlockFails() { + // SYSTEM stage fails. The planner then returns ABORT() (because SYSTEM block becomes + // terminal+hasFailures). Downstream blocks (LEGACY, DEFAULT) must stay NOT_STARTED. + ExecutableStage systemExec = mockExecutableStage("flamingock-system-stage"); + + StageExecutor stageExecutor = mock(StageExecutor.class); + StageExecutionException systemFailure = StageExecutionException.fromResult( + new RuntimeException("system boom"), + failedStageResult("flamingock-system-stage"), + "sys-c1"); + when(stageExecutor.executeStage(eq(systemExec), any(), any())).thenThrow(systemFailure); + + ExecutionPlan planSystem = mockPlanInvokingConsumerWith(systemExec); + ExecutionPlan abortPlan = ExecutionPlan.ABORT(); + + ExecutionPlanner planner = mock(ExecutionPlanner.class); + when(planner.getNextExecution(any(PipelineRun.class))) + .thenReturn(planSystem, abortPlan); + + LoadedPipeline pipeline = mockMultiBlockPipeline(); + ExecuteApplyOperation operation = buildOperation(planner, stageExecutor); + + StagedExecuteOperationException ex = assertThrows( + StagedExecuteOperationException.class, + () -> operation.execute(new ExecuteArgs(pipeline))); + + ExecuteResponseData response = ex.getResult(); + assertEquals(ExecutionStatus.FAILED, response.getStatus()); + assertEquals(3, response.getTotalStages()); + assertEquals(0, response.getCompletedStages()); + assertEquals(1, response.getFailedStages()); + + // Per-stage: SYSTEM failed, LEGACY + DEFAULT untouched (NOT_STARTED). + StageResult systemResult = findStage(response, "flamingock-system-stage"); + assertTrue(systemResult.getState().isFailed(), + "SYSTEM stage should be Failed after its only executable stage failed"); + + StageResult legacyResult = findStage(response, "flamingock-legacy-stage"); + assertTrue(legacyResult.getState().isNotStarted(), + "LEGACY stage must remain NOT_STARTED — its block is downstream of a failed block"); + + StageResult defaultResult = findStage(response, "changes"); + assertTrue(defaultResult.getState().isNotStarted(), + "DEFAULT stage must remain NOT_STARTED — its block is downstream of a failed block"); + + // Executor was called exactly once — only for SYSTEM. + verify(stageExecutor, times(1)).executeStage(any(), any(), any()); + } + + // ─────────────────────────── Helpers ─────────────────────────── + + private static StageResult findStage(ExecuteResponseData response, String name) { + return response.getStages().stream() + .filter(s -> name.equals(s.getStageName())) + .findFirst() + .orElseThrow(() -> new AssertionError("Stage not found in response: " + name)); + } + + private static ExecutableStage mockExecutableStage(String name) { + ExecutableStage stage = mock(ExecutableStage.class); + when(stage.getName()).thenReturn(name); + doNothing().when(stage).validate(); + return stage; + } + + private static StageExecutor.Output successOutput(String stageName) { + StageResult result = StageResult.builder() + .stageId(stageName) + .stageName(stageName) + .state(StageState.COMPLETED) + .build(); + StageExecutor.Output output = mock(StageExecutor.Output.class); + when(output.getResult()).thenReturn(result); + return output; + } + + private static StageResult failedStageResult(String stageName) { + return StageResult.builder() + .stageId(stageName) + .stageName(stageName) + .state(StageState.failed(null)) + .build(); + } + + private static ExecutionPlan mockPlanInvokingConsumerWith(ExecutableStage executableStage) { + ExecutionPlan plan = mock(ExecutionPlan.class); + when(plan.isAborted()).thenReturn(false); + when(plan.isExecutionRequired()).thenReturn(true); + doNothing().when(plan).close(); + doAnswer(invocation -> { + @SuppressWarnings("unchecked") + TriConsumer consumer = + invocation.getArgument(0); + consumer.accept("exec-1", null, executableStage); + return null; + }).when(plan).applyOnEach(any()); + return plan; + } + + private static ExecutionPlan mockContinuePlan() { + ExecutionPlan plan = mock(ExecutionPlan.class); + when(plan.isAborted()).thenReturn(false); + when(plan.isExecutionRequired()).thenReturn(false); + doNothing().when(plan).close(); + return plan; + } + + private static LoadedPipeline mockMultiBlockPipeline() { + AbstractLoadedStage systemLoaded = mockNamedTypedLoadedStage("flamingock-system-stage", StageType.SYSTEM); + AbstractLoadedStage legacyLoaded = mockNamedTypedLoadedStage("flamingock-legacy-stage", StageType.LEGACY); + AbstractLoadedStage defaultLoaded = mockNamedTypedLoadedStage("changes", StageType.DEFAULT); + + LoadedPipeline pipeline = mock(LoadedPipeline.class); + when(pipeline.getSystemStage()).thenReturn(java.util.Optional.of(systemLoaded)); + when(pipeline.getStages()).thenReturn(Arrays.asList(legacyLoaded, defaultLoaded)); + doNothing().when(pipeline).validate(); + return pipeline; + } + + private static AbstractLoadedStage mockNamedTypedLoadedStage(String name, StageType type) { + AbstractLoadedStage stage = mock(AbstractLoadedStage.class); + when(stage.getName()).thenReturn(name); + when(stage.getType()).thenReturn(type); + when(stage.getChanges()).thenReturn(Collections.emptyList()); + return stage; + } + private static ExecuteApplyOperation buildOperation(ExecutionPlanner planner, StageExecutor stageExecutor) { return new ExecuteApplyOperation( RunnerId.fromString("test"), diff --git a/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/ExecuteApplyOperationTest.java b/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/ExecuteApplyOperationTest.java index 566dba663..cf516ba5c 100644 --- a/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/ExecuteApplyOperationTest.java +++ b/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/ExecuteApplyOperationTest.java @@ -222,7 +222,6 @@ private StageResult createFailedStageResult(String stageId, String failedChangeI private ExecutionPlan mockNoExecutionRequiredPlan() { ExecutionPlan plan = mock(ExecutionPlan.class); when(plan.isExecutionRequired()).thenReturn(false); - doNothing().when(plan).validate(); doNothing().when(plan).close(); return plan; } @@ -230,7 +229,6 @@ private ExecutionPlan mockNoExecutionRequiredPlan() { private ExecutionPlan mockPlanInvokingConsumerWith(ExecutableStage executableStage) { ExecutionPlan plan = mock(ExecutionPlan.class); when(plan.isExecutionRequired()).thenReturn(true); - doNothing().when(plan).validate(); doNothing().when(plan).close(); doAnswer(invocation -> { @SuppressWarnings("unchecked") diff --git a/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/ValidateApplyOperationTest.java b/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/ValidateApplyOperationTest.java index 6574471f7..301e3f7ef 100644 --- a/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/ValidateApplyOperationTest.java +++ b/core/flamingock-core/src/test/java/io/flamingock/internal/core/operation/ValidateApplyOperationTest.java @@ -167,7 +167,6 @@ void shouldThrowPendingChangesExceptionWhenPendingChangesExist() throws Exceptio private ExecutionPlan mockNoPendingPlan() { ExecutionPlan plan = mock(ExecutionPlan.class); when(plan.isExecutionRequired()).thenReturn(false); - doNothing().when(plan).validate(); doNothing().when(plan).close(); return plan; } @@ -180,7 +179,6 @@ private ExecutionPlan mockPendingPlan(List stages) { ExecutionPlan plan = mock(ExecutionPlan.class); when(plan.isExecutionRequired()).thenReturn(true); when(plan.getExecutableStages()).thenReturn(stages); - doNothing().when(plan).validate(); doNothing().when(plan).close(); return plan; } diff --git a/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunTest.java b/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunTest.java index 7c8729c4d..c9b12206e 100644 --- a/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunTest.java +++ b/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunTest.java @@ -28,7 +28,6 @@ import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -120,27 +119,6 @@ void markStageFailedWithThrowableSynthesisesErrorInfo() { assertEquals("boom", info.getMessage()); } - @Test - void markPipelineFailedIsIdempotentFirstWins() { - AbstractLoadedStage a = mockStage("alpha"); - PipelineRun pipelineRun = PipelineRun.of(java.util.Collections.singletonList(a)); - - pipelineRun.markPipelineFailed(new RuntimeException("first")); - pipelineRun.markPipelineFailed(new IllegalStateException("second")); - - ErrorInfo info = pipelineRun.getPipelineError().get(); - assertEquals("RuntimeException", info.getErrorType()); - assertEquals("first", info.getMessage()); - } - - @Test - void getPipelineErrorIsEmptyUntilMarked() { - AbstractLoadedStage a = mockStage("alpha"); - PipelineRun pipelineRun = PipelineRun.of(java.util.Collections.singletonList(a)); - - assertFalse(pipelineRun.getPipelineError().isPresent()); - } - @Test void markStageBlockedFromMISetsBlockedForMIState() { AbstractLoadedChange alphaChange = mockChange("alpha-c1"); diff --git a/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunToResponseTest.java b/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunToResponseTest.java index ce1eceeee..df76a8a46 100644 --- a/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunToResponseTest.java +++ b/core/flamingock-core/src/test/java/io/flamingock/internal/core/pipeline/run/PipelineRunToResponseTest.java @@ -52,7 +52,6 @@ void emptyRunYieldsSuccessWithZeroCounts() { assertEquals(0, response.getCompletedStages()); assertEquals(0, response.getFailedStages()); assertEquals(0, response.getTotalChanges()); - assertNull(response.getError()); assertNotNull(response.getStartTime()); assertNotNull(response.getEndTime()); } @@ -78,7 +77,6 @@ void twoCompletedStagesYieldsSuccessAndCountersRollUp() { assertEquals(3, response.getAppliedChanges()); // 2 + 1 assertEquals(1, response.getSkippedChanges()); // 1 + 0 assertEquals(0, response.getFailedChanges()); - assertNull(response.getError()); } @Test @@ -105,60 +103,15 @@ void oneCompletedAndOneFailedYieldsFailedAndPipelineErrorMatchesStage() { assertEquals(2, response.getTotalChanges()); // 1 applied + 1 failed assertEquals(1, response.getAppliedChanges()); assertEquals(1, response.getFailedChanges()); - ErrorInfo error = response.getError(); + + // Per-stage error info lives on the StageResult's state, accessed via getErrorInfo(). + ErrorInfo error = response.getStages().get(1).getState().getErrorInfo().get(); assertEquals(java.util.Collections.singletonList("change-b1"), error.getChangeIds()); assertEquals("beta", error.getStageId()); assertEquals("RuntimeException", error.getErrorType()); assertEquals("boom", error.getMessage()); } - @Test - void pipelineFailedWithNoStageFailuresYieldsFailedAndPipelineError() { - AbstractLoadedStage stageA = mockStage("alpha"); - PipelineRun pipelineRun = PipelineRun.of(java.util.Collections.singletonList(stageA)); - - pipelineRun.start(); - pipelineRun.markPipelineFailed(new RuntimeException("lock not acquired")); - pipelineRun.stop(); - - ExecuteResponseData response = pipelineRun.toResponse(); - - assertEquals(ExecutionStatus.FAILED, response.getStatus()); - // Stage was NOT_STARTED — it appears in response.stages with state=NOT_STARTED, but - // it doesn't count toward failedStages/completedStages. - assertEquals(1, response.getTotalStages()); - assertEquals(0, response.getCompletedStages()); - assertEquals(0, response.getFailedStages()); - assertTrue(response.getStages().get(0).getState().isNotStarted()); - ErrorInfo error = response.getError(); - assertEquals("RuntimeException", error.getErrorType()); - assertEquals("lock not acquired", error.getMessage()); - } - - @Test - void stageErrorTakesPrecedenceOverPipelineError() { - AbstractLoadedStage stageA = mockStage("alpha"); - PipelineRun pipelineRun = PipelineRun.of(java.util.Collections.singletonList(stageA)); - - RuntimeException stageCause = new RuntimeException("stage boom"); - StageExecutionException stageException = StageExecutionException.fromResult( - stageCause, failedStageResult("alpha"), "change-1"); - - pipelineRun.start(); - pipelineRun.markStageFailed("alpha", stageException); - pipelineRun.markPipelineFailed(new IllegalStateException("late pipeline error")); - pipelineRun.stop(); - - ExecuteResponseData response = pipelineRun.toResponse(); - - assertEquals(ExecutionStatus.FAILED, response.getStatus()); - ErrorInfo error = response.getError(); - // Stage-level error wins over pipeline-level - assertEquals("RuntimeException", error.getErrorType()); - assertEquals("stage boom", error.getMessage()); - assertEquals("alpha", error.getStageId()); - } - @Test void blockedForMIStageIsTrackedInResponseWithBlockedState() { AbstractLoadedChange c1 = mockChange("c1"); @@ -181,9 +134,8 @@ void blockedForMIStageIsTrackedInResponseWithBlockedState() { assertEquals(1, response.getFailedStages()); assertTrue(response.getStages().get(0).getState().isBlockedForManualIntervention()); assertEquals(1, response.getStages().get(0).getState().getRecoveryIssues().size()); - // Pipeline-level failure surfaces via pipelineError as today. + // BlockedForMI is a Failed subtype → counts toward failedStages → status FAILED. assertEquals(ExecutionStatus.FAILED, response.getStatus()); - assertNotNull(response.getError()); } @Test diff --git a/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/ExecutionPlanTest.java b/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/ExecutionPlanTest.java index 3109b5993..0e87f8672 100644 --- a/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/ExecutionPlanTest.java +++ b/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/ExecutionPlanTest.java @@ -15,7 +15,6 @@ */ package io.flamingock.internal.core.plan; -import io.flamingock.internal.common.core.error.FlamingockException; import io.flamingock.internal.common.core.recovery.action.ChangeAction; import io.flamingock.internal.core.change.executable.ExecutableChange; import io.flamingock.internal.core.pipeline.execution.ExecutableStage; @@ -25,7 +24,8 @@ import java.util.Arrays; import java.util.Collections; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -34,29 +34,17 @@ class ExecutionPlanTest { @Test @DisplayName("ABORT plan should not require execution") void abortPlanShouldNotRequireExecution() { - ExecutionPlan plan = ExecutionPlan.ABORT(Collections.singletonList( - stageWith(mockChange("change-1", ChangeAction.APPLY)) - )); + ExecutionPlan plan = ExecutionPlan.ABORT(); assertFalse(plan.isExecutionRequired()); } @Test @DisplayName("ABORT plan should be marked as aborted") void abortPlanShouldBeAborted() { - ExecutionPlan plan = ExecutionPlan.ABORT(Collections.emptyList()); + ExecutionPlan plan = ExecutionPlan.ABORT(); assertTrue(plan.isAborted()); } - @Test - @DisplayName("ABORT plan should throw generic FlamingockException on validate") - void abortPlanShouldThrowFlamingockExceptionOnValidate() { - ExecutionPlan plan = ExecutionPlan.ABORT(Collections.singletonList( - stageWith(mockChange("change-1", ChangeAction.APPLY)) - )); - FlamingockException ex = assertThrows(FlamingockException.class, plan::validate); - assertEquals("Execution aborted by the execution planner", ex.getMessage()); - } - @Test @DisplayName("newExecution plan should not be aborted") void newExecutionPlanShouldNotBeAborted() { @@ -68,7 +56,7 @@ void newExecutionPlanShouldNotBeAborted() { @Test @DisplayName("CONTINUE plan should not be aborted") void continuePlanShouldNotBeAborted() { - ExecutionPlan plan = ExecutionPlan.CONTINUE(Collections.emptyList()); + ExecutionPlan plan = ExecutionPlan.CONTINUE(); assertFalse(plan.isAborted()); } diff --git a/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlannerTest.java b/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlannerTest.java index 5f8fbabf7..6f256949c 100644 --- a/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlannerTest.java +++ b/core/flamingock-core/src/test/java/io/flamingock/internal/core/plan/community/CommunityExecutionPlannerTest.java @@ -156,8 +156,8 @@ void shouldSkipBlockedForMIStagesAndPlanRemaining() { } @Test - @DisplayName("Should return CONTINUE without acquiring lock when every remaining stage is already failed") - void shouldReturnContinueWhenAllRemainingStagesAreFailed() { + @DisplayName("Should return ABORT without acquiring lock when every stage in the only block has failed") + void shouldReturnAbortWhenAllStagesInTheOnlyBlockHaveFailed() { AbstractLoadedChange change = mockLoadedChange("change-1"); AbstractLoadedStage stage = mockStage("stage-1", change); @@ -168,7 +168,9 @@ void shouldReturnContinueWhenAllRemainingStagesAreFailed() { ExecutionPlan plan = planner.getNextExecution(pipelineRun); - assertFalse(plan.isAborted()); + // The only block is now terminal+hasFailures → ABORT. Lock is not acquired (block + // selection happens before the audit dance). + assertTrue(plan.isAborted()); assertFalse(plan.isExecutionRequired()); verify(lockService, never()).upsert(any(), any(), anyLong()); } @@ -190,6 +192,108 @@ void shouldReturnContinueWithoutLockWhenAllChangesApplied() { verify(lockService, never()).upsert(any(), any(), anyLong()); } + @Test + @DisplayName("Should return CONTINUE when every block is terminal+successful (multi-block, all done)") + void shouldReturnContinueWhenAllBlocksAreSuccessful() { + AbstractLoadedChange systemChange = mockLoadedChange("sys-c1"); + AbstractLoadedChange userChange = mockLoadedChange("user-c1"); + AbstractLoadedStage systemStage = mockTypedStage("flamingock-system-stage", io.flamingock.api.StageType.SYSTEM, systemChange); + AbstractLoadedStage userStage = mockTypedStage("changes", io.flamingock.api.StageType.DEFAULT, userChange); + + PipelineRun pipelineRun = PipelineRun.of(java.util.Arrays.asList(systemStage, userStage)); + // Both stages completed in this run. + pipelineRun.markStageCompleted("flamingock-system-stage", io.flamingock.internal.common.core.response.data.StageResult.builder() + .stageId("flamingock-system-stage").stageName("flamingock-system-stage") + .state(io.flamingock.internal.common.core.response.data.StageState.COMPLETED).build()); + pipelineRun.markStageCompleted("changes", io.flamingock.internal.common.core.response.data.StageResult.builder() + .stageId("changes").stageName("changes") + .state(io.flamingock.internal.common.core.response.data.StageState.COMPLETED).build()); + + ExecutionPlan plan = planner.getNextExecution(pipelineRun); + + assertFalse(plan.isAborted()); + assertFalse(plan.isExecutionRequired()); + verify(lockService, never()).upsert(any(), any(), anyLong()); + verify(auditReader, never()).getAuditSnapshotByChangeId(); + } + + @Test + @DisplayName("Should return ABORT when an earlier block is terminal+hasFailures (block dependency)") + void shouldReturnAbortWhenAnEarlierBlockHasFailures() { + AbstractLoadedChange systemChange = mockLoadedChange("sys-c1"); + AbstractLoadedChange userChange = mockLoadedChange("user-c1"); + AbstractLoadedStage systemStage = mockTypedStage("flamingock-system-stage", io.flamingock.api.StageType.SYSTEM, systemChange); + AbstractLoadedStage userStage = mockTypedStage("changes", io.flamingock.api.StageType.DEFAULT, userChange); + + PipelineRun pipelineRun = PipelineRun.of(java.util.Arrays.asList(systemStage, userStage)); + // System block fails — earlier block's failure must block downstream work. + pipelineRun.markStageFailed("flamingock-system-stage", new RuntimeException("system stage exploded")); + + ExecutionPlan plan = planner.getNextExecution(pipelineRun); + + assertTrue(plan.isAborted()); + verify(lockService, never()).upsert(any(), any(), anyLong()); + verify(auditReader, never()).getAuditSnapshotByChangeId(); + } + + @Test + @DisplayName("Should synthetically complete a block with no executable work (changes already applied per audit) and plan from the next block") + void shouldAdvancePastBlockWithNoExecutableWorkAndPlanFromNextBlock() { + AbstractLoadedChange legacyChange = mockLoadedChange("legacy-c1"); + AbstractLoadedChange userChange = mockLoadedChange("user-c1"); + AbstractLoadedStage legacyStage = mockTypedStage("flamingock-legacy-stage", io.flamingock.api.StageType.LEGACY, legacyChange); + AbstractLoadedStage userStage = mockTypedStage("changes", io.flamingock.api.StageType.DEFAULT, userChange); + + PipelineRun pipelineRun = PipelineRun.of(java.util.Arrays.asList(legacyStage, userStage)); + // Legacy stages are NOT_STARTED in the runtime view but the audit says their changes are + // already applied (a previous run / another instance). The planner must advance past the + // legacy block, synthetically marking it Completed, then plan from the user (DEFAULT) block. + Map snapshot = new HashMap<>(); + snapshot.put("legacy-c1", buildAuditEntry("legacy-c1", AuditEntry.Status.APPLIED, null)); + when(auditReader.getAuditSnapshotByChangeId()).thenReturn(snapshot); + when(lockService.upsert(any(), any(), anyLong())) + .thenReturn(new LockAcquisition(RunnerId.fromString("test-runner"), 60000L)); + + ExecutionPlan plan = planner.getNextExecution(pipelineRun); + + // Legacy block was inspected, found no executable work, and synthetically completed. + assertTrue(pipelineRun.getStageRun("flamingock-legacy-stage").getState().isCompleted(), + "Legacy stage should be synthetically marked Completed when its changes are already applied per audit"); + // Planner advanced to the DEFAULT block and acquired the lock for actual work. + assertFalse(plan.isAborted()); + assertTrue(plan.isExecutionRequired()); + verify(legacyStage, atLeastOnce()).applyActions(any()); + verify(userStage, atLeastOnce()).applyActions(any()); + verify(lockService, atLeastOnce()).upsert(any(), any(), anyLong()); + } + + @Test + @DisplayName("Should plan from active block, skipping earlier successful blocks (block dependency)") + void shouldPlanFromActiveBlockSkippingEarlierSuccessfulBlock() { + AbstractLoadedChange systemChange = mockLoadedChange("sys-c1"); + AbstractLoadedChange userChange = mockLoadedChange("user-c1"); + AbstractLoadedStage systemStage = mockTypedStage("flamingock-system-stage", io.flamingock.api.StageType.SYSTEM, systemChange); + AbstractLoadedStage userStage = mockTypedStage("changes", io.flamingock.api.StageType.DEFAULT, userChange); + + PipelineRun pipelineRun = PipelineRun.of(java.util.Arrays.asList(systemStage, userStage)); + // System block done; DEFAULT block (user stages) still pending. + pipelineRun.markStageCompleted("flamingock-system-stage", io.flamingock.internal.common.core.response.data.StageResult.builder() + .stageId("flamingock-system-stage").stageName("flamingock-system-stage") + .state(io.flamingock.internal.common.core.response.data.StageState.COMPLETED).build()); + + when(auditReader.getAuditSnapshotByChangeId()).thenReturn(Collections.emptyMap()); + when(lockService.upsert(any(), any(), anyLong())) + .thenReturn(new LockAcquisition(RunnerId.fromString("test-runner"), 60000L)); + + ExecutionPlan plan = planner.getNextExecution(pipelineRun); + + assertFalse(plan.isAborted()); + assertTrue(plan.isExecutionRequired()); + // The completed system stage was skipped — only the user-block stage was action-built. + verify(systemStage, never()).applyActions(any()); + verify(userStage, atLeastOnce()).applyActions(any()); + } + private static AbstractLoadedChange mockLoadedChange(String id) { AbstractLoadedChange change = mock(AbstractLoadedChange.class); when(change.getId()).thenReturn(id); @@ -201,8 +305,17 @@ private static AbstractLoadedChange mockLoadedChange(String id) { } private static AbstractLoadedStage mockStage(String name, AbstractLoadedChange... changes) { + return mockTypedStage(name, null, changes); + } + + private static AbstractLoadedStage mockTypedStage(String name, + io.flamingock.api.StageType type, + AbstractLoadedChange... changes) { AbstractLoadedStage stage = mock(AbstractLoadedStage.class); when(stage.getName()).thenReturn(name); + if (type != null) { + when(stage.getType()).thenReturn(type); + } List changeList = java.util.Arrays.asList(changes); when(stage.getChanges()).thenReturn(changeList);