Skip to content

Commit b083e4f

Browse files
gy2006gy2006
committed
improve circular dep check
1 parent f6dddad commit b083e4f

File tree

5 files changed

+264
-12
lines changed

5 files changed

+264
-12
lines changed

‎pom.xml‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
<jackson.version>2.17.3</jackson.version>
2929
<wiremock-spring-boot.version>3.7.0</wiremock-spring-boot.version>
3030
<httpclient5.version>5.3.1</httpclient5.version>
31+
<git-commit-id-maven-plugin.version>5.0.1</git-commit-id-maven-plugin.version>
3132
</properties>
3233

3334
<dependencyManagement>
@@ -159,6 +160,12 @@
159160
<groupId>org.apache.httpcomponents.client5</groupId>
160161
<artifactId>httpclient5</artifactId>
161162
<version>${httpclient5.version}</version>
163+
<exclusions>
164+
<exclusion>
165+
<groupId>commons-logging</groupId>
166+
<artifactId>commons-logging</artifactId>
167+
</exclusion>
168+
</exclusions>
162169
</dependency>
163170

164171
<dependency>
@@ -178,6 +185,12 @@
178185
<artifactId>wiremock-spring-boot</artifactId>
179186
<version>${wiremock-spring-boot.version}</version>
180187
<scope>test</scope>
188+
<exclusions>
189+
<exclusion>
190+
<groupId>com.google.errorprone</groupId>
191+
<artifactId>error_prone_annotations</artifactId>
192+
</exclusion>
193+
</exclusions>
181194
</dependency>
182195

183196
<dependency>
@@ -214,6 +227,26 @@
214227
<groupId>org.projectlombok</groupId>
215228
<artifactId>lombok</artifactId>
216229
</exclude>
230+
<execution>
231+
<goals>
232+
<goal>repackage</goal>
233+
</goals>
234+
</execution>
235+
<execution>
236+
<id>initialize</id>
237+
<goals>
238+
<goal>build-info</goal>
239+
</goals>
240+
<phase>initialize</phase>
241+
<configuration>
242+
<additionalProperties>
243+
<encoding.source>UTF-8</encoding.source>
244+
<encoding.reporting>UTF-8</encoding.reporting>
245+
<java.source>${maven.compiler.source}</java.source>
246+
<java.target>${maven.compiler.target}</java.target>
247+
</additionalProperties>
248+
</configuration>
249+
</execution>
217250
</excludes>
218251
</configuration>
219252
</plugin>
@@ -255,6 +288,34 @@
255288
</execution>
256289
</executions>
257290
</plugin>
291+
292+
<plugin>
293+
<groupId>io.github.git-commit-id</groupId>
294+
<artifactId>git-commit-id-maven-plugin</artifactId>
295+
<version>${git-commit-id-maven-plugin.version}</version>
296+
<configuration>
297+
<generateGitPropertiesFile>true</generateGitPropertiesFile>
298+
<generateGitPropertiesFilename>${project.build.outputDirectory}/META-INF/git.properties</generateGitPropertiesFilename>
299+
<commitIdGenerationMode>full</commitIdGenerationMode>
300+
<dateFormat>yyyy-MM-dd'T'HH:mm:ssXXX</dateFormat>
301+
<includeOnlyProperties>
302+
<includeOnlyProperty>^git.commit.id.full</includeOnlyProperty>
303+
<includeOnlyProperty>^git.commit.time$</includeOnlyProperty>
304+
<includeOnlyProperty>^git.branch</includeOnlyProperty>
305+
<includeOnlyProperty>^git.tags</includeOnlyProperty>
306+
</includeOnlyProperties>
307+
<verbose>false</verbose>
308+
</configuration>
309+
<executions>
310+
<execution>
311+
<id>get-the-git-infos</id>
312+
<goals>
313+
<goal>revision</goal>
314+
</goals>
315+
<phase>initialize</phase>
316+
</execution>
317+
</executions>
318+
</plugin>
258319
</plugins>
259320
</build>
260321
</project>

‎src/main/java/com/flowci/yaml/business/impl/ParseYamlV2Impl.java‎

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,7 @@
1212
import org.springframework.stereotype.Component;
1313
import org.springframework.util.CollectionUtils;
1414

15-
import java.util.HashMap;
16-
import java.util.HashSet;
17-
import java.util.List;
18-
import java.util.Set;
15+
import java.util.*;
1916

