Merge lp:~robru/bileto/no-more-sqlite-sessions into lp:bileto

[Robert Bruce Park]

Use JSON instead of Sqlite for storing sessions on disk.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:214
http://jenkins.qa.ubuntu.com/job/bileto-ci/13/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/bileto-ci/13/rebuild

review: Approve (continuous-integration)

[Robert Bruce Park (robru)]

Alright, this is looking good in staging, just needs test coverage.

[lp:~robru/bileto/no-more-sqlite-sessions]

215. By Robert Bruce Park on 2015-08-01

Note to self.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:215
http://jenkins.qa.ubuntu.com/job/bileto-ci/14/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/bileto-ci/14/rebuild

review: Needs Fixing (continuous-integration)

[lp:~robru/bileto/no-more-sqlite-sessions]

216. By Robert Bruce Park on 2015-08-01

Whitespace fix.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:216
http://jenkins.qa.ubuntu.com/job/bileto-ci/15/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/bileto-ci/15/rebuild

review: Approve (continuous-integration)

[lp:~robru/bileto/no-more-sqlite-sessions]

217. By Robert Bruce Park on 2015-08-03

Drop CallableAttributeProxy and PersistedObjectProxy due to disuse.

218. By Robert Bruce Park on 2015-08-03

Initial session tests.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:218
http://jenkins.qa.ubuntu.com/job/bileto-ci/16/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/bileto-ci/16/rebuild

review: Approve (continuous-integration)

[lp:~robru/bileto/no-more-sqlite-sessions]

219. By Robert Bruce Park on 2015-08-03

Simplify session implementation.

220. By Robert Bruce Park on 2015-08-03

MOAR simple.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:220
http://jenkins.qa.ubuntu.com/job/bileto-ci/17/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/bileto-ci/17/rebuild

review: Approve (continuous-integration)

[Robert Bruce Park (robru)]

Looking really good in staging, no more IO error tracebacks as far as I can see.

review: Approve

[lp:~robru/bileto/no-more-sqlite-sessions]

221. By Robert Bruce Park on 2015-08-03

Fail under 98% test coverage.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:221
http://jenkins.qa.ubuntu.com/job/bileto-ci/18/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/bileto-ci/18/rebuild

review: Approve (continuous-integration)

=== modified file 'Makefile'
--- Makefile	2015-06-16 17:47:00 +0000
+++ Makefile	2015-08-03 06:41:49 +0000
@@ -2,7 +2,7 @@
 	find -name '*.py' | xargs pyflakes3
 	find -name '*.py' | xargs pep8
 	python3 -m coverage run -m nose2
-	python3 -m coverage report --include="t*/*" --show-missing --fail-under=92
+	python3 -m coverage report --include="t*/*" --show-missing --fail-under=98
 
 devel:
 	./run.py

=== added file 'tests/test_sessions.py'
--- tests/test_sessions.py	1970-01-01 00:00:00 +0000
+++ tests/test_sessions.py	2015-08-03 06:41:49 +0000
@@ -0,0 +1,32 @@
+from pickle import dumps
+
+from tests.tests import BiletoTestCase
+
+from tickets.app import app
+from tickets.sessions import PickleSession
+
+
+class SessionTestCase(BiletoTestCase):
+    def test_save(self):
+        sesh = PickleSession(app.session_interface.directory, 'foo')
+        sesh['foo'] = 'bar'
+        with open(sesh.path, 'rb') as pickled:
+            self.assertEqual(pickled.read(), dumps(dict(foo='bar')))
+
+    def test_read(self):
+        sesh = PickleSession(app.session_interface.directory, 'bar')
+        with open(sesh.path, 'wb') as pickled:
+            pickled.write(dumps(dict(baz='qux')))
+        self.assertEqual(sesh['baz'], 'qux')
+        self.assertEqual(len(sesh), 1)
+
+    def test_setters(self):
+        # Initialize once just to get the path
+        sesh = PickleSession(app.session_interface.directory, 'grill')
+        with open(sesh.path, 'wb') as pickled:
+            pickled.write(dumps(dict(baz='qux')))
+        # Re-initialize with data in place.
+        sesh = PickleSession(app.session_interface.directory, 'grill')
+        sesh['foo'] = 'bar'
+        with open(sesh.path, 'rb') as pickled:
+            self.assertEqual(pickled.read(), dumps(dict(foo='bar', baz='qux')))

