Repository: festinuz/gecaso

Issue:
Make BaseStorage Async
As suggested by [robin-gvx](https://www.reddit.com/r/Python/comments/6on8in/feedback_required_for_a_small_cache_library/dkirf2p/l) on reddit:

> library relies heavily on asyncio, but for some reason get and set aren't required to be coroutines, but are coerced to be coroutines by the library. Why? If you're going to use them asynchronously anyway, why not require the user to write get and set as coroutines?

Perhaps abstract methods of BaseStorage should be defined as async methods.

Discussion:

Patch:
diff --git a/README.md b/README.md
index 9db7af6..b7d39df 100644
--- a/README.md
+++ b/README.md
@@ -1,6 +1,6 @@
 # Gecaso
 
-![master branch status](https://api.travis-ci.org/festinuz/gecaso.svg?branch=master)
+[![PyPI version](https://badge.fury.io/py/gecaso.svg)](https://badge.fury.io/py/gecaso) ![master branch status](https://api.travis-ci.org/festinuz/gecaso.svg?branch=master)
 
 Gecaso provides you with the tools that help with creating cache for your specific task.
 
@@ -30,14 +30,14 @@ class RedisStorage(gecaso.BaseStorage):
     def __init__(self, redis_url):
         self._storage = redis.from_url(redis_url)
 
-    def get(self, key):
+    async def get(self, key):
         value, params = self.unpack(self._storage[key])
         return value
 
-    def set(self, key, value, **params):
+    async def set(self, key, value, **params):
         self._storage[key] = self.pack(value, **params)
 
-    def remove(self, *keys):
+    async def remove(self, *keys):
         self._storage.remove(*keys)
 
 redis_storage = RedisStorage('valid_redis_url')
@@ -59,35 +59,38 @@ Install gecaso with pip:
 Note that at the time, gecaso only supports versions of python that are >=3.5
 
 ## Usage Guide
-##### Gecaso was created to be a simple solution that can be easily expanded to cover any needs of its users. Below is everything there is to know about using Gecaso. As such, there are only two objects that you need to know:
+##### Gecaso was created to be a simple solution that can be easily expanded to cover any needs of its users. Below is everything there is to know about using Gecaso.
 
-#### 1) "cached" function
+#### 1) "gecaso.cached" function
 This function is a wrapper that helps to set up cache for any synchronus or asynchronous function. It takes single positional argument, which must be an instance of class that is inherited from **BaseStorage**. It can also optionally be provided with a keyword argument **loop** which must be an instance of an event loop. Any keyword arguments provided besides **loop** will be passed to the **set** method of storage instance whenever it is being called.
 
-#### 2) "BaseStorage" class
-Any storage provided to "cached" function shoud be inherited from this class. Base storage has 6 methods.
+#### 2) "gecaso.BaseStorage" class
+Any storage provided to "cached" function should be inherited from this class. Base storage has 6 methods.
 
-* **get(self, key)**:  Abstract method that should be overriden. Can be synchronus or asynchronous. MUST raise KeyError if key is not present in storage. If data was packed using **pack** method before being stored, it must be unpacked using **unpack** method.
+* **get(self, key)**:  Abstract method that should be overridden. Must be asynchronous. MUST raise KeyError if key is not present in storage. If data was packed using **gecaso.pack** function before being stored, it must be unpacked using **gecaso.unpack** function.
 
-* **set(self, key, value, \*\*params)**: Abstract method that should be overriden. Can be synchronus or asynchronous. It is recomended to pack provided value using **pack** method before storing it in storage.
+* **set(self, key, value, \*\*params)**: Abstract method that should be overridden. Must be asynchronous. It is recommended to pack provided value using **gecaso.pack** function before storing it in storage.
 
-* **remove(self, \*keys)**: Abstract method that should be overriden. Should delete every key that is passed in *keys* parameter and exisits in storage.
+* **remove(self, \*keys)**: Abstract method that should be overridden. Must be asynchronous. Should delete every key that is passed in *keys* parameter and exists in storage.
 
-* **pack(self, value, \*\*params)**: Returns representation of object with fields named *data* and *params* as bytes object.
 
-* **unpack(self, value)**: Unpacks bytes object that was packed with **pack** method and returns *tuple(data, params)*.
+#### 3) Helper functions
+* **gecaso.pack(value, \*\*params)**: Returns representation of object with fields named *data* and *params* as bytes object. Useful when creating a custom storage as it allows to store almost anything as 'bytes'.
 
-* **verified_get(self, value, \*\*params)**: Helper function to run verification functions of storage. When provided with value and params, it will try to run a method of the class named **vfunc_PARAM** with value specified for that param for every param in *\*\*params*. See *Storage creation* for example of usage.
+* **gecaso.unpack(value)**: Unpacks bytes object that was packed with **pack** method and returns *tuple(data, params)*.
 
 
+#### 4) Storages provided by gecaso library
+* **gecaso.MemoryStorage** storages all the data in RAM. Can be used as a default storage.
+* **gecaso.LRUStorage** is a simplified implementation that provides LRU cache functionality. Storage passed to its *\_\_init\_\_()* method will be used used to store values. This effectively allows to wrap any preexisting storage in *gecaso.LRUStorage* to get LRU cache functionality for that storage.
+
 ### Storage creation
 ##### The process of using gecaso library usually includes creating a storage that fits your specific task the most. Here is a step by step example that should help you understand this process.
 
-Lets say that we want to have in-memory cache with the option of specifying *"time to live"* for cached results. Here are the steps we would take:
+Lets say that we want to have a simple in-memory cache. Here are the steps we would take:
 
 1) Import gecaso and create the base of our class:
 ```python
-import time
 import gecaso
 
 class LocalMemoryStorage(gecaso.BaseStorage):
@@ -95,40 +98,29 @@ class LocalMemoryStorage(gecaso.BaseStorage):
         self._storage = dict()  # Python's dict is a nice basic storage of data
 ```
 
-2) Override **set**, **get** and **remove** methods of gecaso.BaseStorage:
+2) Override async methods **set**, **get** and **remove** of gecaso.BaseStorage:
 ```python
-    def set(self, key, value, ttl=None):  # We dont want any additional parameters besides time to live
+    async def set(self, key, value):  # We don't want any additional parameters
         params = dict()
-        if ttl:  # We want ttl to be an optional parameter
-            params['ttl'] = time.time() + ttl  # Calculate time_of_death,after which result is considered invalid
-        self._storage[key] = self.pack(value, **params)  # Using BaseStorage.pack method
+        self._storage[key] = gecaso.pack(value, **params)
 
-    def get(self, key):
+    async def get(self, key):
         self.data = self._storage[key]  # If key is not present this will raise KeyError
-        value, params = self.unpack(self._storage[key])  # params can optionally contain ttl
-        return self.verified_get(value, **params)  # Using BaseStorage.verified_get method to verify ttl
+        value, params = gecaso.unpack(self._storage[key])
+        return value
 
-    def remove(self, *keys):
+    async def remove(self, *keys):
         for key in keys:
             self._storage.pop(key, None)  # Not going to throw error if some of the keys do not exists
 ```
-At this point the get method wont work properly because we called **verified_get** at the end of it. This method tries to call class method for every parameter it got and will break since we are trying to pass it our **ttl** parameter but it cant find the verifying function that this parameter should represent.
-
-3) Write a verifying function for our ttl parameter:
-```python
-    def vfunc_ttl(self, time_of_death):
-    """Note that the name of this function is equivalent of 'vfunc_' + parameter_name """
-        return time_of_death > time.time()
-```
-
-And now we're done! Whenever we request a value using **get** method it will eventually call **verified_get** which will return value if all the verifying functions returned true and will raise **KeyError** otherwise.
 
