mutable/retrieve: handle the case where self._read_length is 0.

Note that the downloader will still fetch a segment for a zero-length
read, which is wasteful. Fixing that isn't specifically required to fix
#1512, but it should probably be fixed before 1.9.

diff --git a/src/allmydata/mutable/retrieve.py b/src/allmydata/mutable/retrieve.py
index 7498a134dd014ddb243e06236905d4668382c4b4..f6c99d0ff10224fb1b956bc7791aca5fc9bf7d3a 100644 (file)
--- a/src/allmydata/mutable/retrieve.py
+++ b/src/allmydata/mutable/retrieve.py
@@ -404,18 +404,24 @@ class Retrieve:
             self._start_segment = 0
 
 
-        if self._read_length:
+        # If self._read_length is None, then we want to read the whole
+        # file. Otherwise, we want to read only part of the file, and
+        # need to figure out where to stop reading.
+        if self._read_length is not None:
             # our end segment is the last segment containing part of the
             # segment that we were asked to read.
             self.log("got read length %d" % self._read_length)
-            end_data = self._offset + self._read_length
+            if self._read_length != 0:
+                end_data = self._offset + self._read_length
 
-            # We don't actually need to read the byte at end_data, but
-            # the one before it.
-            end = (end_data - 1) // self._segment_size
+                # We don't actually need to read the byte at end_data,
+                # but the one before it.
+                end = (end_data - 1) // self._segment_size
 
-            assert end < self._num_segments
-            self._last_segment = end
+                assert end < self._num_segments
+                self._last_segment = end
+            else:
+                self._last_segment = self._start_segment
             self.log("got end segment: %d" % self._last_segment)
         else:
             self._last_segment = self._num_segments - 1

diff --git a/src/allmydata/test/test_mutable.py b/src/allmydata/test/test_mutable.py
index 147f7de5adf383224ff525d0a566e8df5b66228f..95a21354203bbc1c614b10a8288bf37b9a085adb 100644 (file)
--- a/src/allmydata/test/test_mutable.py
+++ b/src/allmydata/test/test_mutable.py
@@ -3305,6 +3305,7 @@ class Version(GridTestMixin, unittest.TestCase, testutil.ShouldFailMixin, \
     def test_partial_read_ending_one_byte_after_segment_boundary(self):
         return self._test_partial_read(mathutil.next_multiple(128 * 1024, 3)-50, 51)
 
+    # XXX factor these into a single upload after they pass
     def test_partial_read_zero_length_at_start(self):
         return self._test_partial_read(0, 0)
 
@@ -3314,11 +3315,6 @@ class Version(GridTestMixin, unittest.TestCase, testutil.ShouldFailMixin, \
     def test_partial_read_zero_length_at_segment_boundary(self):
         return self._test_partial_read(mathutil.next_multiple(128 * 1024, 3), 0)
 
-    # XXX factor these into a single upload after they pass
-    _broken = "zero-length reads of mutable files don't work"
-    test_partial_read_zero_length_at_start.todo = _broken
-    test_partial_read_zero_length_in_middle.todo = _broken
-    test_partial_read_zero_length_at_segment_boundary.todo = _broken
 
     def _test_read_and_download(self, node, expected):
         d = node.get_best_readable_version()