diff --git a/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/Stage.java b/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/Stage.java index 1dfc23e67d..6e2655fb1d 100644 --- a/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/Stage.java +++ b/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/Stage.java @@ -38,7 +38,6 @@ import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Sets; import com.netflix.spinnaker.kork.exceptions.SpinnakerException; import com.netflix.spinnaker.orca.ExecutionStatus; import com.netflix.spinnaker.orca.jackson.OrcaObjectMapper; @@ -46,17 +45,7 @@ import de.huxhorn.sulky.ulid.ULID; import java.io.IOException; import java.io.Serializable; -import java.util.ArrayList; -import java.util.Base64; -import java.util.Collection; -import java.util.HashMap; -import java.util.HashSet; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.Optional; -import java.util.Set; +import java.util.*; import java.util.function.Predicate; import java.util.stream.Stream; import javax.annotation.Nonnull; @@ -366,14 +355,10 @@ public Task taskById(String taskId) { */ public List ancestors() { if (execution != null) { - Set visited = Sets.newHashSetWithExpectedSize(execution.getStages().size()); - return ImmutableList.builder() - .add(this) - .addAll(getAncestorsImpl(visited, false)) - .build(); - } else { - return emptyList(); + return getAncestorsImpl(false); } + + return emptyList(); } /** @@ -385,81 +370,73 @@ public List ancestors() { */ public List directAncestors() { if (execution != null) { - Set visited = Sets.newHashSetWithExpectedSize(execution.getStages().size()); - return ImmutableList.builder() - .add(this) - .addAll(getAncestorsImpl(visited, true)) - .build(); - } else { - return emptyList(); + return getAncestorsImpl(true); } + + return emptyList(); } /** * Worker method to get the list of all ancestors (parents and, optionally, prerequisite stages) * of the current stage. * - * @param visited list of visited nodes * @param directParentOnly true to only include direct parents of the stage, false to also include * stages this stage depends on (via requisiteRefIds) * @return list of ancestor stages */ - private List getAncestorsImpl(Set visited, boolean directParentOnly) { - visited.add(this.refId); + private ImmutableList getAncestorsImpl(boolean directParentOnly) { + Set visited = new HashSet<>(); + ImmutableList.Builder allAncestorStages = ImmutableList.builder(); + Queue toVisit = new ArrayDeque<>(); - if (!requisiteStageRefIds.isEmpty() && !directParentOnly) { - // Get stages this stage depends on via requisiteStageRefIds: - List previousStages = - execution.getStages().stream() - .filter(it -> requisiteStageRefIds.contains(it.refId)) - .filter(it -> !visited.contains(it.refId)) - .collect(toList()); - List syntheticStages = - execution.getStages().stream() - .filter( - s -> - previousStages.stream() - .map(Stage::getId) - .anyMatch(id -> id.equals(s.parentStageId))) - .collect(toList()); - return ImmutableList.builder() - .addAll(previousStages) - .addAll(syntheticStages) - .addAll( - previousStages.stream() - .flatMap(it -> it.getAncestorsImpl(visited, directParentOnly).stream()) - .collect(toList())) - .build(); - } else if (parentStageId != null && !visited.contains(parentStageId)) { - // Get parent stages, but exclude already visited ones: - - List ancestors = new ArrayList<>(); - if (getSyntheticStageOwner() == SyntheticStageOwner.STAGE_AFTER) { - ancestors.addAll( + toVisit.add(this); + + while (!toVisit.isEmpty()) { + Stage curStage = toVisit.remove(); + + if (visited.contains(curStage.id)) { + continue; + } + + visited.add(curStage.id); + allAncestorStages.add(curStage); + + // 1. Find all stages we depend on through requisiteStageRefIds and push them on the queue + if (!curStage.requisiteStageRefIds.isEmpty() && !directParentOnly) { + List refIdPriorStages = execution.getStages().stream() - .filter( - it -> - parentStageId.equals(it.parentStageId) - && it.getSyntheticStageOwner() == SyntheticStageOwner.STAGE_BEFORE) - .collect(toList())); + .filter(it -> curStage.requisiteStageRefIds.contains(it.refId)) + .collect(toList()); + + toVisit.addAll(refIdPriorStages); } - ancestors.addAll( - execution.getStages().stream() - .filter(it -> it.id.equals(parentStageId)) - .findFirst() - .>map( - parent -> - ImmutableList.builder() - .add(parent) - .addAll(parent.getAncestorsImpl(visited, directParentOnly)) - .build()) - .orElse(emptyList())); - - return ancestors; - } else { - return emptyList(); + // 2. If we have a parent stage + if (curStage.parentStageId != null && !visited.contains(curStage.parentStageId)) { + // 2.1 AND we are an STAGE_AFTER, find all the stages with same parent as us but that are + // marked STAGE_BEFORE + if (curStage.getSyntheticStageOwner() == SyntheticStageOwner.STAGE_AFTER) { + toVisit.addAll( + execution.getStages().stream() + .filter( + it -> + curStage.parentStageId.equals(it.parentStageId) + && it.getSyntheticStageOwner() == SyntheticStageOwner.STAGE_BEFORE) + .collect(toList())); + } + + // 2.2 Add our direct parent to the queue + try { + toVisit.add(curStage.getParent()); + } catch (IllegalArgumentException e) { + // It's not really possible for the parent not to exist.. But unittests beg to differ + // Not logging anything here since not having a parent would have failed and logged in + // other places + } + } } + + return allAncestorStages.build(); } /**