From 6d52a4c18ff85785b6729073158eedd128158c61 Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Thu, 28 Apr 2016 20:15:21 +1000 Subject: [PATCH] More low-hanging query optimisations --- registrasion/controllers/cart.py | 37 ++++++++++++++++---------- registrasion/controllers/conditions.py | 35 +++++++++++++----------- registrasion/controllers/flag.py | 22 +++++++-------- registrasion/views.py | 7 +++-- 4 files changed, 58 insertions(+), 43 deletions(-) diff --git a/registrasion/controllers/cart.py b/registrasion/controllers/cart.py index 7e1e9072..83e08ef8 100644 --- a/registrasion/controllers/cart.py +++ b/registrasion/controllers/cart.py @@ -8,6 +8,7 @@ from django.core.exceptions import ObjectDoesNotExist from django.core.exceptions import ValidationError from django.db import transaction from django.db.models import Max +from django.db.models import Q from django.utils import timezone from registrasion.exceptions import CartValidationError @@ -84,6 +85,8 @@ class CartController(object): revision is increased. ''' + # TODO cache carts mid-batch? + ctrl = cls.for_user(user) _id = ctrl.cart.id @@ -180,22 +183,28 @@ class CartController(object): # Validate that the limits we're adding are OK self._test_limits(all_product_quantities) + new_items = [] + products = [] for product, quantity in product_quantities: - try: - product_item = commerce.ProductItem.objects.get( - cart=self.cart, - product=product, - ) - product_item.quantity = quantity - product_item.save() - except ObjectDoesNotExist: - commerce.ProductItem.objects.create( - cart=self.cart, - product=product, - quantity=quantity, - ) + products.append(product) - items_in_cart.filter(quantity=0).delete() + if quantity == 0: + continue + + item = commerce.ProductItem( + cart=self.cart, + product=product, + quantity=quantity, + ) + new_items.append(item) + + to_delete = ( + Q(quantity=0) | + Q(product__in=products) + ) + + items_in_cart.filter(to_delete).delete() + commerce.ProductItem.objects.bulk_create(new_items) def _test_limits(self, product_quantities): ''' Tests that the quantity changes we intend to make do not violate diff --git a/registrasion/controllers/conditions.py b/registrasion/controllers/conditions.py index 0a2b3c4a..2480a4fc 100644 --- a/registrasion/controllers/conditions.py +++ b/registrasion/controllers/conditions.py @@ -154,12 +154,17 @@ class CategoryConditionController(IsMetByFilter, ConditionController): product from a category invoking that item's condition in one of their carts. ''' - items = commerce.ProductItem.objects.filter(cart__user=user) - items = items.exclude(cart__status=commerce.Cart.STATUS_RELEASED) - items = items.select_related("product", "product__category") - categories = [item.product.category for item in items] + in_user_carts = Q( + enabling_category__product__productitem__cart__user=user + ) + released = commerce.Cart.STATUS_RELEASED + in_released_carts = Q( + enabling_category__product__productitem__cart__status=released + ) + queryset = queryset.filter(in_user_carts) + queryset = queryset.exclude(in_released_carts) - return queryset.filter(enabling_category__in=categories) + return queryset class ProductConditionController(IsMetByFilter, ConditionController): @@ -171,12 +176,15 @@ class ProductConditionController(IsMetByFilter, ConditionController): ''' Returns all of the items from queryset where the user has a product invoking that item's condition in one of their carts. ''' - items = commerce.ProductItem.objects.filter(cart__user=user) - items = items.exclude(cart__status=commerce.Cart.STATUS_RELEASED) - items = items.select_related("product", "product__category") - products = [item.product for item in items] + in_user_carts = Q(enabling_products__productitem__cart__user=user) + released = commerce.Cart.STATUS_RELEASED + in_released_carts = Q( + enabling_products__productitem__cart__status=released + ) + queryset = queryset.filter(in_user_carts) + queryset = queryset.exclude(in_released_carts) - return queryset.filter(enabling_products__in=products) + return queryset class TimeOrStockLimitConditionController( @@ -287,9 +295,4 @@ class VoucherConditionController(IsMetByFilter, ConditionController): ''' Returns all of the items from queryset where the user has entered a voucher that invokes that item's condition in one of their carts. ''' - carts = commerce.Cart.objects.filter( - user=user, - ) - vouchers = [cart.vouchers.all() for cart in carts] - - return queryset.filter(voucher__in=itertools.chain(*vouchers)) + return queryset.filter(voucher__cart__user=user) diff --git a/registrasion/controllers/flag.py b/registrasion/controllers/flag.py index a67a0b94..aa11d53e 100644 --- a/registrasion/controllers/flag.py +++ b/registrasion/controllers/flag.py @@ -4,6 +4,7 @@ import operator from collections import defaultdict from collections import namedtuple from django.db.models import Count +from django.db.models import Q from .conditions import ConditionController @@ -83,18 +84,16 @@ class FlagController(object): # Get all products covered by this condition, and the products # from the categories covered by this condition - cond_products = condition.products.all() - from_category = inventory.Product.objects.filter( - category__in=condition.categories.all(), - ).all() - all_products = cond_products | from_category + + ids = [product.id for product in products] + all_products = inventory.Product.objects.filter(id__in=ids) + cond = ( + Q(flagbase_set=condition) | + Q(category__in=condition.categories.all()) + ) + + all_products = all_products.filter(cond) all_products = all_products.select_related("category") - # Remove the products that we aren't asking about - all_products = [ - product - for product in all_products - if product in products - ] if quantities: consumed = sum(quantities[i] for i in all_products) @@ -221,6 +220,7 @@ _ConditionsCount = namedtuple( ) +# TODO: this should be cacheable. class FlagCounter(_FlagCounter): @classmethod diff --git a/registrasion/views.py b/registrasion/views.py index 41c4a0d6..a7917406 100644 --- a/registrasion/views.py +++ b/registrasion/views.py @@ -445,14 +445,17 @@ def _handle_products(request, category, products, prefix): def _set_quantities_from_products_form(products_form, current_cart): quantities = list(products_form.product_quantities()) - + id_to_quantity = dict(i[:2] for i in quantities) pks = [i[0] for i in quantities] products = inventory.Product.objects.filter( id__in=pks, ).select_related("category") + + + # TODO: This is fundamentally dumb product_quantities = [ - (products.get(pk=i[0]), i[1]) for i in quantities + (product, id_to_quantity[product.id]) for product in products ] field_names = dict( (i[0][0], i[1][2]) for i in zip(product_quantities, quantities)