Merge pull request #55 from ecometrica/fix_keyerror

Fix KeyError

CHANGELOG.rst

@@ -2,6 +2,13 @@
 Release Notes
 =============
 
+1.6.0
+-----
+* Fix KeyError from _get_site_by_id
+* Drop support for Django 1.7
+* Remove unnecessary cache type warnings
+* Remove deprecated SiteIDHook
+
 1.5.0
 -----
 * Support Django 2.0 (PR #47 and #60)

README.rst

@@ -62,13 +62,6 @@ Add to your settings.py TEMPLATES loaders in the OPTIONS section::
         ...
     ]
 
-Or for Django <= 1.7, add to settings.py TEMPLATES_LOADERS::
-
-    TEMPLATE_LOADERS = ( 
-        'multisite.template.loaders.filesystem.Loader',
-        'django.template.loaders.app_directories.Loader',
-    ) 
-
 Edit settings.py MIDDLEWARE (MIDDLEWARE_CLASSES for Django < 1.10)::
 
     MIDDLEWARE = (
@@ -155,8 +148,7 @@ settings.py::
 Templates
 ---------
 If required, create template subdirectories for domain level templates (in a
-location specified in settings.TEMPLATES['DIRS'], or in settings.TEMPLATE_DIRS
-for Django <=1.7).
+location specified in settings.TEMPLATES['DIRS'].
 
 Multisite's template loader will look for templates in folders with the names of
 domains, such as::

multisite/__version__.py

@@ -1 +1 @@
-__version__ = '1.5.0'
+__version__ = '1.6.0'

multisite/hacks.py

@@ -2,7 +2,6 @@
 from __future__ import absolute_import
 
 import sys
-from warnings import warn
 
 from django.conf import settings
 from django.db.models.signals import post_save, post_delete
@@ -22,6 +21,7 @@ def use_framework_for_site_cache():
 
     # Patch the SiteManager class
     models.SiteManager.clear_cache = SiteManager_clear_cache
+    models.SiteManager._get_site_by_id = SiteManager_get_site_by_id
 
     # Hooks to update SiteCache
     post_save.connect(site_cache._site_changed_hook, sender=models.Site)
@@ -35,6 +35,20 @@ def SiteManager_clear_cache(self):
     models.SITE_CACHE.clear()
 
 
+# Override SiteManager._get_site_by_id
+def SiteManager_get_site_by_id(self, site_id):
+    """
+    Patch _get_site_by_id to retrieve the site from the cache at the
+    beginning of the method to avoid a race condition.
+    """
+    models = sys.modules.get(self.__class__.__module__)
+    site = models.SITE_CACHE.get(site_id)
+    if site is None:
+        site = self.get(pk=site_id)
+        models.SITE_CACHE[site_id] = site
+    return site
+
+
 class SiteCache(object):
     """Wrapper for SITE_CACHE that assigns a key_prefix."""
 
@@ -49,28 +63,12 @@ def __init__(self, cache=None):
                 settings.CACHES[cache_alias].get('KEY_PREFIX', '')
             )
             cache = caches[cache_alias]
-            self._warn_cache_backend(cache, cache_alias)
         else:
             self._key_prefix = getattr(
                 settings, 'CACHE_MULTISITE_KEY_PREFIX', cache.key_prefix
             )
         self._cache = cache
 
-    def _warn_cache_backend(self, cache, cache_alias):
-        from django.core.cache.backends.dummy import DummyCache
-        from django.core.cache.backends.db import DatabaseCache
-        from django.core.cache.backends.filebased import FileBasedCache
-        from django.core.cache.backends.locmem import LocMemCache
-
-        if isinstance(cache, (LocMemCache, FileBasedCache)):
-            warn(("'%s' cache is %s, which may cause stale caches." %
-                  (cache_alias, type(cache).__name__)),
-                 RuntimeWarning, stacklevel=3)
-        elif isinstance(cache, (DatabaseCache, DummyCache)):
-            warn(("'%s' is %s, causing extra database queries." %
-                  (cache_alias, type(cache).__name__)),
-                 RuntimeWarning, stacklevel=3)
-
     def _get_cache_key(self, key):
         return 'sites.%s.%s' % (self.key_prefix, key)
 

multisite/tests.py

@@ -44,7 +44,6 @@
 from .hosts import ALLOWED_HOSTS, AllowedHosts, IterableLazyObject
 from .middleware import CookieDomainMiddleware, DynamicSiteMiddleware
 from .models import Alias
-from .threadlocals import SiteIDHook
 
 
 class RequestFactory(DjangoRequestFactory):
@@ -478,14 +477,12 @@ def test_compare_site_ids(self):
 
     def test_compare_differing_types(self):
         self.site_id.set(1)
-        # SiteIDHook <op> int
         self.assertNotEqual(self.site_id, '1')
         self.assertFalse(self.site_id == '1')
         self.assertTrue(self.site_id < '1')
         self.assertTrue(self.site_id <= '1')
         self.assertFalse(self.site_id > '1')
         self.assertFalse(self.site_id >= '1')
