Improve HTTP/1.1 response send file implementation

Motivation:

The implementation of HTTP/1.1 send file can be simplied by using two arguments FileChannel/RandomAccessFile instead of trying to abstract them.

In addition we can set the response content type earlier when a file name is provided.

Changes:

Make the Http1xServerResponse#sendFileInternal accept both FileChannel/RandomAccessFile as those are the only arguments we can actually use.

Set the response content type when a file name is available and to application/octet-stream in the generic case.

Add test for sending RandomAccessFile instances.

Author: vietj

vertx-core/src/main/java/io/vertx/core/http/HttpHeaders.java

@@ -354,6 +354,12 @@ public interface HttpHeaders {
   @GenIgnore(GenIgnore.PERMITTED_TYPE)
   CharSequence APPLICATION_X_WWW_FORM_URLENCODED = HttpHeaderValues.APPLICATION_X_WWW_FORM_URLENCODED;
 
+  /**
+   * application/application/octet-stream header value
+   */
+  @GenIgnore(GenIgnore.PERMITTED_TYPE)
+  CharSequence APPLICATION_OCTET_STREAM = HttpHeaderValues.APPLICATION_OCTET_STREAM;
+
   /**
    * multipart/form-data header value
    */

vertx-core/src/main/java/io/vertx/core/http/HttpServerResponse.java

@@ -423,7 +423,7 @@ default Future<Void> sendFile(FileChannel channel, long offset) {
   @GenIgnore(GenIgnore.PERMITTED_TYPE)
   @Unstable
   default Future<Void> sendFile(RandomAccessFile file) {
-    return sendFile(file.getChannel(), 0);
+    return sendFile(file, 0);
   }
 
   /**
@@ -437,7 +437,7 @@ default Future<Void> sendFile(RandomAccessFile file) {
   @GenIgnore(GenIgnore.PERMITTED_TYPE)
   @Unstable
   default Future<Void> sendFile(RandomAccessFile file, long offset) {
-    return sendFile(file.getChannel(), offset, Long.MAX_VALUE);
+    return sendFile(file, offset, Long.MAX_VALUE);
   }
 
   /**
@@ -450,9 +450,7 @@ default Future<Void> sendFile(RandomAccessFile file, long offset) {
    */
   @GenIgnore(GenIgnore.PERMITTED_TYPE)
   @Unstable
-  default Future<Void> sendFile(RandomAccessFile file, long offset, long length) {
-    return sendFile(file.getChannel(), offset, length);
-  }
+  Future<Void> sendFile(RandomAccessFile file, long offset, long length);
 
   /**
    * @return has the response already ended?

vertx-core/src/main/java/io/vertx/core/http/impl/Http1xServerResponse.java

@@ -449,18 +449,14 @@ public Future<Void> sendFile(String filename, long offset, long length) {
     } catch (Exception e) {
       return context.failedFuture(e);
     }
-    Future<Void> result = sendFileInternal(filename, offset,
-      length,
-      MimeMapping::mimeTypeForFilename,
-      (r) -> {
-        try {
-          return r.length();
-        } catch (IOException e) {
-          throw new RuntimeException(e);
-        }
-      },
-      () -> raf,
-      conn::sendFile);
+    if (!headers.contains(HttpHeaders.CONTENT_TYPE)) {
+      CharSequence mimeType = MimeMapping.mimeTypeForFilename(filename);
+      if (mimeType == null) {
+        mimeType = APPLICATION_OCTET_STREAM;
+      }
+      headers.set(CONTENT_TYPE, mimeType);
+    }
+    Future<Void> result = sendFile(raf, offset, length);
     if (result.failed()) {
       try {
         raf.close();
@@ -470,23 +466,35 @@ public Future<Void> sendFile(String filename, long offset, long length) {
     return result;
   }
 
+  @Override
+  public Future<Void> sendFile(RandomAccessFile file, long offset, long length) {
+    if (!headers.contains(HttpHeaders.CONTENT_TYPE)) {
+      headers.set(CONTENT_TYPE, APPLICATION_OCTET_STREAM);
+    }
+    long size;
+    try {
+      size = file.length();
+    } catch (IOException e) {
+      return context.failedFuture(e);
+    }
+    return sendFileInternal(offset, length, size, null, file);
+  }
+
   @Override
   public Future<Void> sendFile(FileChannel channel, long offset, long length) {
-    return sendFileInternal("application/octet-stream", offset,
-      length,
-      Function.identity(),
-      (c) -> {
-        try {
-          return c.size();
-        } catch (IOException e) {
-          throw new RuntimeException(e);
-        }
-      },
-      () -> channel,
-      conn::sendFile);
+    if (!headers.contains(HttpHeaders.CONTENT_TYPE)) {
+      headers.set(CONTENT_TYPE, APPLICATION_OCTET_STREAM);
+    }
+    long size;
+    try {
+      size = channel.size();
+    } catch (IOException e) {
+      return context.failedFuture(e);
+    }
+    return sendFileInternal(offset, length, size, channel, null);
   }
 
-  private <F> Future<Void> sendFileInternal(String nameOrExtension, long offset, long length, Function<String, String> contentTypeMapper, Function<F, Long> lengthSupplier, Supplier<F> fileSupplier, Sender<F> sendFileSupplier) {
+  private <F> Future<Void> sendFileInternal(long offset, long length, long size, FileChannel fileChannel, RandomAccessFile raf) {
     ContextInternal ctx = vertx.getOrCreateContext();
     if (offset < 0) {
       return ctx.failedFuture("offset : " + offset + " (expected: >= 0)");
@@ -500,12 +508,6 @@ private <F> Future<Void> sendFileInternal(String nameOrExtension, long offset, l
         throw new IllegalStateException("Head already written");
       }
 
-      long size;
-      try {
-        size = lengthSupplier.apply(fileSupplier.get());
-      } catch (Exception e) {
-        return ctx.failedFuture(e);
-      }
       long actualLength = Math.min(length, size - offset);
       long actualOffset = Math.min(offset, size);
 
@@ -514,19 +516,18 @@ private <F> Future<Void> sendFileInternal(String nameOrExtension, long offset, l
         return ctx.failedFuture("offset : " + offset + " is larger than the requested file length : " + size);
       }
 
-      if (!headers.contains(HttpHeaders.CONTENT_TYPE)) {
-        String contentType = contentTypeMapper.apply(nameOrExtension);
-        if (contentType != null) {
-          headers.set(HttpHeaders.CONTENT_TYPE, contentType);
-        }
-      }
       prepareHeaders(actualLength);
       bytesWritten = actualLength;
       written = true;
 
       conn.write(new VertxAssembledHttpResponse(head, version, status, headers), null);
 
-      ChannelFuture channelFut = sendFileSupplier.send(fileSupplier.get(), actualOffset, actualLength);
+      ChannelFuture channelFut;
+      if (fileChannel != null) {
+        channelFut = conn.sendFile(fileChannel, actualOffset, actualLength);
+      } else {
+        channelFut = conn.sendFile(raf, actualOffset, actualLength);
+      }
       channelFut.addListener(future -> {
 
         // write an empty last content to let the http encoder know the response is complete
@@ -836,12 +837,4 @@ public HttpServerResponse addCookie(Cookie cookie) {
       return (Set) cookies().removeOrInvalidateAll(name, invalidate);
     }
   }
-
-  @FunctionalInterface
-  private static interface Sender<F> {
-
-    ChannelFuture send(F u, Long v, Long w);
-
-  }
-
 }

vertx-core/src/main/java/io/vertx/core/http/impl/Http2ServerResponse.java

@@ -34,6 +34,7 @@
 import io.vertx.core.spi.observability.HttpResponse;
 import io.vertx.core.streams.ReadStream;
 
+import java.io.RandomAccessFile;
 import java.nio.channels.FileChannel;
 import java.util.Map.Entry;
 import java.util.Set;
@@ -570,6 +571,11 @@ public Future<Void> sendFile(String filename, long offset, long length) {
     });
   }
 
+  @Override
+  public Future<Void> sendFile(RandomAccessFile file, long offset, long length) {
+    return stream.context.failedFuture("HTTP/2 does not support sending random access file for now");
+  }
+
   @Override
   public Future<Void> sendFile(FileChannel channel, long offset, long length) {
     return stream.context.failedFuture("HTTP/2 does not support sending channel for now");

vertx-core/src/test/java/io/vertx/tests/http/HttpTest.java

@@ -2305,114 +2305,70 @@ private void testSendFileWithFailure(BiFunction<HttpServerResponse, File, Future
 
   @Test
   public void testSendFileWithFileChannel() throws Exception {
-    int expected = 16 * 1024 * 1024;
-    File file = TestUtils.tmpFile(".dat", expected);
-    FileChannel channel = FileChannel.open(file.toPath());
-    server.requestHandler(
-        req -> {
-          req.response().sendFile(channel).onFailure(
-            t -> {
-              if (HttpTest.this instanceof Http2Test) {
-                req.response().end();
-              }
-            }
-          );
-        });
-    startServer(testAddress);
-    Object[] res = {0, ""};
-    Object[] r = client.request(requestOptions)
-      .compose(req -> req.send()
-        .compose(resp -> {
-          resp.handler(buff -> {
-            Integer length = (Integer) res[0];
-            length += buff.length();
-            res[0] = length;
-          });
-          res[1] = resp.getHeader("Content-Type");
-          resp.exceptionHandler(this::fail);
-          return resp.end();
-        }))
-      .map(v -> res).await();
-    if (this instanceof Http1xTest){
-      assertEquals((int) r[0], file.length());
-      assertEquals("application/octet-stream", r[1]);
-    }
+    int fileLength = 16 * 1024 * 1024;
+    BiFunction<RandomAccessFile, HttpServerResponse, Future<?>> sender = (file, response) -> response.sendFile(file.getChannel());
+    testSendFileWithFileChannel(fileLength, sender, "application/octet-stream", fileLength);
   }
 
   @Test
   public void testSendFileWithFileChannelAndExtension() throws Exception {
-    int expected = 16 * 1024 * 1024;
-    File file = TestUtils.tmpFile(".dat", expected);
-    FileChannel channel = FileChannel.open(file.toPath());
-    server.requestHandler(
-        req -> {
-          req.response()
-            .putHeader(HttpHeaders.CONTENT_TYPE, "video/mp4")
-            .sendFile(channel).onFailure(
-            t -> {
-              if (HttpTest.this instanceof Http2Test) {
-                req.response().end();
-              }
-            }
-          );
-        });
-    startServer(testAddress);
-    Object[] res = {0, ""};
-    Object[] r = client.request(requestOptions)
-      .compose(req -> req.send()
-        .compose(resp -> {
-          resp.handler(buff -> {
-            Integer length = (Integer) res[0];
-            length += buff.length();
-            res[0] = length;
-          });
-          res[1] = resp.getHeader("Content-Type");
-          resp.exceptionHandler(this::fail);
-          return resp.end();
-        }))
-      .map(v -> res).await();
-    if (this instanceof Http1xTest){
-      assertEquals((int)r[0], file.length());
-      assertEquals(r[1], "video/mp4");
-    }
+    int fileLength = 16 * 1024 * 1024;
+    BiFunction<RandomAccessFile, HttpServerResponse, Future<?>> sender = (file, response) -> response
+      .putHeader(HttpHeaders.CONTENT_TYPE, "video/mp4")
+      .sendFile(file.getChannel());
+    testSendFileWithFileChannel(fileLength, sender, "video/mp4", fileLength);
   }
 
   @Test
   public void testSendFileWithFileChannelRange() throws Exception {
     int fileLength = 16 * 1024 * 1024;
-    File file = TestUtils.tmpFile(".dat", fileLength);
-    FileChannel channel = FileChannel.open(file.toPath());
     int offset = 1024 * 4;
     int expectedRange = fileLength - offset;
-    server.requestHandler(
-        req -> {
-          req.response().putHeader(HttpHeaders.CONTENT_TYPE, "video/mp4");
-          req.response().sendFile(channel, offset, expectedRange).onFailure(
-            t -> {
-              if (HttpTest.this instanceof Http2Test) {
-                req.response().end();
-              }
-            }
-          );
-        });
-    startServer(testAddress);
-    Object[] res = {0, ""};
-    Object[] r = client.request(requestOptions)
-      .compose(req -> req.send()
-        .compose(resp -> {
-          resp.handler(buff -> {
-            Integer length = (Integer) res[0];
-            length += buff.length();
-            res[0] = length;
-          });
-          res[1] = resp.getHeader("Content-Type");
-          resp.exceptionHandler(this::fail);
-          return resp.end();
-        }))
-      .map(v -> res).await();
-    if (this instanceof Http1xTest) {
-      assertEquals(expectedRange, (int) r[0]);
-      assertEquals("video/mp4", r[1]);
+    BiFunction<RandomAccessFile, HttpServerResponse, Future<?>> sender = (file, response) -> response
+      .putHeader(HttpHeaders.CONTENT_TYPE, "video/mp4")
+      .sendFile(file.getChannel(), offset, expectedRange);
+    testSendFileWithFileChannel(fileLength, sender, "video/mp4", expectedRange);
+  }
+
+  @Test
+  public void testSendFileWithRandomAccessFile() throws Exception {
+    int fileLength = 16 * 1024 * 1024;
+    BiFunction<RandomAccessFile, HttpServerResponse, Future<?>> sender = (file, response) -> response.sendFile(file);
+    testSendFileWithFileChannel(fileLength, sender, "application/octet-stream", fileLength);
+  }
+
+  @Test
+  public void testSendFileWithRandomAccessFileAndExtension() throws Exception {
+    int fileLength = 16 * 1024 * 1024;
+    BiFunction<RandomAccessFile, HttpServerResponse, Future<?>> sender = (file, response) -> response
+      .putHeader(HttpHeaders.CONTENT_TYPE, "video/mp4")
+      .sendFile(file);
+    testSendFileWithFileChannel(fileLength, sender, "video/mp4", fileLength);
+  }
+
+  @Test
+  public void testSendFileWithRandomAccessFileRange() throws Exception {
+    int fileLength = 16 * 1024 * 1024;
+    int offset = 1024 * 4;
+    int expectedRange = fileLength - offset;
+    BiFunction<RandomAccessFile, HttpServerResponse, Future<?>> sender = (file, response) -> response
+      .putHeader(HttpHeaders.CONTENT_TYPE, "video/mp4")
+      .sendFile(file, offset, expectedRange);
+    testSendFileWithFileChannel(fileLength, sender, "video/mp4", expectedRange);
+  }
+
+  private void testSendFileWithFileChannel(int flen, BiFunction<RandomAccessFile, HttpServerResponse, Future<?>> sender, String expectedContentType, long expectedLength) throws Exception {
+    Assume.assumeTrue(this instanceof Http1xTest);
+    File file = TestUtils.tmpFile(".dat", flen);
+    try (RandomAccessFile raf = new RandomAccessFile(file, "r")) {
+      server.requestHandler(req -> sender.apply(raf, req.response()).onComplete(onSuccess(v -> testComplete())));
+      startServer(testAddress);
+      Buffer body = client.request(requestOptions)
+        .compose(req -> req.send()
+          .expecting(HttpResponseExpectation.contentType(expectedContentType))
+          .compose(HttpClientResponse::body)).await();
+      assertEquals(body.length(), expectedLength);
+      await();
     }
   }