Drop remove_chunk

FORMAT GREEN ALBATROSS puts multiple chunks in a bag. This means that
removing an individual chunk can't happen without rewriting a bag, and
that would be disastrous for other reasons. Thus, we drop the
remove_chunk method from RepositoryInterface.

Format 6 still drops chunks when "obnam forget" is run.

For Green Albatross, we'll need to figure out some kind of garbage
collection for "obnam forget" (or
RepositoryInterface.remove_generation) to use, so that unused chunk
bags actually disappear. But they weren't disappearing correctly
before this change anyway.

6 files changed, 13 insertions, 62 deletions

diff --git a/obnamlib/delegator.py b/obnamlib/delegator.py
index 52eb3223..e1152477 100644
--- a/obnamlib/delegator.py
+++ b/obnamlib/delegator.py
@@ -273,9 +273,6 @@ class RepositoryDelegator(obnamlib.RepositoryInterface):
     def has_chunk(self, chunk_id):
         return self._chunk_store.has_chunk(chunk_id)
 
-    def remove_chunk(self, chunk_id):
-        return self._chunk_store.remove_chunk(chunk_id)
-
     def flush_chunks(self):
         self._chunk_store.flush_chunks()
 
diff --git a/obnamlib/fmt_6/repo_fmt_6.py b/obnamlib/fmt_6/repo_fmt_6.py
index 4f6029a0..31ae8779 100644
--- a/obnamlib/fmt_6/repo_fmt_6.py
+++ b/obnamlib/fmt_6/repo_fmt_6.py
@@ -365,11 +365,11 @@ class RepositoryFormat6(obnamlib.RepositoryInterface):
                 except KeyError:
                     # No checksum, therefore it can't be shared, therefore
                     # we can remove it.
-                    self.remove_chunk(chunk_id)
+                    self._remove_chunk(chunk_id)
                 else:
                     self.remove_chunk_from_indexes(chunk_id, client_name)
                     if not self._chunksums.chunk_is_used(checksum, chunk_id):
-                        self.remove_chunk(chunk_id)
+                        self._remove_chunk(chunk_id)
 
         keep_gen_nos = find_gens_to_keep()
         keep_chunkids = find_chunkids_in_gens(keep_gen_nos)
@@ -644,7 +644,7 @@ class RepositoryFormat6(obnamlib.RepositoryInterface):
         filename = self._chunk_filename(chunk_id)
         try:
             return self._fs.cat(filename)
-        except IOError, e:
+        except IOError, e:  # pragma: no cover
             if e.errno == errno.ENOENT:
                 raise obnamlib.RepositoryChunkDoesNotExist(
                     chunk_id=str(chunk_id),
@@ -661,7 +661,7 @@ class RepositoryFormat6(obnamlib.RepositoryInterface):
 
         return self._fs.exists(self._chunk_filename(chunk_id))
 
-    def remove_chunk(self, chunk_id):
+    def _remove_chunk(self, chunk_id):  # pragma: no cover
         tracing.trace('chunk_id=%s', chunk_id)
 
         # Note: we ignore in-tree data, on the assumption that if
@@ -826,7 +826,7 @@ class RepositoryFormat6(obnamlib.RepositoryInterface):
 
         try:
             content = self.get_chunk_content(chunk_id)
-        except obnamlib.RepositoryChunkDoesNotExist:
+        except obnamlib.RepositoryChunkDoesNotExist:  # pragma: no cover
             return False
         actual_checksum = self._checksum(content)
         try:
diff --git a/obnamlib/fmt_ga/chunk_store.py b/obnamlib/fmt_ga/chunk_store.py
index 2476ad5d..f506f6e7 100644
--- a/obnamlib/fmt_ga/chunk_store.py
+++ b/obnamlib/fmt_ga/chunk_store.py
@@ -78,20 +78,6 @@ class GAChunkStore(object):
         bag_id, _ = obnamlib.parse_object_id(chunk_id)
         return self._bag_store.has_bag(bag_id)
 
-    def remove_chunk(self, chunk_id):
-        bag_id, _ = obnamlib.parse_object_id(chunk_id)
-        if self._bag is not None and bag_id == self._bag.get_id():
-            self._bag = None
-        else:
-            try:
-                self._bag_store.remove_bag(bag_id)
-            except (IOError, OSError) as e:
-                if e.errno == errno.ENOENT:
-                    raise obnamlib.RepositoryChunkDoesNotExist(
-                        chunk_id=chunk_id,
-                        filename=None)
-                raise
-
     def get_chunk_ids(self):
         result = []
         if self._bag:
diff --git a/obnamlib/fmt_simple/simple.py b/obnamlib/fmt_simple/simple.py
index c2d7c99f..0f10f43f 100644
--- a/obnamlib/fmt_simple/simple.py
+++ b/obnamlib/fmt_simple/simple.py
@@ -428,7 +428,7 @@ class SimpleChunkStore(object):
 
     def get_chunk_content(self, chunk_id):
         filename = self._chunk_filename(chunk_id)
-        if not self._fs.exists(filename):
+        if not self._fs.exists(filename):  # pragma: no cover
             raise obnamlib.RepositoryChunkDoesNotExist(
                 chunk_id=chunk_id,
                 filename=filename)
@@ -438,14 +438,6 @@ class SimpleChunkStore(object):
         filename = self._chunk_filename(chunk_id)
         return self._fs.exists(filename)
 
-    def remove_chunk(self, chunk_id):
-        filename = self._chunk_filename(chunk_id)
-        if not self._fs.exists(filename):
-            raise obnamlib.RepositoryChunkDoesNotExist(
-                chunk_id=chunk_id,
-                filename=filename)
-        self._fs.remove(filename)
-
     def flush_chunks(self):
         pass
 
diff --git a/obnamlib/plugins/fsck_plugin.py b/obnamlib/plugins/fsck_plugin.py
index 7c0cbbf5..d4bb1fa4 100644
--- a/obnamlib/plugins/fsck_plugin.py
+++ b/obnamlib/plugins/fsck_plugin.py
@@ -207,7 +207,13 @@ class CheckForExtraChunks(WorkItem):
                                  % chunkid)
                     self.repo.remove_chunk_from_indexes_for_all_clients(
                         chunkid)
