Skip to content

Commit 8543bf2

Browse files
Fix Azure Mock Issues (#49377) (#49382)
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
1 parent 3fe3571 commit 8543bf2

File tree

3 files changed

+19
-17
lines changed

3 files changed

+19
-17
lines changed

plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobStoreRepositoryTests.java

+6-5
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import com.sun.net.httpserver.HttpExchange;
2626
import com.sun.net.httpserver.HttpHandler;
2727
import fixture.azure.AzureHttpHandler;
28-
import org.apache.lucene.util.LuceneTestCase;
2928
import org.elasticsearch.common.SuppressForbidden;
3029
import org.elasticsearch.common.settings.MockSecureSettings;
3130
import org.elasticsearch.common.settings.Settings;
@@ -41,7 +40,6 @@
4140
import java.util.Collections;
4241
import java.util.Map;
4342

44-
@LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/48978")
4543
@SuppressForbidden(reason = "this test uses a HttpServer to emulate an Azure endpoint")
4644
public class AzureBlobStoreRepositoryTests extends ESMockAPIBasedRepositoryIntegTestCase {
4745

@@ -140,9 +138,12 @@ private static class AzureErroneousHttpHandler extends ErroneousHttpHandler {
140138

141139
@Override
142140
protected void handleAsError(final HttpExchange exchange) throws IOException {
143-
drainInputStream(exchange.getRequestBody());
144-
AzureHttpHandler.sendError(exchange, randomFrom(RestStatus.INTERNAL_SERVER_ERROR, RestStatus.SERVICE_UNAVAILABLE));
145-
exchange.close();
141+
try {
142+
drainInputStream(exchange.getRequestBody());
143+
AzureHttpHandler.sendError(exchange, randomFrom(RestStatus.INTERNAL_SERVER_ERROR, RestStatus.SERVICE_UNAVAILABLE));
144+
} finally {
145+
exchange.close();
146+
}
146147
}
147148

148149
@Override

test/fixtures/azure-fixture/src/main/java/fixture/azure/AzureHttpHandler.java

+4-6
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ public void handle(final HttpExchange exchange) throws IOException {
9292

9393
} else if (Regex.simpleMatch("PUT /" + container + "/*", request)) {
9494
// PUT Blob (see https://docs.microsoft.com/en-us/rest/api/storageservices/put-blob)
95-
final String ifNoneMatch = exchange.getResponseHeaders().getFirst("If-None-Match");
95+
final String ifNoneMatch = exchange.getRequestHeaders().getFirst("If-None-Match");
9696
if ("*".equals(ifNoneMatch)) {
9797
if (blobs.putIfAbsent(exchange.getRequestURI().getPath(), Streams.readFully(exchange.getRequestBody())) != null) {
9898
sendError(exchange, RestStatus.CONFLICT);
@@ -214,12 +214,10 @@ public static void sendError(final HttpExchange exchange, final RestStatus statu
214214
}
215215

216216
final String errorCode = toAzureErrorCode(status);
217-
if (errorCode != null) {
218-
// see Constants.HeaderConstants.ERROR_CODE
219-
headers.add("x-ms-error-code", errorCode);
220-
}
217+
// see Constants.HeaderConstants.ERROR_CODE
218+
headers.add("x-ms-error-code", errorCode);
221219

222-
if (errorCode == null || "HEAD".equals(exchange.getRequestMethod())) {
220+
if ("HEAD".equals(exchange.getRequestMethod())) {
223221
exchange.sendResponseHeaders(status.getStatus(), -1L);
224222
} else {
225223
final byte[] response = ("<?xml version=\"1.0\" encoding=\"UTF-8\"?><Error><Code>" + errorCode + "</Code><Message>"

test/framework/src/main/java/org/elasticsearch/repositories/blobstore/ESMockAPIBasedRepositoryIntegTestCase.java

+9-6
Original file line numberDiff line numberDiff line change
@@ -206,9 +206,12 @@ public void handle(final HttpExchange exchange) throws IOException {
206206
}
207207

208208
protected void handleAsError(final HttpExchange exchange) throws IOException {
209-
drainInputStream(exchange.getRequestBody());
210-
exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, -1);
211-
exchange.close();
209+
try {
210+
drainInputStream(exchange.getRequestBody());
211+
exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, -1);
212+
} finally {
213+
exchange.close();
214+
}
212215
}
213216

214217
protected abstract String requestUniqueId(HttpExchange exchange);
@@ -225,10 +228,10 @@ private static HttpHandler wrap(final HttpHandler handler, final Logger logger)
225228
return exchange -> {
226229
try {
227230
handler.handle(exchange);
228-
} catch (final Exception e) {
231+
} catch (Throwable t) {
229232
logger.error(() -> new ParameterizedMessage("Exception when handling request {} {} {}",
230-
exchange.getRemoteAddress(), exchange.getRequestMethod(), exchange.getRequestURI()), e);
231-
throw e;
233+
exchange.getRemoteAddress(), exchange.getRequestMethod(), exchange.getRequestURI()), t);
234+
throw t;
232235
}
233236
};
234237
}

0 commit comments

Comments
 (0)