Allow setting multiple stockrecords on product, add chunked by defaul…

…t to resources_to_db, don't error on non existent m2m (#50)

* Add children to category resource

* Add chunked by default in odin, do not try creating m2m instances if the through as no id, children field on category model, partner identifier is code, and partner resource/model

* This test no longer raises exception + improve error message.

* Allow passing extra context. in case a context mapper is not enough (eg; you need a map for references)

* Review feedback & add test for multiple stockrecords

* Allow setting chunk size in products_to_db too.

---------

Co-authored-by: Joey Jurjens <joey@highbiza.nl>

oscar_odin/mappings/catalogue.py

@@ -19,6 +19,7 @@
 
 from .context import ProductModelMapperContext
 from .constants import ALL_CATALOGUE_FIELDS, MODEL_IDENTIFIERS_MAPPING
+from ..settings import RESOURCES_TO_DB_CHUNK_SIZE
 
 __all__ = (
     "ProductImageToResource",
@@ -44,6 +45,7 @@
 map_queryset, OscarBaseMapping = get_classes(
     "oscar_odin.mappings.common", ["map_queryset", "OscarBaseMapping"]
 )
+StockRecordToModel = get_class("oscar_odin.mappings.partner", "StockRecordToModel")
 
 # resources
 (
@@ -312,11 +314,22 @@ def categories(self, values) -> List[CategoryModel]:
         return CategoryToModel.apply(values)
 
     @odin.map_list_field(
-        from_field=["price", "availability", "currency", "upc", "partner"]
+        from_field=[
+            "stockrecords",
+            "price",
+            "availability",
+            "currency",
+            "upc",
+            "partner",
+        ]
     )
     def stockrecords(
-        self, price, availability, currency, upc, partner
+        self, stockrecords, price, availability, currency, upc, partner
     ) -> List[StockRecordModel]:
+        # This means to use explicitly set stockrecords, rather than the price related fields.
+        if stockrecords:
+            return StockRecordToModel.apply(stockrecords, context=self.context)
+
         if upc and currency and partner:
             return [
                 StockRecordModel(
@@ -453,6 +466,7 @@ def products_to_db(
     product_mapper=ProductToModel,
     delete_related=False,
     clean_instances=True,
+    chunk_size=RESOURCES_TO_DB_CHUNK_SIZE,
 ) -> Tuple[List[ProductModel], Dict]:
     """Map mulitple products to a model and store them in the database.
 
@@ -468,4 +482,5 @@ def products_to_db(
         context_mapper=ProductModelMapperContext,
         delete_related=delete_related,
         clean_instances=clean_instances,
+        chunk_size=chunk_size,
     )

oscar_odin/mappings/constants.py

@@ -16,7 +16,6 @@
 PRODUCT_DESCRIPTION = "Product.description"
 PRODUCT_META_TITLE = "Product.meta_title"
 PRODUCT_META_DESCRIPTION = "Product.meta_description"
-PRODUCT_PARENT = "Product.parent"
 PRODUCT_IS_DISCOUNTABLE = "Product.is_discountable"
 
 PRODUCTCLASS_SLUG = "ProductClass.slug"
@@ -107,5 +106,5 @@
     StockRecord: ("partner_id", "partner_sku"),
     ProductClass: ("slug",),
     ProductImage: ("code",),
-    Partner: ("slug",),
+    Partner: ("code",),
 }

oscar_odin/mappings/context.py

@@ -371,12 +371,21 @@ def bulk_update_or_create_many_to_many(self):
                         # Delete throughs if no instances are passed for the field
                         to_delete_throughs_product_ids.append(product.id)
                     for instance in instances:
-                        throughs[(product.pk, instance.pk)] = Through(
-                            **{
-                                relation.m2m_field_name(): product,
-                                relation.m2m_reverse_field_name(): instance,
-                            }
-                        )
+                        if instance.pk is not None:
+                            throughs[(product.pk, instance.pk)] = Through(
+                                **{
+                                    relation.m2m_field_name(): product,
+                                    relation.m2m_reverse_field_name(): instance,
+                                }
+                            )
+                        else:
+                            # Instead of failing bulk_create here below, we will add an error.
+                            self.errors.add_error(
+                                OscarOdinException(
+                                    f"Cannot create m2m relationship {Through.__name__} - related model '{relation.related_model.__name__}' is missing a primary key"
+                                ),
+                                instance,
+                            )
 
                 # Delete throughs if no instances are passed for the field
                 if self.delete_related:
@@ -414,14 +423,8 @@ def bulk_update_or_create_many_to_many(self):
                             }
                         ).exclude(id__in=bulk_troughs.values()).delete()
 
-                    try:
-                        # Save only new through models
-                        Through.objects.bulk_create(throughs.values())
-                    except ValueError as e:
-                        raise OscarOdinException(
-                            "Failed creating Trough models for %s. Maybe the related model does NOT exist?"
-                            % relation.name
-                        ) from e
+                    # Save only new through models
+                    Through.objects.bulk_create(throughs.values())
 
     def bulk_save(
         self, instances, fields_to_update, identifier_mapping, clean_instances

oscar_odin/mappings/partner.py

@@ -0,0 +1,41 @@
+import odin
+
+from oscar.core.loading import get_class, get_model
+
+ModelMapping = get_class("oscar_odin.mappings.model_mapper", "ModelMapping")
+OscarBaseMapping = get_class("oscar_odin.mappings.common", "OscarBaseMapping")
+
+# resources
+PartnerResource = get_class("oscar_odin.resources.partner", "PartnerResource")
+StockRecordResource = get_class("oscar_odin.resources.partner", "StockRecordResource")
+
+Partner = get_model("partner", "Partner")
+StockRecord = get_model("partner", "StockRecord")
+
+
+class PartnerModelToResource(OscarBaseMapping):
+    from_obj = Partner
+    to_obj = PartnerResource
+
+
+class PartnerToModel(ModelMapping):
+    from_obj = PartnerResource
+    to_obj = Partner
+
+
+class StockRecordModelToResource(OscarBaseMapping):
+    from_obj = StockRecord
+    to_obj = StockRecordResource
+
+    @odin.map_field
+    def partner(self, partner):
+        return PartnerModelToResource.apply(partner)
+
+
+class StockRecordToModel(ModelMapping):
+    from_obj = StockRecordResource
+    to_obj = StockRecord
+
+    @odin.map_field
+    def partner(self, partner):
+        return PartnerToModel.apply(partner)

oscar_odin/mappings/resources.py

@@ -1,5 +1,8 @@
 from oscar.core.loading import get_class
 
+from ..settings import RESOURCES_TO_DB_CHUNK_SIZE
+from ..utils import chunked
+
 ModelMapperContext = get_class("oscar_odin.mappings.context", "ModelMapperContext")
 validate_resources = get_class("oscar_odin.utils", "validate_resources")
 
@@ -10,10 +13,12 @@ def resources_to_db(
     identifier_mapping,
     model_mapper,
     context_mapper=ModelMapperContext,
+    extra_context=None,
     delete_related=False,
     clean_instances=True,
     skip_invalid_resources=False,
     error_identifiers=None,
+    chunk_size=RESOURCES_TO_DB_CHUNK_SIZE,
 ):
     """Map mulitple resources to a model and store them in the database.
 
@@ -26,22 +31,38 @@ def resources_to_db(
     if not skip_invalid_resources and resource_errors:
         return [], resource_errors
 
-    context = context_mapper(
-        model_mapper.to_obj,
-        delete_related=delete_related,
-        error_identifiers=error_identifiers,
-    )
-    result = model_mapper.apply(valid_resources, context=context)
-
-    try:
-        instances = list(result)
-    except TypeError:  # it is not a list
-        instances = [result]
-
-    saved_resources, errors = context.bulk_save(
-        instances,
-        fields_to_update,
-        identifier_mapping,
-        clean_instances,
-    )
+    saved_resources_pks = []
+    errors = []
+
+    for chunk in chunked(valid_resources, chunk_size):
+        context = context_mapper(
+            model_mapper.to_obj,
+            delete_related=delete_related,
+            error_identifiers=error_identifiers,
+        )
+
+        if extra_context:
+            context.update(extra_context)
+
+        result = model_mapper.apply(chunk, context=context)
+
+        try:
+            instances = list(result)
+        except TypeError:  # it is not a list
+            instances = [result]
+
+        chunk_saved_resources, chunk_errors = context.bulk_save(
+            instances,
+            fields_to_update,
+            identifier_mapping,
+            clean_instances,
+        )
+
+        # Don't store all the model instances in saved_resources, as this could lead to memory issues.
+        for instance in chunk_saved_resources:
+            saved_resources_pks.append(instance.pk)
+
+        errors.extend(chunk_errors)
+
+    saved_resources = model_mapper.to_obj.objects.filter(pk__in=saved_resources_pks)
     return saved_resources, resource_errors + errors

oscar_odin/resources/catalogue.py

@@ -11,6 +11,9 @@
 
 OscarResource = get_class("oscar_odin.resources.base", "OscarResource")
 
+PartnerResource = get_class("oscar_odin.resources.partner", "PartnerResource")
+StockRecordResource = get_class("oscar_odin.resources.partner", "StockRecordResource")
+
 ProductModel = get_model("catalogue", "Product")
 
 
@@ -59,6 +62,9 @@ class CategoryResource(OscarCatalogueResource):
     ancestors_are_public: Optional[bool]
     depth: Optional[int]
     path: Optional[str]
+    children: Optional[List["CategoryResource"]] = odin.ListOf.delayed(
+        lambda: CategoryResource
+    )
 
 
 class ProductClassResource(OscarCatalogueResource):
@@ -70,15 +76,6 @@ class ProductClassResource(OscarCatalogueResource):
     track_stock: Optional[bool]
 
 
-class StockRecordResource(OscarCatalogueResource):
-    id: Optional[int]
-    partner_sku: str
-    num_in_stock: Optional[int]
-    num_allocated: Optional[int]
-    price: Decimal = DecimalField()
-    currency: Optional[str]
-
-
 class ProductAttributeValueResource(OscarCatalogueResource):
     code: str
     value: Any
@@ -102,6 +99,7 @@ class ProductResource(OscarCatalogueResource):
     slug: Optional[str]
     description: Optional[str] = ""
     meta_title: Optional[str]
+    meta_description: Optional[str]
     images: List[ProductImageResource] = odin.Options(empty=True)
     rating: Optional[float]
     is_discountable: bool = True
@@ -116,6 +114,9 @@ class ProductResource(OscarCatalogueResource):
     is_available_to_buy: Optional[bool]
     partner: Optional[Any]
 
+    # optionally, you can add a list of stockrecords instead of the above.
+    stockrecords: Optional[List[StockRecordResource]] = odin.Options(empty=True)
+
     product_class: Optional[ProductClassResource] = None
     attributes: Dict[str, Union[Any, None]]
     categories: List[CategoryResource]

oscar_odin/resources/partner.py

@@ -0,0 +1,35 @@
+from typing import Optional
+from decimal import Decimal
+
+from oscar.core.loading import get_class
+
+from ..fields import DecimalField
+
+OscarResource = get_class("oscar_odin.resources.base", "OscarResource")
+
+
+class OscarPartnerResource(OscarResource, abstract=True):
+    """Base resource for Oscar partner application."""
+
+    class Meta:
+        allow_field_shadowing = True
+        namespace = "oscar.partner"
+
+
+class PartnerResource(OscarPartnerResource):
+    id: Optional[int]
+    name: str
+    code: str
+
+
+class StockRecordResource(OscarPartnerResource):
+    id: Optional[int]
+    partner_sku: str
+    num_in_stock: Optional[int]
+    num_allocated: Optional[int]
+    price: Decimal = DecimalField()
+    currency: Optional[str]
+
+    # It's optional because we allow easy price setting on the product resource by setting the partner model
+    # on the product resource, which in turn is used to create a stockrecord with other price related fields.
+    partner: Optional[PartnerResource]

oscar_odin/settings.py

@@ -0,0 +1,3 @@
+from django.conf import settings
+
+RESOURCES_TO_DB_CHUNK_SIZE = getattr(settings, "RESOURCES_TO_DB_CHUNK_SIZE", 500)

oscar_odin/utils.py

@@ -10,6 +10,8 @@
 from odin.exceptions import ValidationError
 from odin.mapping import MappingResult
 
+from .settings import RESOURCES_TO_DB_CHUNK_SIZE
+
 
 def get_filters(instances, field_names):
     for ui in instances:
@@ -105,3 +107,22 @@ def validate_resources(resources, error_identifiers=None):
         except ValidationError as error:
             errors.add_error(error, resource)
     return valid_resources, errors
+
+
+def chunked(iterable, size=RESOURCES_TO_DB_CHUNK_SIZE, startindex=0):
+    """
+    Divide an interable into chunks of ``size``
+
+    >>> list(chunked("hahahaha", 2))
+    ['ha', 'ha', 'ha', 'ha']
+    >>> list(chunked([1,2,3,4,5,6,7], 3))
+    [[1, 2, 3], [4, 5, 6], [7]]
+    """
+    while True:
+        chunk = iterable[startindex : startindex + size]
+        chunklen = len(chunk)
+        if chunklen:
+            yield chunk
+        if chunklen < size:
+            break
+        startindex += size