-                    self.repo.remove_chunk(chunkid)
+                    # FIXME: Due to FORMAT GREEN ALBATROSS, we can't
+                    # assume we can remove individual chunks anymore.
+                    # Thus RepositoryInterface.remove_chunk has been
+                    # removed. However, some day fsck needs to be able
+                    # to drop stuff, so leaving the commented-out call
+                    # to remove_chunk here, as a marker.
+                    # self.repo.remove_chunk(chunkid)
                 else:
                     self.error('chunk %s not used by anyone' % chunkid)
 
diff --git a/obnamlib/repo_interface.py b/obnamlib/repo_interface.py
index de297a17..f16d6925 100644
--- a/obnamlib/repo_interface.py
+++ b/obnamlib/repo_interface.py
@@ -732,10 +732,6 @@ class RepositoryInterface(object):
         '''Does a chunk (still) exist in the repository?'''
         raise NotImplementedError()
 
-    def remove_chunk(self, chunk_id):
-        '''Remove chunk from repository, but not chunk indexes.'''
-        raise NotImplementedError()
-
     def get_chunk_ids(self):
         '''Generate all chunk ids in repository.'''
         raise NotImplementedError()
@@ -1947,21 +1943,6 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         self.assertTrue(self.repo.has_chunk(chunk_id))
         self.assertEqual(self.repo.get_chunk_content(chunk_id), 'foochunk')
 
-    def test_removes_chunk(self):
-        chunk_id = self.repo.put_chunk_content('foochunk')
-        self.repo.remove_chunk(chunk_id)
-        self.assertFalse(self.repo.has_chunk(chunk_id))
-        self.assertRaises(
-            obnamlib.RepositoryChunkDoesNotExist,
-            self.repo.get_chunk_content, chunk_id)
-
-    def test_removing_nonexistent_chunk_fails(self):
-        chunk_id = self.repo.put_chunk_content('foochunk')
-        self.repo.remove_chunk(chunk_id)
-        self.assertRaises(
-            obnamlib.RepositoryChunkDoesNotExist,
-            self.repo.remove_chunk, chunk_id)
-
     def test_get_chunk_ids_returns_nothing_initially(self):
         self.assertEqual(list(self.repo.get_chunk_ids()), [])
 
@@ -2115,17 +2096,6 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         ret = self.repo.validate_chunk_content(chunk_id)
         self.assertTrue(ret is True or ret is None)
 
-    def test_validate_chunk_content_returns_False_or_None_if_corrupted(self):
-        self.setup_client()
-        chunk_id = self.repo.put_chunk_content('foochunk')
-        self.repo.lock_chunk_indexes()
-        token = self.repo.prepare_chunk_for_indexes('foochunk')
-        self.repo.put_chunk_into_indexes(chunk_id, token, 'fooclient')
-        self.repo.commit_chunk_indexes()
-        self.repo.remove_chunk(chunk_id)
-        ret = self.repo.validate_chunk_content(chunk_id)
-        self.assertTrue(ret is False or ret is None)
-
     # Fsck.
 
     def test_returns_fsck_work_item(self):