From 94c032ed71147b61091cceb91a393ba17637df91 Mon Sep 17 00:00:00 2001 From: Samar Hassan <88422175+samar-hassan@users.noreply.github.com> Date: Thu, 2 May 2024 12:35:53 +0100 Subject: [PATCH] [FEAT] Prefetch attributes to reduce query count (#26) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat :star: prefetch attributes to reduce query count * fix :wrench: use identifiers of model instead of ids * fix :wrench: resolve filters not hashable error * feat :star: fetch product classes from context * fix :wrench: check if product class exists while setting attributes * refactor :package: revert get_filters optimization - Multiple field names not considered for stockrecords - in_bulk used for many_to_many instances tries creating existing through models * feat :star: validate_instances based on clean_instances argument * chore :recycle: latest oscar version * chore :recycle: latest oscar version * chore ♻️ latest oscar version * chore ♻️ latest oscar version * chore ♻️ latest oscar version * chore ♻️ latest oscar version * chore ♻️ latest oscar version * chore :recycle: update poetry lock file --- oscar_odin/mappings/catalogue.py | 6 ++- oscar_odin/mappings/context.py | 79 ++++++++++++++++++++++++++++---- oscar_odin/utils.py | 2 +- poetry.lock | 9 ++-- pyproject.toml | 2 +- 5 files changed, 81 insertions(+), 17 deletions(-) diff --git a/oscar_odin/mappings/catalogue.py b/oscar_odin/mappings/catalogue.py index 99d6923..f6f9661 100644 --- a/oscar_odin/mappings/catalogue.py +++ b/oscar_odin/mappings/catalogue.py @@ -427,6 +427,7 @@ def products_to_db( identifier_mapping=MODEL_IDENTIFIERS_MAPPING, product_mapper=ProductToModel, delete_related=False, + clean_instances=True, ) -> Tuple[List[ProductModel], Dict]: """Map mulitple products to a model and store them in the database. @@ -444,7 +445,10 @@ def products_to_db( ) products, errors = context.bulk_save( - instances, fields_to_update, identifier_mapping + instances, + fields_to_update, + identifier_mapping, + clean_instances, ) return products, errors diff --git a/oscar_odin/mappings/context.py b/oscar_odin/mappings/context.py index 0d54245..8ebb918 100644 --- a/oscar_odin/mappings/context.py +++ b/oscar_odin/mappings/context.py @@ -5,13 +5,17 @@ from django.db.models import Q from django.core.exceptions import ValidationError -from oscar_odin.utils import in_bulk -from oscar_odin.exceptions import OscarOdinException - from oscar.core.loading import get_model +from oscar.apps.catalogue.product_attributes import QuerysetCache + +from ..utils import in_bulk +from ..exceptions import OscarOdinException +from .constants import MODEL_IDENTIFIERS_MAPPING Product = get_model("catalogue", "Product") +ProductClass = get_model("catalogue", "ProductClass") ProductAttributeValue = get_model("catalogue", "ProductAttributeValue") +ProductAttribute = get_model("catalogue", "ProductAttribute") def separate_instances_to_create_and_update(Model, instances, identifier_mapping): @@ -57,6 +61,7 @@ class ModelMapperContext(dict): instance_keys = None Model = None errors = None + clean_instances = True update_related_models_same_type = True @@ -76,20 +81,37 @@ def __init__(self, Model, *args, delete_related=False, **kwargs): def __bool__(self): return True + def prepare_instance_for_validation(self, instance): + return instance + def validate_instances(self, instances, validate_unique=True, fields=None): + if not self.clean_instances: + return instances validated_instances = [] + identities = [] exclude = () if fields and instances: all_fields = instances[0]._meta.fields exclude = [f.name for f in all_fields if f.name not in fields] + try: + identifier = self.identifier_mapping.get(instances[0].__class__)[0] + except (IndexError, TypeError): + identifier = None + for instance in instances: - try: - instance.full_clean(validate_unique=validate_unique, exclude=exclude) - except ValidationError as e: - self.errors.append(e) - else: - validated_instances.append(instance) + if identifier is None or getattr(instance, identifier) not in identities: + if identifier is not None: + identities.append(getattr(instance, identifier)) + try: + instance = self.prepare_instance_for_validation(instance) + instance.full_clean( + validate_unique=validate_unique, exclude=exclude + ) + except ValidationError as e: + self.errors.append(e) + else: + validated_instances.append(instance) return validated_instances @@ -372,9 +394,12 @@ def bulk_update_or_create_many_to_many(self): % relation.name ) from e - def bulk_save(self, instances, fields_to_update, identifier_mapping): + def bulk_save( + self, instances, fields_to_update, identifier_mapping, clean_instances + ): self.fields_to_update = fields_to_update self.identifier_mapping = identifier_mapping + self.clean_instances = clean_instances with transaction.atomic(): self.bulk_update_or_create_foreign_keys() @@ -390,6 +415,28 @@ def bulk_save(self, instances, fields_to_update, identifier_mapping): class ProductModelMapperContext(ModelMapperContext): update_related_models_same_type = False + product_class_identifier = MODEL_IDENTIFIERS_MAPPING[ProductClass][0] + product_class_keys = set() + attributes = defaultdict(list) + + def prepare_instance_for_validation(self, instance): + if hasattr(instance, "attr"): + self.set_product_class_attributes(instance) + return super().prepare_instance_for_validation(instance) + + def set_product_class_attributes(self, instance): + if instance.product_class: + key = getattr(instance.product_class, self.product_class_identifier) + if key and key in self.attributes: + instance.attr.cache.set_attributes(self.attributes[key]) + + def add_instance_to_fk_items(self, field, instance): + if instance is not None and not instance.pk: + self.foreign_key_items[field] += [instance] + if instance.__class__ == ProductClass: + self.product_class_keys.add( + getattr(instance, self.product_class_identifier) + ) @property def get_fk_relations(self): @@ -416,6 +463,7 @@ def bulk_update_or_create_product_attributes(self, instances): for product in instances: product.attr.invalidate() + self.set_product_class_attributes(product) ( to_be_updated, to_be_created, @@ -453,7 +501,18 @@ def bulk_update_or_create_product_attributes(self, instances): validated_attributes_to_create, batch_size=500, ignore_conflicts=False ) + def fetch_product_class_attributes(self): + product_classes = ProductClass.objects.filter( + **{f"{self.product_class_identifier}__in": self.product_class_keys} + ) + + for product_class in product_classes: + self.attributes[ + getattr(product_class, self.product_class_identifier) + ] = QuerysetCache(product_class.attributes.all()) + def bulk_update_or_create_instances(self, instances): + self.fetch_product_class_attributes() super().bulk_update_or_create_instances(instances) self.bulk_update_or_create_product_attributes(instances) diff --git a/oscar_odin/utils.py b/oscar_odin/utils.py index 08cd0d0..9ae3de0 100644 --- a/oscar_odin/utils.py +++ b/oscar_odin/utils.py @@ -22,7 +22,7 @@ def get_filters(instances, field_names): def get_query(instances, field_names): - filters = list(get_filters(instances, field_names)) + filters = list((get_filters(instances, field_names))) query = filters.pop() for query_filter in filters: diff --git a/poetry.lock b/poetry.lock index 91d4957..9da4a5b 100644 --- a/poetry.lock +++ b/poetry.lock @@ -600,13 +600,13 @@ elasticsearch = ["elasticsearch (>=5,<8)"] [[package]] name = "django-oscar" -version = "3.2.4" +version = "3.2.5a2" description = "A domain-driven e-commerce framework for Django" optional = false python-versions = ">=3.8" files = [ - {file = "django-oscar-3.2.4.tar.gz", hash = "sha256:79a48dfd3cd9a6d93636e55bc7cba25abb56c05c200fcd0c0f30face9566a998"}, - {file = "django_oscar-3.2.4-py3-none-any.whl", hash = "sha256:22d2f2725ba273335f12f28bf7ed79f49a63ee2d16bd842e103c90b2a65bb504"}, + {file = "django-oscar-3.2.5a2.tar.gz", hash = "sha256:7748d142751c312c515a8fd71dd67a0db48d0eceb078edf4af53c66973da28eb"}, + {file = "django_oscar-3.2.5a2-py3-none-any.whl", hash = "sha256:c54d80c939bd8fa840970653f364014d8decc4ef5a91e79a33a9a6fed2eb1cff"}, ] [package.dependencies] @@ -1530,6 +1530,7 @@ files = [ {file = "PyYAML-6.0.1-cp311-cp311-win_amd64.whl", hash = "sha256:bf07ee2fef7014951eeb99f56f39c9bb4af143d8aa3c21b1677805985307da34"}, {file = "PyYAML-6.0.1-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:855fb52b0dc35af121542a76b9a84f8d1cd886ea97c84703eaa6d88e37a2ad28"}, {file = "PyYAML-6.0.1-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:40df9b996c2b73138957fe23a16a4f0ba614f4c0efce1e9406a184b6d07fa3a9"}, + {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:a08c6f0fe150303c1c6b71ebcd7213c2858041a7e01975da3a99aed1e7a378ef"}, {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:6c22bec3fbe2524cde73d7ada88f6566758a8f7227bfbf93a408a9d86bcc12a0"}, {file = "PyYAML-6.0.1-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:8d4e9c88387b0f5c7d5f281e55304de64cf7f9c0021a3525bd3b1c542da3b0e4"}, {file = "PyYAML-6.0.1-cp312-cp312-win32.whl", hash = "sha256:d483d2cdf104e7c9fa60c544d92981f12ad66a457afae824d146093b8c294c54"}, @@ -2012,4 +2013,4 @@ test = ["black", "coverage", "poetry", "pylint", "pylint-django", "responses", " [metadata] lock-version = "2.0" python-versions = "^3.8" -content-hash = "74ba7f8e38f299b88cdd0ec80aa408ea03c58a3b2b6ea5f6dd591635b8f42d62" +content-hash = "b51b0fd2bd39059cec5d7732f5c2c91cf30da653bfa9d94dea898095647b177f" diff --git a/pyproject.toml b/pyproject.toml index 83d74d6..898392b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -27,7 +27,7 @@ classifiers = [ [tool.poetry.dependencies] python = "^3.8" -django-oscar = {version = "^3.2.2", allow-prereleases = true} +django-oscar = {version = "^3.2.5a2", allow-prereleases = true} coverage = { version = "^7.3", optional = true } pylint = { version = "^3.0.2", optional = true } black = { version = "^23.11.0", optional = true }