From 94c4570e16324b4c654d4ec3f0f940926064d955 Mon Sep 17 00:00:00 2001 From: Neenu Shaji Date: Fri, 7 May 2021 14:38:03 -0400 Subject: [PATCH] fix: update GoogleJsonError object to accomodate all fields in Invalid parameter exception (#1783) Fixes b/185405327 Invalid parameter error message does not contain details about which parameter is the invalid one. These fields have been added to GoogleJsonError object. --- .../googleapis/json/GoogleJsonError.java | 81 ++++++++++++++++++- .../googleapis/json/GoogleJsonErrorTest.java | 46 ++++++++++- .../json/GoogleJsonResponseExceptionTest.java | 72 ++++++++++++++--- .../api/client/googleapis/json/error.json | 23 ++++++ 4 files changed, 206 insertions(+), 16 deletions(-) create mode 100644 google-api-client/src/test/resources/com/google/api/client/googleapis/json/error.json diff --git a/google-api-client/src/main/java/com/google/api/client/googleapis/json/GoogleJsonError.java b/google-api-client/src/main/java/com/google/api/client/googleapis/json/GoogleJsonError.java index 265407f51..1a530145a 100644 --- a/google-api-client/src/main/java/com/google/api/client/googleapis/json/GoogleJsonError.java +++ b/google-api-client/src/main/java/com/google/api/client/googleapis/json/GoogleJsonError.java @@ -21,6 +21,7 @@ import com.google.api.client.json.JsonObjectParser; import com.google.api.client.util.Data; import com.google.api.client.util.Key; +import com.google.common.collect.ImmutableList; import java.io.IOException; import java.util.Collections; import java.util.List; @@ -183,6 +184,64 @@ public ErrorInfo clone() { } } + public static class Details { + @Key("@type") + private String type; + + @Key private String detail; + @Key private List parameterViolations; + + public String getType() { + return type; + } + + public void setType(String type) { + this.type = type; + } + + public String getDetail() { + return detail; + } + + public void setDetail(String detail) { + this.detail = detail; + } + + public List getParameterViolations() { + return parameterViolations; + } + + /** + * Sets parameterViolations list as immutable to prevent exposing mutable state. + * + * @param parameterViolations + */ + public void setParameterViolations(List parameterViolations) { + this.parameterViolations = ImmutableList.copyOf(parameterViolations); + } + } + + public static class ParameterViolations { + @Key private String parameter; + @Key private String description; + + public String getDescription() { + return description; + } + + public void setDescription(String description) { + this.description = description; + } + + public String getParameter() { + return parameter; + } + + public void setParameter(String parameter) { + this.parameter = parameter; + } + } + /** List of detailed errors or {@code null} for none. */ @Key private List errors; @@ -192,6 +251,9 @@ public ErrorInfo clone() { /** Human-readable explanation of the error or {@code null} for none. */ @Key private String message; + /** Lists type and parameterViolation details of an Exception */ + @Key private List
details; + /** * Returns the list of detailed errors or {@code null} for none. * @@ -202,12 +264,13 @@ public final List getErrors() { } /** - * Sets the list of detailed errors or {@code null} for none. + * Sets the list of detailed errors or {@code null} for none. Sets the list of detailed errors as + * immutable to prevent exposing mutable state. * * @since 1.8 */ public final void setErrors(List errors) { - this.errors = errors; + this.errors = ImmutableList.copyOf(errors); } /** @@ -246,6 +309,20 @@ public final void setMessage(String message) { this.message = message; } + public List
getDetails() { + return details; + } + + /** + * Sets the list of invalid parameter error details as immutable to prevent exposing mutable + * state. + * + * @param details + */ + public void setDetails(List
details) { + this.details = ImmutableList.copyOf(details); + } + @Override public GoogleJsonError set(String fieldName, Object value) { return (GoogleJsonError) super.set(fieldName, value); diff --git a/google-api-client/src/test/java/com/google/api/client/googleapis/json/GoogleJsonErrorTest.java b/google-api-client/src/test/java/com/google/api/client/googleapis/json/GoogleJsonErrorTest.java index 83960bc81..99cc3b1d1 100644 --- a/google-api-client/src/test/java/com/google/api/client/googleapis/json/GoogleJsonErrorTest.java +++ b/google-api-client/src/test/java/com/google/api/client/googleapis/json/GoogleJsonErrorTest.java @@ -27,10 +27,11 @@ import com.google.api.client.testing.http.MockHttpTransport; import com.google.api.client.testing.http.MockLowLevelHttpRequest; import com.google.api.client.testing.http.MockLowLevelHttpResponse; +import java.io.InputStream; import junit.framework.TestCase; /** - * Tests {@link GoogleJsonError}. + * Tests {@link com.google.api.client.googleapis.json.GoogleJsonError}. * * @author Yaniv Inbar */ @@ -51,7 +52,8 @@ public class GoogleJsonErrorTest extends TestCase { public void test_json() throws Exception { JsonParser parser = FACTORY.createJsonParser(ERROR); parser.nextToken(); - GoogleJsonError e = parser.parse(GoogleJsonError.class); + com.google.api.client.googleapis.json.GoogleJsonError e = + parser.parse(com.google.api.client.googleapis.json.GoogleJsonError.class); assertEquals(ERROR, FACTORY.toString(e)); } @@ -70,6 +72,10 @@ static class ErrorTransport extends MockHttpTransport { .setStatusCode(HttpStatusCodes.STATUS_CODE_FORBIDDEN); } + ErrorTransport(MockLowLevelHttpResponse mockLowLevelHttpResponse) { + response = mockLowLevelHttpResponse; + } + @Override public LowLevelHttpRequest buildRequest(String name, String url) { return new MockLowLevelHttpRequest(url).setResponse(response); @@ -82,7 +88,41 @@ public void testParse() throws Exception { transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL); request.setThrowExceptionOnExecuteError(false); HttpResponse response = request.execute(); - GoogleJsonError errorResponse = GoogleJsonError.parse(FACTORY, response); + com.google.api.client.googleapis.json.GoogleJsonError errorResponse = + com.google.api.client.googleapis.json.GoogleJsonError.parse(FACTORY, response); assertEquals(ERROR, FACTORY.toString(errorResponse)); } + + public void testParse_withDetails() throws Exception { + String DETAILS_ERROR = + "{" + + "\"code\":400," + + "\"details\":[{" + + "\"@type\":\"type.googleapis.com/google.dataflow.v1beta3.InvalidTemplateParameters\"," + + "\"parameterViolations\":[{" + + "\"description\":\"Parameter didn't match regex '^[0-9a-zA-Z_]+$'\"," + + "\"parameter\":\"safeBrowsingApiKey\"" + + "}]},{" + + "\"@type\":\"type.googleapis.com/google.rpc.DebugInfo\"," + + "\"detail\":\"test detail\"}]," + + "\"message\":\"The template parameters are invalid.\"," + + "\"status\":\"INVALID_ARGUMENT\"" + + "}"; + InputStream errorContent = GoogleJsonErrorTest.class.getResourceAsStream("error.json"); + HttpTransport transport = + new ErrorTransport( + new MockLowLevelHttpResponse() + .setContent(errorContent) + .setContentType(Json.MEDIA_TYPE) + .setStatusCode(HttpStatusCodes.STATUS_CODE_FORBIDDEN)); + HttpRequest request = + transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL); + request.setThrowExceptionOnExecuteError(false); + HttpResponse response = request.execute(); + com.google.api.client.googleapis.json.GoogleJsonError errorResponse = + com.google.api.client.googleapis.json.GoogleJsonError.parse(FACTORY, response); + + assertEquals(DETAILS_ERROR, FACTORY.toString(errorResponse)); + assertNotNull(errorResponse.getDetails()); + } } diff --git a/google-api-client/src/test/java/com/google/api/client/googleapis/json/GoogleJsonResponseExceptionTest.java b/google-api-client/src/test/java/com/google/api/client/googleapis/json/GoogleJsonResponseExceptionTest.java index 075154cc4..ad28bd0ca 100644 --- a/google-api-client/src/test/java/com/google/api/client/googleapis/json/GoogleJsonResponseExceptionTest.java +++ b/google-api-client/src/test/java/com/google/api/client/googleapis/json/GoogleJsonResponseExceptionTest.java @@ -17,10 +17,13 @@ import com.google.api.client.googleapis.json.GoogleJsonErrorTest.ErrorTransport; import com.google.api.client.http.HttpRequest; import com.google.api.client.http.HttpResponse; +import com.google.api.client.http.HttpStatusCodes; import com.google.api.client.http.HttpTransport; import com.google.api.client.json.Json; import com.google.api.client.testing.http.HttpTesting; import com.google.api.client.testing.http.MockHttpTransport; +import com.google.api.client.testing.http.MockLowLevelHttpResponse; +import java.io.InputStream; import junit.framework.TestCase; /** @@ -37,7 +40,8 @@ public void testFrom_noDetails() throws Exception { request.setThrowExceptionOnExecuteError(false); HttpResponse response = request.execute(); GoogleJsonResponseException ge = - GoogleJsonResponseException.from(GoogleJsonErrorTest.FACTORY, response); + GoogleJsonResponseException.from( + com.google.api.client.googleapis.json.GoogleJsonErrorTest.FACTORY, response); assertNull(ge.getDetails()); assertTrue(ge.getMessage().startsWith("200")); } @@ -49,8 +53,12 @@ public void testFrom_withDetails() throws Exception { request.setThrowExceptionOnExecuteError(false); HttpResponse response = request.execute(); GoogleJsonResponseException ge = - GoogleJsonResponseException.from(GoogleJsonErrorTest.FACTORY, response); - assertEquals(GoogleJsonErrorTest.ERROR, GoogleJsonErrorTest.FACTORY.toString(ge.getDetails())); + GoogleJsonResponseException.from( + com.google.api.client.googleapis.json.GoogleJsonErrorTest.FACTORY, response); + assertEquals( + com.google.api.client.googleapis.json.GoogleJsonErrorTest.ERROR, + com.google.api.client.googleapis.json.GoogleJsonErrorTest.FACTORY.toString( + ge.getDetails())); assertTrue(ge.getMessage().startsWith("403")); } @@ -61,7 +69,8 @@ public void testFrom_detailsMissingContent() throws Exception { request.setThrowExceptionOnExecuteError(false); HttpResponse response = request.execute(); GoogleJsonResponseException ge = - GoogleJsonResponseException.from(GoogleJsonErrorTest.FACTORY, response); + GoogleJsonResponseException.from( + com.google.api.client.googleapis.json.GoogleJsonErrorTest.FACTORY, response); assertNull(ge.getDetails()); assertTrue(ge.getMessage().startsWith("403")); } @@ -73,7 +82,8 @@ public void testFrom_detailsArbitraryJsonContent() throws Exception { request.setThrowExceptionOnExecuteError(false); HttpResponse response = request.execute(); GoogleJsonResponseException ge = - GoogleJsonResponseException.from(GoogleJsonErrorTest.FACTORY, response); + GoogleJsonResponseException.from( + com.google.api.client.googleapis.json.GoogleJsonErrorTest.FACTORY, response); assertNull(ge.getDetails()); assertTrue(ge.getMessage().startsWith("403")); } @@ -85,7 +95,8 @@ public void testFrom_detailsArbitraryXmlContent() throws Exception { request.setThrowExceptionOnExecuteError(false); HttpResponse response = request.execute(); GoogleJsonResponseException ge = - GoogleJsonResponseException.from(GoogleJsonErrorTest.FACTORY, response); + GoogleJsonResponseException.from( + com.google.api.client.googleapis.json.GoogleJsonErrorTest.FACTORY, response); assertNull(ge.getDetails()); assertTrue(ge.getMessage().startsWith("403")); assertTrue(ge.getMessage().contains("")); @@ -98,7 +109,8 @@ public void testFrom_errorNoContentButWithJsonContentType() throws Exception { request.setThrowExceptionOnExecuteError(false); HttpResponse response = request.execute(); GoogleJsonResponseException ge = - GoogleJsonResponseException.from(GoogleJsonErrorTest.FACTORY, response); + GoogleJsonResponseException.from( + com.google.api.client.googleapis.json.GoogleJsonErrorTest.FACTORY, response); assertNull(ge.getDetails()); assertTrue(ge.getMessage().startsWith("403")); } @@ -110,7 +122,8 @@ public void testFrom_errorEmptyContentButWithJsonContentType() throws Exception request.setThrowExceptionOnExecuteError(false); HttpResponse response = request.execute(); GoogleJsonResponseException ge = - GoogleJsonResponseException.from(GoogleJsonErrorTest.FACTORY, response); + GoogleJsonResponseException.from( + com.google.api.client.googleapis.json.GoogleJsonErrorTest.FACTORY, response); assertNull(ge.getDetails()); assertTrue(ge.getMessage().startsWith("403")); } @@ -125,7 +138,8 @@ public void testFrom_detailsErrorObject() throws Exception { request.setThrowExceptionOnExecuteError(false); HttpResponse response = request.execute(); GoogleJsonResponseException ge = - GoogleJsonResponseException.from(GoogleJsonErrorTest.FACTORY, response); + GoogleJsonResponseException.from( + com.google.api.client.googleapis.json.GoogleJsonErrorTest.FACTORY, response); assertNotNull(ge.getDetails()); assertEquals("invalid_token", ge.getDetails().getMessage()); assertTrue(ge.getMessage().contains("403")); @@ -141,7 +155,8 @@ public void testFrom_detailsErrorString() throws Exception { request.setThrowExceptionOnExecuteError(false); HttpResponse response = request.execute(); GoogleJsonResponseException ge = - GoogleJsonResponseException.from(GoogleJsonErrorTest.FACTORY, response); + GoogleJsonResponseException.from( + com.google.api.client.googleapis.json.GoogleJsonErrorTest.FACTORY, response); assertNull(ge.getDetails()); assertTrue(ge.getMessage().contains("403")); assertTrue(ge.getMessage().contains("invalid_token")); @@ -155,8 +170,43 @@ public void testFrom_detailsNoErrorField() throws Exception { request.setThrowExceptionOnExecuteError(false); HttpResponse response = request.execute(); GoogleJsonResponseException ge = - GoogleJsonResponseException.from(GoogleJsonErrorTest.FACTORY, response); + GoogleJsonResponseException.from( + com.google.api.client.googleapis.json.GoogleJsonErrorTest.FACTORY, response); assertNull(ge.getDetails()); assertTrue(ge.getMessage().startsWith("403")); } + + public void testFrom_detailsWithInvalidParameter() throws Exception { + String DETAILS_ERROR = + "{" + + "\"code\":400," + + "\"details\":[{" + + "\"@type\":\"type.googleapis.com/google.dataflow.v1beta3.InvalidTemplateParameters\"," + + "\"parameterViolations\":[{" + + "\"description\":\"Parameter didn't match regex '^[0-9a-zA-Z_]+$'\"," + + "\"parameter\":\"safeBrowsingApiKey\"" + + "}]},{" + + "\"@type\":\"type.googleapis.com/google.rpc.DebugInfo\"," + + "\"detail\":\"test detail\"}]," + + "\"message\":\"The template parameters are invalid.\"," + + "\"status\":\"INVALID_ARGUMENT\"" + + "}"; + InputStream errorContent = + com.google.api.client.googleapis.json.GoogleJsonErrorTest.class.getResourceAsStream( + "error.json"); + HttpTransport transport = + new ErrorTransport( + new MockLowLevelHttpResponse() + .setContent(errorContent) + .setContentType(Json.MEDIA_TYPE) + .setStatusCode(HttpStatusCodes.STATUS_CODE_FORBIDDEN)); + HttpRequest request = + transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL); + request.setThrowExceptionOnExecuteError(false); + HttpResponse response = request.execute(); + GoogleJsonResponseException ge = + GoogleJsonResponseException.from( + com.google.api.client.googleapis.json.GoogleJsonErrorTest.FACTORY, response); + assertNotNull(ge.getDetails().getDetails()); + } } diff --git a/google-api-client/src/test/resources/com/google/api/client/googleapis/json/error.json b/google-api-client/src/test/resources/com/google/api/client/googleapis/json/error.json new file mode 100644 index 000000000..578abec20 --- /dev/null +++ b/google-api-client/src/test/resources/com/google/api/client/googleapis/json/error.json @@ -0,0 +1,23 @@ +{ + "error": { + "code": 400, + "message": "The template parameters are invalid.", + "status": "INVALID_ARGUMENT", + "details": [ + { + "@type": "type.googleapis.com/google.dataflow.v1beta3.InvalidTemplateParameters", + "parameterViolations": [ + { + "parameter": "safeBrowsingApiKey", + "description": "Parameter didn't match regex '^[0-9a-zA-Z_]+$'" + } + ] + }, + { + "@type": "type.googleapis.com/google.rpc.DebugInfo", + "detail": "test detail" + } + ] + } +} +