+And now we're done!
 After the storage class has been created, all that is left to do is create an instance of this class and provide it to **cached** decorator whenever it is being called:
 
 ```python
 local_storage = LocalMemoryStorage()
 
-@gecaso.cached(local_storage, ttl=30)
+@gecaso.cached(local_storage)
 def foo(bar):
     pass
 ```
diff --git a/gecaso/__init__.py b/gecaso/__init__.py
index 5393200..e7ee9ac 100644
--- a/gecaso/__init__.py
+++ b/gecaso/__init__.py
@@ -1,7 +1,7 @@
 from gecaso.cache import cached
-from gecaso.storage import BaseStorage, LocalMemoryStorage, LRUStorage
-from gecaso.utils import asyncify
+from gecaso.storage import BaseStorage, MemoryStorage, LRUStorage
+from gecaso.utils import pack, unpack
 
 
 __all__ = [
-    'cached', 'BaseStorage', 'LocalMemoryStorage', 'LRUStorage', 'asyncify']
+    'cached', 'BaseStorage', 'MemoryStorage', 'LRUStorage', 'pack', 'unpack']
diff --git a/gecaso/cache.py b/gecaso/cache.py
index 66ae76c..55cfb5c 100644
--- a/gecaso/cache.py
+++ b/gecaso/cache.py
@@ -21,17 +21,14 @@ def cached(cache_storage, loop=None, **params):
 
 
 def _cached(cache_storage, loop, **params):
