Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Maksim_Hadalau authored and Maksim_Hadalau committed Dec 19, 2023
1 parent de05ef2 commit 266041c
Showing 8 changed files with 121 additions and 72 deletions.
Original file line number Diff line number Diff line change
@@ -36,7 +36,7 @@ public Future<?> handle(String bucket, String filePath) {

ResourceDescription resource;
try {
resource = ResourceDescription.from(ResourceType.FILE, urlDecodedBucket, decryptedBucket, filePath);
resource = ResourceDescription.fromEncoded(ResourceType.FILE, urlDecodedBucket, decryptedBucket, filePath);
} catch (Exception ex) {
String errorMessage = ex.getMessage() != null ? ex.getMessage() : DEFAULT_RESOURCE_ERROR_MESSAGE.formatted(filePath);
return context.respond(HttpStatus.BAD_REQUEST, errorMessage);
Original file line number Diff line number Diff line change
@@ -180,7 +180,7 @@ private static FileMetadataBase buildFileMetadata(ResourceDescription resource,

private static ResourceDescription getResourceDescription(ResourceType resourceType, String bucketName, String bucketLocation, String absoluteFilePath) {
String relativeFilePath = absoluteFilePath.substring(bucketLocation.length() + resourceType.getFolder().length() + 1);
return ResourceDescription.from(resourceType, bucketName, bucketLocation, relativeFilePath);
return ResourceDescription.fromDecoded(resourceType, bucketName, bucketLocation, relativeFilePath);
}

private static BlobMetadata buildBlobMetadata(String absoluteFilePath, String contentType, String bucketName) {
Original file line number Diff line number Diff line change
@@ -33,10 +33,6 @@ public String buildUserBucket(ProxyContext context) {
throw new IllegalArgumentException("Can't find user bucket. Either user sub or api-key project must be provided");
}

public String buildAbsoluteFilePath(ResourceType resource, String bucket, String path) {
return bucket + resource.getFolder() + PATH_SEPARATOR + path;
}

public boolean isFolder(String path) {
return path.endsWith(PATH_SEPARATOR);
}
76 changes: 39 additions & 37 deletions src/main/java/com/epam/aidial/core/storage/ResourceDescription.java
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
package com.epam.aidial.core.storage;

import com.epam.aidial.core.util.UrlUtil;
import lombok.AccessLevel;
import lombok.AllArgsConstructor;
import lombok.Data;
import org.apache.commons.lang3.StringUtils;

import java.net.URLDecoder;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;
@@ -25,17 +23,19 @@ public class ResourceDescription {

public String getUrl() {
StringBuilder builder = new StringBuilder();
builder.append(urlEncode(bucketName))
builder.append(UrlUtil.encodePath(bucketName))
.append(BlobStorageUtil.PATH_SEPARATOR);
if (parentFolders != null) {

if (!parentFolders.isEmpty()) {
String parentPath = parentFolders.stream()
.map(ResourceDescription::urlEncode)
.map(UrlUtil::encodePath)
.collect(Collectors.joining(BlobStorageUtil.PATH_SEPARATOR));
builder.append(parentPath)
.append(BlobStorageUtil.PATH_SEPARATOR);
}
if (name != null && !isHomeFolder(name)) {
builder.append(urlEncode(name));

if (name != null) {
builder.append(UrlUtil.encodePath(name));

if (isFolder) {
builder.append(BlobStorageUtil.PATH_SEPARATOR);
@@ -47,69 +47,71 @@ public String getUrl() {

public String getAbsoluteFilePath() {
StringBuilder builder = new StringBuilder();
if (parentFolders != null) {
builder.append(bucketLocation)
.append(type.getFolder())
.append(BlobStorageUtil.PATH_SEPARATOR);

if (!parentFolders.isEmpty()) {
builder.append(getParentPath())
.append(BlobStorageUtil.PATH_SEPARATOR);
}
if (name != null && !isHomeFolder(name)) {

if (name != null) {
builder.append(name);

if (isFolder) {
builder.append(BlobStorageUtil.PATH_SEPARATOR);
}
}

return BlobStorageUtil.buildAbsoluteFilePath(type, bucketLocation, builder.toString());
return builder.toString();
}

public String getParentPath() {
return parentFolders == null ? null : String.join(BlobStorageUtil.PATH_SEPARATOR, parentFolders);
return parentFolders.isEmpty() ? null : String.join(BlobStorageUtil.PATH_SEPARATOR, parentFolders);
}

/**
* Creates resource for the given parameters
*
* @param type resource type
* @param bucketName bucket name (encrypted)
* @param bucketLocation bucket location on blob storage; bucket location must end with /
* @param path url encoded relative path; url path is null or empty we treat it as user home
* @param path url encoded relative path; if url path is null or empty we treat it as user home
*/
public static ResourceDescription from(ResourceType type, String bucketName, String bucketLocation, String path) {
public static ResourceDescription fromEncoded(ResourceType type, String bucketName, String bucketLocation, String path) {
// in case empty path - treat it as a home folder
String urlEncodedRelativePath = StringUtils.isBlank(path) ? BlobStorageUtil.PATH_SEPARATOR : path;
verify(bucketLocation.endsWith(BlobStorageUtil.PATH_SEPARATOR), "Bucket location must end with /");

String[] encodedElements = urlEncodedRelativePath.split(BlobStorageUtil.PATH_SEPARATOR);
List<String> elements = Arrays.stream(encodedElements).map(ResourceDescription::urlDecode).toList();
List<String> elements = Arrays.stream(encodedElements).map(UrlUtil::decodePath).toList();
elements.forEach(element ->
verify(isValidFilename(element), "Invalid path provided " + urlEncodedRelativePath)
);
List<String> parentFolders = null;
String name = "/";
if (!elements.isEmpty()) {
name = elements.get(elements.size() - 1);
}
if (elements.size() > 1) {
String parentPath = urlEncodedRelativePath.substring(0, urlEncodedRelativePath.length() - name.length() - 1);
if (!parentPath.isEmpty() && !parentPath.equals(BlobStorageUtil.PATH_SEPARATOR)) {
parentFolders = List.of(parentPath.split(BlobStorageUtil.PATH_SEPARATOR));
}
}

return new ResourceDescription(type, name, parentFolders, urlEncodedRelativePath, bucketName, bucketLocation,
BlobStorageUtil.isFolder(urlEncodedRelativePath));
return from(type, bucketName, bucketLocation, urlEncodedRelativePath, elements, BlobStorageUtil.isFolder(urlEncodedRelativePath));
}

private static boolean isHomeFolder(String path) {
return path.equals(BlobStorageUtil.PATH_SEPARATOR);
}
/**
* @param type resource type
* @param bucketName bucket name (encrypted)
* @param bucketLocation bucket location on blob storage; bucket location must end with /
* @param path url decoded relative path; if url path is null or empty we treat it as user home
*/
public static ResourceDescription fromDecoded(ResourceType type, String bucketName, String bucketLocation, String path) {
// in case empty path - treat it as a home folder
path = StringUtils.isBlank(path) ? BlobStorageUtil.PATH_SEPARATOR : path;
verify(bucketLocation.endsWith(BlobStorageUtil.PATH_SEPARATOR), "Bucket location must end with /");

private static String urlEncode(String value) {
return URLEncoder.encode(value, StandardCharsets.UTF_8);
List<String> elements = Arrays.asList(path.split(BlobStorageUtil.PATH_SEPARATOR));
return from(type, bucketName, bucketLocation, path, elements, BlobStorageUtil.isFolder(path));
}

private static String urlDecode(String value) {
return URLDecoder.decode(value, StandardCharsets.UTF_8);
private static ResourceDescription from(ResourceType type, String bucketName, String bucketLocation,
String originalPath, List<String> paths, boolean isFolder) {
boolean isEmptyElements = paths.isEmpty();
String name = isEmptyElements ? null : paths.get(paths.size() - 1);
List<String> parentFolders = isEmptyElements ? List.of() : paths.subList(0, paths.size() - 1);
return new ResourceDescription(type, name, parentFolders, originalPath, bucketName, bucketLocation, isFolder);
}

private static boolean isValidFilename(String value) {
28 changes: 28 additions & 0 deletions src/main/java/com/epam/aidial/core/util/UrlUtil.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package com.epam.aidial.core.util;

import lombok.experimental.UtilityClass;

import java.net.URI;
import java.net.URISyntaxException;

@UtilityClass
public class UrlUtil {

public String encodePath(String path) {
try {
URI uri = new URI(null, null, path, null);
return uri.toASCIIString();
} catch (URISyntaxException e) {
throw new RuntimeException(e);
}
}

public String decodePath(String path) {
try {
URI uri = new URI(path);
return uri.getPath();
} catch (URISyntaxException e) {
throw new RuntimeException(e);
}
}
}
10 changes: 5 additions & 5 deletions src/test/java/com/epam/aidial/core/FileApiTest.java
Original file line number Diff line number Diff line change
@@ -88,7 +88,7 @@ public void testEmptyFilesList(Vertx vertx, VertxTestContext context) {
WebClient client = WebClient.create(vertx);

FolderMetadata emptyBucketResponse = new FolderMetadata("7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt",
"/", null, "7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt/", List.of());
null, null, "7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt/", List.of());
client.get(serverPort, "localhost", "/v1/files/metadata/7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt/")
.putHeader("Api-key", "proxyKey2")
.as(BodyCodec.json(FolderMetadata.class))
@@ -209,11 +209,11 @@ public void testFileUpload(Vertx vertx, VertxTestContext context) {
WebClient client = WebClient.create(vertx);

FolderMetadata emptyFolderResponse = new FolderMetadata("7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt",
"/", null, "7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt/", List.of());
null, null, "7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt/", List.of());
FileMetadata expectedFileMetadata = new FileMetadata("7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt",
"file.txt", null, "7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt/file.txt", 17, "text/custom");
FolderMetadata expectedFolderMetadata = new FolderMetadata("7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt",
"/", null, "7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt/", List.of(expectedFileMetadata));
null, null, "7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt/", List.of(expectedFileMetadata));

Future.succeededFuture().compose((mapper) -> {
Promise<Void> promise = Promise.promise();
@@ -310,7 +310,7 @@ public void testListFileWithFolder(Vertx vertx, VertxTestContext context) {
WebClient client = WebClient.create(vertx);

FolderMetadata emptyFolderResponse = new FolderMetadata("7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt",
"/", null, "7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt/", List.of());
null, null, "7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt/", List.of());

FileMetadata expectedFileMetadata1 = new FileMetadata("7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt",
"file.txt", null, "7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt/file.txt", 17, "text/custom");
@@ -319,7 +319,7 @@ public void testListFileWithFolder(Vertx vertx, VertxTestContext context) {
FolderMetadata expectedFolder1Metadata = new FolderMetadata("7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt",
"folder1", null, "7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt/folder1/");
FolderMetadata expectedRootFolderMetadata = new FolderMetadata("7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt",
"/", null, "7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt/",
null, null, "7G9WZNcoY26Vy9D7bEgbv6zqbJGfyDp9KZyEbJR4XMZt/",
List.of(expectedFileMetadata1, expectedFolder1Metadata));

Future.succeededFuture().compose((mapper) -> {
Original file line number Diff line number Diff line change
@@ -15,8 +15,8 @@ public class ResourceDescriptionTest {

@Test
public void testHomeFolderDescription() {
ResourceDescription resource = ResourceDescription.from(ResourceType.FILE, "aes-bucket-name", "buckets/location/", "/");
assertEquals("/", resource.getName());
ResourceDescription resource = ResourceDescription.fromEncoded(ResourceType.FILE, "aes-bucket-name", "buckets/location/", "/");
assertNull(resource.getName());
assertEquals("aes-bucket-name", resource.getBucketName());
assertEquals("buckets/location/", resource.getBucketLocation());
assertEquals(ResourceType.FILE, resource.getType());
@@ -25,27 +25,27 @@ public void testHomeFolderDescription() {
assertEquals("/", resource.getOriginalPath());
assertTrue(resource.isFolder());
assertNull(resource.getParentPath());
assertNull(resource.getParentFolders());
assertTrue(resource.getParentFolders().isEmpty());
}

@Test
public void testUserFolderDescription() {
ResourceDescription resource = ResourceDescription.from(ResourceType.FILE, "test-bucket-name", "buckets/location/", "folder1/");
assertEquals("folder1", resource.getName());
ResourceDescription resource = ResourceDescription.fromEncoded(ResourceType.FILE, "test-bucket-name", "buckets/location/", "folder%201/");
assertEquals("folder 1", resource.getName());
assertEquals("test-bucket-name", resource.getBucketName());
assertEquals("buckets/location/", resource.getBucketLocation());
assertEquals(ResourceType.FILE, resource.getType());
assertEquals("test-bucket-name/folder1/", resource.getUrl());
assertEquals("buckets/location/files/folder1/", resource.getAbsoluteFilePath());
assertEquals("folder1/", resource.getOriginalPath());
assertEquals("test-bucket-name/folder%201/", resource.getUrl());
assertEquals("buckets/location/files/folder 1/", resource.getAbsoluteFilePath());
assertEquals("folder%201/", resource.getOriginalPath());
assertTrue(resource.isFolder());
assertNull(resource.getParentPath());
assertNull(resource.getParentFolders());
assertTrue(resource.getParentFolders().isEmpty());
}

@Test
public void testUserFolderDescription2() {
ResourceDescription resource = ResourceDescription.from(ResourceType.FILE, "test-bucket-name", "buckets/location/", "folder1/folder2/");
ResourceDescription resource = ResourceDescription.fromEncoded(ResourceType.FILE, "test-bucket-name", "buckets/location/", "folder1/folder2/");
assertEquals("folder2", resource.getName());
assertEquals("test-bucket-name", resource.getBucketName());
assertEquals("buckets/location/", resource.getBucketLocation());
@@ -60,7 +60,7 @@ public void testUserFolderDescription2() {

@Test
public void testUserFolderDescription3() {
ResourceDescription resource = ResourceDescription.from(ResourceType.FILE, "test-bucket-name", "buckets/location/", "folder1/folder2/folder3/");
ResourceDescription resource = ResourceDescription.fromEncoded(ResourceType.FILE, "test-bucket-name", "buckets/location/", "folder1/folder2/folder3/");
assertEquals("folder3", resource.getName());
assertEquals("test-bucket-name", resource.getBucketName());
assertEquals("buckets/location/", resource.getBucketLocation());
@@ -75,7 +75,7 @@ public void testUserFolderDescription3() {

@Test
public void testFileDescription1() {
ResourceDescription resource = ResourceDescription.from(ResourceType.FILE, "test-bucket-name", "buckets/location/", "file.txt");
ResourceDescription resource = ResourceDescription.fromEncoded(ResourceType.FILE, "test-bucket-name", "buckets/location/", "file.txt");
assertEquals("file.txt", resource.getName());
assertEquals("test-bucket-name", resource.getBucketName());
assertEquals("buckets/location/", resource.getBucketLocation());
@@ -85,12 +85,12 @@ public void testFileDescription1() {
assertEquals("file.txt", resource.getOriginalPath());
assertFalse(resource.isFolder());
assertNull(resource.getParentPath());
assertNull(resource.getParentFolders());
assertTrue(resource.getParentFolders().isEmpty());
}

@Test
public void testFileDescription2() {
ResourceDescription resource = ResourceDescription.from(ResourceType.FILE, "test-bucket-name", "buckets/location/", "folder1/file.txt");
ResourceDescription resource = ResourceDescription.fromEncoded(ResourceType.FILE, "test-bucket-name", "buckets/location/", "folder1/file.txt");
assertEquals("file.txt", resource.getName());
assertEquals("test-bucket-name", resource.getBucketName());
assertEquals("buckets/location/", resource.getBucketLocation());
@@ -105,7 +105,7 @@ public void testFileDescription2() {

@Test
public void testFileDescription3() {
ResourceDescription resource = ResourceDescription.from(ResourceType.FILE, "test-bucket-name", "buckets/location/", "folder1/folder2/file.txt");
ResourceDescription resource = ResourceDescription.fromEncoded(ResourceType.FILE, "test-bucket-name", "buckets/location/", "folder1/folder2/file.txt");
assertEquals("file.txt", resource.getName());
assertEquals("test-bucket-name", resource.getBucketName());
assertEquals("buckets/location/", resource.getBucketLocation());
@@ -121,30 +121,30 @@ public void testFileDescription3() {
@Test
public void testInvalidBucketLocation() {
assertThrows(IllegalArgumentException.class,
() -> ResourceDescription.from(ResourceType.FILE, "bucket-name", "buckets/location", "file.txt"));
() -> ResourceDescription.fromEncoded(ResourceType.FILE, "bucket-name", "buckets/location", "file.txt"));
}

@Test
public void testEmptyRelativePath() {
assertEquals(
ResourceDescription.from(ResourceType.FILE, "bucket", "location/", "/"),
ResourceDescription.from(ResourceType.FILE, "bucket", "location/", "")
ResourceDescription.fromEncoded(ResourceType.FILE, "bucket", "location/", "/"),
ResourceDescription.fromEncoded(ResourceType.FILE, "bucket", "location/", "")
);
assertEquals(
ResourceDescription.from(ResourceType.FILE, "bucket", "location/", "/"),
ResourceDescription.from(ResourceType.FILE, "bucket", "location/", null)
ResourceDescription.fromEncoded(ResourceType.FILE, "bucket", "location/", "/"),
ResourceDescription.fromEncoded(ResourceType.FILE, "bucket", "location/", null)
);
assertEquals(
ResourceDescription.from(ResourceType.FILE, "bucket", "location/", "/"),
ResourceDescription.from(ResourceType.FILE, "bucket", "location/", " ")
ResourceDescription.fromEncoded(ResourceType.FILE, "bucket", "location/", "/"),
ResourceDescription.fromEncoded(ResourceType.FILE, "bucket", "location/", " ")
);
}

@Test
public void testResourceWithInvalidFilename() {
assertThrows(IllegalArgumentException.class,
() -> ResourceDescription.from(ResourceType.FILE, "bucket", "location/", "folde%2F/"));
() -> ResourceDescription.fromEncoded(ResourceType.FILE, "bucket", "location/", "folde%2F/"));
assertThrows(IllegalArgumentException.class,
() -> ResourceDescription.from(ResourceType.FILE, "bucket", "location/", "folder1/file%2F.txt"));
() -> ResourceDescription.fromEncoded(ResourceType.FILE, "bucket", "location/", "folder1/file%2F.txt"));
}
}
Loading

0 comments on commit 266041c

Please sign in to comment.