From c8f0cd43daa6f7b51004951e9e16f5a2f477aca0 Mon Sep 17 00:00:00 2001 From: Michael Bannister Date: Sat, 27 Jan 2018 15:55:53 +0000 Subject: [PATCH] Preserve order of pipelines from YAML when converting to JSON Fixes #4 Changed to use recent release of EsotericSoftware's yamlbeans, rather than the old sanjusoftware fork. This reads yaml mappings into LinkedHashMap, which preserves ordering. (It also opens the door for supporting YAML merge key syntax - issue #55) --- build.gradle | 4 ++-- .../transforms/EnvironmentsTransform.java | 2 +- .../config/yaml/transforms/RootTransform.java | 4 ++-- .../config/yaml/transforms/TaskTransform.java | 8 +++---- .../yaml/transforms/RootTransformTest.java | 17 +++++++++++++ .../resources/parts/roots/pipeline_order.yaml | 24 +++++++++++++++++++ 6 files changed, 50 insertions(+), 9 deletions(-) create mode 100644 src/test/resources/parts/roots/pipeline_order.yaml diff --git a/build.gradle b/build.gradle index 4541ed2d..98ca1766 100644 --- a/build.gradle +++ b/build.gradle @@ -1,5 +1,5 @@ group 'cd.go.plugin.config.yaml' -version '0.6.0' +version '0.6.1' apply plugin: 'java' @@ -20,7 +20,7 @@ dependencies { compile group: 'com.google.code.gson', name: 'gson', version: '2.6.2' compile group: 'commons-io', name: 'commons-io', version: '2.4' compile group: 'org.apache.ant', name: 'ant', version: '1.7.1' - compile group: 'com.github.sanjusoftware', name: 'yamlbeans', version: '1.11' + compile group: 'com.esotericsoftware.yamlbeans', name: 'yamlbeans', version: '1.13' testCompile group: 'junit', name: 'junit', version: '4.11' testCompile group: 'org.mockito', name: 'mockito-core', version: '1.10.19' testCompile group: 'org.hamcrest', name: 'hamcrest-core', version: '1.3' diff --git a/src/main/java/cd/go/plugin/config/yaml/transforms/EnvironmentsTransform.java b/src/main/java/cd/go/plugin/config/yaml/transforms/EnvironmentsTransform.java index cef7885d..50f091af 100644 --- a/src/main/java/cd/go/plugin/config/yaml/transforms/EnvironmentsTransform.java +++ b/src/main/java/cd/go/plugin/config/yaml/transforms/EnvironmentsTransform.java @@ -30,7 +30,7 @@ public JsonObject transform(Map.Entry env) { JsonObject envJson = new JsonObject(); envJson.addProperty(JSON_ENV_NAME_FIELD, envName); Object envObj = env.getValue(); - if ("".equals(envObj)) + if (envObj == null) return envJson; if (!(envObj instanceof Map)) throw new YamlConfigException("Expected environment to be a hash"); diff --git a/src/main/java/cd/go/plugin/config/yaml/transforms/RootTransform.java b/src/main/java/cd/go/plugin/config/yaml/transforms/RootTransform.java index d371676b..adeaf1e1 100644 --- a/src/main/java/cd/go/plugin/config/yaml/transforms/RootTransform.java +++ b/src/main/java/cd/go/plugin/config/yaml/transforms/RootTransform.java @@ -31,7 +31,7 @@ public JsonConfigCollection transform(Object rootObj, String location) { Map rootMap = (Map) rootObj; for (Map.Entry pe : rootMap.entrySet()) { if ("pipelines".equalsIgnoreCase(pe.getKey())) { - if ("".equals(pe.getValue())) + if (pe.getValue() == null) continue; Map pipelines = (Map) pe.getValue(); for (Map.Entry pipe : pipelines.entrySet()) { @@ -44,7 +44,7 @@ public JsonConfigCollection transform(Object rootObj, String location) { } } } else if ("environments".equalsIgnoreCase(pe.getKey())) { - if ("".equals(pe.getValue())) + if (pe.getValue() == null) continue; Map environments = (Map) pe.getValue(); for (Map.Entry env : environments.entrySet()) { diff --git a/src/main/java/cd/go/plugin/config/yaml/transforms/TaskTransform.java b/src/main/java/cd/go/plugin/config/yaml/transforms/TaskTransform.java index c090ecd2..45fbd5fe 100644 --- a/src/main/java/cd/go/plugin/config/yaml/transforms/TaskTransform.java +++ b/src/main/java/cd/go/plugin/config/yaml/transforms/TaskTransform.java @@ -49,6 +49,10 @@ public JsonObject transform(Map.Entry taskEntry) { JsonObject taskJson = new JsonObject(); String taskType = taskEntry.getKey(); + if (taskEntry.getValue() == null) { + taskJson.addProperty(JSON_TASK_TYPE_FIELD, taskType); + return taskJson; + } if (taskEntry.getValue() instanceof String) { if (taskType.equalsIgnoreCase("script")) { taskJson.addProperty(JSON_TASK_TYPE_FIELD, "plugin"); @@ -64,10 +68,6 @@ public JsonObject transform(Map.Entry taskEntry) { taskJson.add(JSON_TASK_PLUGIN_CONFIGURATION_FIELD, pluginConfig); return taskJson; } - if ("".equals(taskEntry.getValue())) { - taskJson.addProperty(JSON_TASK_TYPE_FIELD, taskType); - return taskJson; - } } taskJson.addProperty(JSON_TASK_TYPE_FIELD, taskType); if (!(taskEntry.getValue() instanceof Map)) diff --git a/src/test/java/cd/go/plugin/config/yaml/transforms/RootTransformTest.java b/src/test/java/cd/go/plugin/config/yaml/transforms/RootTransformTest.java index b1f8e215..f256a21b 100644 --- a/src/test/java/cd/go/plugin/config/yaml/transforms/RootTransformTest.java +++ b/src/test/java/cd/go/plugin/config/yaml/transforms/RootTransformTest.java @@ -3,6 +3,7 @@ import cd.go.plugin.config.yaml.JsonConfigCollection; import cd.go.plugin.config.yaml.YamlConfigException; import com.esotericsoftware.yamlbeans.YamlReader; +import com.google.gson.JsonArray; import com.google.gson.JsonObject; import com.google.gson.JsonPrimitive; import org.junit.Before; @@ -58,6 +59,22 @@ public void shouldRecognizeAndUpdateVersion() throws Exception { assertTargetVersion(readRootYaml("version_1").getJsonObject(), 1); assertTargetVersion(readRootYaml("version_2").getJsonObject(), 2); } + + @Test + public void shouldPreserveOrderOfPipelines() throws IOException { + MaterialTransform materialTransform = mock(MaterialTransform.class); + StageTransform stageTransform = mock(StageTransform.class); + EnvironmentVariablesTransform environmentTransform = mock(EnvironmentVariablesTransform.class); + ParameterTransform parameterTransform = mock(ParameterTransform.class); + pipelineTransform = new PipelineTransform(materialTransform, stageTransform, environmentTransform, parameterTransform); + rootTransform = new RootTransform(pipelineTransform, environmentsTransform); + + JsonConfigCollection collection = readRootYaml("pipeline_order"); + JsonArray pipelines = collection.getJsonObject().get("pipelines").getAsJsonArray(); + assertThat(pipelines.get(0).getAsJsonObject().get("name").getAsString(), is("pipe1")); + assertThat(pipelines.get(1).getAsJsonObject().get("name").getAsString(), is("pipe2")); + assertThat(pipelines.get(2).getAsJsonObject().get("name").getAsString(), is("pipe3")); + } @Test(expected = YamlReader.YamlReaderException.class) public void shouldNotTransformRootWhenYAMLHasDuplicateKeys() throws IOException { diff --git a/src/test/resources/parts/roots/pipeline_order.yaml b/src/test/resources/parts/roots/pipeline_order.yaml new file mode 100644 index 00000000..367506dc --- /dev/null +++ b/src/test/resources/parts/roots/pipeline_order.yaml @@ -0,0 +1,24 @@ +environments: + env1: + pipelines: + - pipe1 + +pipelines: + pipe1: + group: simple + materials: + mygit: + stages: + - null + pipe2: + group: simple + materials: + mygit: + stages: + - null + pipe3: + group: simple + materials: + mygit: + stages: + - null