Merge "Let request-id value derive from alternative request-headers"

4 files changed, 209 insertions, 29 deletions

diff --git a/vid-app-common/src/main/java/org/onap/vid/controller/filter/PromiseRequestIdFilter.java b/vid-app-common/src/main/java/org/onap/vid/controller/filter/PromiseRequestIdFilter.java
index c6b18785a..c0fc96e03 100644
--- a/vid-app-common/src/main/java/org/onap/vid/controller/filter/PromiseRequestIdFilter.java
+++ b/vid-app-common/src/main/java/org/onap/vid/controller/filter/PromiseRequestIdFilter.java
@@ -21,6 +21,9 @@
 package org.onap.vid.controller.filter;
 
 
+import static org.apache.commons.lang3.ObjectUtils.defaultIfNull;
+import static org.apache.commons.lang3.StringUtils.equalsIgnoreCase;
+import static org.apache.commons.lang3.StringUtils.isNotEmpty;
 import static org.onap.portalsdk.core.util.SystemProperties.ECOMP_REQUEST_ID;
 
 import com.google.common.collect.ImmutableList;
@@ -28,6 +31,7 @@ import java.io.IOException;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.UUID;
+import java.util.function.Supplier;
 import java.util.regex.Pattern;
 import javax.servlet.FilterChain;
 import javax.servlet.ServletException;
@@ -37,15 +41,28 @@ import javax.servlet.annotation.WebFilter;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletRequestWrapper;
 import javax.servlet.http.HttpServletResponse;