=== modified file 'tests/tests.py'
--- tests/tests.py	2015-06-02 21:26:29 +0000
+++ tests/tests.py	2015-08-03 06:41:49 +0000
@@ -2,6 +2,8 @@
 import unittest
 import tempfile
 
+from shutil import rmtree
+
 from tickets.app import app, db
 
 
@@ -10,6 +12,7 @@
 
     def setUp(self):
         """Create temp database."""
+        os.makedirs(app.session_interface.directory, exist_ok=True)
         self.db_fd, filename = tempfile.mkstemp()
         app.config['DATABASE'] = filename
         app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite:///' + filename
@@ -18,6 +21,7 @@
         db.create_all()
 
     def tearDown(self):
-        """Close & delete temp database."""
+        """Close & delete stuff."""
         os.close(self.db_fd)
         os.unlink(app.config['DATABASE'])
+        rmtree(app.session_interface.directory, ignore_errors=True)

=== modified file 'tickets/sessions.py'
--- tickets/sessions.py	2015-06-18 20:09:08 +0000
+++ tickets/sessions.py	2015-08-03 06:41:49 +0000
@@ -1,151 +1,83 @@
-# Author: Thiago Arruda
-# License: Public Domain
-# Source: http://flask.pocoo.org/snippets/86/
-
 import os
-import sqlite3
 
-from uuid import uuid4
-from pickle import dumps, loads
+from uuid import uuid1
+from pickle import UnpicklingError, dumps, loads
 from contextlib import suppress
 from collections import MutableMapping
 from flask.sessions import SessionInterface, SessionMixin
 
 
-class SqliteSession(MutableMapping, SessionMixin):
+class PickleSession(MutableMapping, SessionMixin):
     """Server-side session implementation.
 
-    Uses sqlite to achieve a disk-backed session such that multiple
+    Uses pickle to achieve a disk-backed session such that multiple
     worker processes can access the same session data.
     """
 
-    _create_sql = (
-        'CREATE TABLE IF NOT EXISTS session '
-        '('
-        '  key TEXT PRIMARY KEY,'
-        '  val BLOB'
-        ')'
-    )
-    _get_sql = 'SELECT val FROM session WHERE key = ?'
-    _set_sql = 'REPLACE INTO session (key, val) VALUES (?, ?)'
-    _del_sql = 'DELETE FROM session WHERE key = ?'
-    _ite_sql = 'SELECT key FROM session'
-    _len_sql = 'SELECT COUNT(*) FROM session'
-
     def __init__(self, directory, sid, *args, **kwargs):
         self.path = os.path.join(directory, sid)
         self.directory = directory
         self.sid = sid
-        self.modified = False
-        self.conn = None
-        if not os.path.exists(self.path):
-            with self._get_conn() as conn:
-                conn.execute(self._create_sql)
-                self.new = True
+        self.read()
 
     def __getitem__(self, key):
-        rv = None
-        with self._get_conn() as conn:
-            for row in conn.execute(self._get_sql, (key,)):
-                rv = loads(row[0])
-                break
-        if rv is None:
-            raise KeyError('Key not in this session')
-        return rv
+        self.read()
+        return self.data[key]
 
     def __setitem__(self, key, value):
-        with self._get_conn() as conn:
-            conn.execute(self._set_sql, (key, dumps(value)))
-        self.modified = True
+        self.data[key] = value
+        self.save()
 
     def __delitem__(self, key):
