Skip to content

Commit a40e29b

Browse files
tomzotomzo
committed
fix handling errors to collect all rather than fail on first one
1 parent 8d6d4af commit a40e29b

File tree

4 files changed

+69
-109
lines changed

4 files changed

+69
-109
lines changed

‎src/com.tw.go.config.json/ConfigDirectoryParser.java‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.tw.go.config.json;
22

3+
import com.google.gson.JsonArray;
34
import com.google.gson.JsonElement;
45
import com.google.gson.JsonObject;
56
import com.google.gson.JsonParseException;
@@ -23,7 +24,6 @@ public ConfigDirectoryParser(ConfigDirectoryScanner scanner, JsonFileParser pars
2324
}
2425

2526
public JsonConfigCollection parseDirectory(File baseDir) throws Exception {
26-
List<PluginError> errors = new ArrayList<>();
2727
JsonConfigCollection config = new JsonConfigCollection();
2828
for (String environmentFile : scanner.getFilesMatchingPattern(baseDir, environmentPattern)) {
2929
try {
@@ -33,14 +33,14 @@ public JsonConfigCollection parseDirectory(File baseDir) throws Exception {
3333
PluginError error = new PluginError(
3434
String.format("Environment file is empty"),
3535
environmentFile);
36-
errors.add(error);
36+
config.addError(error);
3737
}
3838
else if(env.equals(new JsonObject()))
3939
{
4040
PluginError error = new PluginError(
4141
String.format("Environment definition is empty"),
4242
environmentFile);
43-
errors.add(error);
43+
config.addError(error);
4444
}
4545
else
4646
config.addEnvironment(env,environmentFile);
@@ -50,7 +50,7 @@ else if(env.equals(new JsonObject()))
5050
PluginError error = new PluginError(
5151
String.format("Failed to parse environment file as JSON: %s",parseException.getMessage()),
5252
environmentFile);
53-
errors.add(error);
53+
config.addError(error);
5454
}
5555
}
5656

@@ -62,14 +62,14 @@ else if(env.equals(new JsonObject()))
6262
PluginError error = new PluginError(
6363
String.format("Pipeline file is empty"),
6464
pipelineFile);
65-
errors.add(error);
65+
config.addError(error);
6666
}
6767
else if(pipe.equals(new JsonObject()))
6868
{
6969
PluginError error = new PluginError(
7070
String.format("Pipeline definition is empty"),
7171
pipelineFile);
72-
errors.add(error);
72+
config.addError(error);
7373
}
7474
else
7575
config.addPipeline(pipe,pipelineFile);
@@ -79,11 +79,9 @@ else if(pipe.equals(new JsonObject()))
7979
PluginError error = new PluginError(
8080
String.format("Failed to parse pipeline file as JSON: %s",parseException.getMessage()),
8181
pipelineFile);
82-
errors.add(error);
82+
config.addError(error);
8383
}
8484
}
85-
if(!errors.isEmpty())
86-
throw new ConfigDirectoryParseException(errors);
8785

8886
return config;
8987
}

‎src/com.tw.go.config.json/JsonConfigCollection.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
package com.tw.go.config.json;
22

3-
import com.google.gson.JsonArray;
4-
import com.google.gson.JsonElement;
5-
import com.google.gson.JsonObject;
6-
import com.google.gson.JsonPrimitive;
3+
import com.google.gson.*;
74

