src/inventory.py

@@ -7,6 +7,16 @@ from bmconfigparser import config
 from singleton import Singleton
 
 
+def create_inventory_instance(backend="sqlite"):
+    """
+    Create an instance of the inventory class
+    defined in `storage.<backend>`.
+    """
+    return getattr(
+        getattr(storage, backend),
+        "{}Inventory".format(backend.title()))()
+
+
 @Singleton
 class Inventory():
     """
@@ -15,11 +25,7 @@ class Inventory():
     """
     def __init__(self):
         self._moduleName = config.safeGet("inventory", "storage")
-        self._inventoryClass = getattr(
+        self._realInventory = create_inventory_instance(self._moduleName)
-            getattr(storage, self._moduleName),
-            "{}Inventory".format(self._moduleName.title())
-        )
-        self._realInventory = self._inventoryClass()
         self.numberOfInventoryLookupsPerformed = 0
 
     # cheap inheritance copied from asyncore

src/storage/filesystem.py

@@ -2,21 +2,19 @@
 Module for using filesystem (directory with files) for inventory storage
 """
 import logging
-import string
+import os
 import time
 from binascii import hexlify, unhexlify
-from os import listdir, makedirs, path, remove, rmdir
 from threading import RLock
 
 from paths import lookupAppdataFolder
-from storage import InventoryItem, InventoryStorage
+from .storage import InventoryItem, InventoryStorage
 
 logger = logging.getLogger('default')
 
 
 class FilesystemInventory(InventoryStorage):
     """Filesystem for inventory storage"""
-    # pylint: disable=too-many-ancestors, abstract-method
     topDir = "inventory"
     objectDir = "objects"
     metadataFilename = "metadata"
@@ -24,15 +22,15 @@ class FilesystemInventory(InventoryStorage):
 
     def __init__(self):
         super(FilesystemInventory, self).__init__()
-        self.baseDir = path.join(
+        self.baseDir = os.path.join(
             lookupAppdataFolder(), FilesystemInventory.topDir)
-        for createDir in [self.baseDir, path.join(self.baseDir, "objects")]:
+        for createDir in [self.baseDir, os.path.join(self.baseDir, "objects")]:
-            if path.exists(createDir):
+            if os.path.exists(createDir):
-                if not path.isdir(createDir):
+                if not os.path.isdir(createDir):
                     raise IOError(
                         "%s exists but it's not a directory" % createDir)
             else:
-                makedirs(createDir)
+                os.makedirs(createDir)
         # Guarantees that two receiveDataThreads
         # don't receive and process the same message
         # concurrently (probably sent by a malicious individual)
@@ -46,6 +44,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:
@@ -66,18 +67,18 @@ class FilesystemInventory(InventoryStorage):
         with self.lock:
             value = InventoryItem(*value)
             try:
-                makedirs(path.join(
+                os.makedirs(os.path.join(
                     self.baseDir,
                     FilesystemInventory.objectDir,
-                    hexlify(hashval)))
+                    hexlify(hashval).decode()))
             except OSError:
                 pass
             try:
                 with open(
-                    path.join(
+                    os.path.join(
                         self.baseDir,
                         FilesystemInventory.objectDir,
-                        hexlify(hashval),
+                        hexlify(hashval).decode(),
                         FilesystemInventory.metadataFilename,
                     ),
                     "w",
@@ -86,15 +87,15 @@ class FilesystemInventory(InventoryStorage):
                         value.type,
                         value.stream,
                         value.expires,
-                        hexlify(value.tag)))
+                        hexlify(value.tag).decode()))
                 with open(
-                    path.join(
+                    os.path.join(
                         self.baseDir,
                         FilesystemInventory.objectDir,
-                        hexlify(hashval),
+                        hexlify(hashval).decode(),
                         FilesystemInventory.dataFilename,
                     ),
-                    "w",
+                    "wb",
                 ) as f:
                     f.write(value.payload)
             except IOError:
@@ -114,28 +115,28 @@ class FilesystemInventory(InventoryStorage):
                 pass
         with self.lock:
             try:
-                remove(
+                os.remove(
-                    path.join(
+                    os.path.join(
                         self.baseDir,
                         FilesystemInventory.objectDir,
-                        hexlify(hashval),
+                        hexlify(hashval).decode(),
                         FilesystemInventory.metadataFilename))
             except IOError:
                 pass
             try:
-                remove(
+                os.remove(
-                    path.join(
+                    os.path.join(
                         self.baseDir,
                         FilesystemInventory.objectDir,
-                        hexlify(hashval),
+                        hexlify(hashval).decode(),
                         FilesystemInventory.dataFilename))
             except IOError:
                 pass
             try:
-                rmdir(path.join(
+                os.rmdir(os.path.join(
                     self.baseDir,
                     FilesystemInventory.objectDir,
-                    hexlify(hashval)))
+                    hexlify(hashval).decode()))
             except IOError:
                 pass
 
@@ -168,8 +169,6 @@ class FilesystemInventory(InventoryStorage):
                 logger.debug(
                     'error loading %s', hexlify(hashId), exc_info=True)
         self._inventory = newInventory
-#        for i, v in self._inventory.items():
-#            print "loaded stream: %s, %i items" % (i, len(v))
 
     def stream_list(self):
         """Return list of streams"""
@@ -177,17 +176,17 @@ class FilesystemInventory(InventoryStorage):
 
     def object_list(self):
         """Return inventory vectors (hashes) from a directory"""
-        return [unhexlify(x) for x in listdir(path.join(
+        return [unhexlify(x) for x in os.listdir(os.path.join(
             self.baseDir, FilesystemInventory.objectDir))]
 
     def getData(self, hashId):
         """Get object data"""
         try:
             with open(
-                path.join(
+                os.path.join(
                     self.baseDir,
                     FilesystemInventory.objectDir,
-                    hexlify(hashId),
+                    hexlify(hashId).decode(),
                     FilesystemInventory.dataFilename,
                 ),
                 "r",
@@ -200,16 +199,16 @@ class FilesystemInventory(InventoryStorage):
         """Get object metadata"""
         try:
             with open(
-                path.join(
+                os.path.join(
                     self.baseDir,
                     FilesystemInventory.objectDir,
-                    hexlify(hashId),
+                    hexlify(hashId).decode(),
                     FilesystemInventory.metadataFilename,
                 ),
                 "r",
             ) as f:
-                objectType, streamNumber, expiresTime, tag = string.split(
+                objectType, streamNumber, expiresTime, tag = f.read().split(
-                    f.read(), ",", 4)[:4]
+                    ",", 4)[:4]
                 return [
                     int(objectType),
                     int(streamNumber),
@@ -246,10 +245,10 @@ class FilesystemInventory(InventoryStorage):
 
     def unexpired_hashes_by_stream(self, stream):
         """Return unexpired hashes in the inventory for a particular stream"""
-        t = int(time.time())
         try:
-            return [x for x, value in self._inventory[stream].items()
+            return [
-                    if value.expires > t]
+                x for x, value in self._inventory[stream].items()
+                if value.expires > int(time.time())]
         except KeyError:
             return []
 
@@ -259,7 +258,7 @@ class FilesystemInventory(InventoryStorage):
 
     def clean(self):
         """Clean out old items from the inventory"""
-        minTime = int(time.time()) - (60 * 60 * 30)
+        minTime = int(time.time()) - 60 * 60 * 30
         deletes = []
         for streamDict in self._inventory.values():
             for hashId, item in streamDict.items():

src/storage/sqlite.py

@@ -6,10 +6,10 @@ import time
 from threading import RLock
 
 from helper_sql import SqlBulkExecute, sqlExecute, sqlQuery
-from storage import InventoryItem, InventoryStorage
+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__()

src/storage/storage.py

@@ -1,73 +1,47 @@
 """
 Storing inventory items
 """
-import collections
 
-InventoryItem = collections.namedtuple(
+from abc import abstractmethod
-    'InventoryItem', 'type stream payload expires tag')
+from collections import namedtuple
+try:
+    from collections import MutableMapping  # pylint: disable=deprecated-class
+except ImportError:
+    from collections.abc import MutableMapping
 
 
-class Storage(object):  # pylint: disable=too-few-public-methods
+InventoryItem = namedtuple('InventoryItem', 'type stream payload expires tag')
-    """Base class for storing inventory
-    (extendable for other items to store)"""
-    pass
 
 
-class InventoryStorage(Storage, collections.MutableMapping):
+class InventoryStorage(MutableMapping):
-    """Module used for inventory storage"""
+    """
+    Base class for storing inventory
+    (extendable for other items to store)
+    """
 
-    def __init__(self):  # pylint: disable=super-init-not-called
+    def __init__(self):
         self.numberOfInventoryLookupsPerformed = 0
 
-    def __contains__(self, _):
+    @abstractmethod
-        raise NotImplementedError
+    def __contains__(self, item):
-
+        pass
-    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 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, collections.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

src/tests/test_inventory.py

@@ -0,0 +1,58 @@
+"""Tests for inventory"""
+
+import os
+import shutil
+import struct
+import tempfile
+import time
+import unittest
+
+from pybitmessage.storage import storage
+from pybitmessage.addresses import calculateInventoryHash
+
+from .partial import TestPartialRun
+
+
+class TestFilesystemInventory(TestPartialRun):
+    """A test case for the inventory using filesystem backend"""
+
+    @classmethod
+    def setUpClass(cls):
+        cls.home = os.environ['BITMESSAGE_HOME'] = tempfile.mkdtemp()
+        super(TestFilesystemInventory, cls).setUpClass()
+
+        from inventory import create_inventory_instance
+        cls.inventory = create_inventory_instance('filesystem')
+
+    def test_consistency(self):
+        """Ensure the inventory is of proper class"""
+        if os.path.isfile(os.path.join(self.home, 'messages.dat')):
+            # this will likely never happen
+            self.fail("Failed to configure filesystem inventory!")
+
+    def test_appending(self):
+        """Add a sample message to the inventory"""
+        TTL = 24 * 60 * 60
+        embedded_time = int(time.time() + TTL)
+        msg = struct.pack('>Q', embedded_time) + os.urandom(166)
+        invhash = calculateInventoryHash(msg)
+        self.inventory[invhash] = (2, 1, msg, embedded_time, b'')
+
+    @classmethod
+    def tearDownClass(cls):
+        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()