Skip to content

Commit

Permalink
Fix handling of artifact range requests
Browse files Browse the repository at this point in the history
Artifact range requests in HawkBit were impossible to do efficiently, as the
underlying mechanism had no choice but to read the start byte and discard
content both before and after.

To address this situation, we need to introduce a new method to DbArtifact:
getFileInputStream(long, long), which maps to the semantics of a range request.

The next step is to adjust FileStreamingUtil to use it for both simple range
and multi-part requests.

Signed-off-by: Zygmunt Krynicki <[email protected]>
  • Loading branch information
zyga committed Oct 2, 2023
1 parent a420495 commit d2940c9
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,19 @@

import java.io.BufferedInputStream;
import java.io.File;
import java.io.RandomAccessFile;
import java.nio.channels.Channels;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.util.Objects;

import javax.validation.constraints.NotNull;

import com.google.common.io.ByteStreams;
import org.apache.commons.io.input.RandomAccessFileInputStream;

import org.eclipse.hawkbit.artifact.repository.model.AbstractDbArtifact;
import org.eclipse.hawkbit.artifact.repository.model.DbArtifactHash;

Expand Down Expand Up @@ -47,4 +53,19 @@ public InputStream getFileInputStream() {
throw new ArtifactFileNotFoundException(e);
}
}

@Override
// suppress warning, this InputStream needs to be closed by the caller, this
// cannot be closed in this method
@SuppressWarnings("squid:S2095")
public InputStream getFileInputStream(long start, long end) {
try {
var f = new RandomAccessFile(file, "r");
var ch = f.getChannel();
ch.position(start);
return ByteStreams.limit(Channels.newInputStream(ch), end - start + 1);
} catch (final IOException e) {
throw new ArtifactFileNotFoundException(e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,13 @@ public interface DbArtifact {
* @return {@link InputStream} to read from artifact.
*/
InputStream getFileInputStream();

/**
* Creates an {@link InputStream} on a single range of this artifact.
* The caller has to take care of closing the stream.
* Repeatable calls open a new {@link InputStream}.
*
* @return {@link InputStream} to read from artifact.
*/
InputStream getFileInputStream(long start, long end);
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,9 @@ public String getContentType() {
public InputStream getFileInputStream() {
return decryptionFunction.apply(encryptedDbArtifact.getFileInputStream());
}

@Override
public InputStream getFileInputStream(long start, long end) {
return decryptionFunction.apply(encryptedDbArtifact.getFileInputStream(start, end));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,8 @@ private static ResponseEntity<InputStream> handleMultipartRangeRequest(final DbA
final ServletOutputStream to = response.getOutputStream();

for (final ByteRange r : ranges) {
try (final InputStream from = artifact.getFileInputStream()) {

try (final InputStream from = artifact.getFileInputStream(r.getStart(), r.getEnd())) {
// Add multipart boundary and header fields for every range.
to.println();
to.println("--" + ByteRange.MULTIPART_BOUNDARY);
Expand Down Expand Up @@ -316,7 +316,7 @@ private static ResponseEntity<InputStream> handleStandardRangeRequest(final DbAr
response.setContentLengthLong(r.getLength());
response.setStatus(HttpServletResponse.SC_PARTIAL_CONTENT);

try (final InputStream from = artifact.getFileInputStream()) {
try (final InputStream from = artifact.getFileInputStream(r.getStart(), r.getEnd())) {
final ServletOutputStream to = response.getOutputStream();
copyStreams(from, to, progressListener, r.getStart(), r.getLength(), filename);
} catch (final IOException e) {
Expand All @@ -340,8 +340,6 @@ private static long copyStreams(final InputStream from, final OutputStream to,
long total = 0;
int progressPercent = 1;

ByteStreams.skipFully(from, start);

long toRead = length;
boolean toContinue = true;
long shippedSinceLastEvent = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ public String getContentType() {
public InputStream getFileInputStream() {
return new ByteArrayInputStream(CONTENT_BYTES);
}

@Override
public InputStream getFileInputStream(long start, long end) {
return new ByteArrayInputStream(CONTENT_BYTES, (int)start, (int)(end-start));
}
};

@Test
Expand Down

0 comments on commit d2940c9

Please sign in to comment.