Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

File Service - Update Documenation and Verify Temp Storage #59

Merged
merged 12 commits into from
Sep 18, 2023
Merged

File Service - Update Documenation and Verify Temp Storage #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
102263e
Add default value for PASS_CORE_FILE_SERVICE_ROOT_DIR in application.…
tsande16 Sep 12, 2023
11f2c6b
Update README
tsande16 Sep 13, 2023
38f944c
Add test for temp cleanup
tsande16 Sep 13, 2023
2d89e2b
Cleanup FileStorageServiceTest
tsande16 Sep 13, 2023
f4db138
Update javadoc
tsande16 Sep 13, 2023
d8f81e1
Add test FileStorageServiceS3RootTest
tsande16 Sep 13, 2023
1d984ea
Delete FileStorageServiceS3RootTest
tsande16 Sep 14, 2023
b015556
Remove disabled annotation
tsande16 Sep 14, 2023
4230b29
Cleanup
tsande16 Sep 14, 2023
209211e
Update README.md
tsande16 Sep 14, 2023
df41934
Cleanup
tsande16 Sep 14, 2023
d9ad786
Delete application-test-root-S3.yml
tsande16 Sep 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions pass-core-file-service/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,25 @@ The pass-core-file-service is a RESTful service that provides the ability to upl
configured persistence store. The service is currently designed to persist to a filesystem or S3 compatible storage.

## Configuration
The service is configured via environment variables.
The service by default will use a filesystem based persistence store and does not require any additional configuration. If the variable
PASS_CORE_FILE_SERVICE_ROOT_DIR does not have any value, the File Service will default to the system temp folder and
create a temporary root folder of a random value in the system temp. The following environment variables are available
for configuring the service:
The service is configured via environment variables. The service by default will use a filesystem based persistence
store and does not require any additional configuration. If the variable PASS_CORE_FILE_SERVICE_ROOT_DIR does not have
any value, the File Service will default to the system temp folder and create a temporary root folder of a random value
in the system temp. The variable PASS_CORE_FILE_SERVICE_ROOT_DIR is used by both the FILE_SYSTEM and S3 service types.
It is the root directory where temporary files are stored before being persisted to the configured persistence store as
specified by PASS_CORE_FILE_SERVICE_TYPE. If using FILE_SYSTEM as the persistence store, PASS_CORE_FILE_SERVICE_ROOT_DIR
is also the root directory for file persistence. The value for the PASS_CORE_FILE_SERVICE_ROOT_DIR cannot be a S3 bucket,
and it must be a valid path on the local filesystem.

The following environment variables are available for configuring the service:

