From 0d967c31cb1d21feb17aee3d506b8851d8e8dcb9 Mon Sep 17 00:00:00 2001
From: Alistair Coles <alistairncoles@gmail.com>
Date: Fri, 27 Apr 2018 10:18:03 +0100
Subject: [PATCH] Improve test coverage for metadata reclaim

Actually test that the reclaim method does reclaim metadata.

Replace class invocation of _reclaim_metadata() with instance
invocation.

Fix reclaim() docstring.

Change-Id: I7a473e164c8c14b26b195db9a91fea1d2cd5b267
Related-Change: Ied1373362c38bbe7bab84fe4958888b0145e68ba
---
 swift/common/db.py          | 15 +++++++++------
 swift/container/backend.py  |  2 +-
 test/unit/common/test_db.py |  6 +++---
 3 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/swift/common/db.py b/swift/common/db.py
index f21c5e7591..d3186cdd97 100644
--- a/swift/common/db.py
+++ b/swift/common/db.py
@@ -910,12 +910,15 @@ class DatabaseBroker(object):
 
     def reclaim(self, age_timestamp, sync_timestamp):
         """
-        Delete rows from the db_contains_type table that are marked deleted
-        and whose created_at timestamp is < age_timestamp.  Also deletes rows
-        from incoming_sync and outgoing_sync where the updated_at timestamp is
-        < sync_timestamp.
+        Delete reclaimable rows and metadata from the db.
 
-        In addition, this calls the DatabaseBroker's :func:`_reclaim` method.
+        By default this method will delete rows from the db_contains_type table
+        that are marked deleted and whose created_at timestamp is <
+        age_timestamp, and deletes rows from incoming_sync and outgoing_sync
+        where the updated_at timestamp is < sync_timestamp. In addition, this
+        calls the :meth:`_reclaim_metadata` method.
+
+        Subclasses may reclaim other items by overriding :meth:`_reclaim`.
 
         :param age_timestamp: max created_at timestamp of object rows to delete
         :param sync_timestamp: max update_at timestamp of sync rows to delete
@@ -926,7 +929,7 @@ class DatabaseBroker(object):
                 self._commit_puts()
         with self.get() as conn:
             self._reclaim(conn, age_timestamp, sync_timestamp)
-            DatabaseBroker._reclaim_metadata(self, conn, age_timestamp)
+            self._reclaim_metadata(conn, age_timestamp)
             conn.commit()
 
     def _reclaim(self, conn, age_timestamp, sync_timestamp):
diff --git a/swift/container/backend.py b/swift/container/backend.py
index b6ff6cacfa..ef3463e77f 100644
--- a/swift/container/backend.py
+++ b/swift/container/backend.py
@@ -1565,7 +1565,7 @@ class ContainerBroker(DatabaseBroker):
         try:
             conn.execute('''
                 DELETE FROM shard_ranges WHERE deleted = 1 AND timestamp < ?
-                AND name!= ?
+                AND name != ?
             ''', (sync_timestamp, self.path))
         except sqlite3.OperationalError as err:
             if 'no such table: shard_ranges' not in str(err):
diff --git a/test/unit/common/test_db.py b/test/unit/common/test_db.py
index cb400d216f..a80b276835 100644
--- a/test/unit/common/test_db.py
+++ b/test/unit/common/test_db.py
@@ -1133,9 +1133,9 @@ class TestDatabaseBroker(unittest.TestCase):
 
     def test_metadata(self):
         def reclaim(broker, timestamp):
-            with broker.get() as conn:
-                broker._reclaim_metadata(conn, timestamp)
-                conn.commit()
+            # only testing _reclaim_metadata here
+            with patch.object(broker, '_reclaim'):
+                broker.reclaim(timestamp, timestamp)
         # Initializes a good broker for us
         broker = self.get_replication_info_tester(metadata=True)
         # Add our first item