Skip to content

Commit

Permalink
feat: Support If-Match and If-None-Match for get file request #549 (#556
Browse files Browse the repository at this point in the history 
)
  • Loading branch information
@astsiapanay
astsiapanay authored Nov 4, 2024
1 parent 1578f18 commit 8a500af
Show file tree
Hide file tree
Showing 9 changed files with 153 additions and 61 deletions.
6 changes: 4 additions & 2 deletions server/src/main/java/com/epam/aidial/core/server/controller/DownloadFileController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

import com.epam.aidial.core.server.Proxy;
import com.epam.aidial.core.server.ProxyContext;
import com.epam.aidial.core.server.util.ProxyUtil;
import com.epam.aidial.core.server.vertx.stream.InputStreamReader;
import com.epam.aidial.core.storage.http.HttpStatus;
import com.epam.aidial.core.storage.resource.ResourceDescriptor;
import com.epam.aidial.core.storage.util.EtagHeader;
import io.vertx.core.Future;
import io.vertx.core.http.HttpHeaders;
import io.vertx.core.http.HttpServerResponse;
Expand All @@ -22,8 +24,8 @@ protected Future<?> handle(ResourceDescriptor resource, boolean hasWriteAccess)
if (resource.isFolder()) {
return context.respond(HttpStatus.BAD_REQUEST, "Can't download a folder");
}

