Merge fix for Zope Collector issue #530 from 2.5 branch; do not optimize

byte ranges because Acrobat Reader for Windows chokes when overlapping
ranges have been merged.

lib/python/OFS/Image.py

@@ -12,7 +12,7 @@
 ##############################################################################
 """Image object"""
 
-__version__='$Revision: 1.140 $'[11:-2]
+__version__='$Revision: 1.141 $'[11:-2]
 
 import Globals, struct
 from OFS.content_types import guess_content_type
@@ -227,9 +227,8 @@ class File(Persistent, Implicit, PropertyManager,
                     RESPONSE.setStatus(416)
                     return ''
 
-                # Can we optimize?
-                ranges = HTTPRangeSupport.optimizeRanges(ranges, self.size)
-
+                ranges = HTTPRangeSupport.expandRanges(ranges, self.size)
+                                
                 if len(ranges) == 1:
                     # Easy case, set extra header and return partial set.
                     start, end = ranges[0]
@@ -275,9 +274,6 @@ class File(Persistent, Implicit, PropertyManager,
                     return ''
 
                 else:
-                    # When we get here, ranges have been optimized, so they are
-                    # in order, non-overlapping, and start and end values are
-                    # positive integers.
                     boundary = choose_boundary()
 
                     # Calculate the content length
@@ -304,8 +300,11 @@ class File(Persistent, Implicit, PropertyManager,
                             draftprefix, boundary))
                     RESPONSE.setStatus(206) # Partial content
 
-                    pos = 0
                     data = self.data
+                    # The Pdata map allows us to jump into the Pdata chain
+                    # arbitrarily during out-of-order range searching.
+                    pdata_map = {}
+                    pdata_map[0] = data
 
                     for start, end in ranges:
                         RESPONSE.write('\r\n--%s\r\n' % boundary)
@@ -319,7 +318,14 @@ class File(Persistent, Implicit, PropertyManager,
                             RESPONSE.write(data[start:end])
 
                         else:
-                            # Yippee. Linked Pdata objects.
+                            # Yippee. Linked Pdata objects. The following
+                            # calculations allow us to fast-forward through the
+                            # Pdata chain without a lot of dereferencing if we
+                            # did the work already.
+                            closest_pos = start - (start % (1<<16))
+                            pos = min(closest_pos, max(pdata_map.keys()))
+                            data = pdata_map[pos]
+
                             while data is not None:
                                 l = len(data.data)
                                 pos = pos + l
@@ -335,16 +341,18 @@ class File(Persistent, Implicit, PropertyManager,
 
                                         # Send and loop to next range
                                         RESPONSE.write(data[lstart:lend])
-                                        # Back up the position marker, it will
-                                        # be incremented again for the next
-                                        # part.
-                                        pos = pos - l
                                         break
 
                                     # Not yet at the end, transmit what we have.
                                     RESPONSE.write(data[lstart:])
 
                                 data = data.next
+                                # Store a reference to a Pdata chain link so we
+                                # don't have to deref during this request again.
+                                pdata_map[pos] = data
+
+                    # Do not keep the link references around.
+                    del pdata_map
 
                     RESPONSE.write('\r\n--%s--\r\n' % boundary)
                     return ''

lib/python/OFS/tests/testRanges.py

@@ -303,7 +303,7 @@ class TestRequestRange(unittest.TestCase):
 
     # Multiple ranges
     def testAdjacentRanges(self):
-        self.expectMultipleRanges('21-25,10-20', [(10, 21), (21, 26)])
+        self.expectMultipleRanges('21-25,10-20', [(21, 26), (10, 21)])
 
     def testMultipleRanges(self):
         self.expectMultipleRanges('3-7,10-15', [(3, 8), (10, 16)])
@@ -314,7 +314,13 @@ class TestRequestRange(unittest.TestCase):
     def testMultipleRangesBigFile(self):
         self.uploadBigFile()
         self.expectMultipleRanges('3-700,10-15,-10000',
-            [(3, 701), (len(self.data) - 10000, len(self.data))])
+            [(3, 701), (10, 16), (len(self.data) - 10000, len(self.data))])
+
+    def testMultipleRangesBigFileOutOfOrder(self):
+        self.uploadBigFile()
+        self.expectMultipleRanges('10-15,-10000,70000-80000', 
+            [(10, 16), (len(self.data) - 10000, len(self.data)),
+             (70000, 80001)])
 
     def testMultipleRangesBigFileEndOverflow(self):
         self.uploadBigFile()
@@ -327,10 +333,10 @@ class TestRequestRange(unittest.TestCase):
     def testIllegalIfRange(self):
         # We assume that an illegal if-range is to be ignored, just like an
         # illegal if-modified since.
-        self.expectSingleRange('21-25,10-21', 10, 26, if_range='garbage')
+        self.expectSingleRange('10-25', 10, 26, if_range='garbage')
 
     def testEqualIfRangeDate(self):
-        self.expectSingleRange('21-25,10-21', 10, 26,
+        self.expectSingleRange('10-25', 10, 26,
             if_range=self.createLastModifiedDate())
 
     def testIsModifiedIfRangeDate(self):
@@ -338,15 +344,15 @@ class TestRequestRange(unittest.TestCase):
             if_range=self.createLastModifiedDate(offset=-100))
 
     def testIsNotModifiedIfRangeDate(self):
-        self.expectSingleRange('21-25,10-21', 10, 26,
+        self.expectSingleRange('10-25', 10, 26,
             if_range=self.createLastModifiedDate(offset=100))
 
     def testEqualIfRangeEtag(self):
-        self.expectSingleRange('21-25,10-21', 10, 26,
+        self.expectSingleRange('10-25', 10, 26,
             if_range=self.file.http__etag())
 
     def testNotEqualIfRangeEtag(self):
-        self.expectOK('21-25,10-20',
+        self.expectOK('10-25',
             if_range=self.file.http__etag() + 'bar')
 
 

lib/python/ZPublisher/HTTPRangeSupport.py

@@ -19,7 +19,7 @@ flag-interface and some support functions for implementing this functionality.
 For an implementation example, see the File class in OFS/Image.py.
 """
 
-__version__='$Revision: 1.7 $'[11:-2]
+__version__='$Revision: 1.8 $'[11:-2]
 
 import re, sys
 import Interface
@@ -96,12 +96,10 @@ def parseRange(header):
 
     return ranges
 
-def optimizeRanges(ranges, size):
-    """Optimize Range sets, given those sets and the length of the resource.
+def expandRanges(ranges, size):
+    """Expand Range sets, given those sets and the length of the resource.
 
-    Optimisation is done by first expanding relative start values and open
-    ends, then sorting and combining overlapping ranges. We also remove
-    unsatisfiable ranges (where the start lies beyond the size of the resource).
+    Expansion means relative start values and open ends
 
     """
 
@@ -116,31 +114,7 @@ def optimizeRanges(ranges, size):
         if start < size:
             add((start, end))
 
-    ranges = expanded
-    ranges.sort()
-    ranges.reverse()
-    optimized = []
-    add = optimized.append
-    start, end = ranges.pop()
-
-    while ranges:
-        nextstart, nextend = ranges.pop()
-        # If the next range overlaps
-        if nextstart < end:
-            # If it falls within the current range, discard
-            if nextend <= end:
-                continue
-
-            # Overlap, adjust end
-            end = nextend
-        else:
-            add((start, end))
-            start, end = nextstart, nextend
-
-    # Add the remaining optimized range
-    add((start, end))
-
-    return optimized
+    return expanded
 
 class HTTPRangeInterface(Interface.Base):
     """Objects implementing this Interface support the HTTP Range header.

lib/python/ZPublisher/tests/testHTTPRangeSupport.py

@@ -12,7 +12,7 @@
 ##############################################################################
 
 import sys
-from ZPublisher.HTTPRangeSupport import parseRange, optimizeRanges
+from ZPublisher.HTTPRangeSupport import parseRange, expandRanges
 
 import unittest
 
@@ -77,10 +77,10 @@ class TestRangeHeaderParse(unittest.TestCase):
         self.expectSets('bytes=-0', [(sys.maxint, None)])
 
 
-class TestOptimizeRanges(unittest.TestCase):
+class TestExpandRanges(unittest.TestCase):
 
     def expectSets(self, sets, size, expect):
-        result = optimizeRanges(sets, size)
+        result = expandRanges(sets, size)
         self.failUnless(result == expect,
             'Expected %s, got %s' % (`expect`, `result`))
 
@@ -96,26 +96,26 @@ class TestOptimizeRanges(unittest.TestCase):
 
     def testNoOverlapOutOfOrder(self):
         self.expectSets([(1000, 2000), (3000, None), (1, 5)], 5000,
-            [(1, 5), (1000, 2000), (3000, 5000)])
+            [(1000, 2000), (3000, 5000), (1, 5)])
 
     def testOverlapInOrder(self):
         self.expectSets([(1, 10), (8, 20), (25, None)], 5000,
-            [(1, 20), (25, 5000)])
+            [(1, 10), (8, 20), (25, 5000)])
 
     def testOverlapOutOfOrder(self):
         self.expectSets([(25, 50), (8, None), (1, 10)], 5000,
-            [(1, 5000)])
+            [(25, 50), (8, 5000), (1, 10)])
 
     def testAdjacentInOrder(self):
         self.expectSets([(1, 10), (10, 20), (25, 50)], 5000,
             [(1, 10), (10, 20), (25, 50)])
 
     def testAdjacentOutOfOrder(self):
-        self.expectSets([(-5, None), (40, 45)], 50, [(40, 45), (45, 50)])
+        self.expectSets([(-5, None), (40, 45)], 50, [(45, 50), (40, 45)])
 
-    def testOverLapAndOverflow(self):
+    def testOverlapAndOverflow(self):
         # Note that one endpoint lies beyond the end.
-        self.expectSets([(-5, None), (40, 100)], 50, [(40, 50)])
+        self.expectSets([(-5, None), (40, 100)], 50, [(45, 50), (40, 50)])
 
     def testRemoveUnsatisfiable(self):
         self.expectSets([(sys.maxint, None), (10, 20)], 50, [(10, 20)])
@@ -124,7 +124,7 @@ class TestOptimizeRanges(unittest.TestCase):
 def test_suite():
     suite = unittest.TestSuite()
     suite.addTest(unittest.makeSuite(TestRangeHeaderParse, 'test'))
-    suite.addTest(unittest.makeSuite(TestOptimizeRanges, 'test'))
+    suite.addTest(unittest.makeSuite(TestExpandRanges, 'test'))
     return suite
 
 def main():