-import org.apache.commons.lang3.StringUtils;
+import javax.validation.constraints.NotNull;
+import org.onap.vid.logging.Headers;
 import org.springframework.web.filter.GenericFilterBean;
 
 @WebFilter(urlPatterns = "/*")
 public class PromiseRequestIdFilter extends GenericFilterBean {
 
+    // The wrapped request is guaranteed to have the transaction id as the value
+    // of the header PROMISED_HEADER_NAME.
+    // PROMISED_HEADER_NAME is set to ECOMP_REQUEST_ID as long as
+    // org.onap.portalsdk...UserUtils.getRequestId() is using the header
+    // "X-ECOMP-RequestID".
+    private static final String PROMISED_HEADER_NAME = ECOMP_REQUEST_ID;
     private static final String REQUEST_ID_RESPONSE_HEADER = ECOMP_REQUEST_ID + "-echo";
 
-    private static final Pattern uuidRegex = Pattern.compile("[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}");
+    private static final Pattern uuidRegex = Pattern.compile("[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}", Pattern.CASE_INSENSITIVE);
+
+    private final Headers headers;
+
+    public PromiseRequestIdFilter(Headers headers) {
+        this.headers = headers;
+    }
 
     @Override
     public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
@@ -55,7 +72,7 @@ public class PromiseRequestIdFilter extends GenericFilterBean {
             request = wrapIfNeeded(request);
 
             if (response instanceof HttpServletResponse) {
-                final String actualRequestId = ((HttpServletRequest) request).getHeader(ECOMP_REQUEST_ID);
+                final String actualRequestId = ((HttpServletRequest) request).getHeader(PROMISED_HEADER_NAME);
                 ((HttpServletResponse) response).addHeader(REQUEST_ID_RESPONSE_HEADER, actualRequestId);
             }
         }
@@ -65,35 +82,52 @@ public class PromiseRequestIdFilter extends GenericFilterBean {
 
     public ServletRequest wrapIfNeeded(ServletRequest request) {
         final HttpServletRequest httpRequest = (HttpServletRequest) request;
-        final String originalRequestId = incomingRequestId(httpRequest);
 
-        if (isWrapNeeded(originalRequestId)) {
-            request = new PromiseRequestIdRequestWrapper(httpRequest);
+        final String highestPriorityHeader = highestPriorityHeader(httpRequest);
+        final String originalRequestId = httpRequest.getHeader(highestPriorityHeader);
+
+        if (isWrapNeeded(highestPriorityHeader, originalRequestId)) {
+            // Copy originalRequestId to the promised header value
+            request = new PromiseRequestIdRequestWrapper(httpRequest, toUuidOrElse(originalRequestId, UUID::randomUUID));
         }
 
         return request;
     }
 
     private boolean verifyAndValidateUuid(String value) {
-        return uuidRegex.matcher(value).matches();
+        return isNotEmpty(value) && uuidRegex.matcher(value).matches();
     }
 
-    protected boolean isWrapNeeded(String originalRequestId) {
-        return StringUtils.isEmpty(originalRequestId)
-            || !verifyAndValidateUuid(originalRequestId);
+    private boolean isWrapNeeded(String highestPriorityHeader, String originalRequestId) {
+        boolean headerExistsAndValid =
+            equalsIgnoreCase(highestPriorityHeader, PROMISED_HEADER_NAME) && verifyAndValidateUuid(originalRequestId);
+
+        return !headerExistsAndValid;
+    }
+
+    UUID toUuidOrElse(String uuid, Supplier<UUID> uuidSupplier) {
+        if (verifyAndValidateUuid(uuid)) {
+            try {
+                return UUID.fromString(uuid);
+            } catch (IllegalArgumentException e) {
+                return uuidSupplier.get();
+            }
+        } else {
+            return uuidSupplier.get();
+        }
     }
 
-    protected String incomingRequestId(HttpServletRequest httpRequest) {
-        return httpRequest.getHeader(ECOMP_REQUEST_ID);
+    String highestPriorityHeader(HttpServletRequest httpRequest) {
+        return defaultIfNull(headers.highestPriorityHeader(httpRequest), PROMISED_HEADER_NAME);
     }
 
     private static class PromiseRequestIdRequestWrapper extends HttpServletRequestWrapper {
 
         private final UUID requestId;
 
-        PromiseRequestIdRequestWrapper(HttpServletRequest request) {
+        PromiseRequestIdRequestWrapper(HttpServletRequest request, @NotNull UUID requestId) {
             super(request);
-            requestId = UUID.randomUUID();
+            this.requestId = requestId;
         }
 
         @Override
@@ -114,9 +148,9 @@ public class PromiseRequestIdFilter extends GenericFilterBean {
         @Override
         public Enumeration<String> getHeaderNames() {
 
-            if (null == super.getHeader(ECOMP_REQUEST_ID)) {
+            if (null == super.getHeader(PROMISED_HEADER_NAME)) {
                 return Collections.enumeration(ImmutableList.<String>builder()
-                    .add(ECOMP_REQUEST_ID)
+                    .add(PROMISED_HEADER_NAME)
                     .addAll(Collections.list(super.getHeaderNames()))
                     .build());
             }
@@ -125,7 +159,7 @@ public class PromiseRequestIdFilter extends GenericFilterBean {
         }
 
         private boolean isRequestIdHeaderName(String name) {
-            return ECOMP_REQUEST_ID.equalsIgnoreCase(name);
+            return equalsIgnoreCase(name, PROMISED_HEADER_NAME);
         }
     }
 }
diff --git a/vid-app-common/src/main/java/org/onap/vid/logging/Headers.kt b/vid-app-common/src/main/java/org/onap/vid/logging/Headers.kt
new file mode 100644
index 000000000..cc5ebf38c
--- /dev/null
+++ b/vid-app-common/src/main/java/org/onap/vid/logging/Headers.kt
@@ -0,0 +1,20 @@
+package org.onap.vid.logging
+
+import org.onap.portalsdk.core.util.SystemProperties.ECOMP_REQUEST_ID
+import org.springframework.stereotype.Component
+import javax.servlet.http.HttpServletRequest
+
+@Component
+class Headers {
+    fun prioritizedRequestIdHeaders() = listOf(
+            "X-ONAP-RequestID",
+            "X-RequestID",
+            "X-TransactionID",
+            ECOMP_REQUEST_ID
+    )
+
+    fun highestPriorityHeader(httpRequest: HttpServletRequest): String? {
+        val headers = httpRequest.headerNames.asSequence().toSet().map { it.toUpperCase() }
+        return prioritizedRequestIdHeaders().firstOrNull { headers.contains(it.toUpperCase()) }
+    }
+}
diff --git a/vid-app-common/src/test/java/org/onap/vid/controller/PromiseRequestIdFilterTest.java b/vid-app-common/src/test/java/org/onap/vid/controller/filter/PromiseRequestIdFilterTest.java
index 2304d46f3..a1d15f543 100644
--- a/vid-app-common/src/test/java/org/onap/vid/controller/PromiseRequestIdFilterTest.java
+++ b/vid-app-common/src/test/java/org/onap/vid/controller/filter/PromiseRequestIdFilterTest.java
@@ -18,14 +18,16 @@
  * ============LICENSE_END=========================================================
  */
 
-package org.onap.vid.controller;
+package org.onap.vid.controller.filter;
 
 import static org.apache.commons.lang3.StringUtils.equalsIgnoreCase;
+import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.anyOf;
 import static org.hamcrest.Matchers.emptyOrNullString;
 import static org.hamcrest.Matchers.equalToIgnoringCase;
+import static org.hamcrest.Matchers.hasItems;
 import static org.hamcrest.Matchers.not;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.argThat;
@@ -38,6 +40,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.Map;
+import java.util.UUID;
 import java.util.function.Function;
 import javax.servlet.FilterChain;
 import javax.servlet.ServletException;
@@ -49,8 +52,9 @@ import org.mockito.ArgumentCaptor;
 import org.mockito.Mockito;
 import org.mockito.stubbing.Answer;
 import org.onap.portalsdk.core.web.support.UserUtils;
-import org.onap.vid.controller.filter.PromiseRequestIdFilter;
+import org.onap.vid.logging.Headers;
 import org.springframework.mock.web.MockHttpServletResponse;
+import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
 @Test
@@ -60,6 +64,13 @@ public class PromiseRequestIdFilterTest {
     private final String anotherValue = "foo value";
     private final String mixedCaseHeader = "x-ecomp-REQUESTID";
 
+    private static final String onapRequestIdHeader = "x-onap-requestid";
+    private static final String transactionIdHeader = "x-transactionid";
+    private static final String requestIdHeader = "x-requestid";
+
+    private final PromiseRequestIdFilter promiseRequestIdFilter =
+        new PromiseRequestIdFilter(new Headers());
+
     @Test
     public void givenRequestIdHeader_headerValueNotChanged() throws IOException, ServletException {
 
@@ -111,8 +122,119 @@ public class PromiseRequestIdFilterTest {
         buildRequestThenRunThroughFilterAndAssertResultRequestHeaders(incomingRequestHeaders, UserUtils::getRequestId);
     }
 
-    
-    private void buildRequestThenRunThroughFilterAndAssertResultRequestHeaders(
+    @Test
+    public void givenTwoRequestIdHeader_onapHeaderValueIsUsed() throws IOException, ServletException {
+
+        final String onapTxId = "863850e2-8545-4efd-94b8-AFBA5F52B3D5"; // note mixed case
+        final String ecompTxId = "6e8ff89e-88a4-4977-b63f-3142892b6e08";
+
+        final ImmutableMap<String, String> incomingRequestHeaders = ImmutableMap.of(
+            anotherHeader, anotherValue,
+            ECOMP_REQUEST_ID, ecompTxId,
+            onapRequestIdHeader, onapTxId
+        );
+
+        buildRequestThenRunThroughFilterAndAssertResultRequestHeaders(incomingRequestHeaders, specificTxId(onapTxId));
+    }
+
+    @Test
+    public void givenTwoRequestIdHeaderAndHigherPriorityIsMalformed_headerValueIsGenerated() throws IOException, ServletException {
+
+        final String malformedTxId = "6e8ff89e-88a4-4977-b63f-3142892b6e08-";
+        final String anotherTxId = "863850e2-8545-4efd-94b8-afba5f52b3d5";
+
+        final ImmutableMap<String, String> incomingRequestHeaders = ImmutableMap.of(
+            anotherHeader, anotherValue,
+            requestIdHeader, malformedTxId, // requestIdHeader as higher priority than transactionIdHeader
+            transactionIdHeader, anotherTxId
+        );
+
+        HttpServletRequest wrappedRequest =
+            buildRequestThenRunThroughFilterAndAssertResultRequestHeaders(incomingRequestHeaders, UserUtils::getRequestId);
+
+        assertThat(UserUtils.getRequestId(wrappedRequest),
+            not(anyOf(equalTo(malformedTxId), equalTo(anotherTxId)))
+        );
+    }
+
+
+    @Test
+    public void toUuidOrElse_givenValid_yieldSame() {
+        final String someTxId = "729bbd8d-b0c2-4809-a794-DCCCD9CDA2C0"; // note mixed case
+        UUID unexpected = UUID.randomUUID();
+        assertThat(promiseRequestIdFilter.toUuidOrElse(someTxId, () -> unexpected), is(UUID.fromString(someTxId)));
+    }
+
+    @Test
+    public void toUuidOrElse_givenNull_yieldSupplier() {
+        UUID expected = UUID.fromString("729bbd8d-b0c2-4809-a794-dcccd9cda2c0");
+        assertThat(promiseRequestIdFilter.toUuidOrElse(null, () -> expected), is(expected));
+    }
+
+    @Test
+    public void toUuidOrElse_givenMalformed_yieldSupplier() {
+        UUID expected = UUID.fromString("729bbd8d-b0c2-4809-a794-dcccd9cda2c0");
+        assertThat(promiseRequestIdFilter.toUuidOrElse("malformed uuid", () -> expected), is(expected));
+    }
+
+    @DataProvider
+    public static Object[][] severalRequestIdHeaders() {
+        String someTxId = "69fa2575-d7f2-482c-ad1b-53a63ca03617";
+        String anotherTxId = "06de373b-7e19-4357-9bd1-ed95682ae3a4";
+
+        return new Object[][]{
+            {
+                "header is selected when single", transactionIdHeader,
+                ImmutableMap.of(
+                    transactionIdHeader, someTxId
+                )
+            }, {
+                "header is selected when first", onapRequestIdHeader,
+                ImmutableMap.of(
+                    onapRequestIdHeader, someTxId,
+                    "noise-header", anotherTxId,
+                    ECOMP_REQUEST_ID, anotherTxId
+                )
+            }, {
+                "header is selected when last", onapRequestIdHeader,
+                ImmutableMap.of(
+                    ECOMP_REQUEST_ID, anotherTxId,
+                    "noise-header", anotherTxId,
+                    onapRequestIdHeader, someTxId
+                )
+            }, {
+                "header is selected when value is invalid uuid", onapRequestIdHeader,
+                ImmutableMap.of(
+                    onapRequestIdHeader, "invalid-uuid"
+                )
+            }, {
+                "header is selected when no ecomp-request-id", onapRequestIdHeader,
+                ImmutableMap.of(
+                    requestIdHeader, anotherTxId,
+                    onapRequestIdHeader, someTxId
+                )
+            }, {
+                "ECOMP_REQUEST_ID is returned when no request-id header", ECOMP_REQUEST_ID,
+                ImmutableMap.of(
+                    "tsamina-mina", anotherTxId,
+                    "waka-waka", anotherTxId
+                )
+            },
+        };
+    }
+
+    @Test(dataProvider = "severalRequestIdHeaders")
+    public void highestPriorityHeader_givenSeveralRequestIdHeaders_correctHeaderIsUsed(String description, String expectedHeader, Map<String, String> incomingRequestHeaders) {
+        PromiseRequestIdFilter testSubject = promiseRequestIdFilter;
+
+        HttpServletRequest mockedHttpServletRequest = createMockedHttpServletRequest(incomingRequestHeaders);
+
+        assertThat(description,
+            testSubject.highestPriorityHeader(mockedHttpServletRequest), equalToIgnoringCase(expectedHeader));
+    }
+
+
+    private HttpServletRequest buildRequestThenRunThroughFilterAndAssertResultRequestHeaders(
             ImmutableMap<String, String> originalRequestHeaders,
             Function<HttpServletRequest, String> txIdExtractor
     ) throws IOException, ServletException {
@@ -125,7 +247,7 @@ public class PromiseRequestIdFilterTest {
         //
         // doFilter() is the function under test
         //
-        new PromiseRequestIdFilter().doFilter(servletRequest, servletResponse, capturingFilterChain);
+        promiseRequestIdFilter.doFilter(servletRequest, servletResponse, capturingFilterChain);
         //
         //////////////////
 
@@ -135,6 +257,8 @@ public class PromiseRequestIdFilterTest {
 
         assertRequestObjectHeaders(capturedServletRequest, expectedTxId);
         assertResponseObjectHeaders(capturedServletResponse, expectedTxId);
+
+        return (HttpServletRequest) capturedServletRequest;
     }
 
 
@@ -148,14 +272,14 @@ public class PromiseRequestIdFilterTest {
         final HttpServletRequest httpServletRequest = (HttpServletRequest) request;
 
         assertThat(Collections.list(httpServletRequest.getHeaderNames()),
-                containsInAnyOrder(equalToIgnoringCase(ECOMP_REQUEST_ID), equalToIgnoringCase(anotherHeader)));
+                hasItems(equalToIgnoringCase(ECOMP_REQUEST_ID), equalToIgnoringCase(anotherHeader)));
 
         assertThat(httpServletRequest.getHeader(anotherHeader), is(anotherValue));
 
-        assertThat(httpServletRequest.getHeader(ECOMP_REQUEST_ID), is(expectedTxId));
-        assertThat(httpServletRequest.getHeader(mixedCaseHeader), is(expectedTxId));
+        assertThat(httpServletRequest.getHeader(ECOMP_REQUEST_ID), equalToIgnoringCase(expectedTxId));
+        assertThat(httpServletRequest.getHeader(mixedCaseHeader), equalToIgnoringCase(expectedTxId));
 
-        assertThat(UserUtils.getRequestId(httpServletRequest), is(expectedTxId));
+        assertThat(UserUtils.getRequestId(httpServletRequest), equalToIgnoringCase(expectedTxId));
         assertThat(UserUtils.getRequestId(httpServletRequest), is(not(emptyOrNullString())));
     }
 
@@ -164,7 +288,7 @@ public class PromiseRequestIdFilterTest {
         final HttpServletResponse httpServletResponse = (HttpServletResponse) response;
 
         assertThat("header " + REQUEST_ID_HEADER_NAME_IN_RESPONSE.toLowerCase() + " in response must be provided",
-                httpServletResponse.getHeader(REQUEST_ID_HEADER_NAME_IN_RESPONSE), is(txId));
+                httpServletResponse.getHeader(REQUEST_ID_HEADER_NAME_IN_RESPONSE), equalToIgnoringCase(txId));
     }
 
 
diff --git a/vid-app-common/src/test/java/org/onap/vid/mso/rest/OutgoingRequestHeadersTest.java b/vid-app-common/src/test/java/org/onap/vid/mso/rest/OutgoingRequestHeadersTest.java
index ac05ea706..3ea025870 100644
--- a/vid-app-common/src/test/java/org/onap/vid/mso/rest/OutgoingRequestHeadersTest.java
+++ b/vid-app-common/src/test/java/org/onap/vid/mso/rest/OutgoingRequestHeadersTest.java
@@ -51,6 +51,7 @@ import org.onap.vid.aai.util.AAIRestInterface;
 import org.onap.vid.aai.util.ServletRequestHelper;
 import org.onap.vid.aai.util.SystemPropertyHelper;
 import org.onap.vid.controller.filter.PromiseRequestIdFilter;
+import org.onap.vid.logging.Headers;
 import org.onap.vid.testUtils.TestUtils;
 import org.onap.vid.utils.Logging;
 import org.onap.vid.utils.Unchecked;
@@ -65,7 +66,8 @@ import org.testng.annotations.Test;
 
 public class OutgoingRequestHeadersTest {
 
-    private static final PromiseRequestIdFilter promiseRequestIdFilter = new PromiseRequestIdFilter();
+    private static final PromiseRequestIdFilter promiseRequestIdFilter =
+        new PromiseRequestIdFilter(new Headers());
 
 //    @InjectMocks
 //    private RestMsoImplementation restMsoImplementation;