85
public class JsonConfigCollection {
96
private static final int TARGET_VERSION = 1;
7+
private final Gson gson;
108

119
private JsonObject mainObject = new JsonObject();
1210
private JsonArray environments = new JsonArray();
@@ -15,6 +13,8 @@ public class JsonConfigCollection {
1513

1614
public JsonConfigCollection()
1715
{
16+
gson = new Gson();
17+
1818
mainObject.add("target_version",new JsonPrimitive(TARGET_VERSION));
1919
mainObject.add("environments",environments);
2020
mainObject.add("pipelines",pipelines);
@@ -44,4 +44,12 @@ public void addPipeline(JsonElement pipeline,String location) {
4444
public JsonArray getPipelines() {
4545
return pipelines;
4646
}
47+
48+
public JsonArray getErrors() {
49+
return errors;
50+
}
51+
52+
public void addError(PluginError error) {
53+
errors.add(gson.toJsonTree(error));
54+
}
4755
}

‎src/com.tw.go.config.json/JsonConfigPlugin.java‎

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -153,21 +153,11 @@ private GoPluginApiResponse handleParseDirectoryRequest(GoPluginApiRequest reque
153153

154154
return DefaultGoPluginApiResponse.success(gson.toJson(responseJsonObject));
155155
}
156-
catch (ConfigDirectoryParseException e) {
157-
LOGGER.warn(String.format("%s errors occurred while parsing configuration repository.",e.getErrors().size(), e));
158-
JsonObject responseJsonObject = new JsonObject();
159-
responseJsonObject.add("errors", toJsonArray(e.getErrors()));
160-
return DefaultGoPluginApiResponse.success(gson.toJson(responseJsonObject));
161-
}
162156
catch (Exception e) {
163157
LOGGER.error("Unexpected error occurred while parsing configuration repository.", e);
164-
JsonObject responseJsonObject = new JsonObject();
165-
JsonArray errors = new JsonArray();
166-
JsonObject exceptionAsJson = new JsonObject();
167-
exceptionAsJson.addProperty("message",e.getMessage());
168-
errors.add(exceptionAsJson);
169-
responseJsonObject.add("errors", errors);
170-
return DefaultGoPluginApiResponse.error(gson.toJson(responseJsonObject));
158+
JsonConfigCollection config = new JsonConfigCollection();
159+
config.addError(new PluginError(e.toString(),"JSON config plugin"));
160+
return DefaultGoPluginApiResponse.error(gson.toJson(config.getJsonObject()));
171161
}
172162
}
173163

‎test/com.tw.go.config.json/ConfigDirectoryParserTest.java‎

Lines changed: 47 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -74,67 +74,49 @@ public void shouldAppendPipelineFromDirectory() throws Exception
7474
assertThat(result.getPipelines().size(), is(1));
7575
}
7676
@Test
77-
public void shouldThrowErrorsWithLocationWhenInvalidContent() throws Exception
77+
public void shouldAppendErrorsWithLocationWhenInvalidContent() throws Exception
7878
{
7979
createFileWithContent("pipe1.gopipeline.json", this.pipe1String);
8080
createFileWithContent("pipeBad1.gopipeline.json", "bad pipeline");
81-
try {
82-
parser.parseDirectory(directory);
83-
fail("should have thrown");
84-
}
85-
catch (ConfigDirectoryParseException parseException)
86-
{
87-
assertThat(parseException.getErrors().size(),is(1));
88-
assertThat(parseException.getErrors().get(0).getLocation(), is("pipeBad1.gopipeline.json"));
89-
assertThat(parseException.getErrors().get(0).getMessage(), startsWith("Failed to parse pipeline file as JSON: "));
90-
}
81+
JsonConfigCollection result = parser.parseDirectory(directory);
82+
assertThat(result.getErrors().size(),is(1));
83+
assertThat(result.getErrors().get(0).getAsJsonObject().getAsJsonPrimitive("location").getAsString(), is("pipeBad1.gopipeline.json"));
84+
assertThat(result.getErrors().get(0).getAsJsonObject().getAsJsonPrimitive("message").getAsString(), startsWith("Failed to parse pipeline file as JSON: "));
85+
9186
}
9287
@Test
93-
public void shouldThrowAllErrorsWhenManyFilesHaveInvalidContent() throws Exception
88+
public void shouldAppendAllErrorsWhenManyFilesHaveInvalidContent() throws Exception
9489
{
9590
createFileWithContent("pipe1.gopipeline.json", this.pipe1String);
9691
createFileWithContent("pipeBad1.gopipeline.json", "bad pipeline");
9792
createFileWithContent("pipeBad2.gopipeline.json", "bad pipeline 2");
98-
try {
99-
parser.parseDirectory(directory);
100-
fail("should have thrown");
101-
}
102-
catch (ConfigDirectoryParseException parseException)
103-
{
104-
assertThat(parseException.getErrors().size(),is(2));
105-
}
93+
94+
JsonConfigCollection result = parser.parseDirectory(directory);
95+
assertThat(result.getErrors().size(),is(2));
10696
}
10797
@Test
108-
public void shouldThrowErrorWhenPipelineFileIsEmpty() throws Exception
98+
public void shouldAppendErrorWhenPipelineFileIsEmpty() throws Exception
10999
{
110100
createFileWithContent("pipe1.gopipeline.json", this.pipe1String);
111101
createFileWithContent("pipeBad1.gopipeline.json", "");
112-
try {
113-
parser.parseDirectory(directory);
114-
fail("should have thrown");
115-
}
116-
catch (ConfigDirectoryParseException parseException)
117-
{
118-
assertThat(parseException.getErrors().size(),is(1));
119-
assertThat(parseException.getErrors().get(0).getLocation(), is("pipeBad1.gopipeline.json"));
120-
assertThat(parseException.getErrors().get(0).getMessage(), is("Pipeline file is empty"));
121-
}
102+
103+
JsonConfigCollection result = parser.parseDirectory(directory);
104+
assertThat(result.getErrors().size(),is(1));
105+
106+
assertThat(result.getErrors().size(),is(1));
107+
assertThat(result.getErrors().get(0).getAsJsonObject().getAsJsonPrimitive("location").getAsString(), is("pipeBad1.gopipeline.json"));
108+
assertThat(result.getErrors().get(0).getAsJsonObject().getAsJsonPrimitive("message").getAsString(), is("Pipeline file is empty"));
122109
}
123110
@Test
124-
public void shouldThrowErrorWhenPipelineBlockIsEmpty() throws Exception
111+
public void shouldAppendErrorWhenPipelineBlockIsEmpty() throws Exception
125112
{
126113
createFileWithContent("pipe1.gopipeline.json", this.pipe1String);
127114
createFileWithContent("pipeBad1.gopipeline.json", "{}");
128-
try {
129-
parser.parseDirectory(directory);
130-
fail("should have thrown");
131-
}
132-
catch (ConfigDirectoryParseException parseException)
133-
{
134-
assertThat(parseException.getErrors().size(),is(1));
135-
assertThat(parseException.getErrors().get(0).getLocation(), is("pipeBad1.gopipeline.json"));
136-
assertThat(parseException.getErrors().get(0).getMessage(), is("Pipeline definition is empty"));
137-
}
115+
116+
JsonConfigCollection result = parser.parseDirectory(directory);
117+
assertThat(result.getErrors().size(),is(1));
118+
assertThat(result.getErrors().get(0).getAsJsonObject().getAsJsonPrimitive("location").getAsString(), is("pipeBad1.gopipeline.json"));
119+
assertThat(result.getErrors().get(0).getAsJsonObject().getAsJsonPrimitive("message").getAsString(), is("Pipeline definition is empty"));
138120
}
139121

140122

@@ -146,67 +128,49 @@ public void shouldAppendEnvironmentFromDirectory() throws Exception
146128
assertThat(result.getEnvironments().size(), is(1));
147129
}
148130
@Test
149-
public void shouldThrowErrorsWithLocationWhenInvalidContentInEnvironment() throws Exception
131+
public void shouldAppendErrorsWithLocationWhenInvalidContentInEnvironment() throws Exception
150132
{
151133
createFileWithContent("devenv.goenvironment.json", this.pipe1String);
152134
createFileWithContent("badEnv.goenvironment.json", "bad environment");
153-
try {
154-
parser.parseDirectory(directory);
155-
fail("should have thrown");
156-
}
157-
catch (ConfigDirectoryParseException parseException)
158-
{
159-
assertThat(parseException.getErrors().size(),is(1));
160-
assertThat(parseException.getErrors().get(0).getLocation(), is("badEnv.goenvironment.json"));
161-
assertThat(parseException.getErrors().get(0).getMessage(), startsWith("Failed to parse environment file as JSON: "));
162-
}
135+
JsonConfigCollection result = parser.parseDirectory(directory);
136+
assertThat(result.getErrors().size(),is(1));
137+
138+
assertThat(result.getErrors().get(0).getAsJsonObject().getAsJsonPrimitive("location").getAsString(), is("badEnv.goenvironment.json"));
139+
assertThat(result.getErrors().get(0).getAsJsonObject().getAsJsonPrimitive("message").getAsString(), startsWith("Failed to parse environment file as JSON: "));
163140
}
164141
@Test
165-
public void shouldThrowAllErrorsWhenManyEnvironmentFilesHaveInvalidContent() throws Exception
142+
public void shouldAppendAllErrorsWhenManyEnvironmentFilesHaveInvalidContent() throws Exception
166143
{
167144
createFileWithContent("pipe1.gopipeline.json", this.pipe1String);
168145
createFileWithContent("badEnv.goenvironment.json", "bad env");
169146
createFileWithContent("badEnv2.goenvironment.json", "bad env 2");
170-
try {
171-
parser.parseDirectory(directory);
172-
fail("should have thrown");
173-
}
174-
catch (ConfigDirectoryParseException parseException)
175-
{
176-
assertThat(parseException.getErrors().size(),is(2));
177-
}
147+
JsonConfigCollection result = parser.parseDirectory(directory);
148+
149+
assertThat(result.getErrors().size(),is(2));
178150
}
179151
@Test
180-
public void shouldThrowErrorWhenEnvironmentFileIsEmpty() throws Exception
152+
public void shouldAppendErrorWhenEnvironmentFileIsEmpty() throws Exception
181153
{
182154
createFileWithContent("devenv.goenvironment.json", this.devenvString);
183155
createFileWithContent("badEnv.goenvironment.json", "");
184-
try {
185-
parser.parseDirectory(directory);
186-
fail("should have thrown");
187-
}
188-
catch (ConfigDirectoryParseException parseException)
189-
{
190-
assertThat(parseException.getErrors().size(),is(1));
191-
assertThat(parseException.getErrors().get(0).getLocation(), is("badEnv.goenvironment.json"));
192-
assertThat(parseException.getErrors().get(0).getMessage(), is("Environment file is empty"));
193-
}
156+
157+
JsonConfigCollection result = parser.parseDirectory(directory);
158+
assertThat(result.getErrors().size(),is(1));
159+
160+
assertThat(result.getErrors().get(0).getAsJsonObject().getAsJsonPrimitive("location").getAsString(), is("badEnv.goenvironment.json"));
161+
assertThat(result.getErrors().get(0).getAsJsonObject().getAsJsonPrimitive("message").getAsString(), is("Environment file is empty"));
194162
}
195163
@Test
196164
public void shouldThrowErrorWhenEnvironmentBlockIsEmpty() throws Exception
197165
{
198166
createFileWithContent("devenv.goenvironment.json", this.devenvString);
199167
createFileWithContent("badEnv.goenvironment.json", "{}");
200-
try {
201-
parser.parseDirectory(directory);
202-
fail("should have thrown");
203-
}
204-
catch (ConfigDirectoryParseException parseException)
205-
{
206-
assertThat(parseException.getErrors().size(),is(1));
207-
assertThat(parseException.getErrors().get(0).getLocation(), is("badEnv.goenvironment.json"));
208-
assertThat(parseException.getErrors().get(0).getMessage(), is("Environment definition is empty"));
209-
}
168+
169+
JsonConfigCollection result = parser.parseDirectory(directory);
170+
assertThat(result.getErrors().size(),is(1));
171+
172+
assertThat(result.getErrors().get(0).getAsJsonObject().getAsJsonPrimitive("location").getAsString(), is("badEnv.goenvironment.json"));
173+
assertThat(result.getErrors().get(0).getAsJsonObject().getAsJsonPrimitive("message").getAsString(), is("Environment definition is empty"));
210174
}
211175

212176
private void createFileWithContent(String filePath, String content) throws FileNotFoundException, UnsupportedEncodingException {

0 commit comments

Comments
 (0)