You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by ja...@apache.org on 2020/09/08 09:03:38 UTC

[camel] branch master updated: CAMEL-15507: Make vertx-http component error handling consistent with other HTTP client components

This is an automated email from the ASF dual-hosted git repository.

jamesnetherton pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/camel.git


The following commit(s) were added to refs/heads/master by this push:
     new b035806  CAMEL-15507: Make vertx-http component error handling consistent with other HTTP client components
b035806 is described below

commit b035806dd8fcb6fa2d70d2ebe4a151a11b2ac485
Author: James Netherton <ja...@gmail.com>
AuthorDate: Tue Sep 8 09:55:45 2020 +0100

    CAMEL-15507: Make vertx-http component error handling consistent with other HTTP client components
---
 .../vertx/http/DefaultVertxHttpBinding.java        | 28 +++++++++++-----------
 .../component/vertx/http/VertxHttpBinding.java     |  5 ++--
 .../component/vertx/http/VertxHttpHelper.java      | 10 ++++++--
 .../vertx/http/VertxHttpCustomBindingTest.java     |  2 +-
 .../vertx/http/VertxHttpRestProducerTest.java      |  3 ++-
 .../component/vertx/http/VertxHttpSessionTest.java |  5 +++-
 .../http/VertxHttpThrowExceptionOnFailureTest.java | 17 ++++---------
 .../vertx/http/VertxHttpTransferExceptionTest.java |  7 ------
 8 files changed, 37 insertions(+), 40 deletions(-)

