Skip to content

Commit 594a373

Browse files
authored
Add full conditional write support to S3 test fixture (#138542)
In #133030 we added limited support for conditional writes in `S3HttpHandler`, allowing callers to prevent overwriting an existing blob with an `If-None-Match: *` precondition header. This commit extends the implementation to include support for the `If-Match: <etag>` precondition header allowing callers to perform atomic compare-and-set operations which overwrite existing objects.
1 parent bf4655a commit 594a373

File tree

2 files changed

+108
-13
lines changed

2 files changed

+108
-13
lines changed

test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpHandler.java

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import java.util.Set;
4646
import java.util.concurrent.ConcurrentHashMap;
4747
import java.util.concurrent.ConcurrentMap;
48+
import java.util.concurrent.atomic.AtomicReference;
4849
import java.util.regex.Matcher;
4950
import java.util.regex.Pattern;
5051

@@ -248,6 +249,9 @@ public void handle(final HttpExchange exchange) throws IOException {
248249
if (isProtectOverwrite(exchange)) {
249250
throw new AssertionError("If-None-Match: * header is not supported here");
250251
}
252+
if (getRequiredExistingETag(exchange) != null) {
253+
throw new AssertionError("If-Match: * header is not supported here");
254+
}
251255

252256
var sourceBlob = blobs.get(copySource);
253257
if (sourceBlob == null) {
@@ -406,8 +410,9 @@ public void handle(final HttpExchange exchange) throws IOException {
406410
* Update the blob contents if and only if the preconditions in the request are satisfied.
407411
*
408412
* @return {@link RestStatus#OK} if the blob contents were updated, or else a different status code to indicate the error: possibly
409-
* {@link RestStatus#CONFLICT} or {@link RestStatus#PRECONDITION_FAILED} if the object exists and the precondition requires it
410-
* not to.
413+
* {@link RestStatus#CONFLICT} in any case, but if not then either {@link RestStatus#PRECONDITION_FAILED} if the object exists
414+
* but doesn't match the specified precondition, or {@link RestStatus#NOT_FOUND} if the object doesn't exist but is required to
415+
* do so by the precondition.
411416
*
412417
* @see <a href="https://docs.aws.amazon.com/AmazonS3/latest/userguide/conditional-writes.html#conditional-error-response">AWS docs</a>
413418
*/
@@ -418,6 +423,25 @@ private RestStatus updateBlobContents(HttpExchange exchange, String path, BytesR
418423
: ESTestCase.randomFrom(RestStatus.PRECONDITION_FAILED, RestStatus.CONFLICT);
419424
}
420425

426+
final var requireExistingETag = getRequiredExistingETag(exchange);
427+
if (requireExistingETag != null) {
428+
final var responseCode = new AtomicReference<>(RestStatus.OK);
429+
blobs.compute(path, (ignoredPath, existingContents) -> {
430+
if (existingContents != null && requireExistingETag.equals(getEtagFromContents(existingContents))) {
431+
return newContents;
432+
}
433+
434+
responseCode.set(
435+
ESTestCase.randomFrom(
436+
existingContents == null ? RestStatus.NOT_FOUND : RestStatus.PRECONDITION_FAILED,
437+
RestStatus.CONFLICT
438+
)
439+
);
440+
return existingContents;
441+
});
442+
return responseCode.get();
443+
}
444+
421445
blobs.put(path, newContents);
422446
return RestStatus.OK;
423447
}
@@ -598,6 +622,9 @@ private static boolean isProtectOverwrite(final HttpExchange exchange) {
598622
return false;
599623
}
600624

625+
if (exchange.getRequestHeaders().get("If-Match") != null) {
626+
throw new AssertionError("Handling both If-None-Match and If-Match headers is not supported");
627+
}
601628
if (ifNoneMatch.size() != 1) {
602629
throw new AssertionError("multiple If-None-Match headers found: " + ifNoneMatch);
603630
}
@@ -609,6 +636,29 @@ private static boolean isProtectOverwrite(final HttpExchange exchange) {
609636
throw new AssertionError("invalid If-None-Match header: " + ifNoneMatch);
610637
}
611638

639+
@Nullable // if no If-Match header found
640+
private static String getRequiredExistingETag(final HttpExchange exchange) {
641+
final var ifMatch = exchange.getRequestHeaders().get("If-Match");
642+
643+
if (ifMatch == null) {
644+
return null;
645+
}
646+
647+
if (exchange.getRequestHeaders().get("If-None-Match") != null) {
648+
throw new AssertionError("Handling both If-None-Match and If-Match headers is not supported");
649+
}
650+
651+
final var iterator = ifMatch.iterator();
652+
if (iterator.hasNext()) {
653+
final var result = iterator.next();
654+
if (iterator.hasNext() == false) {
655+
return result;
656+
}
657+
}
658+
659+
throw new AssertionError("multiple If-Match headers found: " + ifMatch);
660+
}
661+
612662
MultipartUpload putUpload(String path) {
613663
final var upload = new MultipartUpload(UUIDs.randomBase64UUID(), path);
614664
synchronized (uploads) {

test/fixtures/s3-fixture/src/test/java/fixture/s3/S3HttpHandlerTests.java

Lines changed: 56 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -412,13 +412,29 @@ public void testExtractPartEtags() {
412412
}
413413

414414
public void testPreventObjectOverwrite() {
415+
ensureExactlyOneSuccess(new S3HttpHandler("bucket", "path"), null);
416+
}
417+
418+
public void testConditionalOverwrite() {
415419
final var handler = new S3HttpHandler("bucket", "path");
416420

417-
var tasks = List.of(
418-
createPutObjectTask(handler),
419-
createPutObjectTask(handler),
420-
createMultipartUploadTask(handler),
421-
createMultipartUploadTask(handler)
421+
final var originalBody = new BytesArray(randomAlphaOfLength(50).getBytes(StandardCharsets.UTF_8));
422+
final var originalETag = S3HttpHandler.getEtagFromContents(originalBody);
423+
assertEquals(RestStatus.OK, handleRequest(handler, "PUT", "/bucket/path/blob", originalBody).status());
424+
assertEquals(
425+
new TestHttpResponse(RestStatus.OK, originalBody, addETag(originalETag, TestHttpExchange.EMPTY_HEADERS)),
426+
handleRequest(handler, "GET", "/bucket/path/blob")
427+
);
428+
429+
ensureExactlyOneSuccess(handler, originalETag);
430+
}
431+
432+
private static void ensureExactlyOneSuccess(S3HttpHandler handler, String originalETag) {
433+
final var tasks = List.of(
434+
createPutObjectTask(handler, originalETag),
435+
createPutObjectTask(handler, originalETag),
436+
createMultipartUploadTask(handler, originalETag),
437+
createMultipartUploadTask(handler, originalETag)
422438
);
423439

424440
runInParallel(tasks.size(), i -> tasks.get(i).consumer.run());
@@ -445,13 +461,38 @@ public void testPreventObjectOverwrite() {
445461
);
446462
}
447463

448-
private static TestWriteTask createPutObjectTask(S3HttpHandler handler) {
464+
public void testPutObjectIfMatchWithBlobNotFound() {
465+
final var handler = new S3HttpHandler("bucket", "path");
466+
while (true) {
467+
final var task = createPutObjectTask(handler, randomIdentifier());
468+
task.consumer.run();
469+
if (task.status == RestStatus.NOT_FOUND) {
470+
break;
471+
}
472+
assertEquals(RestStatus.CONFLICT, task.status); // chosen randomly so eventually we will escape the loop
473+
}
474+
}
475+
476+
public void testCompleteMultipartUploadIfMatchWithBlobNotFound() {
477+
final var handler = new S3HttpHandler("bucket", "path");
478+
while (true) {
479+
final var task = createMultipartUploadTask(handler, randomIdentifier());
480+
task.consumer.run();
481+
if (task.status == RestStatus.NOT_FOUND) {
482+
break;
483+
}
484+
assertEquals(RestStatus.CONFLICT, task.status); // chosen randomly so eventually we will escape the loop
485+
}
486+
}
487+
488+
private static TestWriteTask createPutObjectTask(S3HttpHandler handler, @Nullable String originalETag) {
449489
return new TestWriteTask(
450-
(task) -> task.status = handleRequest(handler, "PUT", "/bucket/path/blob", task.body, ifNoneMatchHeader()).status()
490+
(task) -> task.status = handleRequest(handler, "PUT", "/bucket/path/blob", task.body, conditionalWriteHeader(originalETag))
491+
.status()
451492
);
452493
}
453494

454-
private static TestWriteTask createMultipartUploadTask(S3HttpHandler handler) {
495+
private static TestWriteTask createMultipartUploadTask(S3HttpHandler handler, @Nullable String originalETag) {
455496
final var multipartUploadTask = new TestWriteTask(
456497
(task) -> task.status = handleRequest(
457498
handler,
@@ -465,7 +506,7 @@ private static TestWriteTask createMultipartUploadTask(S3HttpHandler handler) {
465506
<PartNumber>1</PartNumber>
466507
</Part>
467508
</CompleteMultipartUpload>""", task.etag)),
468-
ifNoneMatchHeader()
509+
conditionalWriteHeader(originalETag)
469510
).status()
470511
);
471512

@@ -594,9 +635,13 @@ private static Headers contentRangeHeader(long start, long end, long length) {
594635
return headers;
595636
}
596637

597-
private static Headers ifNoneMatchHeader() {
638+
private static Headers conditionalWriteHeader(@Nullable String originalEtag) {
598639
var headers = new Headers();
599-
headers.put("If-None-Match", List.of("*"));
640+
if (originalEtag == null) {
641+
headers.put("If-None-Match", List.of("*"));
642+
} else {
643+
headers.put("If-Match", List.of(originalEtag));
644+
}
600645
return headers;
601646
}
602647

0 commit comments

Comments
 (0)