From ee17f7a12fd059d9c9e081d763b6d7b844d948f0 Mon Sep 17 00:00:00 2001
From: Alexis Lahouze <xals@lahouze.org>
Date: Sat, 18 Jul 2015 11:58:32 +0200
Subject: [PATCH] Improve query for operations.

---
 accountant/api/models/operations.py | 63 +++++++++++++++++------------
 accountant/api/views/entries.py     | 35 ++++++++--------
 2 files changed, 56 insertions(+), 42 deletions(-)

diff --git a/accountant/api/models/operations.py b/accountant/api/models/operations.py
index 771002e..9542c88 100644
--- a/accountant/api/models/operations.py
+++ b/accountant/api/models/operations.py
@@ -16,7 +16,8 @@
 """
 import arrow
 
-from sqlalchemy import func, case, desc, true, false
+from sqlalchemy import func, case, desc, true, false, text
+from sqlalchemy.orm import column_property
 
 from accountant import db
 
@@ -66,6 +67,18 @@ class Operation(db.Model):
         server_default=false()
     )
 
+    sold = column_property(
+        func.sum(
+            case(
+                whens={canceled: text("0")},
+                else_=value
+            )
+        ).over(
+            partition_by=[account_id],
+            order_by=["operation_date", desc("value"), desc("label"), id]
+        ).label("sold")
+    )
+
     def __init__(self, label, value, account_id, operation_date=None,
                  category=None, pointed=False, confirmed=True, canceled=False,
                  scheduled_operation_id=None):
@@ -80,39 +93,37 @@ class Operation(db.Model):
         self.canceled = canceled
 
     @classmethod
-    def get_for_account_and_range(cls, session, account, begin, end):
-        if isinstance(account, int) or isinstance(account, str):
-            account_id = account
-        else:
-            account_id = account.id
-
+    def query(cls, session, begin=None, end=None):
         end = arrow.now().ceil('month').date() if not end else end
 
+        # We have to use a join because the sold is not computed from the
+        # begining.
         base_query = session.query(
-            cls,
-            case(
-                whens={cls.canceled: None},
-                else_=func.sum(
-                    cls.value
-                ).over(
-                    partition_by=[cls.account_id, cls.canceled],
-                    order_by=["operation_date", desc("value"), desc("label")]
-                )
-            ).label("sold")
+            cls.id,
+            cls.sold
         ).subquery()
 
         query = session.query(
-            base_query
-        ).select_from(
-            base_query
+            cls.id,
+            cls.operation_date,
+            cls.label,
+            cls.value,
+            base_query.c.sold,
+            cls.category,
+            cls.scheduled_operation_id,
+            cls.account_id,
+            cls.pointed,
+            cls.confirmed,
+            cls.canceled
+        ).join(
+            base_query, base_query.c.id == cls.id
         ).filter(
-            base_query.c.account_id == account_id,
-            base_query.c.operation_date >= str(begin),
-            base_query.c.operation_date <= str(end)
+            cls.operation_date >= str(begin),
+            cls.operation_date <= str(end)
         ).order_by(
-            desc(base_query.c.operation_date),
-            base_query.c.value,
-            base_query.c.label,
+            desc(cls.operation_date),
+            cls.value,
+            cls.label,
         )
 
         return query
diff --git a/accountant/api/views/entries.py b/accountant/api/views/entries.py
index 7070ec0..7b1b997 100644
--- a/accountant/api/views/entries.py
+++ b/accountant/api/views/entries.py
@@ -18,8 +18,6 @@ import dateutil.parser
 
 from flask.ext.restful import Resource, fields, reqparse, marshal_with_field
 
-from sqlalchemy.orm.exc import NoResultFound
-
 from accountant import session_aware
 
 from .. import api_api
@@ -69,10 +67,13 @@ class OperationListResource(Resource):
     def get(self, session):
         kwargs = range_parser.parse_args()
 
-        return Operation.get_for_account_and_range(session, **kwargs).all()
-
-    def put(self, *args):
-        return self.post()
+        return Operation.query(
+            session,
+            begin=kwargs['begin'],
+            end=kwargs['end'],
+        ).filter(
+            Operation.account_id == kwargs['account']
+        ).all()
 
     @session_aware
     @marshal_with_field(Object(resource_fields))
@@ -83,7 +84,7 @@ class OperationListResource(Resource):
 
         session.add(operation)
 
-        return operation
+        return Operation.query(session).get(operation.id)
 
 
 class OperationResource(Resource):
@@ -93,11 +94,13 @@ class OperationResource(Resource):
         """
         Get operation.
         """
-        try:
-            return Operation.get(session, operation_id)
-        except NoResultFound:
+        operation = Operation.query(session).get(operation_id)
+
+        if not operation:
             return None, 404
 
+        return operation
+
     @session_aware
     @marshal_with_field(Object(resource_fields))
     def post(self, operation_id, session):
@@ -106,9 +109,9 @@ class OperationResource(Resource):
         assert (id not in kwargs or kwargs.id is None
                 or kwargs.id == operation_id)
 
-        try:
-            operation = Operation.get(session, operation_id)
-        except NoResultFound:
+        operation = Operation.query(session).get(operation_id)
+
+        if not operation:
             return None, 404
 
         # SQLAlchemy objects ignore __dict__.update() with merge.
@@ -122,9 +125,9 @@ class OperationResource(Resource):
     @session_aware
     @marshal_with_field(Object(resource_fields))
     def delete(self, operation_id, session):
-        try:
-            operation = Operation.get(session, operation_id)
-        except NoResultFound:
+        operation = Operation.query(session).get(operation_id)
+
+        if not operation:
             return None, 404
 
         session.delete(operation)