diff --git a/components/camel-vertx-http/src/main/java/org/apache/camel/component/vertx/http/DefaultVertxHttpBinding.java b/components/camel-vertx-http/src/main/java/org/apache/camel/component/vertx/http/DefaultVertxHttpBinding.java
index 5ac652a..024e147 100644
--- a/components/camel-vertx-http/src/main/java/org/apache/camel/component/vertx/http/DefaultVertxHttpBinding.java
+++ b/components/camel-vertx-http/src/main/java/org/apache/camel/component/vertx/http/DefaultVertxHttpBinding.java
@@ -131,17 +131,13 @@ public class DefaultVertxHttpBinding implements VertxHttpBinding {
         if (response.succeeded()) {
             Message message = exchange.getMessage();
             VertxHttpConfiguration configuration = endpoint.getConfiguration();
+            boolean statusCodeOk = HttpHelper.isStatusCodeOk(result.statusCode(), configuration.getOkStatusCodeRange());
 
-            populateResponseHeaders(exchange, result, configuration.getHeaderFilterStrategy());
-
-            if (!configuration.isThrowExceptionOnFailure()) {
-                message.setBody(processResponseBody(endpoint, exchange, result.body()));
+            if ((!configuration.isThrowExceptionOnFailure()) || (configuration.isThrowExceptionOnFailure() && statusCodeOk)) {
+                populateResponseHeaders(exchange, result, configuration.getHeaderFilterStrategy());
+                message.setBody(processResponseBody(endpoint, exchange, result));
             } else {
-                if (HttpHelper.isStatusCodeOk(result.statusCode(), configuration.getOkStatusCodeRange())) {
-                    message.setBody(processResponseBody(endpoint, exchange, result.body()));
-                } else {
-                    exchange.setException(handleResponseFailure(endpoint, exchange, result));
-                }
+                exchange.setException(handleResponseFailure(endpoint, exchange, result));
             }
         } else {
             exchange.setException(response.cause());
@@ -171,10 +167,13 @@ public class DefaultVertxHttpBinding implements VertxHttpBinding {
     }
 
     @Override
-    public Object processResponseBody(VertxHttpEndpoint endpoint, Exchange exchange, Buffer responseBody, boolean exceptionOnly)
+    public Object processResponseBody(
+            VertxHttpEndpoint endpoint, Exchange exchange, HttpResponse<Buffer> result, boolean exceptionOnly)
             throws Exception {
+        Buffer responseBody = result.body();
         if (responseBody != null) {
-            if (VertxHttpHelper.isContentTypeMatching(exchange, CONTENT_TYPE_JAVA_SERIALIZED_OBJECT)) {
+            String contentType = result.getHeader(Exchange.CONTENT_TYPE);
+            if (VertxHttpHelper.isContentTypeMatching(CONTENT_TYPE_JAVA_SERIALIZED_OBJECT, contentType)) {
                 boolean transferException = endpoint.getConfiguration().isTransferException();
                 boolean allowJavaSerializedObject = endpoint.getComponent().isAllowJavaSerializedObject();
 
@@ -196,8 +195,9 @@ public class DefaultVertxHttpBinding implements VertxHttpBinding {
         return null;
     }
 
-    public Object processResponseBody(VertxHttpEndpoint endpoint, Exchange exchange, Buffer responseBody) throws Exception {
-        return processResponseBody(endpoint, exchange, responseBody, false);
+    public Object processResponseBody(VertxHttpEndpoint endpoint, Exchange exchange, HttpResponse<Buffer> result)
+            throws Exception {
+        return processResponseBody(endpoint, exchange, result, false);
     }
 
     @Override
@@ -206,7 +206,7 @@ public class DefaultVertxHttpBinding implements VertxHttpBinding {
         VertxHttpConfiguration configuration = endpoint.getConfiguration();
         Throwable exception;
 
-        Object responseBody = processResponseBody(endpoint, exchange, result.body(), true);
+        Object responseBody = processResponseBody(endpoint, exchange, result, true);
         if (responseBody instanceof Throwable) {
             // Use the exception that was deserialized from the response
             exception = (Throwable) responseBody;
diff --git a/components/camel-vertx-http/src/main/java/org/apache/camel/component/vertx/http/VertxHttpBinding.java b/components/camel-vertx-http/src/main/java/org/apache/camel/component/vertx/http/VertxHttpBinding.java
index c7de603..96072f2 100644
--- a/components/camel-vertx-http/src/main/java/org/apache/camel/component/vertx/http/VertxHttpBinding.java
+++ b/components/camel-vertx-http/src/main/java/org/apache/camel/component/vertx/http/VertxHttpBinding.java
@@ -49,9 +49,10 @@ public interface VertxHttpBinding {
     void populateResponseHeaders(Exchange exchange, HttpResponse<Buffer> response, HeaderFilterStrategy headerFilterStrategy);
 
     /**
-     * Processes the received {@link Buffer} response body
+     * Processes the received {@link Buffer} response body in the {@link HttpResponse}
      */
-    Object processResponseBody(VertxHttpEndpoint endpoint, Exchange exchange, Buffer responseBody, boolean exceptionOnly)
+    Object processResponseBody(
+            VertxHttpEndpoint endpoint, Exchange exchange, HttpResponse<Buffer> result, boolean exceptionOnly)
             throws Exception;
 
     /**
diff --git a/components/camel-vertx-http/src/main/java/org/apache/camel/component/vertx/http/VertxHttpHelper.java b/components/camel-vertx-http/src/main/java/org/apache/camel/component/vertx/http/VertxHttpHelper.java
index 7eebb30..98972dd 100644
--- a/components/camel-vertx-http/src/main/java/org/apache/camel/component/vertx/http/VertxHttpHelper.java
+++ b/components/camel-vertx-http/src/main/java/org/apache/camel/component/vertx/http/VertxHttpHelper.java
@@ -145,8 +145,14 @@ public final class VertxHttpHelper {
      * Verifies whether the Content-Type exchange header value matches an expected value
      */
     public static boolean isContentTypeMatching(Exchange exchange, String expected) {
-        String contentType = ExchangeHelper.getContentType(exchange);
-        return contentType != null && contentType.equals(expected);
+        return isContentTypeMatching(expected, ExchangeHelper.getContentType(exchange));
+    }
+
+    /**
+     * Verifies whether the expected Content-Type value matches an expected value
+     */
+    public static boolean isContentTypeMatching(String expected, String actual) {
+        return actual != null && expected.equals(actual);
     }
 
     /**
diff --git a/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpCustomBindingTest.java b/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpCustomBindingTest.java
index 25b2595..43c62bd 100644
--- a/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpCustomBindingTest.java
+++ b/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpCustomBindingTest.java
@@ -66,7 +66,7 @@ public class VertxHttpCustomBindingTest extends VertxHttpTestSupport {
 
             @Override
             public Object processResponseBody(
-                    VertxHttpEndpoint endpoint, Exchange exchange, Buffer responseBody, boolean exceptionOnly)
+                    VertxHttpEndpoint endpoint, Exchange exchange, HttpResponse<Buffer> result, boolean exceptionOnly)
                     throws Exception {
                 return null;
             }
diff --git a/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpRestProducerTest.java b/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpRestProducerTest.java
index d337125..630ef8d 100644
--- a/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpRestProducerTest.java
+++ b/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpRestProducerTest.java
@@ -27,7 +27,8 @@ import org.apache.camel.builder.RouteBuilder;
 import org.apache.camel.component.rest.openapi.RestOpenApiComponent;
 import org.junit.jupiter.api.Test;
 
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public class VertxHttpRestProducerTest extends VertxHttpTestSupport {
 
diff --git a/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpSessionTest.java b/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpSessionTest.java
index afa55af..c930523 100644
--- a/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpSessionTest.java
+++ b/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpSessionTest.java
@@ -27,6 +27,7 @@ import org.apache.camel.Message;
 import org.apache.camel.Processor;
 import org.apache.camel.RoutesBuilder;
 import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.http.base.HttpOperationFailedException;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -47,7 +48,9 @@ public class VertxHttpSessionTest extends VertxHttpTestSupport {
     @Test
     public void testSessionSupport() {
         Exchange result = template.request(getProducerUri() + "/secure?sessionManagement=true&cookieStore=#cookieStore", null);
-        assertEquals(403, result.getMessage().getHeader(Exchange.HTTP_RESPONSE_CODE));
+
+        HttpOperationFailedException exception = result.getException(HttpOperationFailedException.class);
+        assertEquals(403, exception.getStatusCode());
 
         result = template.request(getProducerUri() + "/login?sessionManagement=true&cookieStore=#cookieStore", exchange -> {
             exchange.getMessage().setHeader("username", USERNAME);
diff --git a/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpThrowExceptionOnFailureTest.java b/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpThrowExceptionOnFailureTest.java
index d5e6d3e..c46efb9 100644
--- a/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpThrowExceptionOnFailureTest.java
+++ b/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpThrowExceptionOnFailureTest.java
@@ -37,8 +37,7 @@ public class VertxHttpThrowExceptionOnFailureTest extends VertxHttpTestSupport {
         assertTrue(exchange.isFailed());
 
         Map<String, Object> headers = exchange.getMessage().getHeaders();
-        assertEquals(500, headers.get(Exchange.HTTP_RESPONSE_CODE));
-        assertEquals("Internal Server Error", headers.get(Exchange.HTTP_RESPONSE_TEXT));
+        assertTrue(headers.isEmpty());
 
         HttpOperationFailedException exception = exchange.getException(HttpOperationFailedException.class);
         assertEquals(500, exception.getStatusCode());
@@ -52,8 +51,7 @@ public class VertxHttpThrowExceptionOnFailureTest extends VertxHttpTestSupport {
         assertTrue(exchange.isFailed());
 
         Map<String, Object> headers = exchange.getMessage().getHeaders();
-        assertEquals(500, headers.get(Exchange.HTTP_RESPONSE_CODE));
-        assertEquals("Internal Server Error", headers.get(Exchange.HTTP_RESPONSE_TEXT));
+        assertTrue(headers.isEmpty());
 
         HttpOperationFailedException exception = exchange.getException(HttpOperationFailedException.class);
         assertEquals(500, exception.getStatusCode());
@@ -68,8 +66,7 @@ public class VertxHttpThrowExceptionOnFailureTest extends VertxHttpTestSupport {
         assertTrue(exchange.isFailed());
 
         Map<String, Object> headers = exchange.getMessage().getHeaders();
-        assertEquals(201, headers.get(Exchange.HTTP_RESPONSE_CODE));
-        assertEquals("Created", headers.get(Exchange.HTTP_RESPONSE_TEXT));
+        assertTrue(headers.isEmpty());
 
         HttpOperationFailedException exception = exchange.getException(HttpOperationFailedException.class);
         assertEquals(201, exception.getStatusCode());
@@ -87,13 +84,9 @@ public class VertxHttpThrowExceptionOnFailureTest extends VertxHttpTestSupport {
         });
         assertTrue(exchange.isFailed());
 
-        Map<String, Object> headers = exchange.getMessage().getHeaders();
-        assertEquals(500, headers.get(Exchange.HTTP_RESPONSE_CODE));
-        assertEquals("Internal Server Error", headers.get(Exchange.HTTP_RESPONSE_TEXT));
-
         HttpOperationFailedException exception = exchange.getException(HttpOperationFailedException.class);
-        assertEquals(500, headers.get(Exchange.HTTP_RESPONSE_CODE));
-        assertEquals("Internal Server Error", headers.get(Exchange.HTTP_RESPONSE_TEXT));
+        assertEquals(500, exception.getStatusCode());
+        assertEquals("Internal Server Error", exception.getStatusText());
         assertEquals(getTestServerUrl() + "/redirect", exception.getUri());
     }
 
diff --git a/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpTransferExceptionTest.java b/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpTransferExceptionTest.java
index 96224e3..a9681c4 100644
--- a/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpTransferExceptionTest.java
+++ b/components/camel-vertx-http/src/test/java/org/apache/camel/component/vertx/http/VertxHttpTransferExceptionTest.java
@@ -16,10 +16,7 @@
  */
 package org.apache.camel.component.vertx.http;
 
-import java.util.Map;
-
 import org.apache.camel.Exchange;
-import org.apache.camel.Message;
 import org.apache.camel.RoutesBuilder;
 import org.apache.camel.builder.RouteBuilder;
 import org.junit.jupiter.api.Test;
@@ -39,10 +36,6 @@ public class VertxHttpTransferExceptionTest extends VertxHttpTestSupport {
         assertNotNull(exception);
         assertTrue(exception instanceof IllegalStateException);
         assertEquals("Forced Exception", exception.getMessage());
-
-        Message message = exchange.getMessage();
-        Map<String, Object> headers = message.getHeaders();
-        assertEquals(VertxHttpConstants.CONTENT_TYPE_JAVA_SERIALIZED_OBJECT, headers.get(Exchange.CONTENT_TYPE));
     }
 
     @Override