Skip to content

Commit

Permalink
Implement per-day folder and timestamp as part of filename
Browse files Browse the repository at this point in the history
  • Loading branch information
pvannierop committed Jul 3, 2024
1 parent 744dd42 commit b81fe7e
Show file tree
Hide file tree
Showing 8 changed files with 319 additions and 86 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,11 @@ public class S3StorageProperties {
private String accessKey;
private String secretKey;
private String bucketName;
private String subPath;
private Path path = new Path();

@Data
public static class Path {
private String prefix;
private boolean collectPerDay;
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -27,47 +27,44 @@
import org.springframework.util.Assert;
import org.springframework.web.multipart.MultipartFile;

import javax.annotation.PostConstruct;


@Slf4j
@Service
@ConditionalOnExpression("${file-upload.enabled:false} and 's3' == '${storage.type:}'")
public class S3StorageService extends RandomUuidFilenameStorageService implements StorageService {
public class S3StorageService implements StorageService {

@Autowired
private S3StorageProperties s3StorageProperties;

@Autowired
private MinioClientInitializer bucketClient;
private String subPath = "";

@PostConstruct
public void init() {
if (s3StorageProperties.getSubPath() != null) {
subPath = s3StorageProperties.getSubPath().replaceAll("^/|/$", "");
if (!subPath.isEmpty()) {
subPath = subPath + "/";
}
}
}

public String store(MultipartFile file, String projectId, String subjectId, String topicId) {
Assert.notNull(file, "File must not be null");
Assert.notEmpty(new String[]{projectId, subjectId, topicId}, "Project, subject and topic IDs must not be empty");
String filePath = String.format("%s%s/%s/%s/%s", subPath, projectId, subjectId, topicId, generateRandomFilename(file.getOriginalFilename()));
log.debug("Storing file at path: {}", filePath);

StoragePath filePath = StoragePath.builder()
.prefix(s3StorageProperties.getPath().getPrefix())
.projectId(projectId)
.subjectId(subjectId)
.topicId(topicId)
.collectPerDay(s3StorageProperties.getPath().isCollectPerDay())
.filename(file.getOriginalFilename())
.build();

log.debug("Attempt storing file at path: {}", filePath.getFullPath());

try {
bucketClient.getClient().putObject(PutObjectArgs
.builder()
.bucket(bucketClient.getBucketName())
.object(filePath)
.object(filePath.getFullPath())
.stream(file.getInputStream(), file.getSize(), -1)
.build());
} catch (Exception e) {
throw new RuntimeException("Could not store file", e);
}
return filePath;
return filePath.getPathInTopicDir();
}

}
168 changes: 168 additions & 0 deletions src/main/java/org/radarbase/appserver/service/storage/StoragePath.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
package org.radarbase.appserver.service.storage;

import org.springframework.util.Assert;

import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.util.UUID;
import java.util.stream.Collectors;
import java.util.stream.Stream;

/**
* Represents path on Object Storage for uploaded files.
* <p>
* The storage path is constructed to include required arguments (projectId, subjectId, topicId and filename)
* and optional arguments (prefix, collectPerDay, folderTimestampPattern, fileTimestampPattern). The path will follow
* the format: prefix/projectId/subjectId/topicId/[day folder]/timestamp_filename.extension.
* The day folder is included if collectPerDay is set to true. File extensions are converted to lowercase.
* </p>
*
* <p>
* Usage example:
* </p>
* <pre>
* StoragePath path = StoragePath.builder()
* .prefix("uploads")
* .projectId("project1")
* .subjectId("subjectA")
* .topicId("topicX")
* .collectPerDay(true)
* .filename("example.txt")
* .build();
*
* System.out.println(path.getFullPath();
* 'uploads/project1/subjectA/topicX/20220101/20220101_example.txt'
*
* System.out.println(path.getPathInTopicDir()
* '20220101/20220101_example.txt'
* </pre>
*/
public class StoragePath {

private String pathInBucket;
private String pathInTopicDirectory;

public StoragePath (String pathInBucket, String pathInTopicDirectory) {
this.pathInBucket = pathInBucket;
this.pathInTopicDirectory = pathInTopicDirectory;
}

public String getFullPath() {
return pathInBucket;
}

public String getPathInTopicDir() {
return pathInTopicDirectory;
}

public static Builder builder() {
return new Builder();
}

public static class Builder {

private String prefix = "";
private String filename = "";
private boolean collectPerDay = false;
private String projectId = "";
private String subjectId = "";
private String topicId = "";
private String folderTimestampPattern = "yyyyMMdd";
private String fileTimestampPattern = "yyyyMMddHHmmss";
private String dirSep = "/";

public Builder filename(String filename) {
this.filename = filename;
return this;
}

public Builder prefix(String prefix) {
Assert.notNull(prefix, "Prefix must not be null");
this.prefix = prefix;
return this;
}

public Builder collectPerDay(boolean collectPerDay) {
this.collectPerDay = collectPerDay;
return this;
}

public Builder projectId(String projectId) {
Assert.notNull(projectId, "Project Id must not be null");
this.projectId = projectId;
return this;
}

public Builder subjectId(String subjectId) {
Assert.notNull(subjectId, "Subject Id must not be null");
this.subjectId = subjectId;
return this;
}

public Builder topicId(String topicId) {
Assert.notNull(topicId, "Topic Id must not be null");
this.topicId = topicId;
return this;
}

public Builder dayFolderPattern(String dayFolderPattern) {
Assert.notNull(dayFolderPattern, "Day folder pattern must not be null");
this.folderTimestampPattern = dayFolderPattern;
return this;
}

public Builder fileTimestampPattern(String fileTimestampPattern) {
Assert.notNull(fileTimestampPattern, "File timestamp pattern must not be null");
this.fileTimestampPattern = fileTimestampPattern;
return this;
}

public StoragePath build() {
Assert.isTrue(!filename.isBlank(), "Filename must be set.");
Assert.isTrue(!projectId.isBlank(), "Project Id must be set.");
Assert.isTrue(!subjectId.isBlank(), "Subject Id must be set.");
Assert.isTrue(!topicId.isBlank(), "Topic Id must be set.");

String pathInTopicDir = Stream.of(
this.collectPerDay ? getDayFolder() : "",
// Storing files under their original filename is a security risk, as it can be used to
// overwrite existing files. We generate a random filename server-side to mitigate this risk.
// See https://owasp.org/www-community/vulnerabilities/Unrestricted_File_Upload
generateRandomFilename(this.filename)
).filter(s -> !s.isBlank())
.collect(Collectors.joining(this.dirSep));

String fullPath = Stream.of(
this.prefix,
projectId,
subjectId,
topicId,
pathInTopicDir
).filter(s -> !s.isBlank())
.collect(Collectors.joining(this.dirSep));

return new StoragePath(fullPath, pathInTopicDir);
}

private String generateRandomFilename(String originalFilename) {
String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern(this.fileTimestampPattern));
return timestamp + "_" + UUID.randomUUID() + getFileExtension(originalFilename);
}

private String getDayFolder() {
return LocalDate.now().format(DateTimeFormatter.ofPattern(this.folderTimestampPattern));
}

private String getFileExtension(String originalFilename) {
int lastDot = originalFilename.lastIndexOf('.');
if (lastDot < 0) {
return "";
} else {
return originalFilename.substring(lastDot).toLowerCase();
}
}

}

}
9 changes: 7 additions & 2 deletions src/main/resources/application-prod.properties
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,13 @@ security.github.cache.retryDuration=60

# DATA UPLOAD
file-upload.enabled=true
storage.type=s3 # can be 's3'
# Can be only 's3' at the moment.
storage.type=s3
storage.s3.url=http://localhost:9000
storage.s3.access-key=access-key
storage.s3.secret-key=secret-key
storage.s3.bucket-name=radar
storage.s3.bucket-name=radar
# Fixed prefix path in the output bucket (e.g., https://s3.amazonaws.com/bucket-name/<prefix>/subject-id/topic-id/...)
storage.s3.path.prefix=
# Collect uploaded files per day in a separate folder (e.g., https://s3.amazonaws.com/bucket-name/subject-id/topic-id/<20240220>/...)
storage.s3.path.collect-per-day=true
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@
@WebMvcTest(UploadController.class)
@AutoConfigureMockMvc(addFilters = false)
@TestPropertySource(properties = {
"file-upload.enabled=true"
"file-upload.enabled=true",
"security.radar.managementportal.enabled=false"
})
class UploadControllerTest {

Expand All @@ -70,7 +71,7 @@ void setUp() {
@Test
void testUploadFile() throws Exception {
String uri = String.format(
"/projects/%s/users/%s/topics/%s/upload", PROJECT_ID, SUBJECT_ID, TOPIC_ID);
"/projects/%s/users/%s/files/topics/%s/upload", PROJECT_ID, SUBJECT_ID, TOPIC_ID);
mockMvc
.perform(
MockMvcRequestBuilders
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,11 @@
package org.radarbase.appserver.service.storage;

import io.minio.MinioClient;
import io.minio.errors.ErrorResponseException;
import io.minio.errors.InsufficientDataException;
import io.minio.errors.InternalException;
import io.minio.errors.InvalidResponseException;
import io.minio.errors.ServerException;
import io.minio.errors.XmlParserException;
import io.minio.PutObjectArgs;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.radarbase.appserver.config.S3StorageProperties;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
Expand All @@ -39,17 +33,12 @@
import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.web.multipart.MultipartFile;

import java.io.IOException;
import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;

import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.*;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;

@ExtendWith(SpringExtension.class)
@SpringBootTest(
Expand All @@ -58,7 +47,7 @@
"file-upload.enabled=true",
"storage.type=s3",
"storage.s3.bucket-name=my-bucket",
"storage.s3.sub-path=my-sub-path",
"storage.s3.path.prefix=my-sub-path",
}
)
@EnableConfigurationProperties({S3StorageProperties.class})
Expand All @@ -81,7 +70,7 @@ class S3StorageServiceTest {
public void setUp() throws Exception {
given(minioInit.getBucketName()).willReturn("my-bucket");
given(minioInit.getClient()).willReturn(minioClient);
given(minioClient.putObject(any())).willReturn(null);
given(minioClient.putObject(any(PutObjectArgs.class))).willReturn(null);
}

@Test
Expand All @@ -98,13 +87,8 @@ void testInvalidArguments() {
@Test
void testStore() throws Exception {
String path = s3StorageService.store(multipartFile, "projectId", "subjectId", "topicId");
// Make sure that MinioClient.putObject is called with the correct arguments.
verify(minioClient).putObject(argThat(args -> args.bucket().equals("my-bucket")
&& args.object().startsWith("my-sub-path/projectId/subjectId/topicId/")
&& args.object().endsWith(".txt"))
);

assertTrue(path.startsWith("my-sub-path/projectId/subjectId/topicId/"));
verify(minioClient).putObject(any());
assertTrue(path.matches("[0-9]+_[a-z0-9-]+\\.txt"));
}

}
Loading

0 comments on commit b81fe7e

Please sign in to comment.