fix expired object deletion

fixes bug #1257330

Change-Id: I49f645abdeba97eafb3ae42ef9e3684c912cfdc6

swift/common/exceptions.py

@@ -51,6 +51,13 @@ class DiskFileNotExist(DiskFileError):
 
 
 class DiskFileDeleted(DiskFileNotExist):
+
+    def __init__(self, metadata=None):
+        self.metadata = metadata or {}
+        self.timestamp = self.metadata.get('X-Timestamp', 0)
+
+
+class DiskFileExpired(DiskFileDeleted):
     pass
 
 

swift/obj/diskfile.py

@@ -56,7 +56,7 @@ from swift.common.utils import mkdirs, normalize_timestamp, \
 from swift.common.exceptions import DiskFileQuarantined, DiskFileNotExist, \
     DiskFileCollision, DiskFileNoSpace, DiskFileDeviceUnavailable, \
     DiskFileDeleted, DiskFileError, DiskFileNotOpen, PathNotDir, \
-    ReplicationLockTimeout
+    ReplicationLockTimeout, DiskFileExpired
 from swift.common.swob import multi_range_iterator
 
 
@@ -1113,8 +1113,7 @@ class DiskFile(object):
                 # we don't have a data file so we are just going to raise an
                 # exception that we could not find the object, providing the
                 # tombstone's timestamp.
-                exc = DiskFileDeleted()
-                exc.timestamp = metadata['X-Timestamp']
+                exc = DiskFileDeleted(metadata=metadata)
         return exc
 
     def _verify_name_matches_hash(self, data_file):
@@ -1136,7 +1135,7 @@ class DiskFile(object):
                    verify the on-disk size with Content-Length metadata value
         :raises DiskFileCollision: if the metadata stored name does not match
                                    the referenced name of the file
-        :raises DiskFileNotExist: if the object has expired
+        :raises DiskFileExpired: if the object has expired
         :raises DiskFileQuarantined: if data inconsistencies were detected
                                      between the metadata and the file-system
                                      metadata
@@ -1165,7 +1164,7 @@ class DiskFile(object):
                     self._metadata['X-Delete-At']))
         else:
             if x_delete_at <= time.time():
-                raise DiskFileNotExist('Expired')
+                raise DiskFileExpired(metadata=self._metadata)
         try:
             metadata_size = int(self._metadata['Content-Length'])
         except KeyError:

swift/obj/server.py

@@ -34,7 +34,7 @@ from swift.common.constraints import check_object_creation, \
     check_float, check_utf8
 from swift.common.exceptions import ConnectionTimeout, DiskFileQuarantined, \
     DiskFileNotExist, DiskFileCollision, DiskFileNoSpace, DiskFileDeleted, \
-    DiskFileDeviceUnavailable
+    DiskFileDeviceUnavailable, DiskFileExpired
 from swift.obj import ssync_receiver
 from swift.common.http import is_success
 from swift.common.request_helpers import split_and_validate_path
@@ -582,6 +582,10 @@ class ObjectController(object):
             return HTTPInsufficientStorage(drive=device, request=request)
         try:
             orig_metadata = disk_file.read_metadata()
+        except DiskFileExpired as e:
+            orig_timestamp = e.timestamp
+            orig_metadata = e.metadata
+            response_class = HTTPNotFound
         except DiskFileDeleted as e:
             orig_timestamp = e.timestamp
             orig_metadata = {}

test/unit/obj/test_server.py

@@ -2587,6 +2587,62 @@ class TestObjectController(unittest.TestCase):
         finally:
             object_server.time.time = orig_time
 
+    def test_DELETE_if_delete_at_expired_still_deletes(self):
+        test_time = time() + 10
+        test_timestamp = normalize_timestamp(test_time)
+        delete_at_time = int(test_time + 10)
+        delete_at_timestamp = str(delete_at_time)
+        delete_at_container = str(
+            delete_at_time /
+            self.object_controller.expiring_objects_container_divisor *
+            self.object_controller.expiring_objects_container_divisor)
+        req = Request.blank(
+            '/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
+            headers={'X-Timestamp': test_timestamp,
+                     'X-Delete-At': delete_at_timestamp,
+                     'X-Delete-At-Container': delete_at_container,
+                     'Content-Length': '4',
+                     'Content-Type': 'application/octet-stream'})
+        req.body = 'TEST'
+        resp = req.get_response(self.object_controller)
+        self.assertEquals(resp.status_int, 201)
+
+        # sanity
+        req = Request.blank(
+            '/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'GET'},
+            headers={'X-Timestamp': test_timestamp})
+        resp = req.get_response(self.object_controller)
+        self.assertEquals(resp.status_int, 200)
+        self.assertEquals(resp.body, 'TEST')
+        objfile = os.path.join(
+            self.testdir, 'sda1',
+            storage_directory(diskfile.DATADIR, 'p',
+                              hash_path('a', 'c', 'o')),
+            test_timestamp + '.data')
+        self.assert_(os.path.isfile(objfile))
+
+        # move time past expirery
+        with mock.patch('swift.obj.diskfile.time') as mock_time:
+            mock_time.time.return_value = test_time + 100
+            req = Request.blank(
+                '/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'GET'},
+                headers={'X-Timestamp': test_timestamp})
+            resp = req.get_response(self.object_controller)
+            # request will 404
+            self.assertEquals(resp.status_int, 404)
+            # but file still exists
+            self.assert_(os.path.isfile(objfile))
+
+            # make the x-if-delete-at with all the right bits
+            req = Request.blank(
+                '/sda1/p/a/c/o',
+                environ={'REQUEST_METHOD': 'DELETE'},
+                headers={'X-Timestamp': delete_at_timestamp,
+                         'X-If-Delete-At': delete_at_timestamp})
+            resp = req.get_response(self.object_controller)
+            self.assertEquals(resp.status_int, 404)
+            self.assertFalse(os.path.isfile(objfile))
+
     def test_DELETE_if_delete_at(self):
         test_time = time() + 10000
         req = Request.blank(