-    storage_get = utils.asyncify(cache_storage.get)
-    storage_set = utils.asyncify(cache_storage.set)
-
     def wrapper(function):
         async def wrapped_function(*args, **kwargs):
             key = utils.make_key(function, *args, **kwargs)
             try:
-                result = await storage_get(key)
+                result = await cache_storage.get(key)
             except KeyError:
                 result = await function(*args, **kwargs)
-                await storage_set(key, result, **params)
+                await cache_storage.set(key, result, **params)
             return result
 
         def sync_wrapped_function(*args, **kwargs):
diff --git a/gecaso/storage.py b/gecaso/storage.py
index 9d27b73..96b5d27 100644
--- a/gecaso/storage.py
+++ b/gecaso/storage.py
@@ -1,6 +1,5 @@
 import abc
 import time
-import pickle
 
 from . import utils
 
@@ -10,58 +9,37 @@ class BaseStorage(metaclass=abc.ABCMeta):
     from this class.
     """
     @abc.abstractmethod
-    def get(self, key):
+    async def get(self, key):
         """Must throw KeyError if key is not found"""
         pass
 
     @abc.abstractmethod
-    def set(self, key, value, **params):
+    async def set(self, key, value, **params):
         pass
 
     @abc.abstractmethod
-    def remove(self, *keys):
+    async def remove(self, *keys):
         pass
 
-    def pack(self, value, **params):
-        """Packs value and methods into a object which is then converted to
-        bytes using pickle library. Used to simplify storaging because bytes
-        can bestored almost anywhere.
-        """
-        result = utils.Namespace(value=value, params=params)
-        return pickle.dumps(result)
-
-    def unpack(self, value):
-        """Unpacks bytes object packed with 'pack' method. Returns packed value
-        and parameters.
-        """
-        result = pickle.loads(value)
-        return result.value, result.params
-
-    def verified_get(self, value, **params):
-        """Given value and params, returns value if all methods called from
-        params (method name is assumed as 'vfunc_PARAMNAME' and argument is
-        value of param) return 'True'; Else raises KeyError."""
-        if all([getattr(self, 'vfunc_'+f)(v) for f, v in params.items()]):
-            return value
-        else:
-            raise KeyError('Cached result didnt pass verification')
-
 
-class LocalMemoryStorage(BaseStorage):
+class MemoryStorage(BaseStorage):
     def __init__(self):
         self._storage = dict()
 
-    def get(self, key):
-        value, params = self.unpack(self._storage[key])
-        return self.verified_get(value, **params)
+    async def get(self, key):
+        value, params = utils.unpack(self._storage[key])
+        if all([getattr(self, 'vfunc_'+f)(v) for f, v in params.items()]):
+            return value
+        else:
+            raise KeyError('Cached result didnt pass verification')
 
-    def set(self, key, value, ttl=None):
+    async def set(self, key, value, ttl=None):
         params = dict()
         if ttl:
             params['ttl'] = time.time() + ttl
-        self._storage[key] = self.pack(value, **params)
+        self._storage[key] = utils.pack(value, **params)
 
-    def remove(self, *keys):
+    async def remove(self, *keys):
         for key in keys:
             self._storage.pop(key, None)
 
@@ -71,7 +49,7 @@ class LocalMemoryStorage(BaseStorage):
 
 class LRUStorage(BaseStorage):
     """Storage that provides LRUCache functionality when used with 'cached'