proxy.getVertx().executeBlocking(() -> proxy.getResourceService().getResourceStream(resource), false)
EtagHeader etagHeader = ProxyUtil.etag(context.getRequest());
proxy.getVertx().executeBlocking(() -> proxy.getResourceService().getResourceStream(resource, etagHeader), false)
.compose(resourceStream -> {
if (resourceStream == null) {
return context.respond(HttpStatus.NOT_FOUND);
Expand Down
11 changes: 6 additions & 5 deletions server/src/main/java/com/epam/aidial/core/server/controller/ResourceController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,9 @@ private Future<?> getResource(ResourceDescriptor descriptor, boolean hasWriteAcc
return context.respond(HttpStatus.BAD_REQUEST, "Folder not allowed: " + descriptor.getUrl());
}

EtagHeader etagHeader = ProxyUtil.etag(context.getRequest());
Future<Pair<ResourceItemMetadata, String>> responseFuture = (descriptor.getType() == ResourceTypes.APPLICATION)
? getApplicationData(descriptor, hasWriteAccess) : getResourceData(descriptor);
? getApplicationData(descriptor, hasWriteAccess, etagHeader) : getResourceData(descriptor, etagHeader);

responseFuture.onSuccess(pair -> {
context.putHeader(HttpHeaders.ETAG, pair.getKey().getEtag())
Expand All @@ -136,9 +137,9 @@ private Future<?> getResource(ResourceDescriptor descriptor, boolean hasWriteAcc
return Future.succeededFuture();
}

private Future<Pair<ResourceItemMetadata, String>> getApplicationData(ResourceDescriptor descriptor, boolean hasWriteAccess) {
private Future<Pair<ResourceItemMetadata, String>> getApplicationData(ResourceDescriptor descriptor, boolean hasWriteAccess, EtagHeader etagHeader) {
return vertx.executeBlocking(() -> {
Pair<ResourceItemMetadata, Application> result = applicationService.getApplication(descriptor);
Pair<ResourceItemMetadata, Application> result = applicationService.getApplication(descriptor, etagHeader);
ResourceItemMetadata meta = result.getKey();

Application application = result.getValue();
Expand All @@ -151,9 +152,9 @@ private Future<Pair<ResourceItemMetadata, String>> getApplicationData(ResourceDe
}, false);
}

private Future<Pair<ResourceItemMetadata, String>> getResourceData(ResourceDescriptor descriptor) {
private Future<Pair<ResourceItemMetadata, String>> getResourceData(ResourceDescriptor descriptor, EtagHeader etag) {
return vertx.executeBlocking(() -> {
Pair<ResourceItemMetadata, String> result = service.getResourceWithMetadata(descriptor);
Pair<ResourceItemMetadata, String> result = service.getResourceWithMetadata(descriptor, etag);

if (result == null) {
throw new ResourceNotFoundException();
Expand Down
6 changes: 5 additions & 1 deletion server/src/main/java/com/epam/aidial/core/server/service/ApplicationService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,12 @@ public List<Application> getPublicApplications(ProxyContext context) {
}

public Pair<ResourceItemMetadata, Application> getApplication(ResourceDescriptor resource) {
return getApplication(resource, EtagHeader.ANY);
}

public Pair<ResourceItemMetadata, Application> getApplication(ResourceDescriptor resource, EtagHeader etagHeader) {
verifyApplication(resource);
Pair<ResourceItemMetadata, String> result = resourceService.getResourceWithMetadata(resource);
Pair<ResourceItemMetadata, String> result = resourceService.getResourceWithMetadata(resource, etagHeader);

if (result == null) {
throw new ResourceNotFoundException("Application is not found: " + resource.getUrl());
Expand Down
3 changes: 2 additions & 1 deletion server/src/main/java/com/epam/aidial/core/server/service/RuleService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.epam.aidial.core.storage.data.ResourceItemMetadata;
import com.epam.aidial.core.storage.resource.ResourceDescriptor;
import com.epam.aidial.core.storage.service.ResourceService;
import com.epam.aidial.core.storage.util.EtagHeader;
import com.fasterxml.jackson.core.type.TypeReference;
import org.apache.commons.lang3.tuple.Pair;

Expand Down Expand Up @@ -129,7 +130,7 @@ private Map<String, List<Rule>> getCachedRules() {
Pair<Long, Map<String, List<Rule>>> current = cachedRules.get();

if (current == null || current.getKey() != key) {
Pair<ResourceItemMetadata, String> resource = resources.getResourceWithMetadata(PUBLIC_RULES);
Pair<ResourceItemMetadata, String> resource = resources.getResourceWithMetadata(PUBLIC_RULES, EtagHeader.ANY);
Pair<Long, Map<String, List<Rule>>> next = (resource == null)
? Pair.of(Long.MIN_VALUE, decodeRules(null))
: Pair.of(resource.getKey().getUpdatedAt(), decodeRules(resource.getValue()));
Expand Down
42 changes: 40 additions & 2 deletions server/src/test/java/com/epam/aidial/core/server/FileApiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@
import io.vertx.junit5.VertxTestContext;
import lombok.extern.slf4j.Slf4j;
import org.apache.http.HttpHeaders;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;

import java.util.Map;
import java.util.stream.IntStream;

import static org.junit.Assert.assertTrue;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
Expand Down Expand Up @@ -1410,7 +1412,43 @@ public void testIfMatch(Vertx vertx, VertxTestContext context) {
context.succeeding(response -> {
context.verify(() -> {
assertEquals(412, response.statusCode());
assertEquals("ETag 123 is rejected", response.bodyAsString());
assertTrue(response.bodyAsString().startsWith("If-match condition is failed for etag"));
checkpoint.flag();
promise.complete();
});
})
);

return promise.future();
}).compose(mapper -> {
Promise<Void> promise = Promise.promise();
// get the test file with incorrect ETag
client.get(serverPort, "localhost", "/v1/files/7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt/test_file.txt")
.putHeader("Api-key", "proxyKey2")
.putHeader(HttpHeaders.IF_MATCH, "123")
.as(BodyCodec.string())
.send(context.succeeding(response -> {
context.verify(() -> {
assertEquals(412, response.statusCode());
assertTrue(response.body().startsWith("If-match condition is failed for etag"));
checkpoint.flag();
promise.complete();
});
}));

return promise.future();
}).compose(mapper -> {
Promise<Void> promise = Promise.promise();
// get the test file with correct ETag
client.get(serverPort, "localhost", "/v1/files/7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt/test_file.txt")
.putHeader("Api-key", "proxyKey2")
.putHeader(HttpHeaders.IF_MATCH, TEST_FILE_ETAG)
.as(BodyCodec.string())
.send(
context.succeeding(response -> {
context.verify(() -> {
assertEquals(200, response.statusCode());
assertEquals(TEST_FILE_CONTENT, response.body());
checkpoint.flag();
promise.complete();
});
Expand Down Expand Up @@ -1476,7 +1514,7 @@ public void testIfMatch(Vertx vertx, VertxTestContext context) {
.send(context.succeeding(response -> {
context.verify(() -> {
assertEquals(412, response.statusCode());
assertEquals("ETag %s is rejected".formatted(TEST_FILE_ETAG), response.bodyAsString());
assertTrue(response.bodyAsString().startsWith("If-match condition is failed for etag"));
checkpoint.flag();
promise.complete();
});
Expand Down
10 changes: 5 additions & 5 deletions server/src/test/java/com/epam/aidial/core/server/ResourceApiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public void testIfMatch() {
verifyNotExact(response, 200, "\"etag\":\"70edd26b3686de5efcdae93fcc87c2bb\"");

response = resourceRequest(HttpMethod.PUT, "/folder/conversation", CONVERSATION_BODY_2, "if-match", "123");
verifyNotExact(response, 412, "ETag 123 is rejected");
verifyNotExact(response, 412, "If-match condition is failed for etag");

response = resourceRequest(HttpMethod.PUT, "/folder/conversation", CONVERSATION_BODY_2, "if-match", "70edd26b3686de5efcdae93fcc87c2bb");
verifyNotExact(response, 200, "\"etag\":\"82833ed7a10a4f99253fccdef4091ad9\"");
Expand All @@ -132,7 +132,7 @@ public void testIfMatch() {
verifyNotExact(response, 200, "\"etag\":\"82833ed7a10a4f99253fccdef4091ad9\"");

response = resourceRequest(HttpMethod.DELETE, "/folder/conversation", "", "if-match", "123");
verify(response, 412, "ETag 123 is rejected");
verifyNotExact(response, 412, "If-match condition is failed for etag");

response = resourceRequest(HttpMethod.DELETE, "/folder/conversation", "", "if-match", "82833ed7a10a4f99253fccdef4091ad9");
verify(response, 200, "");
Expand Down Expand Up @@ -166,9 +166,9 @@ void testMaxContentSize() {
}

@Test
void testUnsupportedIfNoneMatchHeader() {
Response response = resourceRequest(HttpMethod.PUT, "/folder/big", "1", "if-none-match", "unsupported");
verify(response, 400, "only header if-none-match=* is supported");
void testIfNoneMatch() {
Response response = resourceRequest(HttpMethod.PUT, "/folder/big", CONVERSATION_BODY_1, "if-none-match", "unsupported");
verifyNotExact(response, 200, "big");
}

@Test
Expand Down
32 changes: 16 additions & 16 deletions storage/src/main/java/com/epam/aidial/core/storage/service/ResourceService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,12 +278,12 @@ public boolean hasResource(ResourceDescriptor descriptor) {
}

@Nullable
public Pair<ResourceItemMetadata, String> getResourceWithMetadata(ResourceDescriptor descriptor) {
return getResourceWithMetadata(descriptor, true);
public Pair<ResourceItemMetadata, String> getResourceWithMetadata(ResourceDescriptor descriptor, EtagHeader etag) {
return getResourceWithMetadata(descriptor, etag, true);
}

@Nullable
private Pair<ResourceItemMetadata, String> getResourceWithMetadata(ResourceDescriptor descriptor, boolean lock) {
private Pair<ResourceItemMetadata, String> getResourceWithMetadata(ResourceDescriptor descriptor, EtagHeader etagHeader, boolean lock) {
String redisKey = redisKey(descriptor);
Result result = redisGet(redisKey, true);

Expand All @@ -300,6 +300,7 @@ private Pair<ResourceItemMetadata, String> getResourceWithMetadata(ResourceDescr
}

if (result.exists()) {
etagHeader.validate(result.etag);
return Pair.of(
toResourceItemMetadata(descriptor, result),
new String(result.body, StandardCharsets.UTF_8));
Expand All @@ -310,30 +311,30 @@ private Pair<ResourceItemMetadata, String> getResourceWithMetadata(ResourceDescr

@Nullable
public String getResource(ResourceDescriptor descriptor) {
return getResource(descriptor, true);
return getResource(descriptor, EtagHeader.ANY, true);
}

@Nullable
private String getResource(ResourceDescriptor descriptor, boolean lock) {
Pair<ResourceItemMetadata, String> result = getResourceWithMetadata(descriptor, lock);
private String getResource(ResourceDescriptor descriptor, EtagHeader etag, boolean lock) {
Pair<ResourceItemMetadata, String> result = getResourceWithMetadata(descriptor, etag, lock);
return (result == null) ? null : result.getRight();
}

public ResourceStream getResourceStream(ResourceDescriptor resource) throws IOException {
public ResourceStream getResourceStream(ResourceDescriptor resource, EtagHeader etagHeader) throws IOException {
if (resource.getType().requireCompression()) {
throw new IllegalArgumentException("Streaming is supported for uncompressed resources only");
}

String key = redisKey(resource);
Result result = redisGet(key, true);
if (result != null) {
return ResourceStream.fromResult(result);
return ResourceStream.fromResult(result, etagHeader);
}

try (LockService.Lock ignored = lockService.lock(key)) {
result = redisGet(key, true);
if (result != null) {
return ResourceStream.fromResult(result);
return ResourceStream.fromResult(result, etagHeader);
}

Blob blob = blobStore.load(resource.getAbsoluteFilePath());
Expand All @@ -351,9 +352,10 @@ public ResourceStream getResourceStream(ResourceDescriptor resource) throws IOEx
if (length <= maxSize) {
result = blobToResult(blob, metadata);
redisPut(key, result);
return ResourceStream.fromResult(result);
return ResourceStream.fromResult(result, etagHeader);
}

etagHeader.validate(etag);
return new ResourceStream(payload.openStream(), etag, contentType, length);
}
}
Expand Down Expand Up @@ -457,11 +459,7 @@ public ResourceItemMetadata computeResource(ResourceDescriptor descriptor, EtagH
String redisKey = redisKey(descriptor);

try (var ignore = lockService.lock(redisKey)) {
Pair<ResourceItemMetadata, String> oldResult = getResourceWithMetadata(descriptor, false);

if (oldResult != null) {
etag.validate(oldResult.getKey().getEtag());
}
Pair<ResourceItemMetadata, String> oldResult = getResourceWithMetadata(descriptor, etag, false);

String oldBody = oldResult == null ? null : oldResult.getValue();
String newBody = fn.apply(oldBody);
Expand Down Expand Up @@ -843,11 +841,13 @@ public void close() throws IOException {
}

@Nullable
private static ResourceStream fromResult(Result item) {
private static ResourceStream fromResult(Result item, EtagHeader etagHeader) {
if (!item.exists()) {
return null;
}

etagHeader.validate(item.etag);

return new ResourceStream(
new ByteArrayInputStream(item.body),
item.etag(),
Expand Down
Loading

0 comments on commit 8a500af

Please sign in to comment.