Fix broken basket middleware (#319)

* new release

* Undo broken lazy basket.

Added prepare=False to get_basket because we don't really need to apply
any discounts if we only need to find the basket id.

* Added ApiBasketMiddleWare test and filter in get_anonymous_basket

* removed useless code

oscarapi/basket/operations.py

@@ -74,7 +74,7 @@ def get_anonymous_basket(request):
 
     basket_id = get_basket_id_from_session(request)
     try:
-        basket = Basket.open.get(pk=basket_id)
+        basket = Basket.open.get(pk=basket_id, owner=None)
     except Basket.DoesNotExist:
         basket = None
 
@@ -83,7 +83,6 @@ def get_anonymous_basket(request):
 
 def get_user_basket(user):
     "get basket for a user."
-
     try:
         basket, __ = Basket.open.get_or_create(owner=user)
     except Basket.MultipleObjectsReturned:

oscarapi/middleware.py

@@ -5,7 +5,7 @@
 from django.contrib.sessions.middleware import SessionMiddleware
 from django.core.exceptions import PermissionDenied
 from django.http.response import HttpResponse
-from django.utils.translation import gettext as _
+from django.utils.translation import ugettext as _
 
 from oscar.core.loading import get_class
 
@@ -19,7 +19,6 @@
     get_basket,
 )
 
-from django.utils.functional import SimpleLazyObject, empty
 from oscarapi.utils.loading import get_api_class
 from oscarapi.utils.request import get_domain
 from oscarapi.utils.session import session_id_from_parsed_session_uri, get_session
@@ -157,46 +156,29 @@ class ApiBasketMiddleWare(BasketMiddleware, IsApiRequest):
 
     def __call__(self, request):
         if self.is_api_request(request):
+            request.cookies_to_delete = []
+            # we should make sure that any cookie baskets are turned into
+            # session baskets, since oscarapi uses only baskets from the
+            # session.
+            cookie_key = self.get_cookie_key(request)
 
-            def load_basket_into_session():
-                request.cookies_to_delete = []
-                # we should make sure that any cookie baskets are turned into
-                # session baskets, since oscarapi uses only baskets from the
-                # session.
-                cookie_key = self.get_cookie_key(request)
-
-                basket = self.get_cookie_basket(
-                    cookie_key,
-                    request,
-                    Exception("get_cookie_basket doesn't use the manager argument"),
-                )
-
-                if basket is not None:
-                    # when a basket exists and we are already allowed to access
-                    # this basket
-                    if request_allows_access_to_basket(request, basket):
-                        pass
-                    else:
-                        store_basket_in_session(basket, request.session)
-
-            request.basket = SimpleLazyObject(load_basket_into_session)
+            basket = self.get_cookie_basket(
+                cookie_key,
+                request,
+                Exception("get_cookie_basket doesn't use the manager argument"),
+            )
 
-            response = self.get_response(request)
-            return self.process_response(request, response)
+            if basket is not None:
+                # when a basket exists and we are already allowed to access
+                # this basket
+                if request_allows_access_to_basket(request, basket):
+                    pass
+                else:
+                    store_basket_in_session(basket, request.session)
 
         return super(ApiBasketMiddleWare, self).__call__(request)
 
     def process_response(self, request, response):
