Fix Azure Mock Issues (#49377) (#49381)

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
This commit is contained in:
Armin Braun 2019-11-21 19:57:50 +01:00 committed by GitHub
parent 0fa3b887b7
commit 231d079bf8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 19 additions and 17 deletions

View File

@ -25,7 +25,6 @@ import com.microsoft.azure.storage.blob.BlobRequestOptions;
import com.sun.net.httpserver.HttpExchange; import com.sun.net.httpserver.HttpExchange;
import com.sun.net.httpserver.HttpHandler; import com.sun.net.httpserver.HttpHandler;
import fixture.azure.AzureHttpHandler; import fixture.azure.AzureHttpHandler;
import org.apache.lucene.util.LuceneTestCase;
import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.settings.MockSecureSettings; import org.elasticsearch.common.settings.MockSecureSettings;
import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.Settings;
@ -41,7 +40,6 @@ import java.util.Collection;
import java.util.Collections; import java.util.Collections;
import java.util.Map; 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") @SuppressForbidden(reason = "this test uses a HttpServer to emulate an Azure endpoint")
public class AzureBlobStoreRepositoryTests extends ESMockAPIBasedRepositoryIntegTestCase { public class AzureBlobStoreRepositoryTests extends ESMockAPIBasedRepositoryIntegTestCase {
@ -140,9 +138,12 @@ public class AzureBlobStoreRepositoryTests extends ESMockAPIBasedRepositoryInteg
@Override @Override
protected void handleAsError(final HttpExchange exchange) throws IOException { protected void handleAsError(final HttpExchange exchange) throws IOException {
drainInputStream(exchange.getRequestBody()); try {
AzureHttpHandler.sendError(exchange, randomFrom(RestStatus.INTERNAL_SERVER_ERROR, RestStatus.SERVICE_UNAVAILABLE)); drainInputStream(exchange.getRequestBody());
exchange.close(); AzureHttpHandler.sendError(exchange, randomFrom(RestStatus.INTERNAL_SERVER_ERROR, RestStatus.SERVICE_UNAVAILABLE));
} finally {
exchange.close();
}
} }
@Override @Override

View File

@ -92,7 +92,7 @@ public class AzureHttpHandler implements HttpHandler {
} else if (Regex.simpleMatch("PUT /" + container + "/*", request)) { } else if (Regex.simpleMatch("PUT /" + container + "/*", request)) {
// PUT Blob (see https://docs.microsoft.com/en-us/rest/api/storageservices/put-blob) // 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 ("*".equals(ifNoneMatch)) {
if (blobs.putIfAbsent(exchange.getRequestURI().getPath(), Streams.readFully(exchange.getRequestBody())) != null) { if (blobs.putIfAbsent(exchange.getRequestURI().getPath(), Streams.readFully(exchange.getRequestBody())) != null) {
sendError(exchange, RestStatus.CONFLICT); sendError(exchange, RestStatus.CONFLICT);
@ -214,12 +214,10 @@ public class AzureHttpHandler implements HttpHandler {
} }
final String errorCode = toAzureErrorCode(status); final String errorCode = toAzureErrorCode(status);
if (errorCode != null) { // see Constants.HeaderConstants.ERROR_CODE
// see Constants.HeaderConstants.ERROR_CODE headers.add("x-ms-error-code", errorCode);
headers.add("x-ms-error-code", errorCode);
}
if (errorCode == null || "HEAD".equals(exchange.getRequestMethod())) { if ("HEAD".equals(exchange.getRequestMethod())) {
exchange.sendResponseHeaders(status.getStatus(), -1L); exchange.sendResponseHeaders(status.getStatus(), -1L);
} else { } else {
final byte[] response = ("<?xml version=\"1.0\" encoding=\"UTF-8\"?><Error><Code>" + errorCode + "</Code><Message>" final byte[] response = ("<?xml version=\"1.0\" encoding=\"UTF-8\"?><Error><Code>" + errorCode + "</Code><Message>"

View File

@ -206,9 +206,12 @@ public abstract class ESMockAPIBasedRepositoryIntegTestCase extends ESBlobStoreR
} }
protected void handleAsError(final HttpExchange exchange) throws IOException { protected void handleAsError(final HttpExchange exchange) throws IOException {
drainInputStream(exchange.getRequestBody()); try {
exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, -1); drainInputStream(exchange.getRequestBody());
exchange.close(); exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, -1);
} finally {
exchange.close();
}
} }
protected abstract String requestUniqueId(HttpExchange exchange); protected abstract String requestUniqueId(HttpExchange exchange);
@ -225,10 +228,10 @@ public abstract class ESMockAPIBasedRepositoryIntegTestCase extends ESBlobStoreR
return exchange -> { return exchange -> {
try { try {
handler.handle(exchange); handler.handle(exchange);
} catch (final Exception e) { } catch (Throwable t) {
logger.error(() -> new ParameterizedMessage("Exception when handling request {} {} {}", logger.error(() -> new ParameterizedMessage("Exception when handling request {} {} {}",
exchange.getRemoteAddress(), exchange.getRequestMethod(), exchange.getRequestURI()), e); exchange.getRemoteAddress(), exchange.getRequestMethod(), exchange.getRequestURI()), t);
throw e; throw t;
} }
}; };
} }