- PASS_CORE_FILE_SERVICE_TYPE=`FILE_SYSTEM`
- Currently supports [`FILE_SYSTEM` | `S3`]
- PASS_CORE_FILE_SERVICE_ROOT_DIR=`/path/to/root/dir`
- The root directory of the service that is used to support file uploads and downloads.
- Default: system_tmp/17318424270250529523
- The root directory of the service that is used to support file uploads and downloads and the root directory
for file persistence if using FILE_SYSTEM as the persistence store.
- Default example: system_tmp/17318424270250529523
- PASS_CORE_S3_BUCKET_NAME=`bucket-test-name`
- The name of the S3 bucket to use for file persistence if using S3 as the persistence store.
- PASS_CORE_S3_REPO_PREFIX=`s3-repo-prefix`
- PASS_CORE_S3_ENDPOINT=`http://localhost:9090`
- If using a custom endpoint for S3, this value should be set to the endpoint URL.
Expand Down
7 changes: 3 additions & 4 deletions ...-file-service/src/main/java/org/eclipse/pass/file/service/storage/FileStorageService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@
* A configuration of File System requires that the environment variables are properly set in the env file and
* the respective directories have read/write access. For a S3 configuration to work the client needs the
* following permissions: s3:PutObject, s3:GetObject,s3:DeleteObject, s3:ListBucket, s3:AbortMultipartUpload.
*
* The directory structure for the File System is as follows:
* - rootDir: This is the root directory for the File System. This is set in the
* .env file (PASS_CORE_FILE_SERVICE_ROOT_DIR). If it is not set then the default is the system temp directory.
Expand All @@ -76,7 +75,6 @@
* the rootDir. Both the ocflDir and workDir are required to be on the same mount.
* - tempDir: This is a temporary directory that is used to move files to/from the OCFL repository and staging them
* for download. This is a child of the rootDir.
*
* Note, the S3 OCFL implementation does not cache locally and therefore performs much slower compared to the file
* system implementation, most notably on large files.
*
Expand Down Expand Up @@ -106,6 +104,7 @@ public FileStorageService(StorageConfiguration storageConfiguration,
@Value("${aws.region}") String awsRegion) throws IOException {
StorageProperties storageProperties = storageConfiguration.getStorageProperties();
storageType = storageProperties.getStorageType();
LOG.info("File Service: " + storageType + " Storage Type");

Path rootLoc;
if (storageProperties.getStorageRootDir() == null
Expand All @@ -118,6 +117,7 @@ public FileStorageService(StorageConfiguration storageConfiguration,
} else {
rootLoc = Paths.get(storageProperties.getStorageRootDir());
}
LOG.info("File Service: " + rootLoc + " Storage Root Directory");

// The ocflLoc only needs to be set if the storage type is file system.
// If the storageType is S3 then workLoc and tempLoc are used because they are used with FILE_SYSTEM AND S3
Expand Down Expand Up @@ -145,7 +145,6 @@ public FileStorageService(StorageConfiguration storageConfiguration,
}

if (storageType.equals(StorageServiceType.FILE_SYSTEM)) {
LOG.info("File Service: FILE_SYSTEM Storage Type");
try {
if (!Files.exists(ocflLoc)) {
Files.createDirectory(ocflLoc);
Expand All @@ -162,7 +161,6 @@ public FileStorageService(StorageConfiguration storageConfiguration,
throw new IOException("File Service: Unable to setup File Storage directories: " + e);
}
} else if (storageType.equals(StorageServiceType.S3)) {
LOG.info("File Service: S3 Storage Type");
if (storageProperties.getBucketName().isPresent()) {
bucketName = storageProperties.getBucketName().get();
} else {
Expand Down Expand Up @@ -301,6 +299,7 @@ public StorageFile storeFile(MultipartFile mFile, String userName) throws IOExce
fileExt
);

Files.delete(tempPathAndFileName);
} catch (IOException e) {
LOG.error(e.toString());
throw new IOException("File Service: The file system was unable to store the uploaded file", e);
Expand Down
2 changes: 1 addition & 1 deletion pass-core-main/src/main/resources/application.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ pass:
embed: ${PASS_CORE_EMBED_JMS_BROKER:false}
file-service:
storage-type: ${PASS_CORE_FILE_SERVICE_TYPE:FILE_SYSTEM}
root-dir: ${PASS_CORE_FILE_SERVICE_ROOT_DIR}
root-dir: ${PASS_CORE_FILE_SERVICE_ROOT_DIR:#{null}}
s3-bucket-name: ${PASS_CORE_S3_BUCKET_NAME:pass-core-file}
s3-repo-prefix: ${PASS_CORE_S3_REPO_PREFIX:pass-core-file}
s3-endpoint: ${PASS_CORE_S3_ENDPOINT:#{null}}
Expand Down
2 changes: 1 addition & 1 deletion ...re-main/src/test/java/org/eclipse/pass/file/service/storage/FileStorageServiceS3Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
*/
@ActiveProfiles("test-S3")
class FileStorageServiceS3Test extends FileStorageServiceTest {
private static S3Mock s3MockApi;
private static final S3Mock s3MockApi;
private static final int S3_MOCK_PORT = 8010;

// Set up the S3 mock server before the Application Context is loaded.
Expand Down
123 changes: 76 additions & 47 deletions ...core-main/src/test/java/org/eclipse/pass/file/service/storage/FileStorageServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.nio.file.Paths;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;

import edu.wisc.library.ocfl.api.exception.NotFoundException;
import okhttp3.Credentials;
Expand Down Expand Up @@ -91,7 +92,7 @@ protected void tearDown() throws IOException {
@Test
public void storeFileThatExists() throws IOException {
StorageFile storageFile = storageService.storeFile(new MockMultipartFile("test", "test.txt",
MEDIA_TYPE_TEXT.toString(), "Test Pass-core".getBytes()), USER_NAME);
Objects.requireNonNull(MEDIA_TYPE_TEXT).toString(), "Test Pass-core".getBytes()), USER_NAME);
assertFalse(storageService.getResourceFileRelativePath(storageFile.getId()).isEmpty());
//check that the owner is the same
assertEquals(storageService.getFileOwner(storageFile.getId()), USER_NAME);
Expand All @@ -103,18 +104,35 @@ public void storeFileThatExists() throws IOException {
@Test
void getFileShouldReturnFile() throws IOException {
StorageFile storageFile = storageService.storeFile(new MockMultipartFile("test", "test.txt",
MEDIA_TYPE_TEXT.toString(), "Test Pass-core".getBytes()), USER_NAME);
Objects.requireNonNull(MEDIA_TYPE_TEXT).toString(), "Test Pass-core".getBytes()), USER_NAME);
ByteArrayResource file = storageService.getFile(storageFile.getId());
assertTrue(file.contentLength() > 0);
}

/**
* Test that temporary files are cleaned up after being persisted.
* @throws IOException if there is an error
*/
@Test
void storeFileAndEnsureTempIsCleaned() throws IOException {
storageService.storeFile(new MockMultipartFile("test", "test.txt",
Objects.requireNonNull(MEDIA_TYPE_TEXT).toString(), "Test Pass-core".getBytes()), USER_NAME);

Path tempDir = Paths.get(System.getProperty("java.io.tmpdir"));
String rootDirName = storageConfiguration.getStorageProperties().getStorageRootDir();
String tempDirName = storageConfiguration.getStorageProperties().getStorageTempDir();
int fileCount = Objects.requireNonNull(tempDir.resolve(Paths.get(rootDirName, tempDirName))
.toFile().listFiles()).length;
assertEquals(0, fileCount);
}

/**
* Test file content type is returned.
*/
@Test
void getFileContentTypeShouldReturnContentType() throws IOException {
StorageFile storageFile = storageService.storeFile(new MockMultipartFile("test", "test.txt",
MEDIA_TYPE_TEXT.toString(), "Test Pass-core".getBytes()), USER_NAME);
Objects.requireNonNull(MEDIA_TYPE_TEXT).toString(), "Test Pass-core".getBytes()), USER_NAME);
String contentType = storageService.getFileContentType(storageFile.getId());
assertEquals(MEDIA_TYPE_TEXT.toString(), contentType);
}
Expand Down Expand Up @@ -156,7 +174,7 @@ void storeFileWithDifferentLangFilesNames() {
allCharSets.forEach((k,v) -> {
try {
StorageFile storageFile = storageService.storeFile(new MockMultipartFile("test", v,
MEDIA_TYPE_TEXT.toString(), "Test Pass-core".getBytes()), USER_NAME);
Objects.requireNonNull(MEDIA_TYPE_TEXT).toString(), "Test Pass-core".getBytes()), USER_NAME);
assertFalse(storageService.getResourceFileRelativePath(storageFile.getId()).isEmpty());
} catch (IOException e) {
assertEquals("An exception was thrown in storeFileWithDifferentLangFilesNames. On charset=" + k,
Expand All @@ -171,12 +189,12 @@ void storeFileWithDifferentLangFilesNames() {
@Test
void deleteShouldThrowExceptionFileNotExist() throws IOException {
StorageFile storageFile = storageService.storeFile(new MockMultipartFile("test", "test.txt",
MEDIA_TYPE_TEXT.toString(), "Test Pass-core".getBytes()), USER_NAME);
Objects.requireNonNull(MEDIA_TYPE_TEXT).toString(), "Test Pass-core".getBytes()), USER_NAME);
storageService.deleteFile(storageFile.getId());
Exception exception = assertThrows(NotFoundException.class,
() -> storageService.getResourceFileRelativePath(storageFile.getId()));
String exceptionText = exception.getMessage();
assertTrue(exceptionText.matches("(.)+(was not found){1}(.)+"));
assertTrue(exceptionText.matches("(.)+(was not found)(.)+"));
}

/**
Expand All @@ -188,7 +206,7 @@ void deleteShouldThrowExceptionFileNotExist() throws IOException {
void userHasPermissionToDeleteFile() throws IOException {
boolean hasPermission;
StorageFile storageFile = storageService.storeFile(new MockMultipartFile("test", "test.txt",
MEDIA_TYPE_TEXT.toString(), "Test Pass-core".getBytes()), USER_NAME);
Objects.requireNonNull(MEDIA_TYPE_TEXT).toString(), "Test Pass-core".getBytes()), USER_NAME);
hasPermission = storageService.checkUserDeletePermissions(storageFile.getId(), USER_NAME);
assertTrue(hasPermission);
}
Expand All @@ -200,9 +218,9 @@ void userHasPermissionToDeleteFile() throws IOException {
*/
@Test
void userNoPermissionToDeleteFile() throws IOException {
boolean hasPermission = false;
boolean hasPermission;
StorageFile storageFile = storageService.storeFile(new MockMultipartFile("test", "test.txt",
MEDIA_TYPE_TEXT.toString(), "Test Pass-core".getBytes()), USER_NAME);
Objects.requireNonNull(MEDIA_TYPE_TEXT).toString(), "Test Pass-core".getBytes()), USER_NAME);
hasPermission = storageService.checkUserDeletePermissions(storageFile.getId(), USER_NAME2);
assertFalse(hasPermission);
}
Expand All @@ -215,7 +233,7 @@ void userNoPermissionToDeleteFile() throws IOException {
@Test
void getFileByIdUsingController() throws IOException {
StorageFile storageFile = storageService.storeFile(new MockMultipartFile("test", "test.txt",
MEDIA_TYPE_TEXT.toString(), "Test Pass-core".getBytes()), USER_NAME);
Objects.requireNonNull(MEDIA_TYPE_TEXT).toString(), "Test Pass-core".getBytes()), USER_NAME);

ByteArrayResource file = storageService.getFile(storageFile.getId());
//ensure that the file has been stored by the service
Expand All @@ -230,10 +248,11 @@ void getFileByIdUsingController() throws IOException {
.header("Authorization", credentialsBackend)
.get()
.build();
Response response = client.newCall(request).execute();

assertEquals(HttpStatus.OK.value(), response.code());
assertNotNull(response.body());
try (Response response = client.newCall(request).execute()) {
assertEquals(HttpStatus.OK.value(), response.code());
assertNotNull(response.body());
}
}

/**
Expand All @@ -257,17 +276,18 @@ void uploadFile() throws IOException {
.addHeader("Authorization", credentialsBackend)
.build();

Response response = httpClient.newCall(request).execute();
assertEquals(HttpStatus.CREATED.value(), response.code());
assertNotNull(response.body());
file.delete();
try (Response response = httpClient.newCall(request).execute()) {
assertEquals(HttpStatus.CREATED.value(), response.code());
assertNotNull(response.body());
assertTrue(file.delete());
}
}

/**
* Test upload and download of file with not known content type.
*
* @throws IOException if there is an error
* @throws JSONException
* @throws JSONException if there is an error
*/
@Test
void uploadFileWithUnknownType() throws IOException, JSONException {
Expand All @@ -286,26 +306,29 @@ void uploadFileWithUnknownType() throws IOException, JSONException {
.addHeader("Authorization", credentialsBackend)
.build();

Response response = httpClient.newCall(request).execute();
assertEquals(HttpStatus.CREATED.value(), response.code());
try (Response response = httpClient.newCall(request).execute()) {
assertEquals(HttpStatus.CREATED.value(), response.code());

String id = new JSONObject(Objects.requireNonNull(response.body()).string()).getString("id");

String id = new JSONObject(response.body().string()).getString("id");
// Download
Request dlRequest = new Request.Builder().url(url + "/" + id).
addHeader("Authorization", credentialsBackend).build();

// Download
Request dlRequest = new Request.Builder().url(url + "/" + id).
addHeader("Authorization", credentialsBackend).build();
Response dlResponse = httpClient.newCall(dlRequest).execute();
assertEquals(HttpStatus.OK.value(), dlResponse.code());
try (Response dlResponse = httpClient.newCall(dlRequest).execute()) {
assertEquals(HttpStatus.OK.value(), dlResponse.code());

assertArrayEquals(data, dlResponse.body().bytes());
assertEquals(MEDIA_TYPE_APPLICATION, dlResponse.body().contentType());
assertArrayEquals(data, Objects.requireNonNull(dlResponse.body()).bytes());
assertEquals(MEDIA_TYPE_APPLICATION, dlResponse.body().contentType());
}
}
}

/**
* Test upload and download of a large file
*
* @throws IOException if there is an error
* @throws JSONException
* @throws JSONException if there is an error
*/
@Test
void uploadLargeFile() throws IOException, JSONException {
Expand All @@ -324,19 +347,22 @@ void uploadLargeFile() throws IOException, JSONException {
.addHeader("Authorization", credentialsBackend)
.build();

Response response = httpClient.newCall(request).execute();
assertEquals(HttpStatus.CREATED.value(), response.code());
try (Response response = httpClient.newCall(request).execute()) {
assertEquals(HttpStatus.CREATED.value(), response.code());

String id = new JSONObject(Objects.requireNonNull(response.body()).string()).getString("id");

String id = new JSONObject(response.body().string()).getString("id");
// Download
Request dlRequest = new Request.Builder().url(url + "/" + id).
addHeader("Authorization", credentialsBackend).build();

// Download
Request dlRequest = new Request.Builder().url(url + "/" + id).
addHeader("Authorization", credentialsBackend).build();
Response dlResponse = httpClient.newCall(dlRequest).execute();
assertEquals(HttpStatus.OK.value(), dlResponse.code());
try (Response dlResponse = httpClient.newCall(dlRequest).execute()) {
assertEquals(HttpStatus.OK.value(), dlResponse.code());

assertArrayEquals(data.getBytes(), dlResponse.body().bytes());
assertEquals(MEDIA_TYPE_TEXT, dlResponse.body().contentType());
assertArrayEquals(data.getBytes(), Objects.requireNonNull(dlResponse.body()).bytes());
assertEquals(MEDIA_TYPE_TEXT, dlResponse.body().contentType());
}
}
}

/**
Expand All @@ -358,8 +384,9 @@ void uploadFileMissingFile() throws IOException {
.addHeader("Authorization", credentialsBackend)
.build();

Response response = httpClient.newCall(request).execute();
assertEquals(HttpStatus.BAD_REQUEST.value(), response.code());
try (Response response = httpClient.newCall(request).execute()) {
assertEquals(HttpStatus.BAD_REQUEST.value(), response.code());
}
}

/**
Expand All @@ -382,9 +409,10 @@ void uploadFileMissingFileName() throws IOException {
.addHeader("Authorization", credentialsBackend)
.build();

Response response = httpClient.newCall(request).execute();
assertEquals(HttpStatus.BAD_REQUEST.value(), response.code());
file.delete();
try (Response response = httpClient.newCall(request).execute()) {
assertEquals(HttpStatus.BAD_REQUEST.value(), response.code());
}
assertTrue(file.delete());
}

/**
Expand All @@ -394,7 +422,7 @@ void uploadFileMissingFileName() throws IOException {
@Test
void deleteFileUsingFileServiceController() throws IOException {
StorageFile storageFile = storageService.storeFile(new MockMultipartFile("test", "test.txt",
MEDIA_TYPE_TEXT.toString(), "Test Pass-core".getBytes()), USER_NAME);
Objects.requireNonNull(MEDIA_TYPE_TEXT).toString(), "Test Pass-core".getBytes()), USER_NAME);
String url = getBaseUrl() + "file" + "/" + storageFile.getId();

Request request = new Request.Builder()
Expand All @@ -403,8 +431,9 @@ void deleteFileUsingFileServiceController() throws IOException {
.addHeader("Authorization", credentialsBackend)
.build();

Response response = httpClient.newCall(request).execute();
assertEquals(HttpStatus.OK.value(), response.code());
try (Response response = httpClient.newCall(request).execute()) {
assertEquals(HttpStatus.OK.value(), response.code());
}
}

private File createTestFile() throws IOException {
Expand Down