-        if not hasattr(request, "basket"):
-            return response
-
-        # If the basket was never initialized we can safely return
-        if (
-            isinstance(request.basket, SimpleLazyObject)
-            and request.basket._wrapped is empty  # pylint: disable=protected-access
-        ):
-            return response
-
         if (
             self.is_api_request(request)
             and hasattr(request, "user")
@@ -208,7 +190,7 @@ def process_response(self, request, response):
             # We just have to make sure it is stored as a cookie, because it
             # could have been created by oscarapi.
             cookie_key = self.get_cookie_key(request)
-            basket = get_basket(request)
+            basket = get_basket(request, prepare=False)
             cookie = self.get_basket_hash(basket.id)
 
             # Delete any surplus cookies

oscarapi/tests/unit/testbasket.py

@@ -1,7 +1,7 @@
 import json
 
 from unittest.mock import patch
-
+from django.test import override_settings
 from django.contrib.auth import get_user_model
 from django.urls import reverse
 
@@ -1147,3 +1147,107 @@ def test_get_user_basket_with_multiple_baskets(self):
         user_basket = get_user_basket(user)
         self.assertEqual(Basket.open.count(), 1)
         self.assertEqual(user_basket, Basket.open.first())
+
+
+@override_settings(
+    MIDDLEWARE=(
+        "django.middleware.common.CommonMiddleware",
+        "django.middleware.csrf.CsrfViewMiddleware",
+        "django.contrib.sessions.middleware.SessionMiddleware",
+        "django.contrib.auth.middleware.AuthenticationMiddleware",
+        "django.contrib.messages.middleware.MessageMiddleware",
+        "django.middleware.clickjacking.XFrameOptionsMiddleware",
+        "oscarapi.middleware.ApiBasketMiddleWare",
+        "django.contrib.flatpages.middleware.FlatpageFallbackMiddleware",
+    )
+)
+class ApiBasketMiddleWareTest(APITest):
+    fixtures = [
+        "product",
+        "productcategory",
+        "productattribute",
+        "productclass",
+        "productattributevalue",
+        "category",
+        "attributeoptiongroup",
+        "attributeoption",
+        "stockrecord",
+        "partner",
+        "option",
+    ]
+
+    def test_basket_login_logout(self):
+        self.assertEqual(
+            Basket.objects.count(), 0, "Initially there should be no baskets"
+        )
+
+        # add something to the anonymous basket, so we get a cookie basket
+        url = reverse("basket:add", kwargs={"pk": 1})
+        post_params = {"child_id": 2, "action": "add", "quantity": 5}
+        response = self.client.post(url, post_params, follow=True)
+
+        self.assertEqual(
+            Basket.objects.count(),
+            1,
+            "After posting to the basket, 1 basket should be created.",
+        )
+        self.assertIn(
+            "oscar_open_basket",
+            self.client.cookies,
+            "An basket cookie should have been created",
+        )
+        self.assertStartsWith(self.client.cookies["oscar_open_basket"].value, "1")
+
+        # retrieve the basket with oscarapi.
+        self.response = self.get("api-basket")
+        self.response.assertValueEqual(
+            "owner", None, "The basket should not have an owner"
+        )
+        self.response.assertValueEqual("id", 1)
+        self.assertStartsWith(self.client.cookies["oscar_open_basket"].value, "1")
+
+        # now lets log in with oscarapi
+        response = self.post("api-login", username="nobody", password="nobody")
+        # and lets retrieve the basket
+        self.response = self.get("api-basket")
+        self.response.assertValueEqual(
+            "owner",
+            "http://testserver/api/users/2/",
+            "the basket after login should have an owner",
+        )
+        self.assertEqual(
+            self.client.cookies["oscar_open_basket"].value,
+            "1:Rdm76bzEHM-N1G6WSTj0Zu9ByZ80a8ggxSkqqvGbC6s",
+            "After logging out the cookie unfortunately does not go away",
+        )
+
+        response = self.client.post(url, post_params, follow=True)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(Basket.objects.count(), 2)
+        self.assertEqual(
+            self.client.cookies["oscar_open_basket"].value,
+            "",
+            "The basket cookie should be removed",
+        )
+
+        self.response = self.delete("api-login")
+        self.assertStartsWith(
+            self.client.cookies["oscar_open_basket"].value,
+            "",
+            "After loging out, nothing happened to the basket cookie",
+        )
+
+        self.response = self.get("api-basket")
+        self.response.assertValueEqual(
+            "owner", None, "The logged out user's basket should not have an owner"
+        )
+        self.assertEqual(
+            Basket.objects.count(),
+            3,
+            "A new basket should be created for the anonymous (logged out) user",
+        )
+        self.assertStartsWith(
+            self.client.cookies["oscar_open_basket"].value,
+            "3",
+            "The basket cookie is re-established after accessing the basket when logged out",
+        )

oscarapi/tests/utils.py

@@ -121,6 +121,12 @@ def reload_modules(modules=()):
         for module in modules:
             reload_module(module)
 
+    def assertStartsWith(self, value, startvalue, message=""):
+        if not value.startswith(startvalue):
+            self.fail(
+                "'%s' does not start with '%s'. %s" % (value, startvalue, message)
+            )
+
 
 class ParsedResponse(object):
     def __init__(self, response, unittestcase):

setup.py

@@ -1,6 +1,6 @@
 from setuptools import find_packages, setup
 
-__version__ = "3.2.2"
+__version__ = "3.2.3"
 
 setup(
     # package name in pypi