Fix Azure Mock Issues (elastic#49377)

Fixing a few small issues found in this code: 1. We weren't reading the request headers but the response headers when checking for blob existence in the mocked single upload path 2. Error code can never be `null` removed the dead code that resulted 3. In the logging wrapper we weren't checking for `Throwable` so any failing assertions in the http mock would not show up since they run on a thread managed by the mock http server
original-brownbear · Nov 20, 2019 · 1b2cad1 · 1b2cad1
1 parent bcbb2c9
commit 1b2cad1
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 17 deletions.
diff --git a/...ure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobStoreRepositoryTests.java b/...ure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobStoreRepositoryTests.java
@@ -25,7 +25,6 @@
 import com.sun.net.httpserver.HttpExchange;
 import com.sun.net.httpserver.HttpHandler;
 import fixture.azure.AzureHttpHandler;
-import org.apache.lucene.util.LuceneTestCase;
 import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.settings.MockSecureSettings;
 import org.elasticsearch.common.settings.Settings;
@@ -41,7 +40,6 @@
 import java.util.Collections;
 import java.util.Map;
 
-@LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/48978")
 @SuppressForbidden(reason = "this test uses a HttpServer to emulate an Azure endpoint")
 public class AzureBlobStoreRepositoryTests extends ESMockAPIBasedRepositoryIntegTestCase {
 
@@ -140,9 +138,12 @@ private static class AzureErroneousHttpHandler extends ErroneousHttpHandler {
 
         @Override
         protected void handleAsError(final HttpExchange exchange) throws IOException {
-            drainInputStream(exchange.getRequestBody());
-            AzureHttpHandler.sendError(exchange, randomFrom(RestStatus.INTERNAL_SERVER_ERROR, RestStatus.SERVICE_UNAVAILABLE));
-            exchange.close();
+            try {
+                drainInputStream(exchange.getRequestBody());
+                AzureHttpHandler.sendError(exchange, randomFrom(RestStatus.INTERNAL_SERVER_ERROR, RestStatus.SERVICE_UNAVAILABLE));
+            } finally {
+                exchange.close();
+            }
         }
 
         @Override

diff --git a/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java b/test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java
@@ -92,7 +92,7 @@ public void handle(final HttpExchange exchange) throws IOException {
 
             } else if (Regex.simpleMatch("PUT /" + container + "/*", request)) {
                 // PUT Blob (see https://docs.microsoft.com/en-us/rest/api/storageservices/put-blob)
-                final String ifNoneMatch = exchange.getResponseHeaders().getFirst("If-None-Match");
+                final String ifNoneMatch = exchange.getRequestHeaders().getFirst("If-None-Match");
                 if ("*".equals(ifNoneMatch)) {
                     if (blobs.putIfAbsent(exchange.getRequestURI().getPath(), Streams.readFully(exchange.getRequestBody())) != null) {
                         sendError(exchange, RestStatus.CONFLICT);
@@ -214,12 +214,10 @@ public static void sendError(final HttpExchange exchange, final RestStatus statu
         }
 
         final String errorCode = toAzureErrorCode(status);
-        if (errorCode != null) {
-            // see Constants.HeaderConstants.ERROR_CODE
-            headers.add("x-ms-error-code", errorCode);
-        }
+        // see Constants.HeaderConstants.ERROR_CODE
+        headers.add("x-ms-error-code", errorCode);
 
-        if (errorCode == null || "HEAD".equals(exchange.getRequestMethod())) {
+        if ("HEAD".equals(exchange.getRequestMethod())) {
             exchange.sendResponseHeaders(status.getStatus(), -1L);
         } else {
             final byte[] response = ("<?xml version=\"1.0\" encoding=\"UTF-8\"?><Error><Code>" + errorCode + "</Code><Message>"

diff --git a/.../java/org/elasticsearch/repositories/blobstore/ESMockAPIBasedRepositoryIntegTestCase.java b/.../java/org/elasticsearch/repositories/blobstore/ESMockAPIBasedRepositoryIntegTestCase.java
@@ -195,9 +195,12 @@ public void handle(final HttpExchange exchange) throws IOException {
         }
 
         protected void handleAsError(final HttpExchange exchange) throws IOException {
-            drainInputStream(exchange.getRequestBody());
-            exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, -1);
-            exchange.close();
+            try {
+                drainInputStream(exchange.getRequestBody());
+                exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, -1);
+            } finally {
+                exchange.close();
+            }
         }
 
         protected abstract String requestUniqueId(HttpExchange exchange);
@@ -214,10 +217,10 @@ private static HttpHandler wrap(final HttpHandler handler, final Logger logger)
         return exchange -> {
             try {
                 handler.handle(exchange);
-            } catch (final Exception e) {
+            } catch (Throwable t) {
                 logger.error(() -> new ParameterizedMessage("Exception when handling request {} {} {}",
-                    exchange.getRemoteAddress(), exchange.getRequestMethod(), exchange.getRequestURI()), e);
-                throw e;
+                    exchange.getRemoteAddress(), exchange.getRequestMethod(), exchange.getRequestURI()), t);
+                throw t;
             }
         };
     }