Skip to content

Commit 44437e3

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

File tree

5 files changed

+234
-12
lines changed

5 files changed

+234
-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: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,15 +86,22 @@ void givenYaml_whenParsing_thenReturnFlowObject() {
8686
assertEquals("host", step2Docker.getNetwork());
8787
}
8888

89+
@Test
90+
void givenYaml_whenParsing_thenEnableToGetNextSteps() {
91+
var content = getResourceAsString("yaml/v2_success_parallel_steps.yaml");
92+
var flowV2 = parseYamlV2.invoke(content);
93+
assertNotNull(flowV2);
94+
}
95+
8996
@ParameterizedTest
9097
@CsvSource({
9198
"v2_step_name_invalid.yaml,step name 'step 1' is invalid",
9299
"v2_step_name_duplicate.yaml,step name 'step_1' already exists",
93100
"v2_step_depends_on_not_found.yaml,depends on 'step_not_there' is not found",
94101
"v2_commands_missing.yaml,at least one command under step 'step_1' is required",
95102
"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",
103+
"v2_circular_dependency_on_all_depends.yaml,circular dependency found on step: step_d",
104+
"v2_circular_dependency_with_separate_step.yaml,circular dependency found on step: step_c",
98105
})
99106
void givenInvalidYaml_whenParsing_thenThrowException(String yamlFile, String expectedMsg) {
100107
var content = getResourceAsString("yaml/" + yamlFile);

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

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
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_1
20+
timeout: 3600
21+
allow_failure: true
22+
variables:
23+
FLOW_WORKSPACE: "/home/step/1"
24+
FLOW_VERSION: "2.1"
25+
K8S_PATH: "/k8s/test"
26+
commands:
27+
- name: print
28+
bash: |
29+
echo "step 1 on bash"
30+
pwsh: |
31+
echo "step 1 on powershell"
32+
33+
- name: step_2_A_1
34+
depends_on:
35+
- step_1
36+
docker:
37+
image: "ubuntu:24.04"
38+
ports:
39+
- "6400:6400"
40+
entrypoint: [ "/bin/sh" ]
41+
network: host
42+
commands:
43+
- bash: |
44+
echo "step 2_A_1 on bash"
45+
pwsh: |
46+
echo "step 2_A_1 on powershell"
47+
48+
- name: step_2_A_2
49+
depends_on:
50+
- step_2_A_1
51+
docker:
52+
image: "ubuntu:24.04"
53+
ports:
54+
- "6400:6400"
55+
entrypoint: [ "/bin/sh" ]
56+
network: host
57+
commands:
58+
- bash: |
59+
echo "step 2_A_2 on bash"
60+
pwsh: |
61+
echo "step 2_A_2 on powershell"
62+
63+
- name: step_2_B
64+
depends_on:
65+
- step_1
66+
docker:
67+
image: "ubuntu:24.04"
68+
ports:
69+
- "6400:6400"
70+
entrypoint: [ "/bin/sh" ]
71+
network: host
72+
commands:
73+
- bash: |
74+
echo "step 2_B on bash"
75+
pwsh: |
76+
echo "step 2_B on powershell"
77+
78+
- name: step_3
79+
depends_on:
80+
- step_2_A_2
81+
- step_2_B
82+
docker:
83+
image: "ubuntu:24.04"
84+
ports:
85+
- "6400:6400"
86+
entrypoint: [ "/bin/sh" ]
87+
network: host
88+
commands:
89+
- bash: |
90+
echo "step 3 on bash"
91+
pwsh: |
92+
echo "step 3 on powershell"

0 commit comments

Comments
 (0)