-    wrapper. If 'storage' argument is not provided, LocalMemoryStorage is used
+    wrapper. If 'storage' argument is not provided, MemoryStorage is used
     as default substorage. Any provided storage is expected to be inherited
     from BaseStorage.
     """
@@ -93,18 +71,15 @@ class LRUStorage(BaseStorage):
             self.next.prev = self.prev
 
     def __init__(self, storage=None, maxsize=128):
-        self._storage = storage or LocalMemoryStorage()
+        self._storage = storage or MemoryStorage()
         self._nodes = dict()
         self._maxsize = maxsize
         self._head = LRUStorage.Node()  # This empty node will always be last
-        self.storage_set = utils.asyncify(self._storage.set)
-        self.storage_get = utils.asyncify(self._storage.get)
-        self.storage_remove = utils.asyncify(self._storage.remove)
 
     async def get(self, key):
         node = self._nodes.pop(key)  # Throws KeyError on failure
         node.delete()
-        value = await self.storage_get(key)  # Throws KeyError on failure
+        value = await self._storage.get(key)  # Throws KeyError on failure
         self._nodes[key] = LRUStorage.Node(self._head, key)
         self._head = self._nodes[key]
         return value
@@ -113,7 +88,7 @@ class LRUStorage(BaseStorage):
         if len(self._nodes) > self._maxsize:
             last_node = self._head.prev.prev  # skipping over empty node
             await self.remove(last_node.key)
-        await self.storage_set(key, value, **params)
+        await self._storage.set(key, value, **params)
         self._nodes[key] = LRUStorage.Node(self._head, key)
         self._head = self._nodes[key]
 
@@ -121,4 +96,4 @@ class LRUStorage(BaseStorage):
         for key in keys:
             node = self._nodes.pop(key)
             node.delete()
-        await self.storage_remove(*keys)
+        await self._storage.remove(*keys)
diff --git a/gecaso/utils.py b/gecaso/utils.py
index 965c298..78654cb 100644
--- a/gecaso/utils.py
+++ b/gecaso/utils.py
@@ -1,3 +1,4 @@
+import pickle
 import inspect
 import functools
 
@@ -31,3 +32,20 @@ def asyncify(function):
         else:
             return function(*args, **kwargs)
     return wrap(function, new_function)
+
+
+def pack(value, **params):
+    """Packs value and params into a object which is then converted to
+    bytes using pickle library. Is used to simplify storaging because bytes
+    can bestored almost anywhere.
+    """
+    result = Namespace(value=value, params=params)
+    return pickle.dumps(result)
+
+
+def unpack(value):
+    """Unpacks bytes object packed with 'pack' function. Returns packed value
+    and parameters.
+    """
+    result = pickle.loads(value)
+    return result.value, result.params


Test patch:
diff --git a/tests/test_local.py b/tests/test_local.py
index 1f5df7e..b507bc6 100644
--- a/tests/test_local.py
+++ b/tests/test_local.py
@@ -7,30 +7,7 @@ import pytest
 import gecaso
 
 
-class LocalAsyncMemoryStorage(gecaso.BaseStorage):
-    def __init__(self):
-        self._storage = dict()
-
-    async def get(self, key):
-        value, params = self.unpack(self._storage[key])
-        return self.verified_get(value, **params)
-
-    async def set(self, key, value, ttl=None):
-        params = dict()
-        if ttl:
-            params['ttl'] = time.time() + ttl
-        self._storage[key] = self.pack(value, **params)
-
-    async def remove(self, *keys):
-        for key in keys:
-            self._storage.pop(key, None)
-
-    def vfunc_ttl(self, time_of_death):
-        return time_of_death > time.time()
-
-
-local_storage = gecaso.LocalMemoryStorage()
-local_async_storage = LocalAsyncMemoryStorage()
+local_storage = gecaso.MemoryStorage()
 
 
 def slow_echo(time_to_sleep):
@@ -43,7 +20,7 @@ async def slow_async_echo(time_to_sleep):
     return time_to_sleep
 
 
-@pytest.mark.parametrize("storage", [local_storage, local_async_storage])
+@pytest.mark.parametrize("storage", [local_storage])
 @pytest.mark.parametrize("echo_function", [slow_echo, slow_async_echo])
 @pytest.mark.parametrize("argument", [2])
 def test_local_function_cache(storage, echo_function, argument):
diff --git a/tests/test_lru.py b/tests/test_lru.py
index e4bec67..2691537 100644
--- a/tests/test_lru.py
+++ b/tests/test_lru.py
@@ -7,28 +7,6 @@ import pytest
 import gecaso
 
 
-class LocalAsyncMemoryStorage(gecaso.BaseStorage):
-    def __init__(self):
-        self._storage = dict()
-
-    async def get(self, key):
-        value, params = self.unpack(self._storage[key])
-        return self.verified_get(value, **params)
-
-    async def set(self, key, value, ttl=None):
-        params = dict()
-        if ttl:
-            params['ttl'] = time.time() + ttl
-        self._storage[key] = self.pack(value, **params)
-
-    async def remove(self, *keys):
-        for key in keys:
-            self._storage.pop(key, None)
-
-    def vfunc_ttl(self, time_of_death):
-        return time_of_death > time.time()
-
-
 def slow_echo(time_to_sleep):
     time.sleep(time_to_sleep)
     return time_to_sleep
@@ -39,11 +17,10 @@ async def slow_async_echo(time_to_sleep):
     return time_to_sleep
 
 
-local_storage = gecaso.LocalMemoryStorage()
-local_async_storage = LocalAsyncMemoryStorage()
+local_storage = gecaso.MemoryStorage()
 
 
-@pytest.mark.parametrize("storage", [local_storage, local_async_storage])
+@pytest.mark.parametrize("storage", [local_storage])
 @pytest.mark.parametrize("echo_func", [slow_echo, slow_async_echo])
 def test_lru_cache(storage, echo_func):
     lru_echo = gecaso.cached(gecaso.LRUStorage(storage, maxsize=4))(echo_func)