Skip to content

Commit

Permalink
feat: save file resource content synchronously for jobs [DHIS2-15276] (
Browse files Browse the repository at this point in the history
  • Loading branch information
jbee authored Nov 21, 2023
1 parent 52ff676 commit f5161c5
Show file tree
Hide file tree
Showing 14 changed files with 67 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,8 @@ public enum ErrorCode {
/* File resource */
E6100("Filename not present"),
E6101("File type not allowed"),
E6102("File content could not be stored"),
E6103("File resource appears to have no content"),

/* Users */
E6200("Feedback message recipients user group not defined"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.net.URI;
import java.time.Duration;
import java.util.List;
import java.util.NoSuchElementException;
import java.util.Optional;
Expand Down Expand Up @@ -90,9 +89,31 @@ public interface FileResourceService {
*/
List<FileResourceOwner> findOwnersByStorageKey(@CheckForNull String storageKey);

void saveFileResource(FileResource fileResource, File file);
/**
* Creates the provided file resource and stores the file content asynchronously.
*
* @param fileResource the resource to create
* @param file the content stored asynchronously
*/
void asyncSaveFileResource(FileResource fileResource, File file);

/**
* Creates the provided file resource and stores the content asynchronously.
*
* @param fileResource the resource to create
* @param bytes the content stored asynchronously
* @return the UID of the created file resource
*/
String asyncSaveFileResource(FileResource fileResource, byte[] bytes);

String saveFileResource(FileResource fileResource, byte[] bytes);
/**
* Creates the provided file resource and stores the content synchronously.
*
* @param fileResource the resource to create
* @param bytes the content stored asynchronously
* @return the UID of the created file resource
*/
String syncSaveFileResource(FileResource fileResource, byte[] bytes) throws ConflictException;

void deleteFileResource(String uid);

Expand All @@ -101,10 +122,6 @@ public interface FileResourceService {
@Nonnull
InputStream getFileResourceContent(FileResource fileResource) throws ConflictException;

@Nonnull
InputStream getFileResourceContent(FileResource fileResource, Duration timeout)
throws ConflictException;

/** Copy fileResource content to outputStream and Return File content length */
void copyFileResourceContent(FileResource fileResource, OutputStream outputStream)
throws IOException, NoSuchElementException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@
*/
package org.hisp.dhis.fileresource;

import static java.lang.System.currentTimeMillis;
import static java.time.Duration.ofMillis;
import static java.time.Duration.ofSeconds;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
Expand Down Expand Up @@ -173,7 +169,7 @@ public List<FileResourceOwner> findOwnersByStorageKey(@CheckForNull String stora

@Override
@Transactional
public void saveFileResource(FileResource fileResource, File file) {
public void asyncSaveFileResource(FileResource fileResource, File file) {
validateFileResource(fileResource);

fileResource.setStorageStatus(FileResourceStorageStatus.PENDING);
Expand All @@ -194,7 +190,7 @@ public void saveFileResource(FileResource fileResource, File file) {

@Override
@Transactional
public String saveFileResource(FileResource fileResource, byte[] bytes) {
public String asyncSaveFileResource(FileResource fileResource, byte[] bytes) {
fileResource.setStorageStatus(FileResourceStorageStatus.PENDING);
fileResourceStore.save(fileResource);
entityManager.flush();
Expand All @@ -206,6 +202,22 @@ public String saveFileResource(FileResource fileResource, byte[] bytes) {
return uid;
}

@Override
@Transactional
public String syncSaveFileResource(FileResource fileResource, byte[] bytes)
throws ConflictException {
fileResource.setStorageStatus(FileResourceStorageStatus.PENDING);
fileResourceStore.save(fileResource);
entityManager.flush();

final String uid = fileResource.getUid();

String storageId = fileResourceContentStore.saveFileResourceContent(fileResource, bytes);
if (storageId == null) throw new ConflictException(ErrorCode.E6102);

return uid;
}

@Override
@Transactional
public void deleteFileResource(String uid) {
Expand Down Expand Up @@ -245,26 +257,9 @@ public void deleteFileResource(FileResource fileResource) {
@Override
@Nonnull
public InputStream getFileResourceContent(FileResource fileResource) throws ConflictException {
return getFileResourceContent(fileResource, ofSeconds(10));
}

@Nonnull
@Override
public InputStream getFileResourceContent(FileResource fileResource, java.time.Duration timeout)
throws ConflictException {
String key = fileResource.getStorageKey();
InputStream content = fileResourceContentStore.getFileResourceContent(key);
long since = currentTimeMillis();
while (content == null && !timeout.minus(ofMillis(currentTimeMillis() - since)).isNegative()) {
try {
Thread.sleep(50);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
content = fileResourceContentStore.getFileResourceContent(key);
}
if (content == null)
throw new ConflictException("File resource exists but content input stream was null");
if (content == null) throw new ConflictException(ErrorCode.E6103);
return content;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ private void saveJobData(String uid, MimeType contentType, InputStream content)
FileResourceDomain.JOB_DATA);
fr.setUid(uid);
fr.setAssigned(true);
fileResourceService.saveFileResource(fr, data);
fileResourceService.syncSaveFileResource(fr, data);
} catch (IOException ex) {
throw new ConflictException("Failed to create job data file resource: " + ex.getMessage());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ void verifySaveFile() {

File file = new File("");

subject.saveFileResource(fileResource, file);
subject.asyncSaveFileResource(fileResource, file);

verify(fileResourceStore).save(fileResource);
verify(entityManager).flush();
Expand All @@ -122,7 +122,8 @@ void verifySaveIllegalFileTypeResourceA() {
"very_evil_script.html", "text/html", 1024, "md5", FileResourceDomain.USER_AVATAR);

File file = new File("very_evil_script.html");
assertThrows(IllegalQueryException.class, () -> subject.saveFileResource(fileResource, file));
assertThrows(
IllegalQueryException.class, () -> subject.asyncSaveFileResource(fileResource, file));
}

@Test
Expand All @@ -136,7 +137,8 @@ void verifySaveIllegalFileTypeResourceB() {
FileResourceDomain.MESSAGE_ATTACHMENT);

File file = new File("suspicious_program.rpm");
assertThrows(IllegalQueryException.class, () -> subject.saveFileResource(fileResource, file));
assertThrows(
IllegalQueryException.class, () -> subject.asyncSaveFileResource(fileResource, file));
}

@Test
Expand All @@ -157,7 +159,7 @@ void verifySaveImageFile() {

fileResource.setUid("imageUid1");

subject.saveFileResource(fileResource, file);
subject.asyncSaveFileResource(fileResource, file);

verify(fileResourceStore).save(fileResource);
verify(entityManager).flush();
Expand Down Expand Up @@ -211,7 +213,7 @@ void verifySaveOrgUnitImageFile() {

fileResource.setUid("imageUid1");

subject.saveFileResource(fileResource, file);
subject.asyncSaveFileResource(fileResource, file);

verify(fileResourceStore).save(fileResource);
verify(entityManager).flush();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ private String saveFileResource(FileResource fileResource, byte[] bytes) {

fileResource.setAssigned(true);

String fileResourceUid = fileResourceService.saveFileResource(fileResource, bytes);
String fileResourceUid = fileResourceService.asyncSaveFileResource(fileResource, bytes);

externalFileResource.setFileResource(fileResourceService.getFileResource(fileResourceUid));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ class TrackedEntityServiceTest extends TransactionalIntegrationTest {
protected void setUpTest() throws Exception {

fileResource = createFileResource('F', "fileResource".getBytes());
fileResourceService.saveFileResource(fileResource, "fileResource".getBytes());
fileResourceService.asyncSaveFileResource(fileResource, "fileResource".getBytes());

userService = _userService;
user = createAndAddAdminUser(AUTHORITY_ALL);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,12 +182,12 @@ void testOrphan() {
content = "filecontentA".getBytes(StandardCharsets.UTF_8);
FileResource fileResourceA = createFileResource('A', content);
fileResourceA.setCreated(DateTime.now().minus(Days.ONE).toDate());
String uidA = fileResourceService.saveFileResource(fileResourceA, content);
String uidA = fileResourceService.asyncSaveFileResource(fileResourceA, content);

content = "filecontentB".getBytes(StandardCharsets.UTF_8);
FileResource fileResourceB = createFileResource('A', content);
fileResourceB.setCreated(DateTime.now().minus(Days.ONE).toDate());
String uidB = fileResourceService.saveFileResource(fileResourceB, content);
String uidB = fileResourceService.asyncSaveFileResource(fileResourceB, content);

User userB = makeUser("B");
userB.setAvatar(fileResourceB);
Expand Down Expand Up @@ -259,7 +259,7 @@ private DataValue createFileResourceDataValue(char uniqueChar, byte[] content) {
organisationUnitService.addOrganisationUnit(orgUnit);

FileResource fileResource = createFileResource(uniqueChar, content);
String uid = fileResourceService.saveFileResource(fileResource, content);
String uid = fileResourceService.asyncSaveFileResource(fileResource, content);

DataValue dataValue = createDataValue(fileElement, period, orgUnit, uid, null);
fileResource.setAssigned(true);
Expand All @@ -275,7 +275,7 @@ private DataValue createFileResourceDataValue(char uniqueChar, byte[] content) {
private ExternalFileResource createExternal(char uniqueChar, byte[] content) {
ExternalFileResource externalFileResource = createExternalFileResource(uniqueChar, content);

fileResourceService.saveFileResource(externalFileResource.getFileResource(), content);
fileResourceService.asyncSaveFileResource(externalFileResource.getFileResource(), content);
externalFileResourceService.saveExternalFileResource(externalFileResource);

FileResource fileResource = externalFileResource.getFileResource();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ public FileResource createAndPersistFileResource(char uniqueChar) {
fileResource.setCreated(new Date());
fileResource.setAutoFields();

String fileResourceUid = fileResourceService.saveFileResource(fileResource, content);
String fileResourceUid = fileResourceService.asyncSaveFileResource(fileResource, content);
return fileResourceService.getFileResource(fileResourceUid);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1020,7 +1020,7 @@ void testSaveImage() {
fileResource.setAssigned(false);
fileResource.setCreated(new Date());
fileResource.setAutoFields();
fileResourceService.saveFileResource(fileResource, content);
fileResourceService.asyncSaveFileResource(fileResource, content);
OrganisationUnit orgUnit = createOrganisationUnit('A');
orgUnit.setImage(fileResource);
organisationUnitService.addOrganisationUnit(orgUnit);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,10 @@ void testFileAttributeValues() {
content = "filecontentA".getBytes();
fileResourceA = createFileResource('A', content);
fileResourceA.setContentType("image/jpg");
fileResourceService.saveFileResource(fileResourceA, content);
fileResourceService.asyncSaveFileResource(fileResourceA, content);
content = "filecontentB".getBytes();
fileResourceB = createFileResource('B', content);
fileResourceService.saveFileResource(fileResourceB, content);
fileResourceService.asyncSaveFileResource(fileResourceB, content);
attributeValueA = createTrackedEntityAttributeValue('A', entityInstanceA, attributeA);
attributeValueB = createTrackedEntityAttributeValue('B', entityInstanceB, attributeB);
attributeValueA.setValue(fileResourceA.getUid());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ void testTrackedEntityProgramAttributeFileResourceValue() throws IOException {
FileResourceDomain.DOCUMENT);
fileResource.setUid("Jzf6hHNP7jx");
File file = File.createTempFile("file-resource", "test");
fileResourceService.saveFileResource(fileResource, file);
fileResourceService.asyncSaveFileResource(fileResource, file);
assertFalse(fileResource.isAssigned());
ImportReport importReport =
trackerImportService.importTracker(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ void testTrackedEntityProgramAttributeFileResourceValue() throws IOException {
FileResourceDomain.DOCUMENT);
fileResource.setUid("Jzf6hHNP7jx");
File file = File.createTempFile("file-resource", "test");
fileResourceService.saveFileResource(fileResource, file);
fileResourceService.asyncSaveFileResource(fileResource, file);
assertFalse(fileResource.isAssigned());
TrackerObjects trackerObjects =
fromJson("tracker/validations/te-program_with_tea_fileresource_data.json");
Expand All @@ -105,7 +105,7 @@ void testFileAlreadyAssign() throws IOException {
FileResourceDomain.DOCUMENT);
fileResource.setUid("Jzf6hHNP7jx");
File file = File.createTempFile("file-resource", "test");
fileResourceService.saveFileResource(fileResource, file);
fileResourceService.asyncSaveFileResource(fileResource, file);
assertFalse(fileResource.isAssigned());
TrackerObjects trackerObjects =
fromJson("tracker/validations/te-program_with_tea_fileresource_data.json");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ public FileResource saveFileResource(String uid, MultipartFile file, FileResourc
throw new WebMessageException(
conflict(ErrorCode.E1119, FileResource.class.getSimpleName(), uid));
}
fileResourceService.saveFileResource(fileResource, tmpFile);
fileResourceService.asyncSaveFileResource(fileResource, tmpFile);
return fileResource;
}

Expand Down

0 comments on commit f5161c5

Please sign in to comment.