Range content retrieval with javax.jcr.Binary to improve performance.

Resolves: https://www.pivotaltracker.com/s/projects/684825/stories/71429770
fcrepo · May 27, 2014 · 4da6bb1 · 4da6bb1
1 parent 0973828
commit 4da6bb1
Show file tree

Hide file tree

Showing 5 changed files with 87 additions and 7 deletions.
diff --git a/fcrepo-http-api/src/main/java/org/fcrepo/http/api/ContentExposingResource.java b/fcrepo-http-api/src/main/java/org/fcrepo/http/api/ContentExposingResource.java
@@ -22,13 +22,15 @@
 import org.fcrepo.http.commons.responses.RangeRequestInputStream;
 import org.fcrepo.kernel.Datastream;
 
+import javax.jcr.Binary;
 import javax.jcr.RepositoryException;
 import javax.servlet.http.HttpServletResponse;
 import javax.ws.rs.WebApplicationException;
 import javax.ws.rs.core.CacheControl;
 import javax.ws.rs.core.EntityTag;
 import javax.ws.rs.core.Request;
 import javax.ws.rs.core.Response;
+
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.Date;
@@ -49,6 +51,8 @@ public abstract class ContentExposingResource extends AbstractResource {
 
     protected static final int PARTIAL_CONTENT = 206;
 
+    private static long MAX_BUFFER_SIZE = 10240000;
+
     /**
      * A helper method that does most of the work associated with processing a request
      * for content (or a range of the content) into a Response
@@ -67,8 +71,6 @@ protected Response getDatastreamContentResponse(final Datastream ds, final Strin
         cc.setMustRevalidate(true);
         Response.ResponseBuilder builder;
 
-        final InputStream content = ds.getContent();
-
         if (rangeValue != null && rangeValue.startsWith("bytes")) {
 
             final Range range = Range.convert(rangeValue);
@@ -92,14 +94,36 @@ protected Response getDatastreamContentResponse(final Datastream ds, final Strin
                 builder = status(REQUESTED_RANGE_NOT_SATISFIABLE)
                         .header("Content-Range", contentRangeValue);
             } else {
-                final RangeRequestInputStream rangeInputStream =
-                    new RangeRequestInputStream(content, range.start(), range.size());
-
-                builder = status(PARTIAL_CONTENT).entity(rangeInputStream)
-                        .header("Content-Range", contentRangeValue);
+                final long maxBufferSize = MAX_BUFFER_SIZE; // 10MB max buffer size?
+                final long rangeSize = range.size();
+                final long rangeStart = range.start();
+                final long remainingBytes = contentSize - rangeStart;
+                final long bufSize = rangeSize < remainingBytes ? rangeSize : remainingBytes;
+
+                if (bufSize < maxBufferSize) {
+                    // Small size range content retrieval use javax.jcr.Binary to improve performance
+                    final byte[] buf = new byte[(int) bufSize];
+
+                    final Binary binaryContent = ds.getBinaryContent();
+                    binaryContent.read(buf, rangeStart);
+                    binaryContent.dispose();
+
+                    builder = status(PARTIAL_CONTENT).entity(buf)
+                            .header("Content-Range", contentRangeValue);
+                } else {
+                    // For large range content retrieval, go with the InputStream class to balance
+                    // the memory usage, though this is a rare case in range content retrieval.
+                    final InputStream content = ds.getContent();
+                    final RangeRequestInputStream rangeInputStream =
+                            new RangeRequestInputStream(content, range.start(), range.size());
+
+                        builder = status(PARTIAL_CONTENT).entity(rangeInputStream)
+                                .header("Content-Range", contentRangeValue);
+                }
             }
 
         } else {
+            final InputStream content = ds.getContent();
             builder = ok(content);
         }
 
@@ -177,4 +201,12 @@ protected static void addCacheControlHeaders(final HttpServletResponse servletRe
         }
     }
 
+    /**
+     * Setter that set the max buffer size for range content retrieval,
+     * which could help for unit testing.
+     * @param maxBufferSize
+     */
+    public static void setMaxBufferSize(final long maxBufferSize) {
+        MAX_BUFFER_SIZE = maxBufferSize;
+    }
 }
