storage: refactor is_deletable

circle/storage/models.py

@@ -51,7 +51,7 @@ class DataStore(Model):
     def get_deletable_disks(self):
     def get_deletable_disks(self):
         return [disk.filename for disk in
         return [disk.filename for disk in
                 self.disk_set.filter(
                 self.disk_set.filter(
-                    destroyed__isnull=False) if disk.is_deletable()]
+                    destroyed__isnull=False) if disk.is_deletable]
 
 
 
 
 class Disk(AclBase, TimeStampedModel):
 class Disk(AclBase, TimeStampedModel):
@@ -159,21 +159,20 @@ class Disk(AclBase, TimeStampedModel):
         result = celery.AsyncResult(id=task)
         result = celery.AsyncResult(id=task)
         return result.info.get("percent")
         return result.info.get("percent")
 
 
+    @property
     def is_deletable(self):
     def is_deletable(self):
-        """Returns whether the file can be deleted.
-
-        Checks if all children and the disk itself is destroyed.
+        """True if the associated file can be deleted.
         """
         """
-
+        # Check if all children and the disk itself is destroyed.
         yesterday = timezone.now() - timedelta(days=1)
         yesterday = timezone.now() - timedelta(days=1)
         return (self.destroyed is not None
         return (self.destroyed is not None
-                and self.destroyed < yesterday) and not self.has_active_child()
+                and self.destroyed < yesterday) and self.children_deletable
 
 
-    def has_active_child(self):
-        """Returns if disk has children that are not destroyed.
+    @property
+    def children_deletable(self):
+        """True if all children of the disk are deletable.
         """
         """
-
-        return any((not i.is_deletable() for i in self.derivatives.all()))
+        return all(i.is_deletable for i in self.derivatives.all())
 
 
     @property
     @property
     def is_in_use(self):
     def is_in_use(self):

circle/storage/tests/test_models.py

@@ -25,24 +25,24 @@ class DiskTestCase(TestCase):
 
 
     def test_deletable_not_destroyed(self):
     def test_deletable_not_destroyed(self):
         d = self._disk()
         d = self._disk()
-        assert not d.is_deletable()
+        assert not d.is_deletable
 
 
     def test_deletable_newly_destroyed(self):
     def test_deletable_newly_destroyed(self):
         d = self._disk(destroyed=new)
         d = self._disk(destroyed=new)
-        assert not d.is_deletable()
+        assert not d.is_deletable
 
 
     def test_deletable_no_child(self):
     def test_deletable_no_child(self):
         d = self._disk(destroyed=old)
         d = self._disk(destroyed=old)
-        assert d.is_deletable()
+        assert d.is_deletable
 
 
     def test_deletable_child_not_destroyed(self):
     def test_deletable_child_not_destroyed(self):
         d = self._disk()
         d = self._disk()
         self._disk(base=d, destroyed=old)
         self._disk(base=d, destroyed=old)
         self._disk(base=d)
         self._disk(base=d)
-        assert not d.is_deletable()
+        assert not d.is_deletable
 
 
     def test_deletable_child_newly_destroyed(self):
     def test_deletable_child_newly_destroyed(self):
         d = self._disk(destroyed=old)
         d = self._disk(destroyed=old)
         self._disk(base=d, destroyed=new)
         self._disk(base=d, destroyed=new)
         self._disk(base=d)
         self._disk(base=d)
-        assert not d.is_deletable()
+        assert not d.is_deletable