From 8af1a13e1067ebfe99e4dc3eb307851a400acd27 Mon Sep 17 00:00:00 2001
From: Lee Miller <lee.miller@tutanota.com>
Date: Wed, 21 Sep 2022 01:02:36 +0300
Subject: [PATCH] Simplify storage abstract base definition

---
 src/storage/filesystem.py   |  4 ++-
 src/storage/sqlite.py       |  2 +-
 src/storage/storage.py      | 71 +++++++++++++------------------------
 src/tests/test_inventory.py | 14 ++++++++
 4 files changed, 42 insertions(+), 49 deletions(-)

diff --git a/src/storage/filesystem.py b/src/storage/filesystem.py
index 58e8db19..09408813 100644
--- a/src/storage/filesystem.py
+++ b/src/storage/filesystem.py
@@ -16,7 +16,6 @@ logger = logging.getLogger('default')
 
 class FilesystemInventory(InventoryStorage):
     """Filesystem for inventory storage"""
-    # pylint: disable=too-many-ancestors, abstract-method
     topDir = "inventory"
     objectDir = "objects"
     metadataFilename = "metadata"
@@ -46,6 +45,9 @@ class FilesystemInventory(InventoryStorage):
                 return True
         return False
 
+    def __delitem__(self, hash_):
+        raise NotImplementedError
+
     def __getitem__(self, hashval):
         for streamDict in self._inventory.values():
             try:
diff --git a/src/storage/sqlite.py b/src/storage/sqlite.py
index 10874332..eb5df098 100644
--- a/src/storage/sqlite.py
+++ b/src/storage/sqlite.py
@@ -9,7 +9,7 @@ from helper_sql import SqlBulkExecute, sqlExecute, sqlQuery
 from .storage import InventoryItem, InventoryStorage
 
 
-class SqliteInventory(InventoryStorage):  # pylint: disable=too-many-ancestors
+class SqliteInventory(InventoryStorage):
     """Inventory using SQLite"""
     def __init__(self):
         super(SqliteInventory, self).__init__()
diff --git a/src/storage/storage.py b/src/storage/storage.py
index e60d7dcb..c000132c 100644
--- a/src/storage/storage.py
+++ b/src/storage/storage.py
@@ -2,77 +2,54 @@
 Storing inventory items
 """
 
+from abc import ABCMeta, abstractmethod
 from collections import namedtuple
 try:
     from collections import MutableMapping  # pylint: disable=deprecated-class
 except ImportError:
     from collections.abc import MutableMapping
 
+import six
+
 
 InventoryItem = namedtuple('InventoryItem', 'type stream payload expires tag')
 
 
-class Storage(object):  # pylint: disable=too-few-public-methods
-    """Base class for storing inventory
-    (extendable for other items to store)"""
-    pass
+class InventoryStorage(MutableMapping):
+    """
+    Base class for storing inventory
+    (extendable for other items to store)
+    """
 
-
-class InventoryStorage(Storage, MutableMapping):
-    """Module used for inventory storage"""
-
-    def __init__(self):  # pylint: disable=super-init-not-called
+    def __init__(self):
         self.numberOfInventoryLookupsPerformed = 0
 
-    def __contains__(self, _):
-        raise NotImplementedError
-
-    def __getitem__(self, _):
-        raise NotImplementedError
-
-    def __setitem__(self, _, value):
-        raise NotImplementedError
-
-    def __delitem__(self, _):
-        raise NotImplementedError
-
-    def __iter__(self):
-        raise NotImplementedError
-
-    def __len__(self):
-        raise NotImplementedError
+    @abstractmethod
+    def __contains__(self, item):
+        pass
 
+    @abstractmethod
     def by_type_and_tag(self, objectType, tag):
         """Return objects filtered by object type and tag"""
-        raise NotImplementedError
+        pass
 
+    @abstractmethod
     def unexpired_hashes_by_stream(self, stream):
         """Return unexpired inventory vectors filtered by stream"""
-        raise NotImplementedError
+        pass
 
+    @abstractmethod
     def flush(self):
         """Flush cache"""
-        raise NotImplementedError
+        pass
 
+    @abstractmethod
     def clean(self):
         """Free memory / perform garbage collection"""
-        raise NotImplementedError
+        pass
 
 
-class MailboxStorage(Storage, MutableMapping):
-    """Method for storing mails"""
-
-    def __delitem__(self, key):
-        raise NotImplementedError
-
-    def __getitem__(self, key):
-        raise NotImplementedError
-
-    def __iter__(self):
-        raise NotImplementedError
-
-    def __len__(self):
-        raise NotImplementedError
-
-    def __setitem__(self, key, value):
-        raise NotImplementedError
+@six.add_metaclass(ABCMeta)
+class MailboxStorage(MutableMapping):
+    """An abstract class for storing mails. TODO"""
+    pass
diff --git a/src/tests/test_inventory.py b/src/tests/test_inventory.py
index 9ecb576f..b6d0cc85 100644
--- a/src/tests/test_inventory.py
+++ b/src/tests/test_inventory.py
@@ -7,6 +7,7 @@ import tempfile
 import time
 import unittest
 
+from pybitmessage.storage import storage
 from pybitmessage.addresses import calculateInventoryHash
 
 from .partial import TestPartialRun
@@ -42,3 +43,16 @@ class TestFilesystemInventory(TestPartialRun):
         super(TestFilesystemInventory, cls).tearDownClass()
         cls.inventory.flush()
         shutil.rmtree(os.path.join(cls.home, cls.inventory.topDir))
+
+
+class TestStorageAbstract(unittest.TestCase):
+    """A test case for refactoring of the storage abstract classes"""
+
+    def test_inventory_storage(self):
+        """Check inherited abstract methods"""
+        with self.assertRaisesRegexp(
+            TypeError, "^Can't instantiate abstract class.*"
+            "methods __contains__, __delitem__, __getitem__, __iter__,"
+            " __len__, __setitem__"
+        ):  # pylint: disable=abstract-class-instantiated
+            storage.InventoryStorage()