diff --git a/fcrepo-http-api/src/test/java/org/fcrepo/integration/http/api/FedoraContentIT.java b/fcrepo-http-api/src/test/java/org/fcrepo/integration/http/api/FedoraContentIT.java
@@ -31,6 +31,7 @@
 import org.apache.http.client.methods.HttpPut;
 import org.apache.http.entity.StringEntity;
 import org.apache.http.util.EntityUtils;
+import org.fcrepo.http.api.ContentExposingResource;
 import org.junit.Test;
 
 import com.hp.hpl.jena.graph.Node;
@@ -252,6 +253,26 @@ public void testRangeRequest() throws Exception {
 
     }
 
+    @Test
+    public void testRangeRequestWithSkipBytes() throws Exception {
+        ContentExposingResource.setMaxBufferSize(10);
+        final String pid = getRandomUniquePid();
+        createObject(pid);
+        final HttpPost createDSMethod = postDSMethod(pid, "ds1", "large marbles for everyone");
+        assertEquals(201, getStatus(createDSMethod));
+
+        final HttpGet method_test_get = new HttpGet(serverAddress + pid + "/ds1/fcr:content");
+        method_test_get.setHeader("Range", "bytes=1-21");
+        assertEquals(206, getStatus(method_test_get));
+
+        final HttpResponse response = client.execute(method_test_get);
+        logger.debug("Returned from HTTP GET, now checking content...");
+        assertEquals("Got the wrong content back!", "arge marbles for ever",
+                EntityUtils.toString(response.getEntity()));
+        assertEquals("bytes 1-21/26", response.getFirstHeader("Content-Range").getValue());
+
+    }
+
     @Test
     public void testRangeRequestBadRange() throws Exception {
         final String pid = getRandomUniquePid();

diff --git a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/Datastream.java b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/Datastream.java
@@ -18,6 +18,7 @@
 import java.io.InputStream;
 import java.net.URI;
 
+import javax.jcr.Binary;
 import javax.jcr.Node;
 import javax.jcr.RepositoryException;
 
@@ -36,6 +37,12 @@ public interface Datastream extends FedoraResource {
      */
     InputStream getContent() throws RepositoryException;
 
+    /**
+     * @return The Binary content associated with this datastream.
+     * @throws RepositoryException
+     */
+    Binary getBinaryContent() throws RepositoryException;
+
     /**
      * @return The Node of content associated with this datastream.
      * @throws RepositoryException

diff --git a/fcrepo-kernel/src/main/java/org/fcrepo/kernel/DatastreamImpl.java b/fcrepo-kernel/src/main/java/org/fcrepo/kernel/DatastreamImpl.java
@@ -123,6 +123,14 @@ public InputStream getContent() throws RepositoryException {
         return getContentNode().getProperty(JCR_DATA).getBinary().getStream();
     }
 
+    /*
+     * (non-Javadoc)
+     * @see org.fcrepo.kernel.Datastream#getBinaryContent()
+     */
+    @Override
+    public javax.jcr.Binary getBinaryContent() throws RepositoryException {
+        return getContentNode().getProperty(JCR_DATA).getBinary();
+    }
 
     /*
      * (non-Javadoc)

diff --git a/fcrepo-kernel/src/test/java/org/fcrepo/kernel/DatastreamImplTest.java b/fcrepo-kernel/src/test/java/org/fcrepo/kernel/DatastreamImplTest.java
@@ -38,6 +38,7 @@
 import java.util.Calendar;
 import java.util.Date;
 
+import javax.jcr.Binary;
 import javax.jcr.Node;
 import javax.jcr.Property;
 import javax.jcr.PropertyIterator;
@@ -119,6 +120,17 @@ public void testGetContent() throws RepositoryException, IOException {
         verify(mockContent).getProperty(JCR_DATA);
     }
 
+    @Test
+    public void testGetBinaryContent() throws RepositoryException, IOException {
+        final Binary mockBinary = mock(Binary.class);
+        final Property mockProperty = mock(Property.class);
+        final Node mockContent = getContentNodeMock("abc");
+        when(mockDsNode.getNode(JCR_CONTENT)).thenReturn(mockContent);
+        when(mockContent.getProperty(JCR_DATA)).thenReturn(mockProperty);
+        when(mockProperty.getBinary()).thenReturn(mockBinary);
+        assertEquals(mockBinary, testObj.getBinaryContent());
+    }
+
     @Test
     public void testSetContent() throws RepositoryException,
             InvalidChecksumException {