correctly handle signals in nested transactions

.travis.yml

@@ -9,12 +9,8 @@ matrix:
       env: TOXENV=py,codecov
     - python: 3.4
       env: TOXENV=py,codecov
-    - python: 3.3
-      env: TOXENV=py,py-lowest,codecov
     - python: 2.7
       env: TOXENV=py,codecov
-    - python: 2.6
-      env: TOXENV=py,py-lowest,codecov
     - python: pypy
       env: TOXENV=py,codecov
     - python: nightly

docs/conf.py

@@ -45,7 +45,7 @@
 
 # General information about the project.
 project = u'Flask-SQLAlchemy'
-copyright = u'2010 - {0}, Armin Ronacher'.format(datetime.utcnow().year)
+copyright = u'2010 - {}, Armin Ronacher'.format(datetime.utcnow().year)
 
 # The version info for the project you're documenting, acts as replacement for
 # |version| and |release|, also used in various other places throughout the

docs/config.rst

@@ -130,8 +130,8 @@ specify a `custom constraint naming convention
 <http://docs.sqlalchemy.org/en/latest/core/constraints.html#constraint-naming-conventions>`_
 in conjunction with SQLAlchemy 0.9.2 or higher.
 Doing so is important for dealing with database migrations (for instance using
-`alembic <https://alembic.readthedocs.org>`_ as stated
-`here <http://alembic.readthedocs.org/en/latest/naming.html>`_. Here's an
+`alembic <http://alembic.zzzcomputing.com>`_ as stated
+`here <http://alembic.zzzcomputing.com/en/latest/naming.html>`_. Here's an
 example, as suggested by the SQLAlchemy docs::
 
     from sqlalchemy import MetaData

docs/models.rst

@@ -105,7 +105,7 @@ when SQLAlchemy will load the data from the database:
 
 -   ``'select'`` / ``True`` (which is the default, but explicit is better
     than implicit) means that SQLAlchemy will load the data as necessary
-    in one go using a standard select statement.
+    in one go using a standard ``SELECT`` statement.
 -   ``'joined'`` / ``False`` tells SQLAlchemy to load the relationship in
     the same query as the parent using a ``JOIN`` statement.
 -   ``'subquery'`` works like ``'joined'`` but instead SQLAlchemy will

docs/queries.rst

@@ -26,7 +26,7 @@ It is essentially a beefed up version of a database transaction.  This is
 how it works:
 
 >>> from yourapp import User
->>> me = User('admin', '[email protected]')
+>>> me = User(username='admin', email='[email protected]')
 >>> db.session.add(me)
 >>> db.session.commit()
 
@@ -82,7 +82,7 @@ Retrieve a user by username:
 >>> peter.id
 2
 >>> peter.email
-u'[email protected]'
+'[email protected]'
 
 Same as above but for a non existing username gives `None`:
 
@@ -93,22 +93,22 @@ True
 Selecting a bunch of users by a more complex expression:
 
 >>> User.query.filter(User.email.endswith('@example.com')).all()
-[<User u'admin'>, <User u'guest'>]
+[<User 'admin'>, <User 'guest'>]
 
 Ordering users by something:
 
 >>> User.query.order_by(User.username).all()
-[<User u'admin'>, <User u'guest'>, <User u'peter'>]
+[<User 'admin'>, <User 'guest'>, <User 'peter'>]
 
 Limiting users:
 
 >>> User.query.limit(1).all()
-[<User u'admin'>]
+[<User 'admin'>]
 
 Getting user by primary key:
 
 >>> User.query.get(1)
-<User u'admin'>
+<User 'admin'>
 
 
 Queries in Views

docs/quickstart.rst

@@ -60,9 +60,9 @@ But they are not yet in the database, so let's make sure they are::
 Accessing the data in database is easy as a pie::
 
     >>> User.query.all()
-    [<User u'admin'>, <User u'guest'>]
+    [<User 'admin'>, <User 'guest'>]
     >>> User.query.filter_by(username='admin').first()
-    <User u'admin'>
+    <User 'admin'>
 
 Note how we never defined a ``__init__`` method on the ``User`` class?
 That's because SQLAlchemy adds an implicit constructor to all model
@@ -73,7 +73,7 @@ constructor with those ``**kwargs`` to preserve this behavior::
 
     class Foo(db.Model):
         # ...
-        def __init__(**kwargs):
+        def __init__(self, **kwargs):
             super(Foo, self).__init__(**kwargs)
             # do custom stuff
 
@@ -143,8 +143,8 @@ load all categories and their posts, you could do it like this::
     >>> from sqlalchemy.orm import joinedload
     >>> query = Category.query.options(joinedload('posts'))
     >>> for category in query:
-    ...     print category, category.posts
-    <Category u'Python'> [<Post u'Hello Python!'>, <Post u'Snakes'>]
+    ...     print(category, category.posts)
+    <Category 'Python'> [<Post 'Hello Python!'>, <Post 'Snakes'>]
 
 
 If you want to get a query object for that relationship, you can do so

examples/hello.py

@@ -1,6 +1,6 @@
 from datetime import datetime
 from flask import Flask, request, flash, url_for, redirect, \
-     render_template, abort
+     render_template
 from flask_sqlalchemy import SQLAlchemy
 
 

flask_sqlalchemy/__init__.py

@@ -29,7 +29,7 @@
 from sqlalchemy.orm.session import Session as SessionBase
 
 from flask_sqlalchemy.model import Model
-from ._compat import itervalues, string_types, to_str, xrange
+from ._compat import itervalues, string_types, xrange
 from .model import DefaultMeta
 
 __version__ = '2.3.2'
@@ -166,13 +166,14 @@ class _SessionSignalEvents(object):
     @classmethod
     def register(cls, session):
         if not hasattr(session, '_model_changes'):
-            session._model_changes = {}
+            session._model_changes = []
 
         event.listen(session, 'before_flush', cls.record_ops)
         event.listen(session, 'before_commit', cls.record_ops)
         event.listen(session, 'before_commit', cls.before_commit)
         event.listen(session, 'after_commit', cls.after_commit)
         event.listen(session, 'after_rollback', cls.after_rollback)
+        event.listen(session, 'after_transaction_create', cls.after_transaction_create)
 
     @classmethod
     def unregister(cls, session):
@@ -184,6 +185,7 @@ def unregister(cls, session):
         event.remove(session, 'before_commit', cls.before_commit)
         event.remove(session, 'after_commit', cls.after_commit)
         event.remove(session, 'after_rollback', cls.after_rollback)
+        event.remove(session, 'after_transaction_create', cls.after_transaction_create)
 
     @staticmethod
     def record_ops(session, flush_context=None, instances=None):
@@ -196,28 +198,54 @@ def record_ops(session, flush_context=None, instances=None):
             for target in targets:
                 state = inspect(target)
                 key = state.identity_key if state.has_identity else id(target)
-                d[key] = (target, operation)
+                d[-1][key] = (target, operation)
+
+    @staticmethod
+    def after_transaction_create(session, transaction):
+        if transaction.parent and not transaction.nested:
+            return
+
+        try:
+            d = session._model_changes
+        except AttributeError:
+            return
+
+        d.append({})
 
     @staticmethod
     def before_commit(session):
+        if session.transaction.nested:
+            return
+
         try:
             d = session._model_changes
         except AttributeError:
             return
 
         if d:
-            before_models_committed.send(session.app, changes=list(d.values()))
+            for level in d[1:]:
+                d[0].update(level)
+
+            if d[0]:
+                before_models_committed.send(session.app, changes=list(d[0].values()))
 
     @staticmethod
     def after_commit(session):
+        if session.transaction.nested:
+            return
+
         try:
             d = session._model_changes
         except AttributeError:
             return
 
         if d:
-            models_committed.send(session.app, changes=list(d.values()))
-            d.clear()
+            for level in d[1:]:
+                d[0].update(level)
+
+            if d[0]:
+                models_committed.send(session.app, changes=list(d[0].values()))
+            del d[:]
 
     @staticmethod
     def after_rollback(session):
@@ -226,7 +254,10 @@ def after_rollback(session):
         except AttributeError:
             return
 
-        d.clear()
+        try:
+            del d[-1]
+        except IndexError:
+            pass
 
 
 class _EngineDebuggingSignalEvents(object):
@@ -325,7 +356,7 @@ def __init__(self, query, page, per_page, total, items):
     @property
     def pages(self):
         """The total number of pages"""
-        if self.per_page == 0:
+        if self.per_page == 0 or self.total is None:
             pages = 0
         else:
             pages = int(ceil(self.total / float(self.per_page)))
@@ -427,13 +458,14 @@ def first_or_404(self):
             abort(404)
         return rv
 
-    def paginate(self, page=None, per_page=None, error_out=True, max_per_page=None):
+    def paginate(self, page=None, per_page=None, error_out=True, max_per_page=None, count=True):
         """Returns ``per_page`` items from page ``page``.
 
         If ``page`` or ``per_page`` are ``None``, they will be retrieved from
         the request query. If ``max_per_page`` is specified, ``per_page`` will
         be limited to that value. If there is no request or they aren't in the
-        query, they default to 1 and 20 respectively.
+        query, they default to 1 and 20 respectively. If ``count`` is ``False``,
+        no query to help determine total page count will be run.
 
         When ``error_out`` is ``True`` (default), the following rules will
         cause a 404 response:
@@ -494,8 +526,11 @@ def paginate(self, page=None, per_page=None, error_out=True, max_per_page=None):
             abort(404)
 
         # No need to count if we're on the first page and there are fewer
-        # items than we expected.
-        if page == 1 and len(items) < per_page:
+        # items than we expected or if count is disabled.
+
+        if not count:
+            total = None
+        elif page == 1 and len(items) < per_page:
             total = len(items)
         else:
             total = self.order_by(None).count()

flask_sqlalchemy/model.py

@@ -148,7 +148,7 @@ class Model(object):
     def __repr__(self):
         identity = inspect(self).identity
         if identity is None:
-            pk = "(transient {0})".format(id(self))
+            pk = "(transient {})".format(id(self))
         else:
             pk = ', '.join(to_str(value) for value in identity)
-        return '<{0} {1}>'.format(type(self).__name__, pk)
+        return '<{} {}>'.format(type(self).__name__, pk)

scripts/make-release.py

@@ -26,7 +26,6 @@ def parse_changelog():
             match = re.search('^Version\s+(.*)', line.strip())
             if match is None:
                 continue
-            length = len(match.group(1))
             version = match.group(1).strip()
             if lineiter.next().count('-') != len(match.group(0)):
                 continue

setup.py

@@ -43,10 +43,8 @@
         'Topic :: Software Development :: Libraries :: Python Modules',
         'Programming Language :: Python',
         'Programming Language :: Python :: 2',
-        'Programming Language :: Python :: 2.6',
         'Programming Language :: Python :: 2.7',
         'Programming Language :: Python :: 3',
-        'Programming Language :: Python :: 3.3',
         'Programming Language :: Python :: 3.4',
         'Programming Language :: Python :: 3.5',
         'Programming Language :: Python :: 3.6',

tests/test_pagination.py

@@ -22,6 +22,11 @@ def test_pagination_pages_when_0_items_per_page():
     assert p.pages == 0
 
 
+def test_pagination_pages_when_total_is_none():
+    p = fsa.Pagination(None, 1, 100, None, [])
+    assert p.pages == 0
+
+
 def test_query_paginate(app, db, Todo):
     with app.app_context():
         db.session.add_all([Todo('', '') for _ in range(100)])
@@ -30,7 +35,7 @@ def test_query_paginate(app, db, Todo):
     @app.route('/')
     def index():
         p = Todo.query.paginate()
-        return '{0} items retrieved'.format(len(p.items))
+        return '{} items retrieved'.format(len(p.items))
 
     c = app.test_client()
     # request default
@@ -68,3 +73,11 @@ def test_paginate_min(app, db, Todo):
 
     with pytest.raises(NotFound):
         Todo.query.paginate(per_page=-1)
+
+
+def test_paginate_without_count(app, db, Todo):
+    with app.app_context():
+        db.session.add_all(Todo('', '') for _ in range(20))
+        db.session.commit()
+
+    assert len(Todo.query.paginate(count=False, page=1, per_page=10).items) == 10