Skip to content

Improve error message on error #1238

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 1, 2025
Merged

Improve error message on error #1238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions gitlab4j-api/src/main/java/org/gitlab4j/api/GitLabApiException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package org.gitlab4j.api;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.stream.Collectors;

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

StringBuilder buf = new StringBuilder();
validationErrors = new HashMap<>();
validationErrors = new LinkedHashMap<>();
Iterator<Entry<String, JsonNode>> fields = jsonMessage.fields();
while (fields.hasNext()) {

Expand All @@ -97,14 +97,19 @@ public GitLabApiException(Response response) {
for (JsonNode value : field.getValue()) {
values.add(value.asText());
}

if (values.size() > 0) {
buf.append((buf.length() > 0 ? ", " : "")).append(fieldName);
}
}

if (buf.length() > 0) {
this.message = "The following fields have validation errors: " + buf.toString();
if (!validationErrors.isEmpty()) {
this.message = "The following fields have validation errors: "
+ String.join(", ", validationErrors.keySet()) + "\n"
+ validationErrors.entrySet().stream()
.map(e -> {
return "* " + e.getKey()
+ e.getValue().stream()
.collect(Collectors.joining(
"\n - ", "\n - ", ""));
})
.collect(Collectors.joining("\n"));
}

} else if (jsonMessage.isArray()) {
Expand Down
152 changes: 100 additions & 52 deletions gitlab4j-api/src/test/java/org/gitlab4j/api/TestGitLabApiException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import static org.junit.jupiter.api.Assumptions.assumeTrue;

import java.util.List;
import java.util.Map;
Expand All @@ -14,27 +13,9 @@

import org.gitlab4j.api.models.Project;
import org.gitlab4j.api.models.Visibility;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;

/**
* In order for these tests to run you must set the following properties in ~/test-gitlab4j.properties
*
* TEST_NAMESPACE
* TEST_HOST_URL
* TEST_PRIVATE_TOKEN
*
* If any of the above are NULL, all tests in this class will be skipped.
*/
@Tag("integration")
@ExtendWith(SetupIntegrationTestExtension.class)
@org.junit.jupiter.api.Disabled(
"Integration tests are disabled, see https://github.com/gitlab4j/gitlab4j-api/issues/1165")
public class TestGitLabApiException extends AbstractIntegrationTest {

public class TestGitLabApiException {

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

private static GitLabApi gitLabApi;

public TestGitLabApiException() {
super();
}

@BeforeAll
public static void setup() {
// Must setup the connection to the GitLab test server
gitLabApi = baseTestSetup();

deleteAllTestProjects();
}

@AfterAll
public static void teardown() throws GitLabApiException {
deleteAllTestProjects();
}

private static void deleteAllTestProjects() {
if (gitLabApi != null) {
try {
Project project = gitLabApi.getProjectApi().getProject(TEST_NAMESPACE, TEST_PROJECT_NAME_DUPLICATE);
gitLabApi.getProjectApi().deleteProject(project);
} catch (GitLabApiException ignore) {
}
}
}

@BeforeEach
public void beforeMethod() {
assumeTrue(gitLabApi != null);
}
// public TestGitLabApiException() {
// super();
// }
//
// @BeforeAll
// public static void setup() {
// // Must setup the connection to the GitLab test server
// gitLabApi = baseTestSetup();
//
// deleteAllTestProjects();
// }
//
// @AfterAll
// public static void teardown() throws GitLabApiException {
// deleteAllTestProjects();
// }
//
// private static void deleteAllTestProjects() {
// if (gitLabApi != null) {
// try {
// Project project = gitLabApi.getProjectApi().getProject(TEST_NAMESPACE,
// TEST_PROJECT_NAME_DUPLICATE);
// gitLabApi.getProjectApi().deleteProject(project);
// } catch (GitLabApiException ignore) {
// }
// }
// }
//
// @BeforeEach
// public void beforeMethod() {
// assumeTrue(gitLabApi != null);
// }

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

try {
Expand All @@ -96,6 +80,8 @@ public void testNotFoundError() throws GitLabApiException {
}

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

Project project = new Project()
Expand Down Expand Up @@ -125,6 +111,68 @@ public void testStringMessage() throws GitLabApiException {
assertEquals(TEST_ERROR_MESSAGE, glae.getMessage());
}

@Test
public void testObjectMessage() throws GitLabApiException {
String expectedMessage = ""
+ "The following fields have validation errors: project_namespace.name, name\n"
+ "* project_namespace.name\n"
+ " - has already been taken\n"
+ "* name\n"
+ " - has already been taken";
final MockResponse response = new MockResponse(
Status.BAD_REQUEST,
"{\"message\":{\"project_namespace.name\":[\"has already been taken\"],\"name\":[\"has already been taken\"]}}");
GitLabApiException glae = new GitLabApiException(response);
assertEquals(Status.BAD_REQUEST.getStatusCode(), glae.getHttpStatus());
assertEquals(expectedMessage, glae.getMessage());
assertEquals(2, glae.getValidationErrors().size());
assertEquals(1, glae.getValidationErrors().get("project_namespace.name").size());
assertEquals(
"has already been taken",
glae.getValidationErrors().get("project_namespace.name").get(0));
assertEquals(1, glae.getValidationErrors().get("name").size());
assertEquals(
"has already been taken", glae.getValidationErrors().get("name").get(0));
}

@Test
public void testObjectMessage1() throws GitLabApiException {
String message = "{\n"
+ " \"message\":{\n"
+ " \"f1\":[\n"
+ " \"m1\",\n"
+ " \"m2\"\n"
+ " ],\n"
+ " \"f2\":[\n"
+ " \"n1\",\n"
+ " \"n2\",\n"
+ " \"n3\"\n"
+ " ]\n"
+ " }\n"
+ "}";
String expectedMessage = ""
+ "The following fields have validation errors: f1, f2\n"
+ "* f1\n"
+ " - m1\n"
+ " - m2\n"
+ "* f2\n"
+ " - n1\n"
+ " - n2\n"
+ " - n3";
final MockResponse response = new MockResponse(Status.BAD_REQUEST, message);
GitLabApiException glae = new GitLabApiException(response);
assertEquals(Status.BAD_REQUEST.getStatusCode(), glae.getHttpStatus());
assertEquals(expectedMessage, glae.getMessage());
assertEquals(2, glae.getValidationErrors().size());
assertEquals(2, glae.getValidationErrors().get("f1").size());
assertEquals("m1", glae.getValidationErrors().get("f1").get(0));
assertEquals("m2", glae.getValidationErrors().get("f1").get(1));
assertEquals(3, glae.getValidationErrors().get("f2").size());
assertEquals("n1", glae.getValidationErrors().get("f2").get(0));
assertEquals("n2", glae.getValidationErrors().get("f2").get(1));
assertEquals("n3", glae.getValidationErrors().get("f2").get(2));
}

@Test
public void testArrayMessage() throws GitLabApiException {
final MockResponse response = new MockResponse(Status.BAD_REQUEST, TEST_RESPONSE_JSON_ARRAY);
Expand Down