2017
import static java.lang.String.format;
2118
import static org.springframework.util.CollectionUtils.isEmpty;
@@ -39,6 +36,7 @@ public FlowV2 invoke(String yaml) {
3936
for (var step : flowV2.getSteps()) {
4037
step.setParent(flowV2);
4138
}
39+
4240
validateSteps(flowV2);
4341
buildGraph(flowV2);
4442
return flowV2;
@@ -93,35 +91,59 @@ private void validateCommands(StepV2 step) {
9391

9492
private void buildGraph(FlowV2 flow) {
9593
var steps = flow.getSteps();
94+
95+
// set name - step mapping
9696
var map = new HashMap<String, StepV2>(steps.size());
97+
flow.setStepNameMapping(map);
98+
9799
for (var step : steps) {
98100
map.put(step.getName(), step);
99101
}
100102

101103
for (var step : steps) {
102-
if (step.getDependsOn() == null) {
104+
if (CollectionUtils.isEmpty(step.getDependsOn())) {
103105
continue;
104106
}
105107

106108
for (var dep : step.getDependsOn()) {
107109
var depStep = map.get(dep);
110+
111+
// link next
108112
depStep.getNext().add(step);
109113
}
110114
}
111115

112116
for (var step : steps) {
113-
checkCircularDependencies(List.of(step), new HashSet<>());
117+
checkCircularDependencies(List.of(step), new HashMap<>());
114118
}
115119
}
116120

117-
private void checkCircularDependencies(List<StepV2> steps, Set<StepV2> traversed) {
121+
private void checkCircularDependencies(List<StepV2> steps, HashMap<StepV2, Integer> traversed) {
118122
for (var step : steps) {
119-
if (!traversed.add(step)) {
120-
throw new InvalidYamlException("circular dependency found");
123+
Integer numEncountered = traversed.get(step);
124+
125+
if (numEncountered == null) {
126+
traversed.put(step, 1);
127+
checkCircularDependencies(step.getNext(), traversed);
128+
continue;
121129
}
122130

123-
traversed.add(step);
131+
int numOfDepends = step.getDependsOn() == null ? 0 : step.getDependsOn().size();
132+
if (numOfDepends == 0 || numEncountered > numOfDepends) {
133+
throw new InvalidYamlException("circular dependency found on step: " + step.getName());
134+
}
135+
136+
traversed.put(step, numEncountered + 1);
124137
checkCircularDependencies(step.getNext(), traversed);
125138
}
126139
}
140+
141+
/**
142+
* Ex:
143+
* A <-- B <-- C
144+
* A <-- C
145+
*/
146+
private void checkDuplicatedDependsOn(List<StepV2> steps) {
147+
148+
}
127149
}

‎src/main/java/com/flowci/yaml/model/FlowV2.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,19 @@
11
package com.flowci.yaml.model;
22

3+
import com.fasterxml.jackson.annotation.JsonIgnore;
4+
import jakarta.annotation.Nullable;
35
import lombok.Getter;
46
import lombok.Setter;
7+
import org.springframework.util.CollectionUtils;
8+
import org.springframework.util.StringUtils;
59

10+
import java.util.LinkedList;
611
import java.util.List;
12+
import java.util.Map;
713
import java.util.Set;
814

15+
import static org.springframework.util.CollectionUtils.isEmpty;
16+
917
@Setter
1018
@Getter
1119
public class FlowV2 extends BaseV2 {
@@ -16,4 +24,36 @@ public class FlowV2 extends BaseV2 {
1624
private Set<String> agents;
1725

1826
private List<StepV2> steps;
27+
28+
@JsonIgnore
29+
private Map<String, StepV2> stepNameMapping;
30+
31+
@JsonIgnore
32+
public StepV2 getStep(String name) {
33+
return stepNameMapping.get(name);
34+
}
35+
36+
@JsonIgnore
37+
public List<StepV2> next(@Nullable String current) {
38+
if (!StringUtils.hasText(current)) {
39+
return stepsWithoutDependencies();
40+
}
41+
42+
var step = stepNameMapping.get(current);
43+
if (step == null) {
44+
throw new IllegalArgumentException("Invalid step name: " + current);
45+
}
46+
47+
return step.getNext();
48+
}
49+
50+
private List<StepV2> stepsWithoutDependencies() {
51+
var list = new LinkedList<StepV2>();
52+
for (var step : steps) {
53+
if (isEmpty(step.getDependsOn())) {
54+
list.add(step);
55+
}
56+
}
57+
return list;
58+
}
1959
}

‎src/test/java/com/flowci/yaml/ParseYamlV2Test.java‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,15 +86,48 @@ void givenYaml_whenParsing_thenReturnFlowObject() {
8686
assertEquals("host", step2Docker.getNetwork());
8787
}
8888

89+
@Test
90+
void givenYamlWithParallelSteps_whenParsing_thenReturnFlowObject() {
91+
var content = getResourceAsString("yaml/v2_success_parallel_steps.yaml");
92+
var flowV2 = parseYamlV2.invoke(content);
93+
assertNotNull(flowV2);
94+
95+
// next steps of flow should be the steps without deps
96+
var next = flowV2.next(null);
97+
assertEquals(2, next.size());
98+
assertEquals("step_abc", next.get(0).getName());
99+
assertEquals("step_1", next.get(1).getName());
100+
101+
next = flowV2.next("step_1");
102+
assertEquals(2, next.size());
103+
assertEquals("step_2_A_1", next.get(0).getName());
104+
assertEquals("step_2_B", next.get(1).getName());
105+
106+
next = flowV2.next("step_2_A_1");
107+
assertEquals(1, next.size());
108+
assertEquals("step_2_A_2", next.getFirst().getName());
109+
110+
next = flowV2.next("step_2_A_2");
111+
assertEquals(1, next.size());
112+
assertEquals("step_3", next.getFirst().getName());
113+
114+
next = flowV2.next("step_2_B");
115+
assertEquals(1, next.size());
116+
assertEquals("step_3", next.getFirst().getName());
117+
118+
next = flowV2.next("step_3");
119+
assertEquals(0, next.size());
120+
}
121+
89122
@ParameterizedTest
90123
@CsvSource({
91124
"v2_step_name_invalid.yaml,step name 'step 1' is invalid",
92125
"v2_step_name_duplicate.yaml,step name 'step_1' already exists",
93126
"v2_step_depends_on_not_found.yaml,depends on 'step_not_there' is not found",
94127
"v2_commands_missing.yaml,at least one command under step 'step_1' is required",
95128
"v2_commands_without_script.yaml,bash or powershell is required",
96-
"v2_circular_dependency_on_all_depends.yaml,circular dependency found",
97-
"v2_circular_dependency_with_separate_step.yaml,circular dependency found",
129+
"v2_circular_dependency_on_all_depends.yaml,circular dependency found on step: step_d",
130+
"v2_circular_dependency_with_separate_step.yaml,circular dependency found on step: step_c",
98131
})
99132
void givenInvalidYaml_whenParsing_thenThrowException(String yamlFile, String expectedMsg) {
100133
var content = getResourceAsString("yaml/" + yamlFile);

‎src/test/resources/yaml/v2_success_parallel_steps.yaml‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
agents:
2+
- k8s
3+
- java
4+
5+
condition: |
6+
return $FLOWCI_GIT_BRANCH == "develop" || $FLOWCI_GIT_BRANCH == "master";
7+
8+
variables:
9+
FLOW_WORKSPACE: "/home/flowci"
10+
FLOW_VERSION: "2.0"
11+
12+
# default docker for all steps
13+
docker:
14+
image: "alpine:3"
15+
ports:
16+
- "8080:8080"
17+
18+
steps:
19+
- name: step_abc
20+
commands:
21+
- bash: echo hello
22+
23+
- name: step_1
24+
timeout: 3600
25+
allow_failure: true
26+
variables:
27+
FLOW_WORKSPACE: "/home/step/1"
28+
FLOW_VERSION: "2.1"
29+
K8S_PATH: "/k8s/test"
30+
commands:
31+
- name: print
32+
bash: |
33+
echo "step 1 on bash"
34+
pwsh: |
35+
echo "step 1 on powershell"
36+
37+
- name: step_2_A_1
38+
depends_on:
39+
- step_1
40+
docker:
41+
image: "ubuntu:24.04"
42+
ports:
43+
- "6400:6400"
44+
entrypoint: [ "/bin/sh" ]
45+
network: host
46+
commands:
47+
- bash: |
48+
echo "step 2_A_1 on bash"
49+
pwsh: |
50+
echo "step 2_A_1 on powershell"
51+
52+
- name: step_2_A_2
53+
depends_on:
54+
- step_2_A_1
55+
docker:
56+
image: "ubuntu:24.04"
57+
ports:
58+
- "6400:6400"
59+
entrypoint: [ "/bin/sh" ]
60+
network: host
61+
commands:
62+
- bash: |
63+
echo "step 2_A_2 on bash"
64+
pwsh: |
65+
echo "step 2_A_2 on powershell"
66+
67+
- name: step_2_B
68+
depends_on:
69+
- step_1
70+
docker:
71+
image: "ubuntu:24.04"
72+
ports:
73+
- "6400:6400"
74+
entrypoint: [ "/bin/sh" ]
75+
network: host
76+
commands:
77+
- bash: |
78+
echo "step 2_B on bash"
79+
pwsh: |
80+
echo "step 2_B on powershell"
81+
82+
- name: step_3
83+
depends_on:
84+
- step_2_A_2
85+
- step_2_B
86+
docker:
87+
image: "ubuntu:24.04"
88+
ports:
89+
- "6400:6400"
90+
entrypoint: [ "/bin/sh" ]
91+
network: host
92+
commands:
93+
- bash: |
94+
echo "step 3 on bash"
95+
pwsh: |
96+
echo "step 3 on powershell"

0 commit comments

Comments
 (0)