Skip to content

Commit

Permalink
fix: absolute path as default; normalize parent file path in metadata (
Browse files Browse the repository at this point in the history
…#58) (#57)

Co-authored-by: Maksim_Hadalau <[email protected]>
  • Loading branch information
Maxim-Gadalov and Maksim_Hadalau authored Nov 24, 2023
1 parent 6b80fc6 commit ddf0c04
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
public class DownloadFileController {

private static final String PATH_TYPE_QUERY_PARAMETER = "path";
private static final String ABSOLUTE_PATH_TYPE = "absolute";
private static final String RELATIVE_PATH_TYPE = "relative";

static final String PURPOSE_FILE_QUERY_PARAMETER = "purpose";

Expand All @@ -41,7 +41,7 @@ public class DownloadFileController {
public Future<?> download(String path) {
String pathType = context.getRequest().params().get(PATH_TYPE_QUERY_PARAMETER);
String absoluteFilePath;
if (!ABSOLUTE_PATH_TYPE.equals(pathType)) {
if (RELATIVE_PATH_TYPE.equals(pathType)) {
absoluteFilePath = BlobStorageUtil.buildAbsoluteFilePath(context, path);
} else {
absoluteFilePath = path;
Expand Down
7 changes: 4 additions & 3 deletions src/main/java/com/epam/aidial/core/storage/BlobStorage.java
Original file line number Diff line number Diff line change
Expand Up @@ -164,13 +164,14 @@ private static FileMetadataBase buildFileMetadata(StorageMetadata metadata) {
String[] elements = absoluteFilePath.split(BlobStorageUtil.PATH_SEPARATOR);
String lastElement = elements[elements.length - 1];
String path = absoluteFilePath.substring(0, absoluteFilePath.length() - lastElement.length() - 1);
String normalizedPath = BlobStorageUtil.normalizeParentPath(path);

return switch (metadata.getType()) {
case BLOB ->
new FileMetadata(lastElement, path, metadata.getSize(), ((BlobMetadata) metadata).getContentMetadata().getContentType());
new FileMetadata(lastElement, normalizedPath, metadata.getSize(),
((BlobMetadata) metadata).getContentMetadata().getContentType());
case FOLDER, RELATIVE_PATH ->
new FolderMetadata(BlobStorageUtil.removeLeadingAndTrailingPathSeparators(lastElement),
BlobStorageUtil.removeTrailingPathSeparator(path));
new FolderMetadata(lastElement, normalizedPath);
case CONTAINER -> throw new IllegalArgumentException("Can't list container");
};
}
Expand Down
40 changes: 26 additions & 14 deletions src/main/java/com/epam/aidial/core/storage/BlobStorageUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,16 @@ public String normalizePathForQuery(String path) {
return path.charAt(path.length() - 1) == DELIMITER ? path : path + PATH_SEPARATOR;
}

public String removeLeadingAndTrailingPathSeparators(String path) {
if (path == null || path.isBlank() || path.equals(PATH_SEPARATOR)) {
return "";
}
path = removeLeadingPathSeparator(path);
return removeTrailingPathSeparator(path);
/**
* Normalize parent path for file metadata by adding leading path separator and removing trailing one.
* For example, path Users/User1/files/folder1/ will be transformed to /Users/User1/files/folder1
*
* @return normalized path
*/
public String normalizeParentPath(String path) {
path = removeTrailingPathSeparator(path);

return path.charAt(0) == DELIMITER ? path : PATH_SEPARATOR + path;
}

public String removeLeadingPathSeparator(String path) {
Expand All @@ -53,14 +57,6 @@ public String removeLeadingPathSeparator(String path) {
return path.charAt(0) == DELIMITER ? path.substring(1) : path;
}

public String removeTrailingPathSeparator(String path) {
if (path == null || path.isBlank()) {
return null;
}
int length = path.length();
return path.charAt(length - 1) == DELIMITER ? path.substring(0, length - 1) : path;
}

public String buildFilePath(String fileName, String path) {
path = removeTrailingPathSeparator(path);
return path + PATH_SEPARATOR + fileName;
Expand All @@ -83,4 +79,20 @@ private String buildAbsoluteFilePath(String userSub, String apiKeyId, String pat
return API_KEY_ROOT_DIR_PATTERN.formatted(apiKeyId, path);
}
}

String removeTrailingPathSeparator(String path) {
if (path == null || path.isBlank()) {
return null;
}
int length = path.length();
return path.charAt(length - 1) == DELIMITER ? path.substring(0, length - 1) : path;
}

String removeLeadingAndTrailingPathSeparators(String path) {
if (path == null || path.isBlank() || path.equals(PATH_SEPARATOR)) {
return "";
}
path = removeLeadingPathSeparator(path);
return removeTrailingPathSeparator(path);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public void end(Handler<AsyncResult<Void>> handler) {
}

Buffer lastChunk = chunkBuffer.slice(0, position);
metadata = new FileMetadata(fileName, BlobStorageUtil.removeTrailingPathSeparator(parentPath),
metadata = new FileMetadata(fileName, BlobStorageUtil.normalizeParentPath(parentPath),
bytesHandled, contentType);
if (mpu == null) {
log.info("Resource is too small for multipart upload, sending as a regular blob");
Expand Down
98 changes: 92 additions & 6 deletions src/test/java/com/epam/aidial/core/FileApiTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.auth0.jwt.algorithms.Algorithm;
import com.epam.aidial.core.config.Storage;
import com.epam.aidial.core.data.FileMetadata;
import com.epam.aidial.core.data.FolderMetadata;
import com.epam.aidial.core.storage.BlobStorage;
import io.vertx.core.Future;
import io.vertx.core.Promise;
Expand All @@ -21,7 +22,6 @@
import org.jclouds.filesystem.reference.FilesystemConstants;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand All @@ -31,6 +31,7 @@
import java.util.Properties;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertIterableEquals;
import static org.junit.jupiter.api.Assertions.assertNull;

@ExtendWith(VertxExtension.class)
Expand Down Expand Up @@ -109,7 +110,7 @@ public void testFileUpload(Vertx vertx, VertxTestContext context) {
Checkpoint checkpoint = context.checkpoint(3);
WebClient client = WebClient.create(vertx);

FileMetadata expectedFileMetadata = new FileMetadata("file.txt", "Users/User1/files", 17, "text/custom");
FileMetadata expectedFileMetadata = new FileMetadata("file.txt", "/Users/User1/files", 17, "text/custom");

Future.succeededFuture().compose((mapper) -> {
Promise<Void> promise = Promise.promise();
Expand Down Expand Up @@ -158,7 +159,7 @@ public void testFileUpload(Vertx vertx, VertxTestContext context) {
.send(context.succeeding(response -> {
context.verify(() -> {
assertEquals(200, response.statusCode());
Assertions.assertIterableEquals(JsonArray.of(JsonObject.mapFrom(expectedFileMetadata)), response.body());
assertIterableEquals(JsonArray.of(JsonObject.mapFrom(expectedFileMetadata)), response.body());
checkpoint.flag();
});
}));
Expand All @@ -170,7 +171,7 @@ public void testFileDownload(Vertx vertx, VertxTestContext context) {
Checkpoint checkpoint = context.checkpoint(3);
WebClient client = WebClient.create(vertx);

FileMetadata expectedFileMetadata = new FileMetadata("file.txt", "Users/User1/files/folder1", 17, "text/plain");
FileMetadata expectedFileMetadata = new FileMetadata("file.txt", "/Users/User1/files/folder1", 17, "text/plain");

Future.succeededFuture().compose((mapper) -> {
Promise<Void> promise = Promise.promise();
Expand All @@ -196,6 +197,7 @@ public void testFileDownload(Vertx vertx, VertxTestContext context) {
// download by relative path
client.get(serverPort, "localhost", "/v1/files/folder1/file.txt")
.putHeader("Api-key", "proxyKey2")
.addQueryParam("path", "relative")
.bearerTokenAuthentication(generateJwtToken("User1"))
.as(BodyCodec.string())
.send(context.succeeding(response -> {
Expand All @@ -211,7 +213,6 @@ public void testFileDownload(Vertx vertx, VertxTestContext context) {
}).andThen((result) -> {
// download by absolute path
client.get(serverPort, "localhost", "/v1/files/Users/User1/files/folder1/file.txt")
.addQueryParam("path", "absolute")
.putHeader("Api-key", "proxyKey2")
.bearerTokenAuthentication(generateJwtToken("User2"))
.as(BodyCodec.string())
Expand All @@ -225,12 +226,97 @@ public void testFileDownload(Vertx vertx, VertxTestContext context) {
});
}

@Test
public void testListFileWithFolder(Vertx vertx, VertxTestContext context) {
Checkpoint checkpoint = context.checkpoint(4);
WebClient client = WebClient.create(vertx);

FileMetadata expectedFileMetadata1 = new FileMetadata("file.txt", "/Users/User1/files", 17, "text/custom");
FileMetadata expectedFileMetadata2 = new FileMetadata("file.txt", "/Users/User1/files/folder1", 17, "text/custom");
FolderMetadata expectedFolderMetadata = new FolderMetadata("folder1", "/Users/User1/files");

Future.succeededFuture().compose((mapper) -> {
Promise<Void> promise = Promise.promise();
// verify no files
client.get(serverPort, "localhost", "/v1/files")
.putHeader("Api-key", "proxyKey2")
.bearerTokenAuthentication(generateJwtToken("User1"))
.addQueryParam("purpose", "metadata")
.as(BodyCodec.jsonArray())
.send(context.succeeding(response -> {
context.verify(() -> {
assertEquals(200, response.statusCode());
assertEquals(JsonArray.of(), response.body());
checkpoint.flag();
promise.complete();
});
}));

return promise.future();
}).compose((mapper) -> {
Promise<Void> promise = Promise.promise();
// upload test file1
client.post(serverPort, "localhost", "/v1/files")
.putHeader("Api-key", "proxyKey2")
.bearerTokenAuthentication(generateJwtToken("User1"))
.as(BodyCodec.json(FileMetadata.class))
.sendMultipartForm(generateMultipartForm("file.txt", TEST_FILE_CONTENT, "text/custom"),
context.succeeding(response -> {
context.verify(() -> {
assertEquals(200, response.statusCode());
assertEquals(expectedFileMetadata1, response.body());
checkpoint.flag();
promise.complete();
});
})
);

return promise.future();
}).compose((mapper) -> {
Promise<Void> promise = Promise.promise();
// upload test file2
client.post(serverPort, "localhost", "/v1/files/folder1")
.putHeader("Api-key", "proxyKey2")
.bearerTokenAuthentication(generateJwtToken("User1"))
.as(BodyCodec.json(FileMetadata.class))
.sendMultipartForm(generateMultipartForm("file.txt", TEST_FILE_CONTENT, "text/custom"),
context.succeeding(response -> {
context.verify(() -> {
assertEquals(200, response.statusCode());
assertEquals(expectedFileMetadata2, response.body());
checkpoint.flag();
promise.complete();
});
})
);

return promise.future();
}).andThen((result) -> {
// verify uploaded files can be listed
client.get(serverPort, "localhost", "/v1/files")
.putHeader("Api-key", "proxyKey2")
.bearerTokenAuthentication(generateJwtToken("User1"))
.addQueryParam("purpose", "metadata")
.as(BodyCodec.jsonArray())
.send(context.succeeding(response -> {
context.verify(() -> {
assertEquals(200, response.statusCode());
assertIterableEquals(
JsonArray.of(JsonObject.mapFrom(expectedFileMetadata1),
JsonObject.mapFrom(expectedFolderMetadata)),
response.body());
checkpoint.flag();
});
}));
});
}

@Test
public void testFileDelete(Vertx vertx, VertxTestContext context) {
Checkpoint checkpoint = context.checkpoint(3);
WebClient client = WebClient.create(vertx);

FileMetadata expectedFileMetadata = new FileMetadata("test_file.txt", "Users/User1/files", 17, "text/plain");
FileMetadata expectedFileMetadata = new FileMetadata("test_file.txt", "/Users/User1/files", 17, "text/plain");

Future.succeededFuture().compose((mapper) -> {
Promise<Void> promise = Promise.promise();
Expand Down

0 comments on commit ddf0c04

Please sign in to comment.