Refactor/remove custom MultipartFileSender to use Spring Boot Range requests

dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java

@@ -21,14 +21,15 @@
 import javax.ws.rs.core.Response;
 
 import org.apache.catalina.connector.ClientAbortException;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.Pair;
 import org.apache.logging.log4j.Logger;
 import org.dspace.app.rest.converter.ConverterService;
 import org.dspace.app.rest.exception.DSpaceBadRequestException;
 import org.dspace.app.rest.model.BitstreamRest;
 import org.dspace.app.rest.model.hateoas.BitstreamResource;
 import org.dspace.app.rest.utils.ContextUtil;
-import org.dspace.app.rest.utils.MultipartFileSender;
+import org.dspace.app.rest.utils.HttpHeadersInitializer;
 import org.dspace.app.rest.utils.Utils;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.Bitstream;
@@ -42,6 +43,8 @@
 import org.dspace.usage.UsageEvent;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.data.rest.webmvc.ResourceNotFoundException;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.ResponseEntity;
 import org.springframework.security.access.prepost.PostAuthorize;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.web.bind.annotation.PathVariable;
@@ -98,7 +101,7 @@ public class BitstreamRestController {
 
     @PreAuthorize("hasPermission(#uuid, 'BITSTREAM', 'READ')")
     @RequestMapping( method = {RequestMethod.GET, RequestMethod.HEAD}, value = "content")
-    public void retrieve(@PathVariable UUID uuid, HttpServletResponse response,
+    public ResponseEntity retrieve(@PathVariable UUID uuid, HttpServletResponse response,
                          HttpServletRequest request) throws IOException, SQLException, AuthorizeException {
 
 
@@ -107,7 +110,7 @@ public void retrieve(@PathVariable UUID uuid, HttpServletResponse response,
         Bitstream bit = bitstreamService.find(context, uuid);
         if (bit == null) {
             response.sendError(HttpServletResponse.SC_NOT_FOUND);
-            return;
+            return null;
         }
 
         Long lastModified = bitstreamService.getLastModified(bit);
@@ -117,9 +120,21 @@ public void retrieve(@PathVariable UUID uuid, HttpServletResponse response,
 
         Pair<InputStream, Long> bitstreamTuple = getBitstreamInputStreamAndSize(context, bit);
 
+        if (StringUtils.isBlank(request.getHeader("Range"))) {
+            //We only log a download request when serving a request without Range header. This is because
+            //a browser always sends a regular request first to check for Range support.
+            eventService.fireEvent(
+                new UsageEvent(
+                    UsageEvent.Action.VIEW,
+                    request,
+                    context,
+                    bit));
+        }
+
         // Pipe the bits
-        try (InputStream is = bitstreamTuple.getLeft()) {
-            MultipartFileSender sender = MultipartFileSender
+        InputStream is = bitstreamTuple.getLeft();
+        try {
+            HttpHeadersInitializer httpHeadersInitializer = HttpHeadersInitializer
                     .fromInputStream(is)
                     .withBufferSize(BUFFER_SIZE)
                     .withFileName(name)
@@ -130,39 +145,34 @@ public void retrieve(@PathVariable UUID uuid, HttpServletResponse response,
                     .with(response);
 
             if (lastModified != null) {
-                sender.withLastModified(lastModified);
+                httpHeadersInitializer.withLastModified(lastModified);
             }
 
             //Determine if we need to send the file as a download or if the browser can open it inline
             long dispositionThreshold = configurationService.getLongProperty("webui.content_disposition_threshold");
             if (dispositionThreshold >= 0 && bitstreamTuple.getRight() > dispositionThreshold) {
-                sender.withDisposition(MultipartFileSender.CONTENT_DISPOSITION_ATTACHMENT);
+                httpHeadersInitializer.withDisposition(HttpHeadersInitializer.CONTENT_DISPOSITION_ATTACHMENT);
             }
 
-            if (sender.isNoRangeRequest() && isNotAnErrorResponse(response)) {
-                //We only log a download request when serving a request without Range header. This is because
-                //a browser always sends a regular request first to check for Range support.
-                eventService.fireEvent(
-                        new UsageEvent(
-                                UsageEvent.Action.VIEW,
-                                request,
-                                context,
-                                bit));
-            }
+
+            org.dspace.app.rest.utils.BitstreamResource bitstreamResource =
+                new org.dspace.app.rest.utils.BitstreamResource(is, name, uuid, bit.getSizeBytes());
 
             //We have all the data we need, close the connection to the database so that it doesn't stay open during
             //download/streaming
             context.complete();
 
             //Send the data
-            if (sender.isValid()) {
-                sender.serveResource();
+            if (httpHeadersInitializer.isValid()) {
+                HttpHeaders httpHeaders = httpHeadersInitializer.initialiseHeaders();
+                return ResponseEntity.ok().headers(httpHeaders).body(bitstreamResource);
             }
 
         } catch (ClientAbortException ex) {
             log.debug("Client aborted the request before the download was completed. " +
                           "Client is probably switching to a Range request.", ex);
         }
+        return null;
     }
 
     private Pair<InputStream, Long> getBitstreamInputStreamAndSize(Context context, Bitstream bit)

dspace-server-webapp/src/main/java/org/dspace/app/rest/SitemapRestController.java

@@ -19,11 +19,14 @@
 import org.apache.catalina.connector.ClientAbortException;
 import org.apache.logging.log4j.Logger;
 import org.dspace.app.rest.utils.ContextUtil;
-import org.dspace.app.rest.utils.MultipartFileSender;
+import org.dspace.app.rest.utils.HttpHeadersInitializer;
 import org.dspace.core.Context;
 import org.dspace.services.ConfigurationService;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.core.io.FileSystemResource;
 import org.springframework.data.rest.webmvc.ResourceNotFoundException;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.ResponseEntity;
 import org.springframework.stereotype.Controller;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.PathVariable;
@@ -62,10 +65,11 @@ public class SitemapRestController {
      * @param request  the HTTP request
      * @throws SQLException if db error while completing DSpace context
      * @throws IOException  if IO error surrounding sitemap file
+     * @return
      */
     @GetMapping("/{name}")
-    public void retrieve(@PathVariable String name, HttpServletResponse response,
-        HttpServletRequest request) throws IOException, SQLException {
+    public ResponseEntity retrieve(@PathVariable String name, HttpServletResponse response,
+                                                       HttpServletRequest request) throws IOException, SQLException {
         // Find sitemap with given name in dspace/sitemaps
         File foundSitemapFile = null;
         File sitemapOutputDir = new File(configurationService.getProperty("sitemap.dir"));
@@ -94,7 +98,7 @@ public void retrieve(@PathVariable String name, HttpServletResponse response,
                 "Could not find sitemap file with name " + name + " in " + sitemapOutputDir.getAbsolutePath());
         } else {
             // return found sitemap file
-            this.returnSitemapFile(foundSitemapFile, response, request);
+            return this.returnSitemapFile(foundSitemapFile, response, request);
         }
     }
 
@@ -107,12 +111,13 @@ public void retrieve(@PathVariable String name, HttpServletResponse response,
      * @param request          the HTTP request
      * @throws SQLException if db error while completing DSpace context
      * @throws IOException  if IO error surrounding sitemap file
+     * @return
      */
-    private void returnSitemapFile(File foundSitemapFile, HttpServletResponse response, HttpServletRequest request)
-        throws SQLException, IOException {
+    private ResponseEntity returnSitemapFile(File foundSitemapFile, HttpServletResponse response,
+                                             HttpServletRequest request) throws SQLException, IOException {
         // Pipe the bits
         try (InputStream is = new FileInputStream(foundSitemapFile)) {
-            MultipartFileSender sender = MultipartFileSender
+            HttpHeadersInitializer sender = HttpHeadersInitializer
                 .fromInputStream(is)
                 .withBufferSize(BUFFER_SIZE)
                 .withFileName(foundSitemapFile.getName())
@@ -126,7 +131,7 @@ private void returnSitemapFile(File foundSitemapFile, HttpServletResponse respon
             // Determine if we need to send the file as a download or if the browser can open it inline
             long dispositionThreshold = configurationService.getLongProperty("webui.content_disposition_threshold");
             if (dispositionThreshold >= 0 && foundSitemapFile.length() > dispositionThreshold) {
-                sender.withDisposition(MultipartFileSender.CONTENT_DISPOSITION_ATTACHMENT);
+                sender.withDisposition(HttpHeadersInitializer.CONTENT_DISPOSITION_ATTACHMENT);
             }
 
             Context context = ContextUtil.obtainContext(request);
@@ -137,12 +142,15 @@ private void returnSitemapFile(File foundSitemapFile, HttpServletResponse respon
 
             // Send the data
             if (sender.isValid()) {
-                sender.serveResource();
+                HttpHeaders httpHeaders = sender.initialiseHeaders();
+                return ResponseEntity.ok().headers(httpHeaders).body(new FileSystemResource(foundSitemapFile));
+
             }
 
         } catch (ClientAbortException e) {
             log.debug("Client aborted the request before the download was completed. " +
                       "Client is probably switching to a Range request.", e);
         }
+        return null;
     }
 }

dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/BitstreamResource.java

@@ -0,0 +1,55 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+package org.dspace.app.rest.utils;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.UUID;
+
+import org.springframework.core.io.AbstractResource;
+
+/**
+ * This class acts as a {@link AbstractResource} used by Spring's framework to send the data in a proper and
+ * streamlined way inside the {@link org.springframework.http.ResponseEntity} body.
+ * This class' attributes are being used by Spring's framework in the overridden methods so that the proper
+ * attributes are given and used in the response.
+ */
+public class BitstreamResource extends AbstractResource {
+
+    private InputStream inputStream;
+    private String name;
+    private UUID uuid;
+    private long sizeBytes;
+
+    public BitstreamResource(InputStream inputStream, String name, UUID uuid, long sizeBytes) {
+        this.inputStream = inputStream;
+        this.name = name;
+        this.uuid = uuid;
+        this.sizeBytes = sizeBytes;
+    }
+
+    @Override
+    public String getDescription() {
+        return "bitstream [" + uuid + "]";
+    }
+
+    @Override
+    public InputStream getInputStream() throws IOException {
+        return inputStream;
+    }
+
+    @Override
+    public String getFilename() {
+        return name;
+    }
+
+    @Override
+    public long contentLength() throws IOException {
+        return sizeBytes;
+    }
+}