From 554ad061529a41f4ca3b520f4aa0ded8a10dcda4 Mon Sep 17 00:00:00 2001
From: Steven Loria <sloria1@gmail.com>
Date: Tue, 10 Dec 2013 11:46:19 -0600
Subject: [PATCH 01/17] Add RedisStorage (incomplete) and RedisQuerySet

Only supports inserting and querying
On branch redis
Changes to be committed:
  (use "git reset HEAD <file>..." to unstage)

	modified:   dev-requirements.txt
	modified:   modularodm/storage/__init__.py
	modified:   modularodm/storage/mongostorage.py
	modified:   modularodm/storage/picklestorage.py
	new file:   modularodm/storage/redisstorage.py
	modified:   tests/base.py
	new file:   tests/storage/__init__.py
	new file:   tests/storage/test_redisstorage.py
---
 dev-requirements.txt                |   1 +
 modularodm/storage/__init__.py      |   1 +
 modularodm/storage/mongostorage.py  |   3 +-
 modularodm/storage/picklestorage.py |   6 +-
 modularodm/storage/redisstorage.py  | 126 ++++++++++++++++++++++++++++
 tests/base.py                       |  25 +++++-
 tests/storage/__init__.py           |   0
 tests/storage/test_redisstorage.py  |  66 +++++++++++++++
 8 files changed, 222 insertions(+), 6 deletions(-)
 create mode 100644 modularodm/storage/redisstorage.py
 create mode 100644 tests/storage/__init__.py
 create mode 100644 tests/storage/test_redisstorage.py

diff --git a/dev-requirements.txt b/dev-requirements.txt
index 1e031e9..76e354d 100644
--- a/dev-requirements.txt
+++ b/dev-requirements.txt
@@ -3,3 +3,4 @@ tox
 wheel
 invoke
 sphinx
+redis
diff --git a/modularodm/storage/__init__.py b/modularodm/storage/__init__.py
index 4f2b46c..176cfab 100644
--- a/modularodm/storage/__init__.py
+++ b/modularodm/storage/__init__.py
@@ -2,3 +2,4 @@
 from .mongostorage import MongoStorage
 from .picklestorage import PickleStorage
 from .ephemeralstorage import EphemeralStorage
+from .redisstorage import RedisStorage
diff --git a/modularodm/storage/mongostorage.py b/modularodm/storage/mongostorage.py
index 562b7bf..9901259 100644
--- a/modularodm/storage/mongostorage.py
+++ b/modularodm/storage/mongostorage.py
@@ -56,6 +56,7 @@ def __init__(self, schema, cursor):
         super(MongoQuerySet, self).__init__(schema)
         self.data = cursor
 
+    # TODO: make this the default implementations of BaseQuerySet?
     def __getitem__(self, index, raw=False):
         super(MongoQuerySet, self).__getitem__(index)
         key = self.data[index][self.primary]
@@ -117,7 +118,7 @@ def _ensure_index(self, key):
 
     def __init__(self, db, collection):
         self.collection = collection
-        self.store = db[self.collection]
+        self.store = db[self.collection]  # a mongo collection
 
     def find(self, query=None, **kwargs):
         mongo_query = self._translate_query(query)
diff --git a/modularodm/storage/picklestorage.py b/modularodm/storage/picklestorage.py
index 7df7528..36a97a4 100644
--- a/modularodm/storage/picklestorage.py
+++ b/modularodm/storage/picklestorage.py
@@ -201,10 +201,10 @@ def _match(self, value, query):
         else:
             raise TypeError('Query must be a QueryGroup or Query object.')
 
-    def find(self, query=None, **kwargs):
+    def find(self, query=None, by_pk=False, **kwargs):
         """
         Return generator over query results. Takes optional
-        by_pk keyword argument; if true, return keys rather than
+        by_pk keyword argument; if True, return keys rather than
         values.
 
         """
@@ -214,7 +214,7 @@ def find(self, query=None, **kwargs):
         else:
             for key, value in self.store.items():
                 if self._match(value, query):
-                    if kwargs.get('by_pk'):
+                    if by_pk:
                         yield key
                     else:
                         yield value
diff --git a/modularodm/storage/redisstorage.py b/modularodm/storage/redisstorage.py
new file mode 100644
index 0000000..71e807c
--- /dev/null
+++ b/modularodm/storage/redisstorage.py
@@ -0,0 +1,126 @@
+# -*- coding: utf-8 -*-
+
+from .base import Storage
+from ..query.query import QueryGroup, RawQuery
+from ..query.queryset import BaseQuerySet
+from .picklestorage import operators
+
+
+class RedisQuerySet(BaseQuerySet):
+
+    def __init__(self, schema, data):
+        super(RedisQuerySet,  self).__init__(schema, data)
+        self.data = list(data)
+
+    def __getitem__(self, index, raw=False):
+        super(RedisQuerySet, self).__getitem__(index)
+        key = self.data[index][self.primary]
+        if raw:
+            return key
+        return self.schema.load(key)
+
+    def __iter__(self, raw=False):
+        keys = [obj[self.primary] for obj in self.data]
+        if raw:
+            return keys
+        return (self.schema.load(key) for key in keys)
+
+    def __len__(self):
+        return len(self.data)
+
+    count = __len__
+
+    def __repr__(self):
+        return "<RedisQuerySet: {0}>".format(repr(list(self.data)))
+
+
+class RedisStorage(Storage):
+    '''Storage backend for Redis. Requires redis-py.
+
+    Each record is stored as a Hash keyed by the string:
+        <collection>:<id>
+
+    In addition a set keyed by <collection>_keys stores a set of all primary
+    keys for the collection.
+
+    :param redis.Redis client: The ``redis.Redis`` object from redis-py.
+    :param str collection: The name of the collection, e.g. "user"
+    '''
+    QuerySet = RedisQuerySet
+
+    def __init__(self, client, collection):
+        self.client = client
+        self.collection = collection
+        #: Name of set that stores the primary keys for this collection
+        self._key_set = "{col}_keys".format(col=self.collection)
+
+    def get(self, primary_name, key):
+        """Get a record as a dictionary."""
+        record = self.client.hgetall("{col}:{pk}".format(col=self.collection, pk=key))
+        return record
+
+    def get_by_id(self, id):
+        return self.get(None, id)
+
+    def insert(self, primary_name, key, value):
+        '''Insert a new record.
+
+        :param str primary_name: Name of primary key
+        :param key: The value of the primary key
+        :param dict value: The dictionary of attribute:value pairs
+        '''
+        if primary_name not in value:
+            value = value.copy()
+            value[primary_name] = key
+        # Add to set of primary keys
+        self.client.sadd(self._key_set, key)
+        # <collection>:<primary_key> => Hash of attribute:value pairs
+        self.client.hmset("{col}:{pk}".format(col=self.collection, pk=key),
+                        value)
+        return None
+
+    def _match(self, name, query):
+        """Return whether the Hash named ``name`` matches the ``query``.
+        """
+        # TODO: Duplication of PickleStorage logic. Rethink.
+        if isinstance(query, QueryGroup):
+            matches = [self._match(name, node) for node in query.nodes]
+
+            if query.operator == 'and':
+                return all(matches)
+            elif query.operator == 'or':
+                return any(matches)
+            elif query.operator == 'not':
+                return not any(matches)
+            else:
+                raise ValueError('QueryGroup operator must be <and>, <or>, or <not>.')
+
+        elif isinstance(query, RawQuery):
+            attribute, operator, argument = \
+                query.attribute, query.operator, query.argument
+            attribute_value = self.client.hget(name, attribute)
+            # Use same operators as pickle storage
+            comp_function = operators[operator]
+            return comp_function(attribute_value, argument)
+        else:
+            raise TypeError('Query must be a QueryGroup or Query object.')
+
+    def find(self, query=None, by_pk=False):
+        """Return generator over query results. Takes optional
+        by_pk keyword argument; if True, return keys rather than
+        values.
+        """
+        if query is None:
+            # Yield every object in the collection
+            for primary_key in self.client.smembers(self._key_set):
+                yield self.get_by_id(primary_key)
+        else:
+            for primary_key in self.client.smembers(self._key_set):
+                # The hash name
+                name = "{col}:{pk}".format(col=self.collection, pk=primary_key)
+                if self._match(name, query):
+                    if by_pk:
+                        yield primary_key
+                    else:
+                        record = self.get_by_id(primary_key)
+                        yield record
diff --git a/tests/base.py b/tests/base.py
index 2786577..c0abc48 100644
--- a/tests/base.py
+++ b/tests/base.py
@@ -2,12 +2,15 @@
 import logging
 import inspect
 import os
