diff --git a/dspace-api/src/main/java/org/dspace/content/BitstreamServiceImpl.java b/dspace-api/src/main/java/org/dspace/content/BitstreamServiceImpl.java index 7eb3b5021c..0e70422303 100644 --- a/dspace-api/src/main/java/org/dspace/content/BitstreamServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/BitstreamServiceImpl.java @@ -452,7 +452,7 @@ public class BitstreamServiceImpl extends DSpaceObjectServiceImpl imp return bitstreamDAO.getNotReferencedBitstreams(context); } - public String getLastModified(Bitstream bitstream) { + public Long getLastModified(Bitstream bitstream) { return bitstreamStorageService.getLastModified(bitstream); } } diff --git a/dspace-api/src/main/java/org/dspace/content/service/BitstreamService.java b/dspace-api/src/main/java/org/dspace/content/service/BitstreamService.java index ac5f24278c..7799c48a8a 100644 --- a/dspace-api/src/main/java/org/dspace/content/service/BitstreamService.java +++ b/dspace-api/src/main/java/org/dspace/content/service/BitstreamService.java @@ -203,5 +203,5 @@ public interface BitstreamService extends DSpaceObjectService, DSpace List getNotReferencedBitstreams(Context context) throws SQLException; - public String getLastModified(Bitstream bitstream); + public Long getLastModified(Bitstream bitstream); } diff --git a/dspace-api/src/main/java/org/dspace/storage/bitstore/BitstreamStorageServiceImpl.java b/dspace-api/src/main/java/org/dspace/storage/bitstore/BitstreamStorageServiceImpl.java index 172c9674a7..bc490de36d 100644 --- a/dspace-api/src/main/java/org/dspace/storage/bitstore/BitstreamStorageServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/storage/bitstore/BitstreamStorageServiceImpl.java @@ -339,7 +339,7 @@ public class BitstreamStorageServiceImpl implements BitstreamStorageService, Ini } } - public String getLastModified(Bitstream bitstream) { + public Long getLastModified(Bitstream bitstream) { Map wantedMetadata = new HashMap(); wantedMetadata.put("modified", null); try { @@ -347,7 +347,7 @@ public class BitstreamStorageServiceImpl implements BitstreamStorageService, Ini } catch (IOException e) { log.error(e); } - return wantedMetadata.get("modified").toString(); + return Long.valueOf(wantedMetadata.get("modified").toString()); } /** diff --git a/dspace-api/src/main/java/org/dspace/storage/bitstore/service/BitstreamStorageService.java b/dspace-api/src/main/java/org/dspace/storage/bitstore/service/BitstreamStorageService.java index 8cc7d075bc..2a16e00e8c 100644 --- a/dspace-api/src/main/java/org/dspace/storage/bitstore/service/BitstreamStorageService.java +++ b/dspace-api/src/main/java/org/dspace/storage/bitstore/service/BitstreamStorageService.java @@ -7,16 +7,16 @@ */ package org.dspace.storage.bitstore.service; -import org.dspace.authorize.AuthorizeException; -import org.dspace.content.Bitstream; -import org.dspace.core.Context; - import java.io.IOException; import java.io.InputStream; import java.sql.SQLException; import java.util.Map; import java.util.UUID; +import org.dspace.authorize.AuthorizeException; +import org.dspace.content.Bitstream; +import org.dspace.core.Context; + /** *

* Stores, retrieves and deletes bitstreams. @@ -191,6 +191,11 @@ public interface BitstreamStorageService { public void migrate(Context context, Integer assetstoreSource, Integer assetstoreDestination, boolean deleteOld, Integer batchCommitSize) throws IOException, SQLException, AuthorizeException; - public String getLastModified(Bitstream bitstream); + /** + * Get the last modified timestamp of the file linked to the given bitstream + * @param bitstream The bitstream for which to get the last modified timestamp + * @return The last modified timestamp in milliseconds + */ + public Long getLastModified(Bitstream bitstream); } diff --git a/dspace-spring-rest/src/main/java/org/dspace/app/rest/BitstreamContentRestController.java b/dspace-spring-rest/src/main/java/org/dspace/app/rest/BitstreamContentRestController.java index e002634e87..87bb36d295 100644 --- a/dspace-spring-rest/src/main/java/org/dspace/app/rest/BitstreamContentRestController.java +++ b/dspace-spring-rest/src/main/java/org/dspace/app/rest/BitstreamContentRestController.java @@ -9,15 +9,19 @@ package org.dspace.app.rest; import java.io.IOException; import java.io.InputStream; +import java.sql.SQLException; import java.util.UUID; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.apache.commons.io.IOUtils; +import org.apache.log4j.Logger; import org.dspace.app.rest.model.BitstreamRest; -import org.dspace.app.rest.repository.BitstreamRestRepository; +import org.dspace.app.rest.utils.ContextUtil; import org.dspace.app.rest.utils.MultipartFileSender; +import org.dspace.content.Bitstream; +import org.dspace.content.service.BitstreamService; +import org.dspace.core.Context; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestMapping; @@ -26,47 +30,54 @@ import org.springframework.web.bind.annotation.RestController; /** * This is a specialized controller to provide access to the bitstream binary content - * - * @author Andrea Bollini (andrea.bollini at 4science.it) * */ @RestController @RequestMapping("/api/"+BitstreamRest.CATEGORY +"/"+ BitstreamRest.PLURAL_NAME + "/{uuid:[0-9a-fxA-FX]{8}-[0-9a-fxA-FX]{4}-[0-9a-fxA-FX]{4}-[0-9a-fxA-FX]{4}-[0-9a-fxA-FX]{12}}/content") public class BitstreamContentRestController { + + private static final Logger log = Logger.getLogger(BitstreamContentRestController.class); + @Autowired - private BitstreamRestRepository bitstreamRestRepository; + private BitstreamService bitstreamService; @RequestMapping(method = {RequestMethod.GET, RequestMethod.HEAD}) public void retrieve(@PathVariable UUID uuid, HttpServletResponse response, - HttpServletRequest request) throws IOException { - BitstreamRest bit = bitstreamRestRepository.findOne(uuid); + HttpServletRequest request) throws IOException, SQLException { + + Context context = ContextUtil.obtainContext(request); + + Bitstream bit = bitstreamService.find(context, uuid); if (bit == null) { response.sendError(HttpServletResponse.SC_NOT_FOUND); return; } - // Pipe the bits - InputStream is = bitstreamRestRepository.retrieve(uuid); - - long lastModified = bitstreamRestRepository.getLastModified(uuid); - - String mimetype = bit.getFormat().getMimetype(); + Long lastModified = bitstreamService.getLastModified(bit); + String mimetype = bit.getFormat(context).getMIMEType(); //TODO LOG DOWNLOAD if no range or if last chunk + // Pipe the bits + try(InputStream is = bitstreamService.retrieve(context, bit)) { + MultipartFileSender + .fromInputStream(is) + .withFileName(bit.getName()) + .withLength(bit.getSize()) + .withChecksum(bit.getChecksum()) + .withMimetype(mimetype) + .withLastModified(lastModified) + .with(request) + .with(response) + + .serveResource(); - //MultipartFileSender - try { - MultipartFileSender.fromBitstream(bit).with(request).with(response).withInputStream(is).withMimetype(mimetype).withLastModified(lastModified).serveResource(); } catch (Exception e) { - e.printStackTrace(); - } finally { - IOUtils.closeQuietly(is); + log.error(e.getMessage(), e); + response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, e.getMessage()); } - - } diff --git a/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/BitstreamRestRepository.java b/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/BitstreamRestRepository.java index 810444e33c..dc97f11de7 100644 --- a/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/BitstreamRestRepository.java +++ b/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/BitstreamRestRepository.java @@ -7,6 +7,14 @@ */ package org.dspace.app.rest.repository; +import java.io.IOException; +import java.io.InputStream; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import java.util.UUID; + import org.dspace.app.rest.converter.BitstreamConverter; import org.dspace.app.rest.model.BitstreamRest; import org.dspace.app.rest.model.hateoas.BitstreamResource; @@ -20,14 +28,6 @@ import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.Pageable; import org.springframework.stereotype.Component; -import java.io.IOException; -import java.io.InputStream; -import java.sql.SQLException; -import java.util.ArrayList; -import java.util.Iterator; -import java.util.List; -import java.util.UUID; - /** * This is the repository responsible to manage Bitstream Rest object * @@ -111,14 +111,4 @@ public class BitstreamRestRepository extends DSpaceRestRepository lastModified) { - log.error("If-Modified-Since header should be greater than LastModified. If so, then return 304."); - response.setHeader(ETAG, bitstream.getCheckSum().getValue()); // Required in 304. + log.debug("If-Modified-Since header should be greater than LastModified. If so, then return 304."); + response.setHeader(ETAG, checksum); // Required in 304. response.sendError(HttpServletResponse.SC_NOT_MODIFIED); return; } @@ -201,18 +207,26 @@ public class MultipartFileSender { String ifRange = request.getHeader(IF_RANGE); if (nonNull(ifRange) && !ifRange.equals(fileName)) { try { + //Assume that the If-Range contains a date long ifRangeTime = request.getDateHeader(IF_RANGE); // Throws IAE if invalid. - if (ifRangeTime != -1) { + + if (ifRangeTime == -1 || ifUnmodifiedSince + 1000 <= lastModified) { + //Our file has been updated, send the full range ranges.add(full); } + } catch (IllegalArgumentException ignore) { - ranges.add(full); + //Assume that the If-Range contains an ETag + if (!matches(ifRange, checksum)) { + //Our file has been updated, send the full range + ranges.add(full); + } } } // If any valid If-Range header, then process each part of byte range. if (ranges.isEmpty()) { - log.info("If any valid If-Range header, then process each part of byte range."); + log.debug("If any valid If-Range header, then process each part of byte range."); for (String part : range.substring(6).split(",")) { // Assuming a file with length of 100, the following examples returns bytes at: // 50-80 (50 to 80), 40- (40 to length=100), -20 (length-20=80 to length=100). @@ -228,7 +242,7 @@ public class MultipartFileSender { // Check if Range is syntactically valid. If not, then return 416. if (start > end) { - log.info("Check if Range is syntactically valid. If not, then return 416."); + log.warn("Check if Range is syntactically valid. If not, then return 416."); response.setHeader(CONTENT_RANGE, String.format(BYTES_DINVALID_BYTE_RANGE_FORMAT, length)); // Required in 416. response.sendError(HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE); return; @@ -246,7 +260,7 @@ public class MultipartFileSender { response.setBufferSize(DEFAULT_BUFFER_SIZE); response.setHeader(CONTENT_TYPE, contentType); response.setHeader(ACCEPT_RANGES, BYTES); - response.setHeader(ETAG, bitstream.getCheckSum().getValue()); + response.setHeader(ETAG, checksum); response.setDateHeader(LAST_MODIFIED, lastModified); response.setDateHeader(EXPIRES, System.currentTimeMillis() + DEFAULT_EXPIRE_TIME); @@ -266,7 +280,6 @@ public class MultipartFileSender { log.debug("Content-Disposition : {}", disposition); } - // Content phase if (METHOD_HEAD.equals(request.getMethod())) { log.debug("HEAD request - skipping content"); @@ -275,31 +288,30 @@ public class MultipartFileSender { // Send requested file (part(s)) to client ------------------------------------------------ // Prepare streams. - try (InputStream input = inputStream; - OutputStream output = response.getOutputStream()) { + try (OutputStream output = response.getOutputStream()) { if (ranges.isEmpty() || ranges.get(0) == full) { // Return full file. - log.info("Return full file"); + log.debug("Return full file"); response.setContentType(contentType); response.setHeader(CONTENT_RANGE, String.format(BYTES_RANGE_FORMAT, full.start, full.end, full.total)); response.setHeader(CONTENT_LENGTH, String.valueOf(full.length)); - Range.copy(input, output, length, full.start, full.length); + Range.copy(inputStream, output, length, full.start, full.length); } else if (ranges.size() == 1) { // Return single part of file. Range r = ranges.get(0); - log.info("Return 1 part of file : from ({}) to ({})", r.start, r.end); + log.debug("Return 1 part of file : from ({}) to ({})", r.start, r.end); response.setContentType(contentType); response.setHeader(CONTENT_RANGE, String.format(BYTES_RANGE_FORMAT, r.start, r.end, r.total)); response.setHeader(CONTENT_LENGTH, String.valueOf(r.length)); response.setStatus(HttpServletResponse.SC_PARTIAL_CONTENT); // 206. // Copy single part range. - Range.copy(input, output, length, r.start, r.length); + Range.copy(inputStream, output, length, r.start, r.length); } else { @@ -312,7 +324,7 @@ public class MultipartFileSender { // Copy multi part range. for (Range r : Range.relativize(ranges)) { - log.info("Return multi part of file : from ({}) to ({})", r.start, r.end); + log.debug("Return multi part of file : from ({}) to ({})", r.start, r.end); // Add multipart boundary and header fields for every range. sos.println(); sos.println("--" + MULTIPART_BOUNDARY); @@ -320,7 +332,7 @@ public class MultipartFileSender { sos.println(CONTENT_RANGE + ": " + String.format(BYTES_RANGE_FORMAT, r.start, r.end, r.total)); // Copy single part range of multi part range. - Range.copy(input, output, length, r.start, r.length); + Range.copy(inputStream, output, length, r.start, r.length); } // End with multipart boundary. @@ -352,12 +364,8 @@ public class MultipartFileSender { */ public Range(long start, long end, long total) { this.start = start; -// if (end <= start + DEFAULT_BUFFER_SIZE) { - this.end = end; -// } else { -// this.end = Math.min(start + DEFAULT_BUFFER_SIZE, total - 1); -// } - this.length = this.end - start + 1; + this.end = Math.min(end, Math.min(start + DEFAULT_BUFFER_SIZE, total - 1)); + this.length = this.end - this.start + 1; this.total = total; }