-        # int <op> SiteIDHook
         self.assertNotEqual('1', self.site_id)
         self.assertFalse('1' == self.site_id)
         self.assertFalse('1' < self.site_id)
@@ -547,24 +544,6 @@ def test_deferred_site(self):
                          site.id)
 
 
-@pytest.mark.django_db
-class TestSiteIDHook(TestCase):
-    def test_deprecation_warning(self):
-        with warnings.catch_warnings(record=True) as w:
-            warnings.simplefilter('always')
-            threadlocals.__warningregistry__ = {}
-            SiteIDHook()
-            self.assertTrue(w)
-            self.assertTrue(issubclass(w[-1].category, DeprecationWarning))
-
-    def test_default_value(self):
-        with warnings.catch_warnings():
-            warnings.simplefilter('ignore')
-            site_id = SiteIDHook()
-            self.assertEqual(site_id.default, 1)
-            self.assertEqual(int(site_id), 1)
-
-
 @pytest.mark.django_db
 class AliasTest(TestCase):
     def setUp(self):
@@ -1048,21 +1027,11 @@ class TemplateLoaderTests(TestCase):
 
     def test_get_template_multisite_default_dir(self):
         template = get_template("test.html")
-        if django.VERSION < (1, 8):  # <1.7 render() requires Context instance
-            from django.template.context import Context
-            self.assertEqual(template.render(context=Context()), "Test!")
-        else:
-            self.assertEqual(template.render(), "Test!")
+        self.assertEqual(template.render(), "Test!")
 
     def test_domain_template(self):
         template = get_template("example.html")
-        if django.VERSION < (1, 8):  # <1.7 render() requires Context instance
-            from django.template.context import Context
-            self.assertEqual(
-                template.render(context=Context()), "Test example.com template"
-            )
-        else:
-            self.assertEqual(template.render(), "Test example.com template")
+        self.assertEqual(template.render(), "Test example.com template")
 
     def test_get_template_old_settings(self):
         # tests that we can still get to the template filesystem loader with
@@ -1085,11 +1054,7 @@ def test_get_template_old_settings(self):
                 ]
         ):
             template = get_template("test.html")
-            if django.VERSION < (1, 8):  # <1.7 render() requires Context instance
-                from django.template.context import Context
-                self.assertEqual(template.render(context=Context()), "Test!")
-            else:
-                self.assertEqual(template.render(), "Test!")
+            self.assertEqual(template.render(), "Test!")
 
 
 class UpdatePublicSuffixListCommandTestCase(TestCase):

multisite/threadlocals.py

@@ -154,10 +154,3 @@ def get_default(self):
             qset = Site.objects.only('id')
             self.default = qset.get(domain=self.default_domain).id
         return self.default
-
-
-def SiteIDHook():
-    """Deprecated: Use multisite.SiteID(default=1) for identical behaviour."""
-    warn('Use multisite.SiteID instead of multisite.threadlocals.SiteIDHook',
-         DeprecationWarning, stacklevel=2)
-    return SiteID(default=1)

setup.py

@@ -7,9 +7,9 @@
 
 
 if sys.version_info < (3, 4):
-    install_requires = ['Django>=1.7,<2.0', 'tldextract>=1.2']
+    install_requires = ['Django>=1.8,<2.0', 'tldextract>=1.2']
 else:
-    install_requires = ['Django>=1.7,<2.1', 'tldextract>=1.2']
+    install_requires = ['Django>=1.8,<2.1', 'tldextract>=1.2']
 
 
 def long_description():

tox.ini

@@ -8,10 +8,10 @@ setenv=
     PYTHONPATH = {toxinidir}:{env:PYTHONPATH:}
 usedevelop = True
 envlist =
-    py36-django{2.0, 1.11}
-    py35-django{2.0, 1.11,1.10,1.9,1.8}
-    py34-django{2.0, 1.11,1.10,1.9,1.8,1.7}
-    py27-django{1.11,1.10,1.9,1.8,1.7}
+    py36-django{2.1,2.0,1.11}
+    py35-django{2.1,2.0,1.11,1.10,1.9,1.8}
+    py34-django{2.0,1.11,1.10,1.9,1.8}
+    py27-django{1.11,1.10,1.9,1.8}
 
 [testenv]
 commands = pytest --cov --cov-config .coveragerc --pyargs multisite
@@ -20,13 +20,12 @@ deps =
     pytest
     pytest-cov
     pytest-pythonpath
+    pytest-django
 
     py27: mock
-    django2.0,django1.11,django1.10,django1.9,django1.8: pytest-django
+    django2.1: Django>=2.1,<2.2
     django2.0: Django>=2.0,<2.1
     django1.11: Django>=1.11,<2.0
     django1.10: Django>=1.10,<1.11
     django1.9: Django>=1.9,<1.10
     django1.8: Django>=1.8,<1.9
-    django1.7: pytest-django<3.2.0
-    django1.7: Django>=1.7,<1.8