-import pymongo
 import unittest
 import uuid
 
+import pymongo
+import redis
+
 from modularodm import StoredObject
-from modularodm.storage import MongoStorage, PickleStorage, EphemeralStorage
+from modularodm.storage import (MongoStorage, PickleStorage, EphemeralStorage,
+                                RedisStorage)
 
 logger = logging.getLogger(__name__)
 
@@ -83,6 +86,23 @@ def clean_up_storage(self):
             self.mongo_client.drop_collection(c)
 
 
+class RedisStorageMixin(object):
+    fixture_suffix = "Redis"
+
+    # DB Settings
+    DB_HOST = os.environ.get("REDIS_HOST", 'localhost')
+    DB_PORT = os.environ.get("REDIS_PORT", 6379)
+
+    client = redis.Redis(host=DB_HOST, port=DB_PORT)
+
+    def make_storage(self):
+        collection = str(uuid.uuid4())[:8]
+        return RedisStorage(client=self.client, collection=collection)
+
+    def clean_up_storage(self):
+        self.client.flushall()
+
+
 class MultipleBackendMeta(type):
     def __new__(mcs, name, bases, dct):
 
@@ -101,6 +121,7 @@ def __new__(mcs, name, bases, dct):
             PickleStorageMixin,
             MongoStorageMixin,
             EphemeralStorageMixin,
