Skip to content

Commit

Permalink
fix: wait until file resource content is available (#15686)
Browse files Browse the repository at this point in the history
* fix: wait until file resource content is available

* fix: detect null content for file resources

* fix: wait for file content to become available
  • Loading branch information
jbee authored Nov 14, 2023
1 parent 4ebaa4b commit 014eec0
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,33 @@
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;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import org.hisp.dhis.feedback.ConflictException;
import org.hisp.dhis.feedback.NotFoundException;

/**
* @author Halvdan Hoem Grelland
*/
public interface FileResourceService {

@CheckForNull
FileResource getFileResource(String uid);

/**
* Get the {@link FileResource} with the given ID
*
* @param uid the resource to fetch
* @return the file resource
* @throws NotFoundException when no such file resource exits
*/
@Nonnull
FileResource getExistingFileResource(String uid) throws NotFoundException;

/**
* Lookup a {@link FileResource} by uid and {@link FileResourceDomain}.
*
Expand Down Expand Up @@ -83,7 +98,12 @@ public interface FileResourceService {

void deleteFileResource(FileResource fileResource);

InputStream getFileResourceContent(FileResource fileResource);
@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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,5 @@
public enum FileResourceStorageStatus {
NONE, // No content stored
PENDING, // In transit to store, not available
FAILED, // Storing the resource failed
STORED // Is available from store
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@
*/
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 All @@ -43,7 +47,9 @@
import lombok.RequiredArgsConstructor;
import org.hibernate.SessionFactory;
import org.hisp.dhis.common.IllegalQueryException;
import org.hisp.dhis.feedback.ConflictException;
import org.hisp.dhis.feedback.ErrorCode;
import org.hisp.dhis.feedback.NotFoundException;
import org.hisp.dhis.fileresource.events.BinaryFileSavedEvent;
import org.hisp.dhis.fileresource.events.FileDeletedEvent;
import org.hisp.dhis.fileresource.events.FileSavedEvent;
Expand Down Expand Up @@ -86,6 +92,15 @@ public class DefaultFileResourceService implements FileResourceService {
// FileResourceService implementation
// -------------------------------------------------------------------------

@Nonnull
@Override
@Transactional(readOnly = true)
public FileResource getExistingFileResource(String uid) throws NotFoundException {
FileResource fr = fileResourceStore.getByUid(uid);
if (fr == null) throw new NotFoundException(FileResource.class, uid);
return fr;
}

@Override
@Transactional(readOnly = true)
public FileResource getFileResource(String uid) {
Expand Down Expand Up @@ -228,26 +243,43 @@ public void deleteFileResource(FileResource fileResource) {
}

@Override
@Transactional(readOnly = true)
public InputStream getFileResourceContent(FileResource fileResource) {
return fileResourceContentStore.getFileResourceContent(fileResource.getStorageKey());
@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");
return content;
}

@Override
@Transactional(readOnly = true)
public long getFileResourceContentLength(FileResource fileResource) {
return fileResourceContentStore.getFileResourceContentLength(fileResource.getStorageKey());
}

@Override
@Transactional(readOnly = true)
public void copyFileResourceContent(FileResource fileResource, OutputStream outputStream)
throws IOException, NoSuchElementException {
fileResourceContentStore.copyContent(fileResource.getStorageKey(), outputStream);
}

@Override
@Transactional(readOnly = true)
public byte[] copyFileResourceContent(FileResource fileResource)
throws IOException, NoSuchElementException {
return fileResourceContentStore.copyContent(fileResource.getStorageKey());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ public InputStream getFileResourceContent(String key) {
final Blob blob = getBlob(key);

if (blob == null) {

return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
import org.hisp.dhis.dxf2.webmessage.WebMessage;
import org.hisp.dhis.dxf2.webmessage.WebMessageException;
import org.hisp.dhis.feedback.BadRequestException;
import org.hisp.dhis.feedback.ConflictException;
import org.hisp.dhis.fieldfilter.FieldFilterParams;
import org.hisp.dhis.fieldfilter.FieldFilterService;
import org.hisp.dhis.fileresource.FileResource;
Expand Down Expand Up @@ -205,7 +206,7 @@ public void getAttributeImage(
@RequestParam(required = false) Integer height,
@RequestParam(required = false) ImageFileDimension dimension,
HttpServletResponse response)
throws WebMessageException {
throws WebMessageException, ConflictException {
User user = currentUserService.getCurrentUser();

TrackedEntity trackedEntity = instanceService.getTrackedEntity(teiId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public void execute(JobConfiguration config, JobProgress progress) {
MetadataImportParams params = (MetadataImportParams) config.getJobParameters();
progress.startingStage("Loading file resource");
FileResource data =
progress.runStage(() -> fileResourceService.getFileResource(config.getUid()));
progress.runStage(() -> fileResourceService.getExistingFileResource(config.getUid()));
progress.startingStage("Loading file content");
try (InputStream input =
progress.runStage(() -> fileResourceService.getFileResourceContent(data))) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public void execute(JobConfiguration config, JobProgress progress) {
TrackerImportParams params = (TrackerImportParams) config.getJobParameters();
progress.startingStage("Loading file resource");
FileResource data =
progress.runStage(() -> fileResourceService.getFileResource(config.getUid()));
progress.runStage(() -> fileResourceService.getExistingFileResource(config.getUid()));
progress.startingStage("Loading file content");
try (InputStream input =
progress.runStage(() -> fileResourceService.getFileResourceContent(data))) {
Expand Down

0 comments on commit 014eec0

Please sign in to comment.