Skip to content

Commit bc1d0c7

Browse files
jminijmini
authored
Improve error message on error (gitlab4j#1238)
1 parent fe14e34 commit bc1d0c7

File tree

2 files changed

+114
-61
lines changed

2 files changed

+114
-61
lines changed

‎gitlab4j-api/src/main/java/org/gitlab4j/api/GitLabApiException.java

+14-9
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
package org.gitlab4j.api;
22

33
import java.util.ArrayList;
4-
import java.util.HashMap;
54
import java.util.Iterator;
5+
import java.util.LinkedHashMap;
66
import java.util.List;
77
import java.util.Map;
88
import java.util.Map.Entry;
99
import java.util.Objects;
10+
import java.util.stream.Collectors;
1011

1112
import jakarta.ws.rs.core.MediaType;
1213
import jakarta.ws.rs.core.MultivaluedMap;
@@ -85,8 +86,7 @@ public GitLabApiException(Response response) {
8586
// If the node is an object, then it is validation errors
8687
if (jsonMessage.isObject()) {
8788

88-
StringBuilder buf = new StringBuilder();
89-
validationErrors = new HashMap<>();
89+
validationErrors = new LinkedHashMap<>();
9090
Iterator<Entry<String, JsonNode>> fields = jsonMessage.fields();
9191
while (fields.hasNext()) {
9292

@@ -97,14 +97,19 @@ public GitLabApiException(Response response) {
9797
for (JsonNode value : field.getValue()) {
9898
values.add(value.asText());
9999
}
100-
101-
if (values.size() > 0) {
102-
buf.append((buf.length() > 0 ? ", " : "")).append(fieldName);
103-
}
104100
}
105101

106-
if (buf.length() > 0) {
107-
this.message = "The following fields have validation errors: " + buf.toString();
102+
if (!validationErrors.isEmpty()) {
103+
this.message = "The following fields have validation errors: "
104+
+ String.join(", ", validationErrors.keySet()) + "\n"
105+
+ validationErrors.entrySet().stream()
106+
.map(e -> {
107+
return "* " + e.getKey()
108+
+ e.getValue().stream()
109+
.collect(Collectors.joining(
110+
"\n - ", "\n - ", ""));
111+
})
112+
.collect(Collectors.joining("\n"));
108113
}
109114

110115
} else if (jsonMessage.isArray()) {

‎gitlab4j-api/src/test/java/org/gitlab4j/api/TestGitLabApiException.java

+100-52
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import static org.junit.jupiter.api.Assertions.assertNotNull;
66
import static org.junit.jupiter.api.Assertions.assertTrue;
77
import static org.junit.jupiter.api.Assertions.fail;
8-
import static org.junit.jupiter.api.Assumptions.assumeTrue;
98

109
import java.util.List;
1110
import java.util.Map;
@@ -14,27 +13,9 @@
1413

1514
import org.gitlab4j.api.models.Project;
1615
import org.gitlab4j.api.models.Visibility;
17-
import org.junit.jupiter.api.AfterAll;
18-
import org.junit.jupiter.api.BeforeAll;
19-
import org.junit.jupiter.api.BeforeEach;
20-
import org.junit.jupiter.api.Tag;
2116
import org.junit.jupiter.api.Test;
22-
import org.junit.jupiter.api.extension.ExtendWith;
23-
24-
/**
25-
* In order for these tests to run you must set the following properties in ~/test-gitlab4j.properties
26-
*
27-
* TEST_NAMESPACE
28-
* TEST_HOST_URL
29-
* TEST_PRIVATE_TOKEN
30-
*
31-
* If any of the above are NULL, all tests in this class will be skipped.
32-
*/
33-
@Tag("integration")
34-
@ExtendWith(SetupIntegrationTestExtension.class)
35-
@org.junit.jupiter.api.Disabled(
36-
"Integration tests are disabled, see https://github.com/gitlab4j/gitlab4j-api/issues/1165")
37-
public class TestGitLabApiException extends AbstractIntegrationTest {
17+
18+
public class TestGitLabApiException {
3819

3920
private static final String TEST_PROJECT_NAME_DUPLICATE = "test-gitlab4j-create-project-duplicate";
4021
private static final String TEST_ERROR_MESSAGE =
@@ -45,39 +26,42 @@ public class TestGitLabApiException extends AbstractIntegrationTest {
4526

4627
private static GitLabApi gitLabApi;
4728

48-
public TestGitLabApiException() {
49-
super();
50-
}
51-
52-
@BeforeAll
53-
public static void setup() {
54-
// Must setup the connection to the GitLab test server
55-
gitLabApi = baseTestSetup();
56-
57-
deleteAllTestProjects();
58-
}
59-
60-
@AfterAll
61-
public static void teardown() throws GitLabApiException {
62-
deleteAllTestProjects();
63-
}
64-
65-
private static void deleteAllTestProjects() {
66-
if (gitLabApi != null) {
67-
try {
68-
Project project = gitLabApi.getProjectApi().getProject(TEST_NAMESPACE, TEST_PROJECT_NAME_DUPLICATE);
69-
gitLabApi.getProjectApi().deleteProject(project);
70-
} catch (GitLabApiException ignore) {
71-
}
72-
}
73-
}
74-
75-
@BeforeEach
76-
public void beforeMethod() {
77-
assumeTrue(gitLabApi != null);
78-
}
29+
// public TestGitLabApiException() {
30+
// super();
31+
// }
32+
//
33+
// @BeforeAll
34+
// public static void setup() {
35+
// // Must setup the connection to the GitLab test server
36+
// gitLabApi = baseTestSetup();
37+
//
38+
// deleteAllTestProjects();
39+
// }
40+
//
41+
// @AfterAll
42+
// public static void teardown() throws GitLabApiException {
43+
// deleteAllTestProjects();
44+
// }
45+
//
46+
// private static void deleteAllTestProjects() {
47+
// if (gitLabApi != null) {
48+
// try {
49+
// Project project = gitLabApi.getProjectApi().getProject(TEST_NAMESPACE,
50+
// TEST_PROJECT_NAME_DUPLICATE);
51+
// gitLabApi.getProjectApi().deleteProject(project);
52+
// } catch (GitLabApiException ignore) {
53+
// }
54+
// }
55+
// }
56+
//
57+
// @BeforeEach
58+
// public void beforeMethod() {
59+
// assumeTrue(gitLabApi != null);
60+
// }
7961

8062
@Test
63+
@org.junit.jupiter.api.Disabled(
64+
"Integration tests are disabled, see https://github.com/gitlab4j/gitlab4j-api/issues/1165")
8165
public void testNotFoundError() throws GitLabApiException {
8266

8367
try {
@@ -96,6 +80,8 @@ public void testNotFoundError() throws GitLabApiException {
9680
}
9781

9882
@Test
83+
@org.junit.jupiter.api.Disabled(
84+
"Integration tests are disabled, see https://github.com/gitlab4j/gitlab4j-api/issues/1165")
9985
public void testValidationErrors() throws GitLabApiException {
10086

10187
Project project = new Project()
@@ -125,6 +111,68 @@ public void testStringMessage() throws GitLabApiException {
125111
assertEquals(TEST_ERROR_MESSAGE, glae.getMessage());
126112
}
127113

114+
@Test
115+
public void testObjectMessage() throws GitLabApiException {
116+
String expectedMessage = ""
117+
+ "The following fields have validation errors: project_namespace.name, name\n"
118+
+ "* project_namespace.name\n"
119+
+ " - has already been taken\n"
120+
+ "* name\n"
121+
+ " - has already been taken";
122+
final MockResponse response = new MockResponse(
123+
Status.BAD_REQUEST,
124+
"{\"message\":{\"project_namespace.name\":[\"has already been taken\"],\"name\":[\"has already been taken\"]}}");
125+
GitLabApiException glae = new GitLabApiException(response);
126+
assertEquals(Status.BAD_REQUEST.getStatusCode(), glae.getHttpStatus());
127+
assertEquals(expectedMessage, glae.getMessage());
128+
assertEquals(2, glae.getValidationErrors().size());
129+
assertEquals(1, glae.getValidationErrors().get("project_namespace.name").size());
130+
assertEquals(
131+
"has already been taken",
132+
glae.getValidationErrors().get("project_namespace.name").get(0));
133+
assertEquals(1, glae.getValidationErrors().get("name").size());
134+
assertEquals(
135+
"has already been taken", glae.getValidationErrors().get("name").get(0));
136+
}
137+
138+
@Test
139+
public void testObjectMessage1() throws GitLabApiException {
140+
String message = "{\n"
141+
+ " \"message\":{\n"
142+
+ " \"f1\":[\n"
143+
+ " \"m1\",\n"
144+
+ " \"m2\"\n"
145+
+ " ],\n"
146+
+ " \"f2\":[\n"
147+
+ " \"n1\",\n"
148+
+ " \"n2\",\n"
149+
+ " \"n3\"\n"
150+
+ " ]\n"
151+
+ " }\n"
152+
+ "}";
153+
String expectedMessage = ""
154+
+ "The following fields have validation errors: f1, f2\n"
155+
+ "* f1\n"
156+
+ " - m1\n"
157+
+ " - m2\n"
158+
+ "* f2\n"
159+
+ " - n1\n"
160+
+ " - n2\n"
161+
+ " - n3";
162+
final MockResponse response = new MockResponse(Status.BAD_REQUEST, message);
163+
GitLabApiException glae = new GitLabApiException(response);
164+
assertEquals(Status.BAD_REQUEST.getStatusCode(), glae.getHttpStatus());
165+
assertEquals(expectedMessage, glae.getMessage());
166+
assertEquals(2, glae.getValidationErrors().size());
167+
assertEquals(2, glae.getValidationErrors().get("f1").size());
168+
assertEquals("m1", glae.getValidationErrors().get("f1").get(0));
169+
assertEquals("m2", glae.getValidationErrors().get("f1").get(1));
170+
assertEquals(3, glae.getValidationErrors().get("f2").size());
171+
assertEquals("n1", glae.getValidationErrors().get("f2").get(0));
172+
assertEquals("n2", glae.getValidationErrors().get("f2").get(1));
173+
assertEquals("n3", glae.getValidationErrors().get("f2").get(2));
174+
}
175+
128176
@Test
129177
public void testArrayMessage() throws GitLabApiException {
130178
final MockResponse response = new MockResponse(Status.BAD_REQUEST, TEST_RESPONSE_JSON_ARRAY);

0 commit comments

Comments
 (0)