Optimize handling of attributes

.github/workflows/ci.yml

@@ -10,19 +10,19 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - name: Checkout the repository
-      uses: actions/checkout@v2
+      uses: actions/checkout@v4
       with:
         path: ./src
     - name: Setup Python 3.11
-      uses: actions/setup-python@v2
+      uses: actions/setup-python@v5
       with:
          python-version: '3.11'
     - name: Install all dependencies
       run: make install
     - name: Run all linting
       run: make lint
     - name: Upload src dir as artefact
-      uses: actions/upload-artifact@v2
+      uses: actions/upload-artifact@v4
       with:
         name: src
         path: ./src
@@ -38,12 +38,12 @@ jobs:
         oscar-version: ["3.2"]
     steps:
       - name: Download src dir
-        uses: actions/download-artifact@v2
+        uses: actions/download-artifact@v4
         with:
           name: src
           path: ./src
       - name: Setup Python 3.x
-        uses: actions/setup-python@v2
+        uses: actions/setup-python@v5
         with:
            python-version: ${{ matrix.python-version }}
       - name: Install all dependencies

Jenkinsfile

@@ -0,0 +1,70 @@
+#!/usr/bin/env groovy
+
+pipeline {
+    agent { label 'GEITENPETRA' }
+    options { disableConcurrentBuilds() }
+
+    stages {
+        stage('Build') {
+            steps {
+                withPythonEnv('System-CPython-3.10') {
+                    withEnv(['PIP_INDEX_URL=https://pypi.uwkm.nl/voxyan/oscar/+simple/']) {
+                        pysh "make install"
+                    }
+                }
+            }
+        }
+        stage('Lint') {
+            steps {
+                withPythonEnv('System-CPython-3.10') {
+                    pysh "make lint"
+                }
+            }
+        }
+        stage('Test') {
+            steps {
+                withPythonEnv('System-CPython-3.10') {
+                    pysh "make coverage"
+                }
+            }
+            post {
+                always {
+                    junit allowEmptyResults: true, testResults: '**/nosetests.xml'
+                }
+                success {
+                    step([
+                        $class: 'CoberturaPublisher',
+                        coberturaReportFile: '**/coverage.xml',
+                    ])
+                }
+            }
+        }
+    }
+    post {
+        always {
+            echo 'This will always run'
+        }
+        success {
+            echo 'This will run only if successful'
+            withPythonEnv('System-CPython-3.10') {
+                echo 'This will run only if successful'
+                pysh "version --plugin=wheel -B${env.BUILD_NUMBER} --skip-build"
+                sh "which git"
+                sh "git push --tags"
+            }
+        }
+        failure {
+            emailext subject: "JENKINS-NOTIFICATION: ${currentBuild.currentResult}: Job '${env.JOB_NAME} #${env.BUILD_NUMBER}'",
+                    body: '${SCRIPT, template="groovy-text.template"}',
+                    recipientProviders: [culprits(), brokenBuildSuspects(), brokenTestsSuspects()]
+
+        }
+        unstable {
+            echo 'This will run only if the run was marked as unstable'
+        }
+        changed {
+            echo 'This will run only if the state of the Pipeline has changed'
+            echo 'For example, if the Pipeline was previously failing but is now successful'
+        }
+    }
+}

Makefile

@@ -8,7 +8,7 @@ clean:
 	rm -Rf build/
 
 install:
-	pip install -e .[dev]
+	pip install -e .[dev] --upgrade --upgrade-strategy=eager --pre
 
 sandbox: install
 	python sandbox/manage.py migrate