+            RedisStorageMixin
         ):
             new_name = '{}{}'.format(name, mixin.fixture_suffix)
             frame.f_globals[new_name] = type.__new__(
diff --git a/tests/storage/__init__.py b/tests/storage/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/tests/storage/test_redisstorage.py b/tests/storage/test_redisstorage.py
new file mode 100644
index 0000000..fed57bd
--- /dev/null
+++ b/tests/storage/test_redisstorage.py
@@ -0,0 +1,66 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+import os
+import unittest
+from nose.tools import *  # PEP8 asserts
+
+import redis
+
+from modularodm import StoredObject, fields
+from modularodm.storage import RedisStorage
+from modularodm.query.querydialect import DefaultQueryDialect as Q
+
+class Person(StoredObject):
+    _meta = {"optimistic": True}
+    _id = fields.StringField(primary=True, index=True)
+    name = fields.StringField(required=True)
+
+    def __repr__(self):
+        return "<Person: {0}>".format(self.name)
+
+class TestRedisStorage(unittest.TestCase):
+
+    # DB Settings
+    DB_HOST = os.environ.get("REDIS_HOST", 'localhost')
+    DB_PORT = os.environ.get("REDIS_PORT", 6379)
+
+    client = redis.Redis(host=DB_HOST, port=DB_PORT)
+
+    def setUp(self):
+        Person.set_storage(RedisStorage(client=self.client, collection='people'))
+
+    def tearDown(self):
+        self.client.flushall()
+
+    def test_insert(self):
+        p = Person(name="Foo")
+        p.save()
+        # has an _id
+        assert_true(p._id)
+
+    def test_load(self):
+        p = Person(name="Foo")
+        p.save()
+        retrieved = Person.load(p._id)
+        assert_equal(p, retrieved)
+
+    def test_find_all(self):
+        for i in range(5):
+            p = Person(name="foo".format(i))
+            p.save()
+        all_people = Person.find()
+        assert_equal(len(all_people), 5)
+        assert False
+        assert_equal(all_people[0].name, 'foo')
+
+    def test_find(self):
+        p = Person(name="Foo")
+        p.save()
+        p2 = Person(name="Bar")
+        p2.save()
+        retrieved = Person.find(Q("name", "eq", "Foo"))
+        assert_in(p, retrieved)
+        assert_not_in(p2, retrieved)
+
+if __name__ == '__main__':
+    unittest.main()

From d2570c4a9911a17b9cef06a9c257ba2d1ebe661d Mon Sep 17 00:00:00 2001
From: Steven Loria <sloria1@gmail.com>
Date: Tue, 10 Dec 2013 12:08:21 -0600
Subject: [PATCH 02/17] Update README with instructions for redis

---
 README.rst | 18 +++++++++++++-----
 tasks.py   | 13 +++++++++++--
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/README.rst b/README.rst
index 8b0a3bb..bd7db68 100644
--- a/README.rst
+++ b/README.rst
@@ -5,7 +5,7 @@ modular-odm
 .. image:: https://badge.fury.io/py/modular-odm.png
     :target: http://badge.fury.io/py/modular-odm
 
-A database-agnostic Document-Object Mapper for Python.
+A Document-Object Mapper with support for multiple NoSQL backends.
 
 
 Install
@@ -91,10 +91,10 @@ TODO
 Development
 ===========
 
-Tests require `nose <http://nose.readthedocs.org/en/latest/>`_, `invoke <http://docs.pyinvoke.org/en/latest/>`_, and MongoDB.
+Tests require `nose <http://nose.readthedocs.org/en/latest/>`_, `invoke <http://docs.pyinvoke.org/en/latest/>`_, MongoDB, and redis.
 
-Installing MongoDB
-------------------
+Installing Dependencies
+-----------------------
 
 If you are on MacOSX with `homebrew <http://brew.sh/>`_, run
 
@@ -102,15 +102,23 @@ If you are on MacOSX with `homebrew <http://brew.sh/>`_, run
 
     $ brew update
     $ brew install mongodb
+    $ brew install redis
+
+Then install the Python development requirements with
+
+.. code-block:: bash
+
+    $ pip install -r dev-requirements.txt
 
 Running Tests
 -------------
 
-To start mongodb, run
+You must have both a mongo and redis server running to execute the tests.
 
 .. code-block:: bash
 
     $ invoke mongo
+    $ invoke redis
 
 Run all tests with
 
diff --git a/tasks.py b/tasks.py
index 82cfb39..6c80e63 100644
--- a/tasks.py
+++ b/tasks.py
@@ -6,14 +6,23 @@
 build_dir = os.path.join(docs_dir, '_build')
 
 @task
-def mongo(daemon=False, port=20771):
+def mongo(daemon=False):
     '''Run the mongod process.
     '''
+    port = os.environ.get('MONGO_PORT', 20771)
     cmd = "mongod --port {0}".format(port)
     if daemon:
         cmd += " --fork"
     run(cmd)
 
+
+@task
+def redis():
+    port = os.environ.get("REDIS_PORT", 6379)
+    cmd = "redis-server --port {0}".format(port)
+    run(cmd)
+
+
 @task
 def test(coverage=False, browse=False):
     command = "nosetests"
@@ -27,7 +36,7 @@ def test(coverage=False, browse=False):
 def clean():
     run("rm -rf build")
     run("rm -rf dist")
-    run("rm -rf marshmallow.egg-info")
+    run("rm -rf modularodm.egg-info")
     clean_docs()
     print("Cleaned up.")
 

From 7b82a9e77972727858abefa67c4069977a4395cd Mon Sep 17 00:00:00 2001
From: Steven Loria <sloria1@gmail.com>
Date: Tue, 10 Dec 2013 12:59:13 -0600
Subject: [PATCH 03/17] Add default implementation of find_one

---
 modularodm/storage/base.py         | 16 ++++++++++++---
 modularodm/storage/redisstorage.py |  3 +++
 tests/storage/test_redisstorage.py | 31 ++++++++++++++++++++++++++----
 3 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/modularodm/storage/base.py b/modularodm/storage/base.py
index 62354cf..c2bbcbd 100644
--- a/modularodm/storage/base.py
+++ b/modularodm/storage/base.py
@@ -4,6 +4,7 @@
 from functools import wraps
 import itertools
 
+from modularodm import exceptions
 from ..translators import DefaultTranslator
 
 class KeyExistsException(Exception): pass
@@ -173,11 +174,20 @@ def remove(self, key):
     def flush(self):
         raise NotImplementedError
 
-    def find_one(self, query=None, **kwargs):
-        raise NotImplementedError
-
     def find(self, query=None, **kwargs):
         raise NotImplementedError
 
+    def find_one(self, query=None, **kwargs):
+        results = list(self.find(query))
+        if len(results) == 1:
+            return results[0]
+        elif len(results) == 0:
+            raise exceptions.NoResultsFound()
+        else:
+            raise exceptions.MultipleResultsFound(
+                'Query for find_one must return exactly one result; '
+                'returned {0}'.format(len(results))
+            )
+
     def __repr__(self):
         return str(self.store)
diff --git a/modularodm/storage/redisstorage.py b/modularodm/storage/redisstorage.py
index 71e807c..5099594 100644
--- a/modularodm/storage/redisstorage.py
+++ b/modularodm/storage/redisstorage.py
@@ -124,3 +124,6 @@ def find(self, query=None, by_pk=False):
                     else:
                         record = self.get_by_id(primary_key)
                         yield record
+
+    def __repr__(self):
+        return "<RedisStorage: {0!r}>".format(self.collection)
diff --git a/tests/storage/test_redisstorage.py b/tests/storage/test_redisstorage.py
index fed57bd..a63d313 100644
--- a/tests/storage/test_redisstorage.py
+++ b/tests/storage/test_redisstorage.py
@@ -9,6 +9,7 @@
 from modularodm import StoredObject, fields
 from modularodm.storage import RedisStorage
 from modularodm.query.querydialect import DefaultQueryDialect as Q
+from modularodm import exceptions
 
 class Person(StoredObject):
     _meta = {"optimistic": True}
@@ -25,9 +26,7 @@ class TestRedisStorage(unittest.TestCase):
     DB_PORT = os.environ.get("REDIS_PORT", 6379)
 
     client = redis.Redis(host=DB_HOST, port=DB_PORT)
-
-    def setUp(self):
-        Person.set_storage(RedisStorage(client=self.client, collection='people'))
+    Person.set_storage(RedisStorage(client=client, collection='people'))
 
     def tearDown(self):
         self.client.flushall()
@@ -50,7 +49,6 @@ def test_find_all(self):
             p.save()
         all_people = Person.find()
         assert_equal(len(all_people), 5)
-        assert False
         assert_equal(all_people[0].name, 'foo')
 
     def test_find(self):
@@ -62,5 +60,30 @@ def test_find(self):
         assert_in(p, retrieved)
         assert_not_in(p2, retrieved)
 
+    def test_find_one(self):
+        p = Person(name="Foo")
+        p.save()
+        p2 = Person(name="Bar")
+        p2.save()
+        retrieved = Person.find_one(Q("name", "eq", "Foo"))
+        assert_equal(p, retrieved)
+
+    def test_find_one_raises_error_if_no_records_found(self):
+        p = Person(name="Foo")
+        p.save()
+        assert_raises(exceptions.NoResultsFound,
+            lambda: Person.find_one(Q("name", "eq", "Bar")))
+
+    def test_find_one_raises_error_if_multiple_records_found(self):
+        p = Person(name="Foo")
+        p.save()
+        p2 = Person(name="Foo")
+        p2.save()
+        assert_raises(exceptions.MultipleResultsFound,
+                    lambda: Person.find_one(Q("name", "eq", "Foo")))
+
+    def test_repr(self):
+        assert_equal(repr(Person._storage[0]), "<RedisStorage: 'people'>")
+
 if __name__ == '__main__':
     unittest.main()

From c87eca439ac864febbe15481d771d2f1f50c6beb Mon Sep 17 00:00:00 2001
From: Steven Loria <sloria1@gmail.com>
Date: Tue, 10 Dec 2013 14:59:37 -0600
Subject: [PATCH 04/17] Use get_key for getting a redis key

---
 modularodm/storage/redisstorage.py | 11 +++++++----
 tests/storage/test_redisstorage.py | 10 ++++++++--
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/modularodm/storage/redisstorage.py b/modularodm/storage/redisstorage.py
index 5099594..61f6a23 100644
--- a/modularodm/storage/redisstorage.py
+++ b/modularodm/storage/redisstorage.py
@@ -56,12 +56,16 @@ def __init__(self, client, collection):
 
     def get(self, primary_name, key):
         """Get a record as a dictionary."""
-        record = self.client.hgetall("{col}:{pk}".format(col=self.collection, pk=key))
+        record = self.client.hgetall(self.get_key(key))
         return record
 
     def get_by_id(self, id):
         return self.get(None, id)
 
+    def get_key(self, pk):
+        """Get the redis key for a given primary key."""
+        return u"{0}:{1}".format(self.collection, pk)
+
     def insert(self, primary_name, key, value):
         '''Insert a new record.
 
@@ -75,8 +79,7 @@ def insert(self, primary_name, key, value):
         # Add to set of primary keys
         self.client.sadd(self._key_set, key)
         # <collection>:<primary_key> => Hash of attribute:value pairs
-        self.client.hmset("{col}:{pk}".format(col=self.collection, pk=key),
-                        value)
+        self.client.hmset(self.get_key(key), value)
         return None
 
     def _match(self, name, query):
@@ -117,7 +120,7 @@ def find(self, query=None, by_pk=False):
         else:
             for primary_key in self.client.smembers(self._key_set):
                 # The hash name
-                name = "{col}:{pk}".format(col=self.collection, pk=primary_key)
+                name = self.get_key(primary_key)
                 if self._match(name, query):
                     if by_pk:
                         yield primary_key
diff --git a/tests/storage/test_redisstorage.py b/tests/storage/test_redisstorage.py
index a63d313..520bd8e 100644
--- a/tests/storage/test_redisstorage.py
+++ b/tests/storage/test_redisstorage.py
@@ -26,7 +26,8 @@ class TestRedisStorage(unittest.TestCase):
     DB_PORT = os.environ.get("REDIS_PORT", 6379)
 
     client = redis.Redis(host=DB_HOST, port=DB_PORT)
-    Person.set_storage(RedisStorage(client=client, collection='people'))
+    store = RedisStorage(client=client, collection='people')
+    Person.set_storage(store)
 
     def tearDown(self):
         self.client.flushall()
@@ -83,7 +84,12 @@ def test_find_one_raises_error_if_multiple_records_found(self):
                     lambda: Person.find_one(Q("name", "eq", "Foo")))
 
     def test_repr(self):
-        assert_equal(repr(Person._storage[0]), "<RedisStorage: 'people'>")
+        assert_equal(repr(self.store), "<RedisStorage: 'people'>")
+
+
+    def test_get_key(self):
+        assert_equal(self.store.get_key('abc123'), "people:abc123")
+
 
 if __name__ == '__main__':
     unittest.main()

From bf999bb372e42bfb3d0c406f46ef70478307f76e Mon Sep 17 00:00:00 2001
From: Steven Loria <sloria1@gmail.com>
Date: Tue, 10 Dec 2013 16:26:51 -0600
Subject: [PATCH 05/17] Imlement RedisStorage::remove

On branch redis
Changes to be committed:
  (use "git reset HEAD^1 <file>..." to unstage)

	modified:   modularodm/storage/picklestorage.py
	modified:   modularodm/storage/redisstorage.py
	modified:   tests/storage/test_redisstorage.py
---
 modularodm/storage/picklestorage.py |  2 +-
 modularodm/storage/redisstorage.py  | 54 ++++++++++++++++++++----
 tests/storage/test_redisstorage.py  | 65 ++++++++++++++++++++++-------
 3 files changed, 99 insertions(+), 22 deletions(-)

diff --git a/modularodm/storage/picklestorage.py b/modularodm/storage/picklestorage.py
index 36a97a4..6a79d10 100644
--- a/modularodm/storage/picklestorage.py
+++ b/modularodm/storage/picklestorage.py
@@ -146,7 +146,7 @@ def get(self, primary_name, key):
             return copy.deepcopy(data)
 
     def _remove_by_pk(self, key, flush=True):
-        """Retrieve value from store.
+        """Remove value from store.
 
         :param key: Key
 
diff --git a/modularodm/storage/redisstorage.py b/modularodm/storage/redisstorage.py
index 61f6a23..e45ef44 100644
--- a/modularodm/storage/redisstorage.py
+++ b/modularodm/storage/redisstorage.py
@@ -54,18 +54,26 @@ def __init__(self, client, collection):
         #: Name of set that stores the primary keys for this collection
         self._key_set = "{col}_keys".format(col=self.collection)
 
+    def get_key_set(self):
+        """Return the set of primary keys from the store."""
+        return self.client.smembers(self._key_set)
+
+    def get_key(self, pk):
+        """Get the redis key for a given primary key."""
+        return u"{0}:{1}".format(self.collection, pk)
+
     def get(self, primary_name, key):
-        """Get a record as a dictionary."""
+        """Get a record as a dictionary.
+
+        :param primary_name: The name of the primary key.
+        :param key: The value of the primary key
+        """
         record = self.client.hgetall(self.get_key(key))
         return record
 
     def get_by_id(self, id):
         return self.get(None, id)
 
-    def get_key(self, pk):
-        """Get the redis key for a given primary key."""
-        return u"{0}:{1}".format(self.collection, pk)
-
     def insert(self, primary_name, key, value):
         '''Insert a new record.
 
@@ -115,10 +123,13 @@ def find(self, query=None, by_pk=False):
         """
         if query is None:
             # Yield every object in the collection
-            for primary_key in self.client.smembers(self._key_set):
+            for primary_key in self.get_key_set():
+                print(primary_key)
+                if by_pk:
+                    yield primary_key
                 yield self.get_by_id(primary_key)
         else:
-            for primary_key in self.client.smembers(self._key_set):
+            for primary_key in self.get_key_set():
                 # The hash name
                 name = self.get_key(primary_key)
                 if self._match(name, query):
@@ -128,5 +139,34 @@ def find(self, query=None, by_pk=False):
                         record = self.get_by_id(primary_key)
                         yield record
 
+    def _remove_from_key_set(self, *keys):
+        """Remove primary keys from key set.
+
+        Redis doesn't support removing arbitrary values from a set
+        so overwrite the key_set with the difference betwen the
+        current key_set and the set of keys to remove.
+
+        :param keys: The primary keys to remove
+        """
+        tmp_name = "__modm_tmp__"
+        self.client.sadd(tmp_name, *keys)
+        self.client.sdiffstore(self._key_set, self._key_set, tmp_name)
+        self.client.delete(tmp_name)
+        return None
+
+    def remove(self, *query):
+        # Iterator of primary keys
+        keys_to_remove = list(self.find(*query, by_pk=True))
+        # List of redis keys
+        redis_keys = [self.get_key(key) for key in keys_to_remove]
+        # Remove keys
+        self.client.delete(*redis_keys)
+        self._remove_from_key_set(*keys_to_remove)
+        return None
+
+    def flush(self):
+        pass
+
+
     def __repr__(self):
         return "<RedisStorage: {0!r}>".format(self.collection)
diff --git a/tests/storage/test_redisstorage.py b/tests/storage/test_redisstorage.py
index 520bd8e..78f1625 100644
--- a/tests/storage/test_redisstorage.py
+++ b/tests/storage/test_redisstorage.py
@@ -1,6 +1,7 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
 import os
+import random
 import unittest
 from nose.tools import *  # PEP8 asserts
 
@@ -11,10 +12,13 @@
 from modularodm.query.querydialect import DefaultQueryDialect as Q
 from modularodm import exceptions
 
+random.seed(1)
+
 class Person(StoredObject):
     _meta = {"optimistic": True}
     _id = fields.StringField(primary=True, index=True)
     name = fields.StringField(required=True)
+    age = fields.IntegerField(required=False)
 
     def __repr__(self):
         return "<Person: {0}>".format(self.name)
@@ -29,14 +33,32 @@ class TestRedisStorage(unittest.TestCase):
     store = RedisStorage(client=client, collection='people')
     Person.set_storage(store)
 
+    def setUp(self):
+        self.p1 = Person(name="Foo")
+        self.p1.save()
+        self.p2 = Person(name="Bar")
+        self.p2.save()
+        self.p3 = Person(name="Baz")
+        self.p3.save()
+
     def tearDown(self):
         self.client.flushall()
 
     def test_insert(self):
-        p = Person(name="Foo")
+        self.store.insert("_id", "abc123", {"name": "Steve", "age": 23})
+        # Sets key => hash of attributes
+        name = self.client.hget("people:abc123", "name")
+        assert_equal(name, "Steve")
+        age = int(self.client.hget("people:abc123", "age"))
+        assert_equal(age, 23)
+
+    def test_create_stored_object(self):
+        p = Person(name="Foo", age=23)
         p.save()
         # has an _id
         assert_true(p._id)
+        assert_equal(p.name, "Foo")
+        assert_equal(p.age, 23)
 
     def test_load(self):
         p = Person(name="Foo")
@@ -45,6 +67,7 @@ def test_load(self):
         assert_equal(p, retrieved)
 
     def test_find_all(self):
+        self.client.flushall()
         for i in range(5):
             p = Person(name="foo".format(i))
             p.save()
@@ -53,29 +76,30 @@ def test_find_all(self):
         assert_equal(all_people[0].name, 'foo')
 
     def test_find(self):
-        p = Person(name="Foo")
-        p.save()
-        p2 = Person(name="Bar")
-        p2.save()
         retrieved = Person.find(Q("name", "eq", "Foo"))
-        assert_in(p, retrieved)
-        assert_not_in(p2, retrieved)
+        assert_in(self.p1, retrieved)
+        assert_not_in(self.p2, retrieved)
+
+    def test_find_by_pk(self):
+        pks = list(self.store.find(by_pk=True))
+        for each in Person.find():
+            assert_in(each._primary_key, pks)
+
+        pks = list(self.store.find(Q("name", "eq", self.p1.name), by_pk=True))
+        assert_in(self.p1._primary_key, pks)
 
     def test_find_one(self):
-        p = Person(name="Foo")
-        p.save()
-        p2 = Person(name="Bar")
-        p2.save()
         retrieved = Person.find_one(Q("name", "eq", "Foo"))
-        assert_equal(p, retrieved)
+        assert_equal(self.p1, retrieved)
 
     def test_find_one_raises_error_if_no_records_found(self):
         p = Person(name="Foo")
         p.save()
         assert_raises(exceptions.NoResultsFound,
-            lambda: Person.find_one(Q("name", "eq", "Bar")))
+            lambda: Person.find_one(Q("name", "eq", "notfound")))
 
     def test_find_one_raises_error_if_multiple_records_found(self):
+        self.client.flushall()
         p = Person(name="Foo")
         p.save()
         p2 = Person(name="Foo")
@@ -86,10 +110,23 @@ def test_find_one_raises_error_if_multiple_records_found(self):
     def test_repr(self):
         assert_equal(repr(self.store), "<RedisStorage: 'people'>")
 
-
     def test_get_key(self):
         assert_equal(self.store.get_key('abc123'), "people:abc123")
 
+    def test_remove(self):
+        all_people = list(Person.find())
+        assert_in(self.p1, all_people)
+        Person.remove_one(self.p1._primary_key)
+        all_people = list(Person.find())
+        assert_not_in(self.p1, all_people)
+        # Keys were removed
+        assert_not_in(self.p1._primary_key, self.store.get_key_set())
+        redis_key = self.store.get_key(self.p1._primary_key)
+        assert_not_in(redis_key, self.client.keys())
+
+    def test_get_key_set(self):
+        key_set = self.client.smembers("people_keys")
+        assert_equal(self.store.get_key_set(), key_set)
 
 if __name__ == '__main__':
     unittest.main()

From 36a33a519f5672683e002c393a360fb8e89231e0 Mon Sep 17 00:00:00 2001
From: Steven Loria <sloria1@gmail.com>
Date: Tue, 10 Dec 2013 20:31:11 -0600
Subject: [PATCH 06/17] Implement RedisStorage::update

---
 modularodm/storage/redisstorage.py | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/modularodm/storage/redisstorage.py b/modularodm/storage/redisstorage.py
index e45ef44..74f0523 100644
--- a/modularodm/storage/redisstorage.py
+++ b/modularodm/storage/redisstorage.py
@@ -124,7 +124,6 @@ def find(self, query=None, by_pk=False):
         if query is None:
             # Yield every object in the collection
             for primary_key in self.get_key_set():
-                print(primary_key)
                 if by_pk:
                     yield primary_key
                 yield self.get_by_id(primary_key)
@@ -164,6 +163,17 @@ def remove(self, *query):
         self._remove_from_key_set(*keys_to_remove)
         return None
 
+    def update(self, query, data):
+        """Update multiple records in the store.
+
+        :param query: The query object.
+        :param dict data: Attribute:value pairs.
+        """
+        for primary_key in self.find(query, by_pk=True):
+            redis_key = self.get_key(primary_key)
+            self.client.hmset(redis_key, data)
+        return None
+
     def flush(self):
         pass
 

From 51a75ccf663bfc9c76e6c30992f2e196ddda0d30 Mon Sep 17 00:00:00 2001
From: Steven Loria <sloria1@gmail.com>
Date: Wed, 11 Dec 2013 15:54:20 -0600
Subject: [PATCH 07/17] Implement RedisQuerySet sorting

---
 modularodm/storage/redisstorage.py |  8 ++++
 tests/storage/test_redisstorage.py | 62 +++++++++++++++++++++++++++---
 2 files changed, 64 insertions(+), 6 deletions(-)

diff --git a/modularodm/storage/redisstorage.py b/modularodm/storage/redisstorage.py
index 74f0523..0f53fbb 100644
--- a/modularodm/storage/redisstorage.py
+++ b/modularodm/storage/redisstorage.py
@@ -33,6 +33,14 @@ def __len__(self):
     def __repr__(self):
         return "<RedisQuerySet: {0}>".format(repr(list(self.data)))
 
+    def sort(self, *keys):
+        """Sort data by keys."""
+        for key in keys[::-1]:
+            reverse = key.startswith("-")
+            sort_key = key.lstrip("-")
+            self.data = sorted(self.data, key=lambda rec: rec[sort_key], reverse=reverse)
+        return self
+
 
 class RedisStorage(Storage):
     '''Storage backend for Redis. Requires redis-py.
diff --git a/tests/storage/test_redisstorage.py b/tests/storage/test_redisstorage.py
index 78f1625..4e15edc 100644
--- a/tests/storage/test_redisstorage.py
+++ b/tests/storage/test_redisstorage.py
@@ -9,11 +9,13 @@
 
 from modularodm import StoredObject, fields
 from modularodm.storage import RedisStorage
+from modularodm.storage.redisstorage import RedisQuerySet
 from modularodm.query.querydialect import DefaultQueryDialect as Q
 from modularodm import exceptions
 
 random.seed(1)
 
+
 class Person(StoredObject):
     _meta = {"optimistic": True}
     _id = fields.StringField(primary=True, index=True)
@@ -21,10 +23,9 @@ class Person(StoredObject):
     age = fields.IntegerField(required=False)
 
     def __repr__(self):
-        return "<Person: {0}>".format(self.name)
-
-class TestRedisStorage(unittest.TestCase):
+        return "<Person: {0!r}>".format(self.name)
 
+class RedisTestCase(unittest.TestCase):
     # DB Settings
     DB_HOST = os.environ.get("REDIS_HOST", 'localhost')
     DB_PORT = os.environ.get("REDIS_PORT", 6379)
@@ -33,6 +34,12 @@ class TestRedisStorage(unittest.TestCase):
     store = RedisStorage(client=client, collection='people')
     Person.set_storage(store)
 
+    def tearDown(self):
+        self.client.flushall()
+
+
+class TestRedisStorage(RedisTestCase):
+
     def setUp(self):
         self.p1 = Person(name="Foo")
         self.p1.save()
@@ -41,9 +48,6 @@ def setUp(self):
         self.p3 = Person(name="Baz")
         self.p3.save()
 
-    def tearDown(self):
-        self.client.flushall()
-
     def test_insert(self):
         self.store.insert("_id", "abc123", {"name": "Steve", "age": 23})
         # Sets key => hash of attributes
@@ -128,5 +132,51 @@ def test_get_key_set(self):
         key_set = self.client.smembers("people_keys")
         assert_equal(self.store.get_key_set(), key_set)
 
+    def test_update(self):
+        query = Q("_id", "eq", self.p1._id)
+        self.store.update(query, {"name": "Boo"})
+        # Record as dict
+        record = self.client.hgetall(self.store.get_key(self.p1._id))
+        assert_equal(record['name'], "Boo")
+
+    def test_update_one_stored_object(self):
+        Person.update_one(self.p1, {"name": "Boo"})
+        assert_equal(self.p1.name, "Boo")
+
+    def test_update_multiple(self):
+        self.client.flushall()
+        recs = []
+        for _ in range(5):
+            p = Person(name="Foo")
+            recs.append(p)
+            p.save()
+        self.store.update(Q("name", "eq", "Foo"), {"name": "Boo"})
+        for rec in recs:
+            rec.reload()
+            assert_equal(rec.name, "Boo")
+
+
+class TestRedisQuerySet(RedisTestCase):
+
+    def setUp(self):
+        for i in range(5):
+            p = Person(name="Foo", age=i + 1)
+            p.save()
+        Person(name="Bar", age=6).save()
+        self.qs = Person.find()
+
+    def test_sort(self):
+        sorted_qs = self.qs.sort("age")
+        loaded_objects = [p for p in sorted_qs]
+        expected = sorted(list(Person.find()), key=lambda rec: rec.age)
+        assert_equal(loaded_objects, expected)
+
+    def test_sort_reversed(self):
+        sorted_qs = self.qs.sort("-age")
+        loaded_objects = [p for p in sorted_qs]
+        expected = sorted(list(Person.find()), key=lambda rec: rec.age, reverse=True)
+        assert_equal(loaded_objects, expected)
+
+
 if __name__ == '__main__':
     unittest.main()

From 91c3d6c1f345772a2ead3dc314fbf7407b4a6be6 Mon Sep 17 00:00:00 2001
From: Steven Loria <sloria1@gmail.com>
Date: Wed, 11 Dec 2013 16:17:47 -0600
Subject: [PATCH 08/17] Add limit and offset

---
 modularodm/storage/redisstorage.py | 10 ++++++++++
 tests/storage/test_redisstorage.py |  4 ++++
 2 files changed, 14 insertions(+)

diff --git a/modularodm/storage/redisstorage.py b/modularodm/storage/redisstorage.py
index 0f53fbb..783c8ef 100644
--- a/modularodm/storage/redisstorage.py
+++ b/modularodm/storage/redisstorage.py
@@ -41,6 +41,16 @@ def sort(self, *keys):
             self.data = sorted(self.data, key=lambda rec: rec[sort_key], reverse=reverse)
         return self
 
+    def offset(self, n):
+        """Return the queryset offset by ``n`` items."""
+        self.data = self.data[n:]
+        return self
+
+    def limit(self, n):
+        """Return the queryset limited to ``n`` items."""
+        self.data = self.data[:n]
+        return self
+
 
 class RedisStorage(Storage):
     '''Storage backend for Redis. Requires redis-py.
diff --git a/tests/storage/test_redisstorage.py b/tests/storage/test_redisstorage.py
index 4e15edc..349599b 100644
--- a/tests/storage/test_redisstorage.py
+++ b/tests/storage/test_redisstorage.py
@@ -177,6 +177,10 @@ def test_sort_reversed(self):
         expected = sorted(list(Person.find()), key=lambda rec: rec.age, reverse=True)
         assert_equal(loaded_objects, expected)
 
+    def test_limit(self):
+        limited = self.qs.sort("age").limit(3)
+        assert_equal(len(limited), 3)
+
 
 if __name__ == '__main__':
     unittest.main()

From b88441779bf07e4295419462c601fd539155be94 Mon Sep 17 00:00:00 2001
From: Steven Loria <sloria1@gmail.com>
Date: Wed, 11 Dec 2013 16:30:55 -0600
Subject: [PATCH 09/17] Refactor find()

---
 modularodm/storage/redisstorage.py | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/modularodm/storage/redisstorage.py b/modularodm/storage/redisstorage.py
index 783c8ef..59c66f9 100644
--- a/modularodm/storage/redisstorage.py
+++ b/modularodm/storage/redisstorage.py
@@ -59,7 +59,8 @@ class RedisStorage(Storage):
         <collection>:<id>
 
     In addition a set keyed by <collection>_keys stores a set of all primary
-    keys for the collection.
+    keys for the collection. As a result, the queryset returned by a
+    StoredObject's ``find()`` method will be unordered.
 
     :param redis.Redis client: The ``redis.Redis`` object from redis-py.
     :param str collection: The name of the collection, e.g. "user"
@@ -136,25 +137,20 @@ def _match(self, name, query):
 
     def find(self, query=None, by_pk=False):
         """Return generator over query results. Takes optional
-        by_pk keyword argument; if True, return keys rather than
+        by_pk keyword argument; if ``True``, return keys rather than
         values.
+
+        .. note:: The returned objects are unordered.
         """
         if query is None:
-            # Yield every object in the collection
-            for primary_key in self.get_key_set():
-                if by_pk:
-                    yield primary_key
-                yield self.get_by_id(primary_key)
+            # Generator with every object in the collection
+            return (pkey if by_pk else self.get_by_id(pkey)
+                    for pkey in self.get_key_set())
         else:
-            for primary_key in self.get_key_set():
-                # The hash name
-                name = self.get_key(primary_key)
-                if self._match(name, query):
-                    if by_pk:
-                        yield primary_key
-                    else:
-                        record = self.get_by_id(primary_key)
-                        yield record
+            # Generator with objects filtered by query
+            return (pkey if by_pk else self.get_by_id(pkey)
+                    for pkey in self.get_key_set()
+                    if self._match(self.get_key(pkey), query))
 
     def _remove_from_key_set(self, *keys):
         """Remove primary keys from key set.

From a911f1edb13e3ebb247e4a5c1e6df1ea73076525 Mon Sep 17 00:00:00 2001
From: Steven Loria <sloria1@gmail.com>
Date: Wed, 11 Dec 2013 18:16:07 -0600
Subject: [PATCH 10/17] Serialize values to json for redis storage

---
 modularodm/storage/redisstorage.py | 38 ++++++++++++++++++++++++++----
 tests/storage/test_redisstorage.py | 12 ++++++----
 2 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/modularodm/storage/redisstorage.py b/modularodm/storage/redisstorage.py
index 59c66f9..addf9b8 100644
--- a/modularodm/storage/redisstorage.py
+++ b/modularodm/storage/redisstorage.py
@@ -1,4 +1,6 @@
 # -*- coding: utf-8 -*-
+from bson import json_util
+import json
 
 from .base import Storage
 from ..query.query import QueryGroup, RawQuery
@@ -6,6 +8,18 @@
 from .picklestorage import operators
 
 
+def dumps(val):
+    '''Custom JSON serialization function that allows serialization of special
+    types, e.g. datetimes, UUIDs, etc.
+    '''
+    return json.dumps(val, default=json_util.default)
+
+
+def loads(val):
+    '''Custom JSON deserialization function that handles special types.'''
+    return json.loads(val, object_hook=json_util.object_hook)
+
+
 class RedisQuerySet(BaseQuerySet):
 
     def __init__(self, schema, data):
@@ -30,6 +44,12 @@ def __len__(self):
 
     count = __len__
 
+    def get_key(self, index):
+        return self.__getitem__(index, raw=True)
+
+    def get_keys(self):
+        return list(self.__iter__(raw=True))
+
     def __repr__(self):
         return "<RedisQuerySet: {0}>".format(repr(list(self.data)))
 
@@ -73,6 +93,12 @@ def __init__(self, client, collection):
         #: Name of set that stores the primary keys for this collection
         self._key_set = "{col}_keys".format(col=self.collection)
 
+    def to_storage(self, record):
+        return dict((k, dumps(v)) for k, v in record.iteritems())
+
+    def from_storage(self, record):
+        return dict((key, loads(val)) for key, val in record.iteritems())
+
     def get_key_set(self):
         """Return the set of primary keys from the store."""
         return self.client.smembers(self._key_set)
@@ -87,7 +113,8 @@ def get(self, primary_name, key):
         :param primary_name: The name of the primary key.
         :param key: The value of the primary key
         """
-        record = self.client.hgetall(self.get_key(key))
+        hash_vals = self.client.hgetall(self.get_key(key))
+        record = self.from_storage(hash_vals)
         return record
 
     def get_by_id(self, id):
@@ -106,7 +133,8 @@ def insert(self, primary_name, key, value):
         # Add to set of primary keys
         self.client.sadd(self._key_set, key)
         # <collection>:<primary_key> => Hash of attribute:value pairs
-        self.client.hmset(self.get_key(key), value)
+        storeable = self.to_storage(value)
+        self.client.hmset(self.get_key(key), storeable)
         return None
 
     def _match(self, name, query):
@@ -131,7 +159,7 @@ def _match(self, name, query):
             attribute_value = self.client.hget(name, attribute)
             # Use same operators as pickle storage
             comp_function = operators[operator]
-            return comp_function(attribute_value, argument)
+            return comp_function(loads(attribute_value), argument)
         else:
             raise TypeError('Query must be a QueryGroup or Query object.')
 
@@ -185,12 +213,12 @@ def update(self, query, data):
         """
         for primary_key in self.find(query, by_pk=True):
             redis_key = self.get_key(primary_key)
-            self.client.hmset(redis_key, data)
+            storeable = self.to_storage(data)
+            self.client.hmset(redis_key, storeable)
         return None
 
     def flush(self):
         pass
 
-
     def __repr__(self):
         return "<RedisStorage: {0!r}>".format(self.collection)
diff --git a/tests/storage/test_redisstorage.py b/tests/storage/test_redisstorage.py
index 349599b..090c23c 100644
--- a/tests/storage/test_redisstorage.py
+++ b/tests/storage/test_redisstorage.py
@@ -1,7 +1,9 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
 import os
+import json
 import random
+import datetime
 import unittest
 from nose.tools import *  # PEP8 asserts
 
@@ -21,6 +23,7 @@ class Person(StoredObject):
     _id = fields.StringField(primary=True, index=True)
     name = fields.StringField(required=True)
     age = fields.IntegerField(required=False)
+    created = fields.DateTimeField(default=datetime.datetime.utcnow)
 
     def __repr__(self):
         return "<Person: {0!r}>".format(self.name)
@@ -41,7 +44,7 @@ def tearDown(self):
 class TestRedisStorage(RedisTestCase):
 
     def setUp(self):
-        self.p1 = Person(name="Foo")
+        self.p1 = Person(name="Foo", age=12)
         self.p1.save()
         self.p2 = Person(name="Bar")
         self.p2.save()
@@ -52,7 +55,7 @@ def test_insert(self):
         self.store.insert("_id", "abc123", {"name": "Steve", "age": 23})
         # Sets key => hash of attributes
         name = self.client.hget("people:abc123", "name")
-        assert_equal(name, "Steve")
+        assert_equal(name, json.dumps("Steve"))
         age = int(self.client.hget("people:abc123", "age"))
         assert_equal(age, 23)
 
@@ -134,10 +137,11 @@ def test_get_key_set(self):
 
     def test_update(self):
         query = Q("_id", "eq", self.p1._id)
-        self.store.update(query, {"name": "Boo"})
+        self.store.update(query, {"name": "Boo", 'age': 23})
         # Record as dict
         record = self.client.hgetall(self.store.get_key(self.p1._id))
-        assert_equal(record['name'], "Boo")
+        assert_equal(record['name'], json.dumps("Boo"))
+        assert_equal(record['age'],json.dumps(23))
 
     def test_update_one_stored_object(self):
         Person.update_one(self.p1, {"name": "Boo"})

From 8af22ccb6c88f9f7d77f2a4bad6b8d6fd2bfcc6d Mon Sep 17 00:00:00 2001
From: Steven Loria <sloria1@gmail.com>
Date: Wed, 11 Dec 2013 20:13:54 -0600
Subject: [PATCH 11/17] Fix updating pk of Redis StoredObject On branch redis
 Your branch is up-to-date with 'origin/redis'.

Changes to be committed:
  (use "git reset HEAD <file>..." to unstage)

	modified:   modularodm/storage/redisstorage.py
	modified:   tests/storage/test_redisstorage.py
---
 modularodm/storage/redisstorage.py | 18 +++++++++++++++---
 tests/storage/test_redisstorage.py | 10 ++++++++++
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/modularodm/storage/redisstorage.py b/modularodm/storage/redisstorage.py
index addf9b8..9afc5b8 100644
--- a/modularodm/storage/redisstorage.py
+++ b/modularodm/storage/redisstorage.py
@@ -94,9 +94,19 @@ def __init__(self, client, collection):
         self._key_set = "{col}_keys".format(col=self.collection)
 
     def to_storage(self, record):
+        """Convert a python dictionary to a dictionary that can be stored
+        in as a redis hash. Values must be serialized to JSON.
+
+        :param dict record: Dictionary representation of the record.
+        """
         return dict((k, dumps(v)) for k, v in record.iteritems())
 
     def from_storage(self, record):
+        """Convert a dictionary retrieved from the redis store to a native
+        Python dictionary.
+
+        :param dict record: A dictionary (hash) retrieved from the redis store.
+        """
         return dict((key, loads(val)) for key, val in record.iteritems())
 
     def get_key_set(self):
@@ -113,9 +123,11 @@ def get(self, primary_name, key):
         :param primary_name: The name of the primary key.
         :param key: The value of the primary key
         """
-        hash_vals = self.client.hgetall(self.get_key(key))
-        record = self.from_storage(hash_vals)
-        return record
+        hash_obj = self.client.hgetall(self.get_key(key))
+        if hash_obj:
+            return self.from_storage(hash_obj)
+        else:  # If HASH is empty, return None
+            return None
 
     def get_by_id(self, id):
         return self.get(None, id)
diff --git a/tests/storage/test_redisstorage.py b/tests/storage/test_redisstorage.py
index 090c23c..2397330 100644
--- a/tests/storage/test_redisstorage.py
+++ b/tests/storage/test_redisstorage.py
@@ -147,6 +147,16 @@ def test_update_one_stored_object(self):
         Person.update_one(self.p1, {"name": "Boo"})
         assert_equal(self.p1.name, "Boo")
 
+    def test_updating_pk(self):
+        p = Person(name="Steve")
+        old_key = p._primary_key
+        p._id = 'mykey'
+        p.save()
+        StoredObject._clear_caches()
+        assert_true(Person.load("mykey") is not None)
+        assert_true(Person.load(old_key) is None)
+        assert_equal(p._id, 'mykey')
+
     def test_update_multiple(self):
         self.client.flushall()
         recs = []

From ecb655c2670e3a31a6b9a9cb34ce1a51d5a72108 Mon Sep 17 00:00:00 2001
From: Steven Loria <sloria1@gmail.com>
Date: Wed, 11 Dec 2013 20:16:21 -0600
Subject: [PATCH 12/17] Rename to/from_storage to avoid ambiguity On branch
 redis Your branch is ahead of 'origin/redis' by 1 commit.   (use "git push"
 to publish your local commits)

Changes to be committed:
  (use "git reset HEAD <file>..." to unstage)

	modified:   modularodm/storage/redisstorage.py
---
 modularodm/storage/redisstorage.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/modularodm/storage/redisstorage.py b/modularodm/storage/redisstorage.py
index 9afc5b8..f2d26c1 100644
--- a/modularodm/storage/redisstorage.py
+++ b/modularodm/storage/redisstorage.py
@@ -93,7 +93,7 @@ def __init__(self, client, collection):
         #: Name of set that stores the primary keys for this collection
         self._key_set = "{col}_keys".format(col=self.collection)
 
-    def to_storage(self, record):
+    def from_native(self, record):
         """Convert a python dictionary to a dictionary that can be stored
         in as a redis hash. Values must be serialized to JSON.
 
@@ -101,7 +101,7 @@ def to_storage(self, record):
         """
         return dict((k, dumps(v)) for k, v in record.iteritems())
 
-    def from_storage(self, record):
+    def to_native(self, record):
         """Convert a dictionary retrieved from the redis store to a native
         Python dictionary.
 
@@ -125,7 +125,7 @@ def get(self, primary_name, key):
         """
         hash_obj = self.client.hgetall(self.get_key(key))
         if hash_obj:
-            return self.from_storage(hash_obj)
+            return self.to_native(hash_obj)
         else:  # If HASH is empty, return None
             return None
 
@@ -145,7 +145,7 @@ def insert(self, primary_name, key, value):
         # Add to set of primary keys
         self.client.sadd(self._key_set, key)
         # <collection>:<primary_key> => Hash of attribute:value pairs
-        storeable = self.to_storage(value)
+        storeable = self.from_native(value)
         self.client.hmset(self.get_key(key), storeable)
         return None
 
@@ -225,7 +225,7 @@ def update(self, query, data):
         """
         for primary_key in self.find(query, by_pk=True):
             redis_key = self.get_key(primary_key)
-            storeable = self.to_storage(data)
+            storeable = self.from_native(data)
             self.client.hmset(redis_key, storeable)
         return None
 

From 939a11ad40eb71ec3a4c1f191b259a4df6c1e31d Mon Sep 17 00:00:00 2001
From: Steven Loria <sloria1@gmail.com>
Date: Thu, 12 Dec 2013 14:42:07 -0600
Subject: [PATCH 13/17] Hack to handle datatypes returned by Redis which are
 different from the datatypes expected by an ODM query

---
 modularodm/storage/redisstorage.py | 15 ++++++++++++---
 modularodm/storedobject.py         |  2 ++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/modularodm/storage/redisstorage.py b/modularodm/storage/redisstorage.py
index f2d26c1..2278a70 100644
--- a/modularodm/storage/redisstorage.py
+++ b/modularodm/storage/redisstorage.py
@@ -169,9 +169,18 @@ def _match(self, name, query):
             attribute, operator, argument = \
                 query.attribute, query.operator, query.argument
             attribute_value = self.client.hget(name, attribute)
-            # Use same operators as pickle storage
             comp_function = operators[operator]
-            return comp_function(loads(attribute_value), argument)
+            # Datatype returned by redis-py might not be the type expected by a query
+            data_type = getattr(query, "datatype", None)
+            val = loads(attribute_value)
+            # Coerce val to be the correct datatype
+            if data_type:
+                if getattr(query, "use_list", False):
+                    val = [data_type(v) for v in val]
+                else:
+                    val = data_type(val)
+            # Use same operators as pickle storage
+            return comp_function(val, argument)
         else:
             raise TypeError('Query must be a QueryGroup or Query object.')
 
@@ -196,7 +205,7 @@ def _remove_from_key_set(self, *keys):
         """Remove primary keys from key set.
 
         Redis doesn't support removing arbitrary values from a set
-        so overwrite the key_set with the difference betwen the
+        so overwrite the key_set with the difference between the
         current key_set and the set of keys to remove.
 
         :param keys: The primary keys to remove
diff --git a/modularodm/storedobject.py b/modularodm/storedobject.py
index 0781336..5e92091 100644
--- a/modularodm/storedobject.py
+++ b/modularodm/storedobject.py
@@ -913,11 +913,13 @@ def _process_query(cls, query):
 
         if isinstance(query, RawQuery):
             field = cls._fields.get(query.attribute)
+            query.use_list = field._list
             if field is None:
                 return
             if field._is_foreign:
                 if getattr(query.argument, '_fields', None):
                     if field._is_abstract:
+                        query.datatype = tuple
                         query.argument = (
                             query.argument._primary_key,
                             query.argument._name,

From 7f8fd9c81287919fe036e133c358b9ccf0888bec Mon Sep 17 00:00:00 2001
From: Steven Loria <sloria1@gmail.com>
Date: Thu, 12 Dec 2013 15:14:15 -0600
Subject: [PATCH 14/17] Move dumps and loads to modularodm.utils

---
 modularodm/storage/redisstorage.py | 15 +--------------
 modularodm/utils.py                | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 14 deletions(-)
 create mode 100644 modularodm/utils.py

diff --git a/modularodm/storage/redisstorage.py b/modularodm/storage/redisstorage.py
index 2278a70..7a097ed 100644
--- a/modularodm/storage/redisstorage.py
+++ b/modularodm/storage/redisstorage.py
@@ -1,25 +1,12 @@
 # -*- coding: utf-8 -*-
-from bson import json_util
-import json
 
+from modularodm.utils import dumps, loads
 from .base import Storage
 from ..query.query import QueryGroup, RawQuery
 from ..query.queryset import BaseQuerySet
 from .picklestorage import operators
 
 
-def dumps(val):
-    '''Custom JSON serialization function that allows serialization of special
-    types, e.g. datetimes, UUIDs, etc.
-    '''
-    return json.dumps(val, default=json_util.default)
-
-
-def loads(val):
-    '''Custom JSON deserialization function that handles special types.'''
-    return json.loads(val, object_hook=json_util.object_hook)
-
-
 class RedisQuerySet(BaseQuerySet):
 
     def __init__(self, schema, data):
diff --git a/modularodm/utils.py b/modularodm/utils.py
new file mode 100644
index 0000000..0c798cf
--- /dev/null
+++ b/modularodm/utils.py
@@ -0,0 +1,15 @@
+# -*- coding: utf-8 -*-
+from bson import json_util
+import json
+
+
+def dumps(val):
+    '''Custom JSON serialization function that allows serialization of special
+    types, e.g. datetimes, UUIDs, etc.
+    '''
+    return json.dumps(val, default=json_util.default)
+
+
+def loads(val):
+    '''Custom JSON deserialization function that handles special types.'''
+    return json.loads(val, object_hook=json_util.object_hook)

From e27b9d88d8cc77a4a64d4218a565636239f06c78 Mon Sep 17 00:00:00 2001
From: Steven Loria <sloria1@gmail.com>
Date: Thu, 12 Dec 2013 15:20:17 -0600
Subject: [PATCH 15/17] Update README with redis instructions

---
 README.rst | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/README.rst b/README.rst
index bd7db68..59f2c40 100644
--- a/README.rst
+++ b/README.rst
@@ -16,8 +16,8 @@ Install
     $ pip install modular-odm
 
 
-Example Usage with MongoDB
-==========================
+Example Usage
+=============
 
 Defining Models
 ---------------
@@ -49,6 +49,8 @@ Defining Models
 Setting the Storage Backend
 ---------------------------
 
+For **MongoDB**:
+
 .. code-block:: python
 
     from pymongo import MongoClient
@@ -59,6 +61,21 @@ Setting the Storage Backend
     User.set_storage(storage.MongoStorage(db, collection="user"))
     Comment.set_storage(storage.MongoStorage(db, collection="comment"))
 
+
+For **Redis**:
+
+.. note::
+    To use modularodm with Redis, you must have `redis-py <https://github.com/andymccurdy/redis-py>`_ installed.
+
+.. code-block:: python
+
+    from redis import Redis
+    from modularodm import storage
+
+    db = redis.Redis()
+    User.set_storage(storage.RedisStorage(db, collection="user"))
+    Comment.set_storage(storage.RedisStorage(db, collection="comment"))
+
 Creating and Querying
 ---------------------
 

From ef3831a6a9c1ac3f1f56a5950041f32f70170189 Mon Sep 17 00:00:00 2001
From: Steven Loria <sloria1@gmail.com>
Date: Thu, 12 Dec 2013 15:25:45 -0600
Subject: [PATCH 16/17] Remove abstract implementation of repr

Wrongly assumed concrete implementations have a store attribute
---
 modularodm/storage/base.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/modularodm/storage/base.py b/modularodm/storage/base.py
index 385f0cc..5c84e31 100644
--- a/modularodm/storage/base.py
+++ b/modularodm/storage/base.py
@@ -209,6 +209,3 @@ def find_one(self, query=None, **kwargs):
                 'Query for find_one must return exactly one result; '
                 'returned {0}'.format(len(results))
             )
-
-    def __repr__(self):
-        return str(self.store)

From b5286dba870c7c9705859a70d1ae07952941b0c5 Mon Sep 17 00:00:00 2001
From: Steven Loria <sloria1@gmail.com>
Date: Sun, 15 Dec 2013 23:20:26 -0600
Subject: [PATCH 17/17] Move some redisstorage tests to multidatabase query
 tests

---
 .gitignore                           |  7 +---
 tests/queries/test_simple_queries.py | 51 +++++++++++++++++++++--
 tests/storage/test_redisstorage.py   | 61 ----------------------------
 3 files changed, 49 insertions(+), 70 deletions(-)

diff --git a/.gitignore b/.gitignore
index 9ef6d59..d9366d3 100644
--- a/.gitignore
+++ b/.gitignore
@@ -48,10 +48,7 @@ nosetests.xml
 # Coverage
 cover
 
-# OSF-specific
-#######################
-Site/Cache/*
-Site/Uploads/*
-
 # Readme build
 README.html
+
+dump.rdb
diff --git a/tests/queries/test_simple_queries.py b/tests/queries/test_simple_queries.py
index 5723df7..3db37b3 100644
--- a/tests/queries/test_simple_queries.py
+++ b/tests/queries/test_simple_queries.py
@@ -1,8 +1,10 @@
 # -*- coding: utf-8 -*-
 import logging
+import datetime
 
-from modularodm import exceptions, StoredObject
-from modularodm.fields import IntegerField
+from nose.tools import *  # PEP8 asserts
+
+from modularodm import fields, exceptions, StoredObject
 from modularodm.query.query import RawQuery as Q
 
 from tests.base import ModularOdmTestCase
@@ -14,9 +16,19 @@ class BasicQueryTestCase(ModularOdmTestCase):
 
     def define_objects(self):
         class Foo(StoredObject):
-            _id = IntegerField(primary=True)
+            _id = fields.IntegerField(primary=True)
+
+        class Person(StoredObject):
+            _meta = {"optimistic": True}
+            _id = fields.StringField(primary=True, index=True)
+            name = fields.StringField(required=True)
+            age = fields.IntegerField(required=False)
+            created = fields.DateTimeField(default=datetime.datetime.utcnow)
 
-        return Foo,
+            def __repr__(self):
+                return "<Person: {0!r}>".format(self.name)
+
+        return Foo, Person
 
     def set_up_objects(self):
         self.foos = []
@@ -26,6 +38,13 @@ def set_up_objects(self):
             foo.save()
             self.foos.append(foo)
 
+        self.p1 = self.Person(name="Foo", age=12)
+        self.p1.save()
+        self.p2 = self.Person(name="Bar")
+        self.p2.save()
+        self.p3 = self.Person(name="Baz")
+        self.p3.save()
+
     def test_load_by_pk(self):
         """ Given a known primary key, ``.get(pk)``  should return the object.
         """
@@ -279,3 +298,27 @@ def test_sort_sort_offset(self):
             [27, 28, 29]
         )
 
+    def test_find(self):
+        retrieved = self.Person.find(Q("name", "eq", "Foo"))
+        assert_in(self.p1, retrieved)
+        assert_not_in(self.p2, retrieved)
+
+    def test_create_stored_object(self):
+        p = self.Person(name="Foo", age=23)
+        p.save()
+        # has an _id
+        assert_true(p._id)
+        assert_equal(p.name, "Foo")
+        assert_equal(p.age, 23)
+
+    def test_update_one_stored_object(self):
+        self.Person.update_one(self.p1, {"name": "Boo"})
+        assert_equal(self.p1.name, "Boo")
+
+    def test_find_one_raises_error_if_no_records_found(self):
+        p = self.Person(name="Foo")
+        p2 = self.Person(name="Foo")
+        p.save()
+        p2.save()
+        assert_raises(exceptions.NoResultsFound,
+            lambda: self.Person.find_one(Q("name", "eq", "notfound")))
diff --git a/tests/storage/test_redisstorage.py b/tests/storage/test_redisstorage.py
index 2397330..be01c82 100644
--- a/tests/storage/test_redisstorage.py
+++ b/tests/storage/test_redisstorage.py
@@ -59,34 +59,6 @@ def test_insert(self):
         age = int(self.client.hget("people:abc123", "age"))
         assert_equal(age, 23)
 
-    def test_create_stored_object(self):
-        p = Person(name="Foo", age=23)
-        p.save()
-        # has an _id
-        assert_true(p._id)
-        assert_equal(p.name, "Foo")
-        assert_equal(p.age, 23)
-
-    def test_load(self):
-        p = Person(name="Foo")
-        p.save()
-        retrieved = Person.load(p._id)
-        assert_equal(p, retrieved)
-
-    def test_find_all(self):
-        self.client.flushall()
-        for i in range(5):
-            p = Person(name="foo".format(i))
-            p.save()
-        all_people = Person.find()
-        assert_equal(len(all_people), 5)
-        assert_equal(all_people[0].name, 'foo')
-
-    def test_find(self):
-        retrieved = Person.find(Q("name", "eq", "Foo"))
-        assert_in(self.p1, retrieved)
-        assert_not_in(self.p2, retrieved)
-
     def test_find_by_pk(self):
         pks = list(self.store.find(by_pk=True))
         for each in Person.find():
@@ -95,25 +67,6 @@ def test_find_by_pk(self):
         pks = list(self.store.find(Q("name", "eq", self.p1.name), by_pk=True))
         assert_in(self.p1._primary_key, pks)
 
-    def test_find_one(self):
-        retrieved = Person.find_one(Q("name", "eq", "Foo"))
-        assert_equal(self.p1, retrieved)
-
-    def test_find_one_raises_error_if_no_records_found(self):
-        p = Person(name="Foo")
-        p.save()
-        assert_raises(exceptions.NoResultsFound,
-            lambda: Person.find_one(Q("name", "eq", "notfound")))
-
-    def test_find_one_raises_error_if_multiple_records_found(self):
-        self.client.flushall()
-        p = Person(name="Foo")
-        p.save()
-        p2 = Person(name="Foo")
-        p2.save()
-        assert_raises(exceptions.MultipleResultsFound,
-                    lambda: Person.find_one(Q("name", "eq", "Foo")))
-
     def test_repr(self):
         assert_equal(repr(self.store), "<RedisStorage: 'people'>")
 
@@ -143,20 +96,6 @@ def test_update(self):
         assert_equal(record['name'], json.dumps("Boo"))
         assert_equal(record['age'],json.dumps(23))
 
-    def test_update_one_stored_object(self):
-        Person.update_one(self.p1, {"name": "Boo"})
-        assert_equal(self.p1.name, "Boo")
-
-    def test_updating_pk(self):
-        p = Person(name="Steve")
-        old_key = p._primary_key
-        p._id = 'mykey'
-        p.save()
-        StoredObject._clear_caches()
-        assert_true(Person.load("mykey") is not None)
-        assert_true(Person.load(old_key) is None)
-        assert_equal(p._id, 'mykey')
-
     def test_update_multiple(self):
         self.client.flushall()
         recs = []