[cxf] branch 3.3.x-fixes updated (4492eba -> 8783ef8)

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[cxf] branch 3.3.x-fixes updated (4492eba -> 8783ef8)

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

reta pushed a change to branch 3.3.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git.


    from 4492eba  CXF-8376: Misleading Error Message From UriBuilder.path(Method) (#727)
     new 0665e4c  CXF-8367: Micrometer always reports outcome=UNKNOWN on success cases (#726)
     new 8783ef8  Recording .gitmergeinfo Changes

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .gitmergeinfo                                      |  2 +
 .../java/org/apache/cxf/message/MessageUtils.java  | 51 ++++++++++++++++++++++
 .../metrics/micrometer/provider/StandardTags.java  | 14 +++---
 .../provider/jaxws/JaxwsFaultCodeProvider.java     |  9 ++--
 .../micrometer/provider/StandardTagsTest.java      | 25 +++--------
 .../provider/jaxws/JaxwsFaultCodeProviderTest.java | 34 ++++++++++++++-
 .../transport/http/AbstractHTTPDestination.java    | 40 ++---------------
 7 files changed, 103 insertions(+), 72 deletions(-)

Reply | Threaded
Open this post in threaded view
|

[cxf] 01/02: CXF-8367: Micrometer always reports outcome=UNKNOWN on success cases (#726)

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

reta pushed a commit to branch 3.3.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git

commit 0665e4c6db56056ed40b04732eb24b20cd5c3ffa
Author: Andriy Redko <[hidden email]>
AuthorDate: Fri Nov 20 08:55:44 2020 -0500

    CXF-8367: Micrometer always reports outcome=UNKNOWN on success cases (#726)
   
    (cherry picked from commit c4e1d0ef71d4a86e5dc641c4a793cf294ac59509)
---
 .../java/org/apache/cxf/message/MessageUtils.java  | 51 ++++++++++++++++++++++
 .../metrics/micrometer/provider/StandardTags.java  | 14 +++---
 .../provider/jaxws/JaxwsFaultCodeProvider.java     |  9 ++--
 .../micrometer/provider/StandardTagsTest.java      | 25 +++--------
 .../provider/jaxws/JaxwsFaultCodeProviderTest.java | 34 ++++++++++++++-
 .../transport/http/AbstractHTTPDestination.java    | 40 ++---------------
 6 files changed, 101 insertions(+), 72 deletions(-)

diff --git a/core/src/main/java/org/apache/cxf/message/MessageUtils.java b/core/src/main/java/org/apache/cxf/message/MessageUtils.java
index 34937f9..043c6af 100644
--- a/core/src/main/java/org/apache/cxf/message/MessageUtils.java
+++ b/core/src/main/java/org/apache/cxf/message/MessageUtils.java
@@ -20,6 +20,7 @@
 package org.apache.cxf.message;
 
 import java.lang.reflect.Method;
+import java.net.HttpURLConnection;
 import java.util.Optional;
 import java.util.logging.Logger;
 
@@ -205,4 +206,54 @@ public final class MessageUtils {
         }
         return Optional.ofNullable(method);
     }
+    
+    /**
+     * Gets the response code from the message and tries to deduct one if it
+     * is not set yet.
+     * @param message message to get response code from
+     * @return response code (or deducted value assuming success)
+     */
+    public static int getReponseCodeFromMessage(Message message) {
+        Integer i = (Integer)message.get(Message.RESPONSE_CODE);
+        if (i != null) {
+            return i.intValue();
+        }
+        int code = hasNoResponseContent(message) ? HttpURLConnection.HTTP_ACCEPTED : HttpURLConnection.HTTP_OK;
+        // put the code in the message so that others can get it
+        message.put(Message.RESPONSE_CODE, code);
+        return code;
+    }
+
+    /**
+     * Determines if the current message has no response content.
+     * The message has no response content if either:
+     *  - the request is oneway and the current message is no partial
+     *    response or an empty partial response.
+     *  - the request is not oneway but the current message is an empty partial
+     *    response.
+     * @param message
+     * @return
+     */
+    public static boolean hasNoResponseContent(Message message) {
+        final boolean ow = isOneWay(message);
+        final boolean pr = MessageUtils.isPartialResponse(message);
+        final boolean epr = MessageUtils.isEmptyPartialResponse(message);
+
+        //REVISIT may need to provide an option to choose other behavior?
+        // old behavior not suppressing any responses  => ow && !pr
+        // suppress empty responses for oneway calls   => ow && (!pr || epr)
+        // suppress additionally empty responses for decoupled twoway calls =>
+        return (ow && !pr) || epr;
+    }
+    
+    /**
+     * Checks if the message is oneway or not
+     * @param message the message under consideration
+     * @return true if the message has been marked as oneway
+     */
+    public static boolean isOneWay(Message message) {
+        final Exchange ex = message.getExchange();
+        return ex != null && ex.isOneWay();
+    }
+
 }
diff --git a/rt/features/metrics/src/main/java/org/apache/cxf/metrics/micrometer/provider/StandardTags.java b/rt/features/metrics/src/main/java/org/apache/cxf/metrics/micrometer/provider/StandardTags.java
index 07247a5..f62d0ce 100644
--- a/rt/features/metrics/src/main/java/org/apache/cxf/metrics/micrometer/provider/StandardTags.java
+++ b/rt/features/metrics/src/main/java/org/apache/cxf/metrics/micrometer/provider/StandardTags.java
@@ -24,6 +24,7 @@ import java.util.Optional;
 
 import org.apache.cxf.common.util.StringUtils;
 import org.apache.cxf.message.Message;
+import org.apache.cxf.message.MessageUtils;
 
 import io.micrometer.core.instrument.Tag;
 
@@ -65,10 +66,8 @@ public class StandardTags {
 
     public Tag status(Message response) {
         return ofNullable(response)
-                .map(e -> e.get(Message.RESPONSE_CODE))
-                .filter(e -> e instanceof Integer)
-                .map(e -> (Integer) e)
-                .map(String::valueOf)
+                .map(e -> MessageUtils.getReponseCodeFromMessage(response))
+                .map(code -> Integer.toString(code))
                 .map(status -> Tag.of("status", status))
                 .orElse(STATUS_UNKNOWN);
     }
@@ -90,11 +89,8 @@ public class StandardTags {
     }
 
     public Tag outcome(Message response) {
-        Optional<Integer> statusCode =
-                ofNullable(response)
-                        .map(e -> e.get(Message.RESPONSE_CODE))
-                        .filter(e -> e instanceof Integer)
-                        .map(e -> (Integer) e);
+        Optional<Integer> statusCode = ofNullable(response)
+           .map(e -> MessageUtils.getReponseCodeFromMessage(response));
         if (!statusCode.isPresent()) {
             return OUTCOME_UNKNOWN;
         }
diff --git a/rt/features/metrics/src/main/java/org/apache/cxf/metrics/micrometer/provider/jaxws/JaxwsFaultCodeProvider.java b/rt/features/metrics/src/main/java/org/apache/cxf/metrics/micrometer/provider/jaxws/JaxwsFaultCodeProvider.java
index 7b9f7f0..9e11c50 100644
--- a/rt/features/metrics/src/main/java/org/apache/cxf/metrics/micrometer/provider/jaxws/JaxwsFaultCodeProvider.java
+++ b/rt/features/metrics/src/main/java/org/apache/cxf/metrics/micrometer/provider/jaxws/JaxwsFaultCodeProvider.java
@@ -26,20 +26,17 @@ public class JaxwsFaultCodeProvider {
     
     public String getFaultCode(Exchange ex, boolean client) {
         FaultMode fm = ex.get(FaultMode.class);
+        // We check OutFaultMessage/InFaultMessage only because some features propagate the
+        // fault mode using InMessage/OutMessage (which may not end-up with a fault), for
+        // example check MAPAggregatorImpl.
         if (client) {
             if (fm == null && ex.getInFaultMessage() != null) {
                 fm = ex.getInFaultMessage().get(FaultMode.class);
             }
-            if (fm == null && ex.getOutMessage() != null) {
-                fm = ex.getOutMessage().get(FaultMode.class);
-            }
         } else {
             if (fm == null && ex.getOutFaultMessage() != null) {
                 fm = ex.getOutFaultMessage().get(FaultMode.class);
             }
-            if (fm == null && ex.getInMessage() != null) {
-                fm = ex.getInMessage().get(FaultMode.class);
-            }
         }
         
         if (fm == null) {
diff --git a/rt/features/metrics/src/test/java/org/apache/cxf/metrics/micrometer/provider/StandardTagsTest.java b/rt/features/metrics/src/test/java/org/apache/cxf/metrics/micrometer/provider/StandardTagsTest.java
index 3159d02..ca41d51 100644
--- a/rt/features/metrics/src/test/java/org/apache/cxf/metrics/micrometer/provider/StandardTagsTest.java
+++ b/rt/features/metrics/src/test/java/org/apache/cxf/metrics/micrometer/provider/StandardTagsTest.java
@@ -192,26 +192,27 @@ public class StandardTagsTest {
     }
 
     @Test
-    public void testStatusReturnWithUnknownWhenStatusIsNull() {
+    public void testStatusReturnWith200WhenResponseCodeIsNotSet() {
         // given
 
         // when
         Tag actual = underTest.status(response);
 
         // then
-        assertThat(actual, is(Tag.of(STATUS_METRIC_NAME, "UNKNOWN")));
+        assertThat(actual, is(Tag.of(STATUS_METRIC_NAME, "200")));
     }
 
     @Test
-    public void testStatusReturnWithUnknownWhenResponseCodeIsNotInteger() {
+    public void testStatusReturnWith202WhenResponseCodeIsNullAndResponseIsPartial() {
         // given
-        doReturn("").when(request).get(Message.RESPONSE_CODE);
+        doReturn(null).when(request).get(Message.RESPONSE_CODE);
+        doReturn(true).when(request).get(Message.EMPTY_PARTIAL_RESPONSE_MESSAGE);
 
         // when
         Tag actual = underTest.status(request);
 
         // then
-        assertThat(actual, is(Tag.of(STATUS_METRIC_NAME, "UNKNOWN")));
+        assertThat(actual, is(Tag.of(STATUS_METRIC_NAME, "202")));
     }
 
     @Test
@@ -327,18 +328,6 @@ public class StandardTagsTest {
         Tag actual = underTest.outcome(response);
 
         // then
-        assertThat(actual, is(Tag.of(OUTCOME_METRIC_NAME, "UNKNOWN")));
-    }
-
-    @Test
-    public void testOutcomeReturnWithUnknownWhenMethodIsNotInteger() {
-        // given
-        doReturn(new Object()).when(response).get(Message.BASE_PATH);
-
-        // when
-        Tag actual = underTest.outcome(response);
-
-        // then
-        assertThat(actual, is(Tag.of(OUTCOME_METRIC_NAME, "UNKNOWN")));
+        assertThat(actual, is(Tag.of(OUTCOME_METRIC_NAME, "SUCCESS")));
     }
 }
diff --git a/rt/features/metrics/src/test/java/org/apache/cxf/metrics/micrometer/provider/jaxws/JaxwsFaultCodeProviderTest.java b/rt/features/metrics/src/test/java/org/apache/cxf/metrics/micrometer/provider/jaxws/JaxwsFaultCodeProviderTest.java
index 15de3bc..741c9a4 100644
--- a/rt/features/metrics/src/test/java/org/apache/cxf/metrics/micrometer/provider/jaxws/JaxwsFaultCodeProviderTest.java
+++ b/rt/features/metrics/src/test/java/org/apache/cxf/metrics/micrometer/provider/jaxws/JaxwsFaultCodeProviderTest.java
@@ -77,6 +77,21 @@ public class JaxwsFaultCodeProviderTest {
         // then
         assertThat(actual, equalTo(RUNTIME_FAULT_STRING));
     }
+    
+    @Test
+    public void testFaultModeIsNotPresentButInFaultModeIsPresentThenShouldReturnThat() {
+        // given
+        doReturn(null).when(ex).get(FaultMode.class);
+        doReturn(message).when(ex).getInFaultMessage();
+        doReturn(RUNTIME_FAULT).when(message).get(FaultMode.class);
+
+        // when
+        String actual = underTest.getFaultCode(ex, true);
+
+        // then
+        assertThat(actual, equalTo(RUNTIME_FAULT_STRING));
+    }
+
 
     @Test
     public void testFaultModeIsNotPresentButOutFaultModeIsMissingThenShouldReturnNull() {
@@ -92,7 +107,7 @@ public class JaxwsFaultCodeProviderTest {
     }
 
     @Test
-    public void testNeitherFaultModeNorOutFaultModePresentsThenShouldReturnInMessagesFaultMode() {
+    public void testNeitherFaultModeNorOutFaultModePresentsThenShouldNotReturnInMessageFaultMode() {
         // given
         doReturn(null).when(ex).get(FaultMode.class);
         doReturn(null).when(ex).getOutFaultMessage();
@@ -103,7 +118,22 @@ public class JaxwsFaultCodeProviderTest {
         String actual = underTest.getFaultCode(ex, false);
 
         // then
-        assertThat(actual, equalTo(RUNTIME_FAULT_STRING));
+        assertThat(actual, is(nullValue()));
+    }
+    
+    @Test
+    public void testNeitherFaultModeNorOutFaultModePresentsThenShouldNotReturnOutMessageFaultMode() {
+        // given
+        doReturn(null).when(ex).get(FaultMode.class);
+        doReturn(null).when(ex).getInFaultMessage();
+        doReturn(message).when(ex).getOutMessage();
+        doReturn(RUNTIME_FAULT).when(message).get(FaultMode.class);
+
+        // when
+        String actual = underTest.getFaultCode(ex, true);
+
+        // then
+        assertThat(actual, is(nullValue()));
     }
 
     @Test
diff --git a/rt/transports/http/src/main/java/org/apache/cxf/transport/http/AbstractHTTPDestination.java b/rt/transports/http/src/main/java/org/apache/cxf/transport/http/AbstractHTTPDestination.java
index 2beaf93..2be88b3 100644
--- a/rt/transports/http/src/main/java/org/apache/cxf/transport/http/AbstractHTTPDestination.java
+++ b/rt/transports/http/src/main/java/org/apache/cxf/transport/http/AbstractHTTPDestination.java
@@ -22,7 +22,6 @@ package org.apache.cxf.transport.http;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.net.HttpURLConnection;
 import java.net.ServerSocket;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
@@ -235,8 +234,7 @@ public abstract class AbstractHTTPDestination
      * @return true iff the message has been marked as oneway
      */
     protected final boolean isOneWay(Message message) {
-        Exchange ex = message.getExchange();
-        return ex != null && ex.isOneWay();
+        return MessageUtils.isOneWay(message);
     }
 
     public void invoke(final ServletConfig config,
@@ -632,7 +630,7 @@ public abstract class AbstractHTTPDestination
 
         HttpServletResponse response = getHttpResponseFromMessage(outMessage);
 
-        int responseCode = getReponseCodeFromMessage(outMessage);
+        int responseCode = MessageUtils.getReponseCodeFromMessage(outMessage);
         if (responseCode >= 300) {
             String ec = (String)outMessage.get(Message.ERROR_MESSAGE);
             if (!StringUtils.isEmpty(ec)) {
@@ -645,7 +643,7 @@ public abstract class AbstractHTTPDestination
 
         outMessage.put(RESPONSE_HEADERS_COPIED, "true");
 
-        if (hasNoResponseContent(outMessage)) {
+        if (MessageUtils.hasNoResponseContent(outMessage)) {
             response.setContentLength(0);
             response.flushBuffer();
             closeResponseOutputStream(response);
@@ -669,38 +667,6 @@ public abstract class AbstractHTTPDestination
         }
     }
 
-    private int getReponseCodeFromMessage(Message message) {
-        Integer i = (Integer)message.get(Message.RESPONSE_CODE);
-        if (i != null) {
-            return i.intValue();
-        }
-        int code = hasNoResponseContent(message) ? HttpURLConnection.HTTP_ACCEPTED : HttpURLConnection.HTTP_OK;
-        // put the code in the message so that others can get it
-        message.put(Message.RESPONSE_CODE, code);
-        return code;
-    }
-
-    /**
-     * Determines if the current message has no response content.
-     * The message has no response content if either:
-     *  - the request is oneway and the current message is no partial
-     *    response or an empty partial response.
-     *  - the request is not oneway but the current message is an empty partial
-     *    response.
-     * @param message
-     * @return
-     */
-    private boolean hasNoResponseContent(Message message) {
-        final boolean ow = isOneWay(message);
-        final boolean pr = MessageUtils.isPartialResponse(message);
-        final boolean epr = MessageUtils.isEmptyPartialResponse(message);
-
-        //REVISIT may need to provide an option to choose other behavior?
-        // old behavior not suppressing any responses  => ow && !pr
-        // suppress empty responses for oneway calls   => ow && (!pr || epr)
-        // suppress additionally empty responses for decoupled twoway calls =>
-        return (ow && !pr) || epr;
-    }
 
     private HttpServletResponse getHttpResponseFromMessage(Message message) throws IOException {
         Object responseObj = message.get(HTTP_RESPONSE);

Reply | Threaded
Open this post in threaded view
|

[cxf] 02/02: Recording .gitmergeinfo Changes

reta
In reply to this post by reta
This is an automated email from the ASF dual-hosted git repository.

reta pushed a commit to branch 3.3.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git

commit 8783ef870058ae56f668c2c6675c73020ab592cf
Author: reta <[hidden email]>
AuthorDate: Fri Nov 20 09:20:45 2020 -0500

    Recording .gitmergeinfo Changes
---
 .gitmergeinfo | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.gitmergeinfo b/.gitmergeinfo
index 45904f6..0c481f1 100644
--- a/.gitmergeinfo
+++ b/.gitmergeinfo
@@ -543,6 +543,7 @@ M 7f733555945f1faed66b0b6163d24889d4cf60fc
 M 83c9d2701fbf4ed54bd704ca7c4cf67f57e1048d
 M 84bc3830fb3d320f843b43d86e90d8a7e7e871d4
 M 8b11d6b26369650e5f4c6f3730dbb97d60927d9b
+M 8c4f04855a5d8623daff2aa8a8856367879c624b
 M 8d10e2ed1a2b2d56b0d1ad750bf576a1f06bb084
 M 90cad919d00dad2aaeede8f8475c14e2c2e76ad0
 M 9115be37d5a5a76b747c03452bbbf7fb2da7be56
@@ -607,4 +608,5 @@ M f5919bbdb97dfaedba6847d1e8d27e42f2de843d
 M f6ee8880f961974da0aa2406d415627f6a382aaa
 M fa1238f03711538460b62aec581e035f237f7a81
 M fb4b78dbc40ff9b56eb53abe3aca02865ef8672d
+M fb7821164ec6d7fb98dc3f8caea67465db230a14
 M fc54b7fc73d4189a18b2ab92a91875b779dfaf52