-        with self._get_conn() as conn:
-            conn.execute(self._del_sql, (key,))
-        self.modified = True
+        del self.data[key]
+        self.save()
 
     def __iter__(self):
-        with self._get_conn() as conn:
-            for row in conn.execute(self._ite_sql):
-                yield str(row[0])
+        return iter(self.data)
 
     def __len__(self):
+        return len(self.data)
+
+    def read(self):
+        """Load pickle from (ram)disk."""
         try:
-            with self._get_conn() as conn:
-                for row in conn.execute(self._len_sql):
-                    return row[0]
-        except sqlite3.OperationalError:
-            return 0
-
-    def _get_conn(self):
-        if not self.conn:
-            self.conn = sqlite3.connect(self.path)
-        return self.conn
-
-    # These proxy classes are needed in order
-    # for this session implementation to work properly.
-    # That is because sometimes flask will chain method calls
-    # with session'setdefault' calls.
-    # Eg: session.setdefault('_flashes', []).append(1)
-    # With these proxies, the changes made by chained
-    # method calls will be persisted back to the sqlite
-    # database.
-    class CallableAttributeProxy(object):
-        def __init__(self, session, key, obj, attr):
-            self.session = session
-            self.key = key
-            self.obj = obj
-            self.attr = attr
-
-        def __call__(self, *args, **kwargs):
-            rv = self.attr(*args, **kwargs)
-            self.session[self.key] = self.obj
-            return rv
-
-    class PersistedObjectProxy(object):
-        def __init__(self, session, key, obj):
-            self.session = session
-            self.key = key
-            self.obj = obj
-
-        def __getattr__(self, name):
-            attr = getattr(self.obj, name)
-            if callable(attr):
-                return SqliteSession.CallableAttributeProxy(
-                    self.session, self.key, self.obj, attr)
-            return attr
-
-    def setdefault(self, key, value):
-        if key not in self:
-            self[key] = value
-            self.modified = True
-        return SqliteSession.PersistedObjectProxy(
-            self, key, self[key])
+            with open(self.path, 'rb') as blob:
+                self.data = loads(blob.read())
+        except (FileNotFoundError, ValueError, EOFError, UnpicklingError):
+            self.data = {}
+
+    def save(self):
+        """Dump pickle to (ram)disk atomically."""
+        new_name = '{}.new'.format(self.path)
+        with open(new_name, 'wb') as blob:
+            blob.write(dumps(self.data))
+        os.rename(new_name, self.path)
+
+    # Note: This shouldn't be a problem, but if you find poor interactions
+    # between sessions and message "flashes" (a flask feature not used at this
+    # time), you may need to resurrect the CallableAttributeProxy and
+    # PersistedObjectProxy classes which I've dropped due to disuse.
 
 
 class SqliteSessionInterface(SessionInterface):
     """Basic SessionInterface which uses the SqliteSession."""
 
     def __init__(self, directory):
-        directory = os.path.abspath(directory)
-        if not os.path.exists(directory):
-            os.mkdir(directory)
-        self.directory = directory
+        self.directory = os.path.abspath(directory)
+        os.makedirs(self.directory, exist_ok=True)
 
     def open_session(self, app, request):
-        sid = request.cookies.get(app.session_cookie_name)
-        if not sid:
-            sid = str(uuid4())
-        rv = SqliteSession(self.directory, sid)
-        return rv
+        sid = request.cookies.get(
+            app.session_cookie_name) or '{}-{}'.format(uuid1(), os.getpid())
+        return PickleSession(self.directory, sid)
 
     def save_session(self, app, session, response):
         domain = self.get_cookie_domain(app)
         if not session:
             with suppress(FileNotFoundError):
                 os.unlink(session.path)
-            if session.modified:
-                response.delete_cookie(
-                    app.session_cookie_name, domain=domain)
+            response.delete_cookie(
+                app.session_cookie_name, domain=domain)
             return
         cookie_exp = self.get_expiration_time(app, session)
         response.set_cookie(