@@ -43,15 +43,11 @@ publish_release_testpypi: build_release
 publish_release: build_release
 	twine upload dist/*
 
-lint.installed:
-	pip install -e .[lint]
-	touch $@
-
-lint: lint.installed
+lint:
 	black --check --exclude "migrations/*" oscarapi/
 	pylint setup.py oscarapi/
 
-black: lint.installed
+black:
 	black --exclude "/migrations/" oscarapi/
 
 uwsgi:

oscarapi/serializers/admin/product.py

@@ -145,12 +145,15 @@ def update(self, instance, validated_data):
             if (
                 self.partial
             ):  # we need to clean up all the attributes with wrong product class
+                attribute_codes = product_class.attributes.values_list(
+                    "code", flat=True
+                )
                 for attribute_value in instance.attribute_values.exclude(
                     attribute__product_class=product_class
                 ):
                     code = attribute_value.attribute.code
                     if (
-                        code in pclass_option_codes
+                        code in attribute_codes
                     ):  # if the attribute exist also on the new product class, update the attribute
                         attribute_value.attribute = product_class.attributes.get(
                             code=code

oscarapi/serializers/fields.py

@@ -1,6 +1,7 @@
 # pylint: disable=W0212, W0201, W0632
 import logging
 import operator
+import warnings
 
 from os.path import basename, join
 from urllib.parse import urlsplit, parse_qs
@@ -15,8 +16,10 @@
 from rest_framework.fields import get_attribute
 
 from oscar.core.loading import get_model, get_class
+from oscarapi.utils.deprecations import RemovedInFutureRelease
 
 from oscarapi import settings
+from oscarapi.utils.attributes import AttributeFieldBase, attribute_details
 from oscarapi.utils.loading import get_api_class
 from oscarapi.utils.exists import bound_unique_together_get_or_create
 from .exceptions import FieldError
@@ -27,7 +30,6 @@
 create_from_breadcrumbs = get_class("catalogue.categories", "create_from_breadcrumbs")
 entity_internal_value = get_api_class("serializers.hooks", "entity_internal_value")
 RetrieveFileMixin = get_api_class(settings.FILE_DOWNLOADER_MODULE, "RetrieveFileMixin")
-attribute_details = operator.itemgetter("code", "value")
 
 
 class TaxIncludedDecimalField(serializers.DecimalField):
@@ -93,7 +95,7 @@ def use_pk_only_optimization(self):
         return False
 
 
-class AttributeValueField(serializers.Field):
+class AttributeValueField(AttributeFieldBase, serializers.Field):
     """
     This field is used to handle the value of the ProductAttributeValue model
 
@@ -103,80 +105,56 @@ class AttributeValueField(serializers.Field):
     """
 
     def __init__(self, **kwargs):
+        warnings.warn(
+            "AttributeValueField is deprecated and will be removed in a future version of oscarapi",
+            RemovedInFutureRelease,
+            stacklevel=2,
+        )
         # this field always needs the full object
         kwargs["source"] = "*"
-        kwargs["error_messages"] = {
-            "no_such_option": _("{code}: Option {value} does not exist."),
-            "invalid": _("Wrong type, {error}."),
-            "attribute_validation_error": _(
-                "Error assigning `{value}` to {code}, {error}."
-            ),
-            "attribute_required": _("Attribute {code} is required."),
-            "attribute_missing": _(
-                "No attribute exist with code={code}, "
-                "please define it in the product_class first."
-            ),
-            "child_without_parent": _(
-                "Can not find attribute if product_class is empty and "
-                "parent is empty as well, child without parent?"
-            ),
-        }
         super(AttributeValueField, self).__init__(**kwargs)
 
     def get_value(self, dictionary):
         # return all the data because this field uses everything
         return dictionary
 
+    def to_product_attribute(self, data):
+        if "product" in data:
+            # we need the attribute to determine the type of the value
+            return ProductAttribute.objects.get(
+                code=data["code"], product_class__products__id=data["product"]
+            )
+        elif "product_class" in data and data["product_class"] is not None:
+            return ProductAttribute.objects.get(
+                code=data["code"], product_class__slug=data.get("product_class")
+            )
+        elif "parent" in data:
+            return ProductAttribute.objects.get(
+                code=data["code"], product_class__products__id=data["parent"]
+            )
+
+    def to_attribute_type_value(self, attribute, code, value):
+        internal_value = super().to_attribute_type_value(attribute, code, value)
+        if attribute.type in [
+            attribute.IMAGE,
+            attribute.FILE,
+        ]:
+            image_field = ImageUrlField()
+            image_field._context = self.context
+            internal_value = image_field.to_internal_value(value)
+
+        return internal_value
+
     def to_internal_value(self, data):  # noqa
         assert "product" in data or "product_class" in data or "parent" in data
 
         try:
             code, value = attribute_details(data)
             internal_value = value
 
-            if "product" in data:
-                # we need the attribute to determine the type of the value
-                attribute = ProductAttribute.objects.get(
-                    code=code, product_class__products__id=data["product"]
-                )
-            elif "product_class" in data and data["product_class"] is not None:
-                attribute = ProductAttribute.objects.get(
-                    code=code, product_class__slug=data.get("product_class")
-                )
-            elif "parent" in data:
-                attribute = ProductAttribute.objects.get(
-                    code=code, product_class__products__id=data["parent"]
-                )
+            attribute = self.to_product_attribute(data)
 
-            if attribute.required and value is None:
-                self.fail("attribute_required", code=code)
-
-            # some of these attribute types need special processing, or their
-            # validation will fail
-            if attribute.type == attribute.OPTION:
-                internal_value = attribute.option_group.options.get(option=value)
-            elif attribute.type == attribute.MULTI_OPTION:
-                if attribute.required and not value:
-                    self.fail("attribute_required", code=code)
-                internal_value = attribute.option_group.options.filter(option__in=value)
-                if len(value) != internal_value.count():
-                    non_existing = set(value) - set(
-                        internal_value.values_list("option", flat=True)
-                    )
-                    non_existing_as_error = ",".join(sorted(non_existing))
-                    self.fail("no_such_option", value=non_existing_as_error, code=code)
-            elif attribute.type == attribute.DATE:
-                date_field = serializers.DateField()
-                internal_value = date_field.to_internal_value(value)
-            elif attribute.type == attribute.DATETIME:
-                date_field = serializers.DateTimeField()
-                internal_value = date_field.to_internal_value(value)
-            elif attribute.type == attribute.ENTITY:
-                internal_value = entity_internal_value(attribute, value)
-            elif attribute.type in [attribute.IMAGE, attribute.FILE]:
-                image_field = ImageUrlField()
-                image_field._context = self.context
-                internal_value = image_field.to_internal_value(value)
+            internal_value = self.to_attribute_type_value(attribute, code, value)
 
             # the rest of the attribute types don't need special processing
             try:
@@ -221,10 +199,14 @@ def to_representation(self, value):
             return value.value.option
         elif obj_type == value.attribute.MULTI_OPTION:
             return value.value.values_list("option", flat=True)
-        elif obj_type == value.attribute.FILE:
-            return value.value.url
-        elif obj_type == value.attribute.IMAGE:
-            return value.value.url
+        elif obj_type in [value.attribute.FILE, value.attribute.IMAGE]:
+            if not value.value:
+                return None
+            url = value.value.url
+            request = self.context.get("request", None)
+            if request is not None:
+                url = request.build_absolute_uri(url)
+            return url
         elif obj_type == value.attribute.ENTITY:
             if hasattr(value.value, "json"):
                 return value.value.json()