From 941caa30d9f4216675d37f7cdc9ae7523b22db14 Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sat, 30 Apr 2016 20:30:21 +1000 Subject: [PATCH 01/20] Replaces ProductController.attach_user_remainders with ProductController.user_remainders --- registrasion/controllers/cart.py | 6 ++---- registrasion/controllers/product.py | 25 +++++++------------------ 2 files changed, 9 insertions(+), 22 deletions(-) diff --git a/registrasion/controllers/cart.py b/registrasion/controllers/cart.py index dbf7e8a0..2ff9f171 100644 --- a/registrasion/controllers/cart.py +++ b/registrasion/controllers/cart.py @@ -217,16 +217,14 @@ class CartController(object): errors = [] # Pre-annotate products - products = [p for (p, q) in product_quantities] - r = ProductController.attach_user_remainders(self.cart.user, products) - with_remainders = dict((p, p) for p in r) + remainders = ProductController.user_remainders(self.cart.user) # Test each product limit here for product, quantity in product_quantities: if quantity < 0: errors.append((product, "Value must be zero or greater.")) - limit = with_remainders[product].remainder + limit = remainders[product.id] if quantity > limit: errors.append(( diff --git a/registrasion/controllers/product.py b/registrasion/controllers/product.py index 610c7f0d..0e2e984f 100644 --- a/registrasion/controllers/product.py +++ b/registrasion/controllers/product.py @@ -38,14 +38,13 @@ class ProductController(object): r = CategoryController.attach_user_remainders(user, categories) cat_quants = dict((c, c) for c in r) - r = ProductController.attach_user_remainders(user, all_products) - prod_quants = dict((p, p) for p in r) + product_remainders = ProductController.user_remainders(user) passed_limits = set( product for product in all_products if cat_quants[product.category].remainder > 0 - if prod_quants[product].remainder > 0 + if product_remainders[product.id] > 0 ) failed_and_messages = FlagController.test_flags( @@ -59,17 +58,15 @@ class ProductController(object): return out @classmethod - def attach_user_remainders(cls, user, products): + def user_remainders(cls, user): ''' Return: - queryset(inventory.Product): A queryset containing items from - ``product``, with an extra attribute -- remainder = the amount of - this item that is remaining. + Mapping[int->int]: A dictionary that maps the product ID to the + user's remainder for that product. ''' - ids = [product.id for product in products] - products = inventory.Product.objects.filter(id__in=ids) + products = inventory.Product.objects.all() cart_filter = ( Q(productitem__cart__user=user) & @@ -93,12 +90,4 @@ class ProductController(object): products = products.annotate(remainder=remainder) - return products - - def user_quantity_remaining(self, user): - ''' Returns the quantity of this product that the user add in the - current cart. ''' - - with_remainders = self.attach_user_remainders(user, [self.product]) - - return with_remainders[0].remainder + return dict((product.id, product.remainder) for product in products) From c6fdfa496e844ad4dde24e06c9fd03ff30b36484 Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sat, 30 Apr 2016 20:30:44 +1000 Subject: [PATCH 02/20] Replaces CategoryController.attach_user_remainders with user_remainders --- registrasion/controllers/cart.py | 5 ++--- registrasion/controllers/category.py | 21 ++++++--------------- registrasion/controllers/product.py | 7 ++----- 3 files changed, 10 insertions(+), 23 deletions(-) diff --git a/registrasion/controllers/cart.py b/registrasion/controllers/cart.py index 2ff9f171..283c3119 100644 --- a/registrasion/controllers/cart.py +++ b/registrasion/controllers/cart.py @@ -240,12 +240,11 @@ class CartController(object): by_cat[product.category].append((product, quantity)) # Pre-annotate categories - r = CategoryController.attach_user_remainders(self.cart.user, by_cat) - with_remainders = dict((cat, cat) for cat in r) + remainders = CategoryController.user_remainders(self.cart.user) # Test each category limit here for category in by_cat: - limit = with_remainders[category].remainder + limit = remainders[category.id] # Get the amount so far in the cart to_add = sum(i[1] for i in by_cat[category]) diff --git a/registrasion/controllers/category.py b/registrasion/controllers/category.py index 9db8ca9e..4681f48b 100644 --- a/registrasion/controllers/category.py +++ b/registrasion/controllers/category.py @@ -39,17 +39,16 @@ class CategoryController(object): return set(i.category for i in available) @classmethod - def attach_user_remainders(cls, user, categories): + def user_remainders(cls, user): ''' Return: - queryset(inventory.Product): A queryset containing items from - ``categories``, with an extra attribute -- remainder = the amount - of items from this category that is remaining. + Mapping[int->int]: A dictionary that maps the category ID to the + user's remainder for that category. + ''' - ids = [category.id for category in categories] - categories = inventory.Category.objects.filter(id__in=ids) + categories = inventory.Category.objects.all() cart_filter = ( Q(product__productitem__cart__user=user) & @@ -73,12 +72,4 @@ class CategoryController(object): categories = categories.annotate(remainder=remainder) - return categories - - def user_quantity_remaining(self, user): - ''' Returns the quantity of this product that the user add in the - current cart. ''' - - with_remainders = self.attach_user_remainders(user, [self.category]) - - return with_remainders[0].remainder + return dict((cat.id, cat.remainder) for cat in categories) diff --git a/registrasion/controllers/product.py b/registrasion/controllers/product.py index 0e2e984f..0810902b 100644 --- a/registrasion/controllers/product.py +++ b/registrasion/controllers/product.py @@ -34,16 +34,13 @@ class ProductController(object): if products is not None: all_products = set(itertools.chain(all_products, products)) - categories = set(product.category for product in all_products) - r = CategoryController.attach_user_remainders(user, categories) - cat_quants = dict((c, c) for c in r) - + category_remainders = CategoryController.user_remainders(user) product_remainders = ProductController.user_remainders(user) passed_limits = set( product for product in all_products - if cat_quants[product.category].remainder > 0 + if category_remainders[product.category.id] > 0 if product_remainders[product.id] > 0 ) From b3491cab8e1fc59a1ef0ab94230ad84907e8a924 Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sat, 30 Apr 2016 20:42:41 +1000 Subject: [PATCH 03/20] _filtered_flags now no longer cares about products for filtering. It just does everything. --- registrasion/controllers/flag.py | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/registrasion/controllers/flag.py b/registrasion/controllers/flag.py index aa11d53e..97456478 100644 --- a/registrasion/controllers/flag.py +++ b/registrasion/controllers/flag.py @@ -47,8 +47,6 @@ class FlagController(object): a list is returned containing all of the products that are *not enabled*. ''' - print "GREPME: test_flags()" - if products is not None and product_quantities is not None: raise ValueError("Please specify only products or " "product_quantities") @@ -62,7 +60,7 @@ class FlagController(object): if products: # Simplify the query. - all_conditions = cls._filtered_flags(user, products) + all_conditions = cls._filtered_flags(user) else: all_conditions = [] @@ -160,7 +158,7 @@ class FlagController(object): return error_fields @classmethod - def _filtered_flags(cls, user, products): + def _filtered_flags(cls, user): ''' Returns: @@ -171,16 +169,7 @@ class FlagController(object): types = list(ConditionController._controllers()) flagtypes = [i for i in types if issubclass(i, conditions.FlagBase)] - # Get all flags for the products and categories. - prods = ( - product.flagbase_set.all() - for product in products - ) - cats = ( - category.flagbase_set.all() - for category in set(product.category for product in products) - ) - all_flags = reduce(operator.or_, itertools.chain(prods, cats)) + all_flags = conditions.FlagBase.objects.all() all_subsets = [] From 162a1f23dd2e54ddd2404657a4774dd393a4a9db Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sun, 1 May 2016 08:47:53 +1000 Subject: [PATCH 04/20] _filtered_discounts is now called _filtered_clauses, and it no longer cares about specific products or categories --- registrasion/controllers/discount.py | 53 +++++++++++++--------------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/registrasion/controllers/discount.py b/registrasion/controllers/discount.py index f4f88ed2..f0df1b07 100644 --- a/registrasion/controllers/discount.py +++ b/registrasion/controllers/discount.py @@ -50,7 +50,22 @@ class DiscountController(object): categories and products. The discounts also list the available quantity for this user, not including products that are pending purchase. ''' - filtered_clauses = cls._filtered_discounts(user, categories, products) + filtered_clauses = cls._filtered_clauses(user, categories, products) + + # clauses that match provided categories + categories = set(categories) + # clauses that match provided products + products = set(products) + # clauses that match categories for provided products + product_categories = set(product.category for product in products) + # (Not relevant: clauses that match products in provided categories) + all_categories = categories | product_categories + + filtered_clauses = ( + clause for clause in filtered_clauses + if hasattr(clause, 'product') and clause.product in products or + hasattr(clause, 'category') and clause.category in all_categories + ) discounts = [] @@ -84,7 +99,7 @@ class DiscountController(object): return discounts @classmethod - def _filtered_discounts(cls, user, categories, products): + def _filtered_clauses(cls, user): ''' Returns: @@ -98,37 +113,17 @@ class DiscountController(object): i for i in types if issubclass(i, conditions.DiscountBase) ] - # discounts that match provided categories - category_discounts = conditions.DiscountForCategory.objects.filter( - category__in=categories - ) - # discounts that match provided products - product_discounts = conditions.DiscountForProduct.objects.filter( - product__in=products - ) - # discounts that match categories for provided products - product_category_discounts = conditions.DiscountForCategory.objects - product_category_discounts = product_category_discounts.filter( - category__in=(product.category for product in products) - ) - # (Not relevant: discounts that match products in provided categories) - - product_discounts = product_discounts.select_related( + product_clauses = conditions.DiscountForProduct.objects.all() + product_clauses = product_clauses.select_related( "product", "product__category", ) - - all_category_discounts = ( - category_discounts | product_category_discounts - ) - all_category_discounts = all_category_discounts.select_related( + category_clauses = conditions.DiscountForCategory.objects.all() + category_clauses = category_clauses.select_related( "category", ) - valid_discounts = conditions.DiscountBase.objects.filter( - Q(discountforproduct__in=product_discounts) | - Q(discountforcategory__in=all_category_discounts) - ) + valid_discounts = conditions.DiscountBase.objects.all() all_subsets = [] @@ -145,8 +140,8 @@ class DiscountController(object): from_filter = dict((i.id, i) for i in filtered_discounts) clause_sets = ( - product_discounts.filter(discount__in=filtered_discounts), - all_category_discounts.filter(discount__in=filtered_discounts), + product_clauses.filter(discount__in=filtered_discounts), + category_clauses.filter(discount__in=filtered_discounts), ) clause_sets = ( From 78a41970ea48428dde22d5af5b17564be063bd17 Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sat, 30 Apr 2016 21:42:02 +1000 Subject: [PATCH 05/20] Adds design for BatchController --- registrasion/controllers/batch.py | 85 ++++++++++++++++++++++++++++ registrasion/controllers/discount.py | 6 +- 2 files changed, 88 insertions(+), 3 deletions(-) create mode 100644 registrasion/controllers/batch.py diff --git a/registrasion/controllers/batch.py b/registrasion/controllers/batch.py new file mode 100644 index 00000000..2e5e0241 --- /dev/null +++ b/registrasion/controllers/batch.py @@ -0,0 +1,85 @@ +import contextlib +import functools + + +class BatchController(object): + ''' Batches are sets of operations where certain queries for users may be + repeated, but are also unlikely change within the boundaries of the batch. + + Batches are keyed per-user. You can mark the edge of the batch with the + ``batch`` context manager. If you nest calls to ``batch``, only the + outermost call will have the effect of ending the batch. + + Batches store results for functions wrapped with ``memoise``. These results + for the user are flushed at the end of the batch. + + If a return for a memoised function has a callable attribute called + ``end_batch``, that attribute will be called at the end of the batch. + + ''' + + _user_caches = {} + + @classmethod + @contextlib.contextmanager + def batch(cls, user): + ''' Marks the entry point for a batch for the given user. ''' + pass + # TODO: store nesting count *inside* the cache object. You know it + # makes sense. + + @classmethod + def memoise(cls, func): + ''' Decorator that stores the result of the stored function in the + user's results cache until the batch completes. + + Arguments: + func (callable(user, *a, **k)): The function whose results we want + to store. ``user`` must be the first argument; this is used as + the cache key. + + Returns: + callable(user, *a, **k): The memosing version of ``func``. + + ''' + + @functools.wraps(func) + def f(user, *a, **k): + + cache = cls.get_cache(user) + if func not in cache: + cache[func] = func(user, *a, **k) + + return cache[func] + + return f + + @classmethod + def get_cache(cls, user): + if user not in cls._user_caches: + return {} # Return blank cache here, we'll just discard :) + + return cls._user_caches[user] + + +''' +TODO: memoise CartController.for_user +TODO: memoise user_remainders (Product, Category) +TODO: memoise _filtered_flags +TODO: memoise FlagCounter.count() (doesn't take user, but it'll do for now) +TODO: memoise _filtered_discounts + +Tests: +- Correct nesting behaviour + - do we get different cache objects every time we get a cache in non-batched + contexts? + - do we get the same cache object for nested caches? + - do we get different cache objects when we back out of a batch and enter a + new one +- are cache clears independent for different users? +- ``end_batch`` behaviour for CartController (use for_user *A LOT*) + - discounts not calculated until outermost batch point exits. + - Revision number shouldn't change until outermost batch point exits. +- Make sure memoisation ONLY happens when we're in a batch. + +''' diff --git a/registrasion/controllers/discount.py b/registrasion/controllers/discount.py index f0df1b07..29bc1ec6 100644 --- a/registrasion/controllers/discount.py +++ b/registrasion/controllers/discount.py @@ -50,7 +50,7 @@ class DiscountController(object): categories and products. The discounts also list the available quantity for this user, not including products that are pending purchase. ''' - filtered_clauses = cls._filtered_clauses(user, categories, products) + filtered_clauses = cls._filtered_clauses(user) # clauses that match provided categories categories = set(categories) @@ -103,8 +103,8 @@ class DiscountController(object): ''' Returns: - Sequence[discountbase]: All discounts that passed the filter - function. + Sequence[DiscountForProduct | DiscountForCategory]: All clauses + that passed the filter function. ''' From eb29e7cd0977103c1d8b5ad6cb128cfa9cac8ac4 Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sun, 1 May 2016 10:46:40 +1000 Subject: [PATCH 06/20] Adds test cases for basic batch cacheing behaviour --- registrasion/tests/test_batch.py | 83 ++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 registrasion/tests/test_batch.py diff --git a/registrasion/tests/test_batch.py b/registrasion/tests/test_batch.py new file mode 100644 index 00000000..95c3fbc0 --- /dev/null +++ b/registrasion/tests/test_batch.py @@ -0,0 +1,83 @@ +import datetime +import pytz + +from django.core.exceptions import ValidationError + +from controller_helpers import TestingCartController +from test_cart import RegistrationCartTestCase + +from registrasion.controllers.batch import BatchController +from registrasion.controllers.discount import DiscountController +from registrasion.controllers.product import ProductController +from registrasion.models import commerce +from registrasion.models import conditions + +UTC = pytz.timezone('UTC') + + +class BatchTestCase(RegistrationCartTestCase): + + def test_no_caches_outside_of_batches(self): + cache_1 = BatchController.get_cache(self.USER_1) + cache_2 = BatchController.get_cache(self.USER_2) + + # Identity testing is important here + self.assertIsNot(cache_1, cache_2) + + def test_cache_clears_at_batch_exit(self): + with BatchController.batch(self.USER_1): + cache_1 = BatchController.get_cache(self.USER_1) + + cache_2 = BatchController.get_cache(self.USER_1) + + self.assertIsNot(cache_1, cache_2) + + def test_caches_identical_within_nestings(self): + with BatchController.batch(self.USER_1): + cache_1 = BatchController.get_cache(self.USER_1) + + with BatchController.batch(self.USER_2): + cache_2 = BatchController.get_cache(self.USER_1) + + cache_3 = BatchController.get_cache(self.USER_1) + + self.assertIs(cache_1, cache_2) + self.assertIs(cache_2, cache_3) + + def test_caches_are_independent_for_different_users(self): + with BatchController.batch(self.USER_1): + cache_1 = BatchController.get_cache(self.USER_1) + + with BatchController.batch(self.USER_2): + cache_2 = BatchController.get_cache(self.USER_2) + + self.assertIsNot(cache_1, cache_2) + + def test_cache_clears_are_independent_for_different_users(self): + with BatchController.batch(self.USER_1): + cache_1 = BatchController.get_cache(self.USER_1) + + with BatchController.batch(self.USER_2): + cache_2 = BatchController.get_cache(self.USER_2) + + with BatchController.batch(self.USER_2): + cache_3 = BatchController.get_cache(self.USER_2) + + cache_4 = BatchController.get_cache(self.USER_1) + + self.assertIs(cache_1, cache_4) + self.assertIsNot(cache_1, cache_2) + self.assertIsNot(cache_2, cache_3) + + def test_new_caches_for_new_batches(self): + with BatchController.batch(self.USER_1): + cache_1 = BatchController.get_cache(self.USER_1) + + with BatchController.batch(self.USER_1): + cache_2 = BatchController.get_cache(self.USER_1) + + with BatchController.batch(self.USER_1): + cache_3 = BatchController.get_cache(self.USER_1) + + self.assertIs(cache_2, cache_3) + self.assertIsNot(cache_1, cache_2) From ddedf54c42cfccaac5c438c638ae76fcfa34d6fc Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sun, 1 May 2016 10:36:52 +1000 Subject: [PATCH 07/20] Adds batch context manager behaviour --- registrasion/controllers/batch.py | 45 +++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/registrasion/controllers/batch.py b/registrasion/controllers/batch.py index 2e5e0241..00001a86 100644 --- a/registrasion/controllers/batch.py +++ b/registrasion/controllers/batch.py @@ -19,14 +19,37 @@ class BatchController(object): ''' _user_caches = {} + _NESTING_KEY = "nesting_count" @classmethod @contextlib.contextmanager def batch(cls, user): ''' Marks the entry point for a batch for the given user. ''' - pass - # TODO: store nesting count *inside* the cache object. You know it - # makes sense. + + cls._enter_batch_context(user) + try: + yield + finally: + # Make sure we clean up in case of errors. + cls._exit_batch_context(user) + + @classmethod + def _enter_batch_context(cls, user): + if user not in cls._user_caches: + cls._user_caches[user] = cls._new_cache() + + cache = cls._user_caches[user] + cache[cls._NESTING_KEY] += 1 + + @classmethod + def _exit_batch_context(cls, user): + cache = cls._user_caches[user] + cache[cls._NESTING_KEY] -= 1 + + if cache[cls._NESTING_KEY] == 0: + # TODO: Handle batch end cases + + del cls._user_caches[user] @classmethod def memoise(cls, func): @@ -57,10 +80,17 @@ class BatchController(object): @classmethod def get_cache(cls, user): if user not in cls._user_caches: - return {} # Return blank cache here, we'll just discard :) + # Return blank cache here, we'll just discard :) + return cls._new_cache() return cls._user_caches[user] + @classmethod + def _new_cache(cls): + ''' Returns a new cache dictionary. ''' + cache = {} + cache[cls._NESTING_KEY] = 0 + return cache ''' TODO: memoise CartController.for_user @@ -70,13 +100,6 @@ TODO: memoise FlagCounter.count() (doesn't take user, but it'll do for now) TODO: memoise _filtered_discounts Tests: -- Correct nesting behaviour - - do we get different cache objects every time we get a cache in non-batched - contexts? - - do we get the same cache object for nested caches? - - do we get different cache objects when we back out of a batch and enter a - new one -- are cache clears independent for different users? - ``end_batch`` behaviour for CartController (use for_user *A LOT*) - discounts not calculated until outermost batch point exits. - Revision number shouldn't change until outermost batch point exits. From 27ab44ec4415613584c51d5f4bb05ac769211a58 Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sun, 1 May 2016 10:42:18 +1000 Subject: [PATCH 08/20] test cases for memoisation --- registrasion/tests/test_batch.py | 39 ++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/registrasion/tests/test_batch.py b/registrasion/tests/test_batch.py index 95c3fbc0..51aa19dd 100644 --- a/registrasion/tests/test_batch.py +++ b/registrasion/tests/test_batch.py @@ -81,3 +81,42 @@ class BatchTestCase(RegistrationCartTestCase): self.assertIs(cache_2, cache_3) self.assertIsNot(cache_1, cache_2) + + def test_memoisation_happens_in_batch_context(self): + with BatchController.batch(self.USER_1): + output_1 = self._memoiseme(self.USER_1) + + with BatchController.batch(self.USER_1): + output_2 = self._memoiseme(self.USER_1) + + self.assertIs(output_1, output_2) + + def test_memoisaion_does_not_happen_outside_batch_context(self): + output_1 = self._memoiseme(self.USER_1) + output_2 = self._memoiseme(self.USER_1) + + self.assertIsNot(output_1, output_2) + + def test_memoisation_is_user_independent(self): + with BatchController.batch(self.USER_1): + output_1 = self._memoiseme(self.USER_1) + with BatchController.batch(self.USER_2): + output_2 = self._memoiseme(self.USER_2) + output_3 = self._memoiseme(self.USER_1) + + self.assertIsNot(output_1, output_2) + self.assertIs(output_1, output_3) + + def test_memoisation_clears_outside_batches(self): + with BatchController.batch(self.USER_1): + output_1 = self._memoiseme(self.USER_1) + + with BatchController.batch(self.USER_1): + output_2 = self._memoiseme(self.USER_1) + + self.assertIsNot(output_1, output_2) + + @classmethod + @BatchController.memoise + def _memoiseme(self, user): + return object() From a267b60eb9f39c723c910ca92cd90533d1749602 Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sun, 1 May 2016 11:12:35 +1000 Subject: [PATCH 09/20] Makes memoise work properly --- registrasion/controllers/batch.py | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/registrasion/controllers/batch.py b/registrasion/controllers/batch.py index 00001a86..1a7f2d2e 100644 --- a/registrasion/controllers/batch.py +++ b/registrasion/controllers/batch.py @@ -1,6 +1,8 @@ import contextlib import functools +from django.contrib.auth.models import User + class BatchController(object): ''' Batches are sets of operations where certain queries for users may be @@ -54,26 +56,36 @@ class BatchController(object): @classmethod def memoise(cls, func): ''' Decorator that stores the result of the stored function in the - user's results cache until the batch completes. + user's results cache until the batch completes. Keyword arguments are + not yet supported. Arguments: - func (callable(user, *a, **k)): The function whose results we want - to store. ``user`` must be the first argument; this is used as - the cache key. + func (callable(*a)): The function whose results we want + to store. The positional arguments, ``a``, are used as cache + keys. Returns: - callable(user, *a, **k): The memosing version of ``func``. + callable(*a): The memosing version of ``func``. ''' @functools.wraps(func) - def f(user, *a, **k): + def f(*a): + for arg in a: + if isinstance(arg, User): + user = arg + break + else: + raise ValueError("One position argument must be a User") + + func_key = (func, tuple(a)) cache = cls.get_cache(user) - if func not in cache: - cache[func] = func(user, *a, **k) - return cache[func] + if func_key not in cache: + cache[func_key] = func(*a) + + return cache[func_key] return f From 3db1256895aa5086621335e42134a1ffd5b06965 Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sun, 1 May 2016 11:21:55 +1000 Subject: [PATCH 10/20] Adds test for end_batch functionality --- registrasion/tests/test_batch.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/registrasion/tests/test_batch.py b/registrasion/tests/test_batch.py index 51aa19dd..590f7f04 100644 --- a/registrasion/tests/test_batch.py +++ b/registrasion/tests/test_batch.py @@ -120,3 +120,26 @@ class BatchTestCase(RegistrationCartTestCase): @BatchController.memoise def _memoiseme(self, user): return object() + + def test_batch_end_functionality_is_called(self): + + class Ender(object): + end_count = 0 + def end_batch(self): + self.end_count += 1 + + @BatchController.memoise + def get_ender(user): + return Ender() + + # end_batch should get called once on exiting the batch + with BatchController.batch(self.USER_1): + ender = get_ender(self.USER_1) + self.assertEquals(1, ender.end_count) + + # end_batch should get called once on exiting the batch + # no matter how deep the object gets cached + with BatchController.batch(self.USER_1): + with BatchController.batch(self.USER_1): + ender = get_ender(self.USER_1) + self.assertEquals(1, ender.end_count) From 5929c0af3cec5548f92a9cf026ec01e261cb57bb Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sun, 1 May 2016 11:25:48 +1000 Subject: [PATCH 11/20] Adds end_batch functionality --- registrasion/controllers/batch.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/registrasion/controllers/batch.py b/registrasion/controllers/batch.py index 1a7f2d2e..3cf95c8a 100644 --- a/registrasion/controllers/batch.py +++ b/registrasion/controllers/batch.py @@ -49,7 +49,11 @@ class BatchController(object): cache[cls._NESTING_KEY] -= 1 if cache[cls._NESTING_KEY] == 0: - # TODO: Handle batch end cases + + for key in cache: + item = cache[key] + if hasattr(item, 'end_batch') and callable(item.end_batch): + item.end_batch() del cls._user_caches[user] @@ -115,6 +119,4 @@ Tests: - ``end_batch`` behaviour for CartController (use for_user *A LOT*) - discounts not calculated until outermost batch point exits. - Revision number shouldn't change until outermost batch point exits. -- Make sure memoisation ONLY happens when we're in a batch. - ''' From 94ceaa3bb10fccea5b77e0e90dced7a722d878ab Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sun, 1 May 2016 11:55:14 +1000 Subject: [PATCH 12/20] Adds test case for CartController batching --- registrasion/tests/test_batch.py | 1 - registrasion/tests/test_cart.py | 56 ++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/registrasion/tests/test_batch.py b/registrasion/tests/test_batch.py index 590f7f04..70370799 100644 --- a/registrasion/tests/test_batch.py +++ b/registrasion/tests/test_batch.py @@ -122,7 +122,6 @@ class BatchTestCase(RegistrationCartTestCase): return object() def test_batch_end_functionality_is_called(self): - class Ender(object): end_count = 0 def end_batch(self): diff --git a/registrasion/tests/test_cart.py b/registrasion/tests/test_cart.py index 790c1df9..a4f6a5eb 100644 --- a/registrasion/tests/test_cart.py +++ b/registrasion/tests/test_cart.py @@ -12,6 +12,7 @@ from registrasion.models import commerce from registrasion.models import conditions from registrasion.models import inventory from registrasion.models import people +from registrasion.controllers.batch import BatchController from registrasion.controllers.product import ProductController from controller_helpers import TestingCartController @@ -360,3 +361,58 @@ class BasicCartTests(RegistrationCartTestCase): def test_available_products_respects_product_limits(self): self.__available_products_test(self.PROD_4, 6) + + def test_cart_controller_batching(self): + # - that for_user is memoised + with BatchController.batch(self.USER_1): + cart = TestingCartController.for_user(self.USER_1) + cart_2 = TestingCartController.for_user(self.USER_1) + self.assertIs(cart, cart_2) + + # - that the revision only increments on modifications + rev_0 = cart.cart.revision + with BatchController.batch(self.USER_1): + # Memoise the cart + same_cart = TestingCartController.for_user(self.USER_1) + # Do nothing on exit + rev_1 = self.reget(cart.cart).revision + self.assertEqual(rev_0, rev_1) + + # - that the revision only increments on the outside of a batch + rev_0 = cart.cart.revision + with BatchController.batch(self.USER_1): + # Memoise the cart + same_cart = TestingCartController.for_user(self.USER_1) + same_cart.add_to_cart(self.PROD_1, 1) + rev_1 = self.reget(same_cart.cart).revision + + rev_2 = self.reget(cart.cart).revision + cart.set_quantity(self.PROD_1, 0) + + self.assertEqual(rev_0, rev_1) + self.assertNotEqual(rev_0, rev_2) + + # - that discounts are only calculated on modifications o/s batch + def count_discounts(cart): + return cart.cart.discountitem_set.count() + + count_0 = count_discounts(cart.cart) + self.make_discount_ceiling("FLOOZLE") + with BatchController.batch(self.USER_1): + # Memoise the cart + same_cart = TestingCartController.for_user(self.USER_1) + + with BatchController.batch(self.USER_1): + # Memoise the cart + same_cart_2 = TestingCartController.for_user(self.USER_1) + + same_cart_2.add_to_cart(self.PROD_1, 1) + count_1 = count_discounts(same_cart_2.cart) + + count_2 = count_discounts(same_cart.cart) + + count_3 = count_discounts(cart.cart) + self.assertEqual(0, count_0) + self.assertEqual(0, count_1) + self.assertEqual(0, count_2) + self.assertEqual(1, count_1) From ad2de6e9d40ddfe0610769f8007ac13106f95a10 Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sun, 1 May 2016 12:05:51 +1000 Subject: [PATCH 13/20] Breaks cart batching tests into multiple tests --- registrasion/tests/test_cart.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/registrasion/tests/test_cart.py b/registrasion/tests/test_cart.py index a4f6a5eb..7b804f64 100644 --- a/registrasion/tests/test_cart.py +++ b/registrasion/tests/test_cart.py @@ -362,24 +362,29 @@ class BasicCartTests(RegistrationCartTestCase): def test_available_products_respects_product_limits(self): self.__available_products_test(self.PROD_4, 6) - def test_cart_controller_batching(self): + def test_cart_controller_for_user_is_memoised(self): # - that for_user is memoised with BatchController.batch(self.USER_1): cart = TestingCartController.for_user(self.USER_1) cart_2 = TestingCartController.for_user(self.USER_1) self.assertIs(cart, cart_2) - # - that the revision only increments on modifications + def test_cart_revision_does_not_increment_if_not_modified(self): + cart = TestingCartController.for_user(self.USER_1) rev_0 = cart.cart.revision + with BatchController.batch(self.USER_1): # Memoise the cart same_cart = TestingCartController.for_user(self.USER_1) # Do nothing on exit + rev_1 = self.reget(cart.cart).revision self.assertEqual(rev_0, rev_1) - # - that the revision only increments on the outside of a batch + def test_cart_revision_only_increments_at_end_of_batches(self): + cart = TestingCartController.for_user(self.USER_1) rev_0 = cart.cart.revision + with BatchController.batch(self.USER_1): # Memoise the cart same_cart = TestingCartController.for_user(self.USER_1) @@ -387,17 +392,18 @@ class BasicCartTests(RegistrationCartTestCase): rev_1 = self.reget(same_cart.cart).revision rev_2 = self.reget(cart.cart).revision - cart.set_quantity(self.PROD_1, 0) self.assertEqual(rev_0, rev_1) self.assertNotEqual(rev_0, rev_2) - # - that discounts are only calculated on modifications o/s batch + def test_cart_discounts_only_calculated_at_end_of_batches(self): def count_discounts(cart): return cart.cart.discountitem_set.count() - count_0 = count_discounts(cart.cart) + cart = TestingCartController.for_user(self.USER_1) self.make_discount_ceiling("FLOOZLE") + count_0 = count_discounts(cart.cart) + with BatchController.batch(self.USER_1): # Memoise the cart same_cart = TestingCartController.for_user(self.USER_1) From 3717adb262908c8daac542830761912ed3814587 Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sun, 1 May 2016 12:19:20 +1000 Subject: [PATCH 14/20] Squash this and last two --- registrasion/tests/test_cart.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/registrasion/tests/test_cart.py b/registrasion/tests/test_cart.py index 7b804f64..619b9074 100644 --- a/registrasion/tests/test_cart.py +++ b/registrasion/tests/test_cart.py @@ -402,7 +402,7 @@ class BasicCartTests(RegistrationCartTestCase): cart = TestingCartController.for_user(self.USER_1) self.make_discount_ceiling("FLOOZLE") - count_0 = count_discounts(cart.cart) + count_0 = count_discounts(cart) with BatchController.batch(self.USER_1): # Memoise the cart @@ -413,12 +413,13 @@ class BasicCartTests(RegistrationCartTestCase): same_cart_2 = TestingCartController.for_user(self.USER_1) same_cart_2.add_to_cart(self.PROD_1, 1) - count_1 = count_discounts(same_cart_2.cart) + count_1 = count_discounts(same_cart_2) - count_2 = count_discounts(same_cart.cart) + count_2 = count_discounts(same_cart) + + count_3 = count_discounts(cart) - count_3 = count_discounts(cart.cart) self.assertEqual(0, count_0) self.assertEqual(0, count_1) self.assertEqual(0, count_2) - self.assertEqual(1, count_1) + self.assertEqual(1, count_3) From 3d635521ebf68c8a9fa7bd98e1bc5c85085f8ec4 Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sun, 1 May 2016 12:19:36 +1000 Subject: [PATCH 15/20] CartController now uses BatchController memoisation --- registrasion/controllers/cart.py | 69 ++++++-------------------------- 1 file changed, 13 insertions(+), 56 deletions(-) diff --git a/registrasion/controllers/cart.py b/registrasion/controllers/cart.py index 283c3119..d0a9f057 100644 --- a/registrasion/controllers/cart.py +++ b/registrasion/controllers/cart.py @@ -16,6 +16,7 @@ from registrasion.models import commerce from registrasion.models import conditions from registrasion.models import inventory +from.batch import BatchController from .category import CategoryController from .discount import DiscountController from .flag import FlagController @@ -34,10 +35,11 @@ def _modifies_cart(func): def inner(self, *a, **k): self._fail_if_cart_is_not_active() with transaction.atomic(): - with CartController.operations_batch(self.cart.user) as mark: - mark.mark = True # Marker that we've modified the cart + with BatchController.batch(self.cart.user): + # Mark the version of self in the batch cache as modified + memoised = self.for_user(self.cart.user) + memoised._modified_by_batch = True return func(self, *a, **k) - return inner @@ -47,6 +49,7 @@ class CartController(object): self.cart = cart @classmethod + @BatchController.memoise def for_user(cls, user): ''' Returns the user's current cart, or creates a new cart if there isn't one ready yet. ''' @@ -64,59 +67,6 @@ class CartController(object): ) return cls(existing) - # Marks the carts that are currently in batches - _FOR_USER = {} - _BATCH_COUNT = collections.defaultdict(int) - _MODIFIED_CARTS = set() - - class _ModificationMarker(object): - pass - - @classmethod - @contextlib.contextmanager - def operations_batch(cls, user): - ''' Marks the boundary for a batch of operations on a user's cart. - - These markers can be nested. Only on exiting the outermost marker will - a batch be ended. - - When a batch is ended, discounts are recalculated, and the cart's - revision is increased. - ''' - - if user not in cls._FOR_USER: - _ctrl = cls.for_user(user) - cls._FOR_USER[user] = (_ctrl, _ctrl.cart.id) - - ctrl, _id = cls._FOR_USER[user] - - cls._BATCH_COUNT[_id] += 1 - try: - success = False - - marker = cls._ModificationMarker() - yield marker - - if hasattr(marker, "mark"): - cls._MODIFIED_CARTS.add(_id) - - success = True - finally: - - cls._BATCH_COUNT[_id] -= 1 - - # Only end on the outermost batch marker, and only if - # it excited cleanly, and a modification occurred - modified = _id in cls._MODIFIED_CARTS - outermost = cls._BATCH_COUNT[_id] == 0 - if modified and outermost and success: - ctrl._end_batch() - cls._MODIFIED_CARTS.remove(_id) - - # Clear out the cache on the outermost operation - if outermost: - del cls._FOR_USER[user] - def _fail_if_cart_is_not_active(self): self.cart.refresh_from_db() if self.cart.status != commerce.Cart.STATUS_ACTIVE: @@ -144,6 +94,13 @@ class CartController(object): self.cart.time_last_updated = timezone.now() self.cart.reservation_duration = max(reservations) + + def end_batch(self): + ''' Calls ``_end_batch`` if a modification has been performed in the + previous batch. ''' + if hasattr(self,'_modified_by_batch'): + self._end_batch() + def _end_batch(self): ''' Performs operations that occur occur at the end of a batch of product changes/voucher applications etc. From efb73e7a682f937934c71d381ec2606182ff4800 Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sun, 1 May 2016 12:42:06 +1000 Subject: [PATCH 16/20] Memoises everything else that needs to be memoised. --- registrasion/controllers/batch.py | 13 ------------- registrasion/controllers/category.py | 2 ++ registrasion/controllers/discount.py | 6 ++++-- registrasion/controllers/flag.py | 8 +++++--- registrasion/controllers/product.py | 2 ++ 5 files changed, 13 insertions(+), 18 deletions(-) diff --git a/registrasion/controllers/batch.py b/registrasion/controllers/batch.py index 3cf95c8a..defd741e 100644 --- a/registrasion/controllers/batch.py +++ b/registrasion/controllers/batch.py @@ -107,16 +107,3 @@ class BatchController(object): cache = {} cache[cls._NESTING_KEY] = 0 return cache - -''' -TODO: memoise CartController.for_user -TODO: memoise user_remainders (Product, Category) -TODO: memoise _filtered_flags -TODO: memoise FlagCounter.count() (doesn't take user, but it'll do for now) -TODO: memoise _filtered_discounts - -Tests: -- ``end_batch`` behaviour for CartController (use for_user *A LOT*) - - discounts not calculated until outermost batch point exits. - - Revision number shouldn't change until outermost batch point exits. -''' diff --git a/registrasion/controllers/category.py b/registrasion/controllers/category.py index 4681f48b..4adf09b6 100644 --- a/registrasion/controllers/category.py +++ b/registrasion/controllers/category.py @@ -7,6 +7,7 @@ from django.db.models import Sum from django.db.models import When from django.db.models import Value +from .batch import BatchController class AllProducts(object): pass @@ -39,6 +40,7 @@ class CategoryController(object): return set(i.category for i in available) @classmethod + @BatchController.memoise def user_remainders(cls, user): ''' diff --git a/registrasion/controllers/discount.py b/registrasion/controllers/discount.py index 29bc1ec6..108ed29b 100644 --- a/registrasion/controllers/discount.py +++ b/registrasion/controllers/discount.py @@ -1,6 +1,8 @@ import itertools -from conditions import ConditionController +from .batch import BatchController +from .conditions import ConditionController + from registrasion.models import commerce from registrasion.models import conditions @@ -10,7 +12,6 @@ from django.db.models import Sum from django.db.models import Value from django.db.models import When - class DiscountAndQuantity(object): ''' Represents a discount that can be applied to a product or category for a given user. @@ -99,6 +100,7 @@ class DiscountController(object): return discounts @classmethod + @BatchController.memoise def _filtered_clauses(cls, user): ''' diff --git a/registrasion/controllers/flag.py b/registrasion/controllers/flag.py index 97456478..879d85f0 100644 --- a/registrasion/controllers/flag.py +++ b/registrasion/controllers/flag.py @@ -6,6 +6,7 @@ from collections import namedtuple from django.db.models import Count from django.db.models import Q +from .batch import BatchController from .conditions import ConditionController from registrasion.models import conditions @@ -115,7 +116,7 @@ class FlagController(object): if not met and product not in messages: messages[product] = message - total_flags = FlagCounter.count() + total_flags = FlagCounter.count(user) valid = {} @@ -158,6 +159,7 @@ class FlagController(object): return error_fields @classmethod + @BatchController.memoise def _filtered_flags(cls, user): ''' @@ -209,11 +211,11 @@ _ConditionsCount = namedtuple( ) -# TODO: this should be cacheable. class FlagCounter(_FlagCounter): @classmethod - def count(cls): + @BatchController.memoise + def count(cls, user): # Get the count of how many conditions should exist per product flagbases = conditions.FlagBase.objects diff --git a/registrasion/controllers/product.py b/registrasion/controllers/product.py index 0810902b..4210bd7c 100644 --- a/registrasion/controllers/product.py +++ b/registrasion/controllers/product.py @@ -9,6 +9,7 @@ from django.db.models import Value from registrasion.models import commerce from registrasion.models import inventory +from .batch import BatchController from .category import CategoryController from .flag import FlagController @@ -55,6 +56,7 @@ class ProductController(object): return out @classmethod + @BatchController.memoise def user_remainders(cls, user): ''' From 3ab5ac32cadc69dbb03a3c4056de534848749b9c Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sun, 1 May 2016 12:44:07 +1000 Subject: [PATCH 17/20] Part of CartController->BatchController memoisation --- registrasion/views.py | 48 ++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/registrasion/views.py b/registrasion/views.py index a4dcceac..3d2a3c04 100644 --- a/registrasion/views.py +++ b/registrasion/views.py @@ -5,9 +5,10 @@ from registrasion import util from registrasion.models import commerce from registrasion.models import inventory from registrasion.models import people -from registrasion.controllers.discount import DiscountController +from registrasion.controllers.batch import BatchController from registrasion.controllers.cart import CartController from registrasion.controllers.credit_note import CreditNoteController +from registrasion.controllers.discount import DiscountController from registrasion.controllers.invoice import InvoiceController from registrasion.controllers.product import ProductController from registrasion.exceptions import CartValidationError @@ -170,18 +171,18 @@ def guided_registration(request): category__in=cats, ).select_related("category") - available_products = set(ProductController.available_products( - request.user, - products=all_products, - )) + with BatchController.batch(request.user): + available_products = set(ProductController.available_products( + request.user, + products=all_products, + )) - if len(available_products) == 0: - # We've filled in every category - attendee.completed_registration = True - attendee.save() - return next_step + if len(available_products) == 0: + # We've filled in every category + attendee.completed_registration = True + attendee.save() + return next_step - with CartController.operations_batch(request.user): for category in cats: products = [ i for i in available_products @@ -345,20 +346,21 @@ def product_category(request, category_id): category_id = int(category_id) # Routing is [0-9]+ category = inventory.Category.objects.get(pk=category_id) - products = ProductController.available_products( - request.user, - category=category, - ) - - if not products: - messages.warning( - request, - "There are no products available from category: " + category.name, + with BatchController.batch(request.user): + products = ProductController.available_products( + request.user, + category=category, ) - return redirect("dashboard") - p = _handle_products(request, category, products, PRODUCTS_FORM_PREFIX) - products_form, discounts, products_handled = p + if not products: + messages.warning( + request, + "There are no products available from category: " + category.name, + ) + return redirect("dashboard") + + p = _handle_products(request, category, products, PRODUCTS_FORM_PREFIX) + products_form, discounts, products_handled = p if request.POST and not voucher_handled and not products_form.errors: # Only return to the dashboard if we didn't add a voucher code From 9ca25e5986ba3f9709219a24e6db8e33f27ef49c Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sun, 1 May 2016 13:15:19 +1000 Subject: [PATCH 18/20] Makes sure that the cache is not disturbed by calling end_batch --- registrasion/controllers/batch.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/registrasion/controllers/batch.py b/registrasion/controllers/batch.py index defd741e..579d8970 100644 --- a/registrasion/controllers/batch.py +++ b/registrasion/controllers/batch.py @@ -49,13 +49,23 @@ class BatchController(object): cache[cls._NESTING_KEY] -= 1 if cache[cls._NESTING_KEY] == 0: + cls._call_end_batch_methods(user) + del cls._user_caches[user] - for key in cache: + @classmethod + def _call_end_batch_methods(cls, user): + cache = cls._user_caches[user] + ended = set() + while True: + keys = set(cache.keys()) + if ended == keys: + break + keys_to_end = keys - ended + for key in keys_to_end: item = cache[key] if hasattr(item, 'end_batch') and callable(item.end_batch): item.end_batch() - - del cls._user_caches[user] + ended = ended | keys_to_end @classmethod def memoise(cls, func): From b9b50c68466b7b8d8914de2b476ca28d9e03ff0d Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sun, 1 May 2016 14:07:29 +1000 Subject: [PATCH 19/20] Bug fixes and query optimisations in flag.py and discount.py --- registrasion/controllers/discount.py | 2 ++ registrasion/controllers/flag.py | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/registrasion/controllers/discount.py b/registrasion/controllers/discount.py index 108ed29b..79785a64 100644 --- a/registrasion/controllers/discount.py +++ b/registrasion/controllers/discount.py @@ -117,12 +117,14 @@ class DiscountController(object): product_clauses = conditions.DiscountForProduct.objects.all() product_clauses = product_clauses.select_related( + "discount", "product", "product__category", ) category_clauses = conditions.DiscountForCategory.objects.all() category_clauses = category_clauses.select_related( "category", + "discount", ) valid_discounts = conditions.DiscountBase.objects.all() diff --git a/registrasion/controllers/flag.py b/registrasion/controllers/flag.py index 879d85f0..29b5be6d 100644 --- a/registrasion/controllers/flag.py +++ b/registrasion/controllers/flag.py @@ -85,6 +85,8 @@ class FlagController(object): # from the categories covered by this condition ids = [product.id for product in products] + + # TODO: This is re-evaluated a lot. all_products = inventory.Product.objects.filter(id__in=ids) cond = ( Q(flagbase_set=condition) | @@ -181,7 +183,7 @@ class FlagController(object): flags = ctrl.pre_filter(flags, user) all_subsets.append(flags) - return itertools.chain(*all_subsets) + return list(itertools.chain(*all_subsets)) ConditionAndRemainder = namedtuple( From abe8c12b05e3f70146379852ec5ef134d47c720e Mon Sep 17 00:00:00 2001 From: Christopher Neugebauer Date: Sun, 1 May 2016 19:12:40 +1000 Subject: [PATCH 20/20] Simplifies flag and discount filter functions --- registrasion/controllers/discount.py | 4 +--- registrasion/controllers/flag.py | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/registrasion/controllers/discount.py b/registrasion/controllers/discount.py index 79785a64..984fe214 100644 --- a/registrasion/controllers/discount.py +++ b/registrasion/controllers/discount.py @@ -127,12 +127,10 @@ class DiscountController(object): "discount", ) - valid_discounts = conditions.DiscountBase.objects.all() - all_subsets = [] for discounttype in discounttypes: - discounts = discounttype.objects.filter(id__in=valid_discounts) + discounts = discounttype.objects.all() ctrl = ConditionController.for_type(discounttype) discounts = ctrl.pre_filter(discounts, user) all_subsets.append(discounts) diff --git a/registrasion/controllers/flag.py b/registrasion/controllers/flag.py index 29b5be6d..77d6476d 100644 --- a/registrasion/controllers/flag.py +++ b/registrasion/controllers/flag.py @@ -173,12 +173,10 @@ class FlagController(object): types = list(ConditionController._controllers()) flagtypes = [i for i in types if issubclass(i, conditions.FlagBase)] - all_flags = conditions.FlagBase.objects.all() - all_subsets = [] for flagtype in flagtypes: - flags = flagtype.objects.filter(id__in=all_flags) + flags = flagtype.objects.all() ctrl = ConditionController.for_type(flagtype) flags = ctrl.pre_filter(flags, user) all_subsets.append(flags)