Bladeren bron

Try to prevent circular import issues

Registering receivers is a tricky business pre-Django 1.7 as they get
registered at the same time when models get registered, but require the
sending model to already be in the app cache. It's surprising we don't
see more issues!

Recommendations:
* Avoid calls to get_model in modules that get called during app init at
  all cost
* Don't use get_model in your own code. Just import directly.
* Switch to Django 1.7 :)

This commit probably fixes the issue in #1205. But get_model calls to
fetch the category, product image and stock record will all fail,
because receivers listen to their signals.

This commit also moves the catalogue receiver into it's own receivers
module, because that's how we do it elsewhere. That won't make a
difference to the issues though.
master
Maik Hoepfel 11 jaren geleden
bovenliggende
commit
5f23648d9e

+ 4
- 0
docs/source/topics/class_loading_explained.rst Bestand weergeven

64
 the class is overridden, it will not require code changes. Care should be taken
64
 the class is overridden, it will not require code changes. Care should be taken
65
 when doing this, as this is a tricky trade-off between maintainability and
65
 when doing this, as this is a tricky trade-off between maintainability and
66
 added complexity.
66
 added complexity.
67
+Please note that we cannot recommend ever using ``get_model`` in your own code.
68
+Especially pre-Django 1.7, model initialisation is a tricky process and it's
69
+easy to run into circular import issues.
70
+
67
 
71
 
68
 Testing
72
 Testing
69
 -------
73
 -------

+ 5
- 13
oscar/apps/customer/history.py Bestand weergeven

6
 
6
 
7
 from oscar.core.loading import get_class
7
 from oscar.core.loading import get_class
8
 
8
 
9
-Product = get_model('catalogue', 'Product')
10
 product_viewed = get_class('catalogue.signals', 'product_viewed')
9
 product_viewed = get_class('catalogue.signals', 'product_viewed')
11
 
10
 
12
 
11
 
16
     """
15
     """
17
     ids = extract(request)
16
     ids = extract(request)
18
 
17
 
18
+    # Needs to live in local scope because receivers in this module get
19
+    # registered during model initialisation
20
+    # TODO Move this back to global scope once Django < 1.7 support is removed
21
+    Product = get_model('catalogue', 'Product')
22
+
19
     # Reordering as the ID order gets messed up in the query
23
     # Reordering as the ID order gets messed up in the query
20
     product_dict = Product.browsable.in_bulk(ids)
24
     product_dict = Product.browsable.in_bulk(ids)
21
     ids.reverse()
25
     ids.reverse()
67
         json.dumps(updated_ids),
71
         json.dumps(updated_ids),
68
         max_age=settings.OSCAR_RECENTLY_VIEWED_COOKIE_LIFETIME,
72
         max_age=settings.OSCAR_RECENTLY_VIEWED_COOKIE_LIFETIME,
69
         httponly=True)
73
         httponly=True)
70
-
71
-
72
-# Receivers
73
-
74
-@receiver(product_viewed)
75
-def receive_product_view(sender, product, user, request, response, **kwargs):
76
-    """
77
-    Receiver to handle viewing single product pages
78
-
79
-    Requires the request and response objects due to dependence on cookies
80
-    """
81
-    return update(product, request, response)

+ 1
- 1
oscar/apps/customer/models.py Bestand weergeven

26
 
26
 
27
 
27
 
28
 if django.VERSION < (1, 7):
28
 if django.VERSION < (1, 7):
29
-    from oscar.apps.customer.history import *  # noqa
29
+    from .receivers import *  # noqa
30
     from .alerts import receivers  # noqa
30
     from .alerts import receivers  # noqa

+ 17
- 0
oscar/apps/customer/receivers.py Bestand weergeven

1
+from django.dispatch import receiver
2
+
3
+from oscar.core.loading import get_class
4
+
5
+from . import history
6
+
7
+product_viewed = get_class('catalogue.signals', 'product_viewed')
8
+
9
+
10
+@receiver(product_viewed)
11
+def receive_product_view(sender, product, user, request, response, **kwargs):
12
+    """
13
+    Receiver to handle viewing single product pages
14
+
15
+    Requires the request and response objects due to dependence on cookies
16
+    """
17
+    return history.update(product, request, response)

+ 9
- 0
oscar/core/loading.py Bestand weergeven

278
         This is merely a thin wrapper around Django's get_model function.
278
         This is merely a thin wrapper around Django's get_model function.
279
         Raises LookupError if model isn't found.
279
         Raises LookupError if model isn't found.
280
         """
280
         """
281
+
282
+        # The snippet below is not useful in production, but helpful to
283
+        # investigate circular import issues
284
+        # from django.db.models.loading import app_cache_ready
285
+        # if not app_cache_ready():
286
+        #     print(
287
+        #         "%s.%s accessed before app cache is fully populated!" %
288
+        #         (app_label, model_name))
289
+
281
         model = django_get_model(app_label, model_name, *args, **kwargs)
290
         model = django_get_model(app_label, model_name, *args, **kwargs)
282
         if model is None:
291
         if model is None:
283
             raise LookupError(
292
             raise LookupError(

Laden…
Annuleren
Opslaan