Switches to EscapedFragmentMiddleware, fixes tests.

skoczen · Mar 3, 2015 · 4e993a5 · 4e993a5
1 parent 7b04b89
commit 4e993a5
Show file tree

Hide file tree

Showing 8 changed files with 82 additions and 44 deletions.
diff --git a/.gitignore b/.gitignore
@@ -36,3 +36,5 @@ README.html
 shelf.db
 .idea/*
 venv
+
+django_seo_js/.DS_Store
diff --git a/README.md b/README.md
@@ -38,7 +38,7 @@ Quick-links:
     ```python
     # If in doubt, just include both.  Details below.
     MIDDLEWARE_CLASSES = (
-        'django_seo_js.middleware.HashBangMiddleware',  # If you're using #!
+        'django_seo_js.middleware.EscapedFragmentMiddleware',  # If you're using #!
         'django_seo_js.middleware.UserAgentMiddleware',  # If you want to detect by user agent
     ) + MIDDLEWARE_CLASSES
 
@@ -244,7 +244,8 @@ Original development was at GreenKahuna (now defunct.)
 
 ### 0.3.1 - March 3, 2015
 
-* Bugfix to user agent middleware not respecting `ENABLED`, thanks to [rchrd2](https://github.com/rchrd2).
+* **Deprecation**: `django_seo_js.middleware.HashBangMiddleware` is now called `django_seo_js.middleware.EscapedFragmentMiddleware`, to fix confusion.  `HashBangMiddleware` will be removed in 0.5.  Which I would bet is probably late 2015, early 2016.  You'll see a log warning from now on.  Thanks to [thoop](https://github.com/thoop) for the report.
+* Bugfix to user agent middleware not respecting `ENABLED`, thanks to [rchrd2](https://github.com/rchrd2). Also reported by [denisvlr](https://github.com/denisvlr).
 * New (backwards-compatible) `build_absolute_uri` method that can be overridden, thanks to [chazcb](https://github.com/chazcb).
 * Removed Google, Yahoo, and Bing from the default `USER_AGENTS`, since they now support the escaped fragment protocol (and leaving them in can cause a cloaking penalty.)  Thanks to [thoop](https://github.com/thoop) for pointing this out.
 

diff --git a/django_seo_js/middleware/__init__.py b/django_seo_js/middleware/__init__.py
@@ -1,2 +1,3 @@
+from .escaped_fragment import EscapedFragmentMiddleware
 from .hashbang import HashBangMiddleware
 from .useragent import UserAgentMiddleware
diff --git a/django_seo_js/middleware/escaped_fragment.py b/django_seo_js/middleware/escaped_fragment.py
@@ -0,0 +1,36 @@
+from django_seo_js import settings
+from django_seo_js.backends import SelectedBackend
+from django_seo_js.helpers import request_should_be_ignored
+
+import logging
+logger = logging.getLogger(__name__)
+
+
+class EscapedFragmentMiddleware(SelectedBackend):
+    def process_request(self, request):
+        if not settings.ENABLED:
+            return
+
+        if request_should_be_ignored(request):
+            return
+
+        if "_escaped_fragment_" not in request.GET:
+            return
+
+        url = self.backend.build_absolute_uri(request)
+        try:
+            return self.backend.get_response_for_url(url)
+        except Exception as e:
+            logger.exception(e)
+
+
+class HashBangMiddleware(EscapedFragmentMiddleware):
+
+    def __init__(self, *args, **kwargs):
+        logging.info(
+            "Deprecation note: HashBangMiddleware has been renamed EscapedFragmentMiddleware,"
+            " for more clarity. Upgrade your MIDDLEWARE_CLASSES to \n"
+            "   'django_seo_js.middleware.EscapedFragmentMiddleware'"
+            " when you get a chance. HashBangMiddleware will be removed in v0.5"
+        )
+        super(HashBangMiddleware, self).__init__(*args, **kwargs)
diff --git a/django_seo_js/middleware/hashbang.py b/django_seo_js/middleware/hashbang.py
@@ -1,24 +1,4 @@
-from django_seo_js import settings
-from django_seo_js.backends import SelectedBackend
-from django_seo_js.helpers import request_should_be_ignored
+from .escaped_fragment import HashBangMiddleware
 
 import logging
 logger = logging.getLogger(__name__)
-
-
-class HashBangMiddleware(SelectedBackend):
-    def process_request(self, request):
-        if not settings.ENABLED:
-            return
-
-        if request_should_be_ignored(request):
-            return
-
-        if "_escaped_fragment_" not in request.GET:
-            return
-
-        url = self.backend.build_absolute_uri(request)
-        try:
-            return self.backend.get_response_for_url(url)
-        except Exception as e:
-            logger.exception(e)
diff --git a/django_seo_js/tests/test_helpers.py b/django_seo_js/tests/test_helpers.py
@@ -1,19 +1,19 @@
 from django.test import TestCase
 
 from django_seo_js.tests.utils import override_settings
-from django_seo_js.middleware import HashBangMiddleware
+from django_seo_js.middleware import EscapedFragmentMiddleware
 
 
 class HelpersTest(TestCase):
 
     @override_settings(BACKEND='django_seo_js.backends.TestBackend')
     def test_update_the_render_cache(self):
         from django_seo_js.helpers import update_cache_for_url
-        self.middleware = HashBangMiddleware()
+        self.middleware = EscapedFragmentMiddleware()
         self.assertEqual(update_cache_for_url("http://example.com"), True)
 
     @override_settings(BACKEND='django_seo_js.backends.TestBackend', ENABLED=False)
     def test_update_skips_if_disabled(self):
         from django_seo_js.helpers import update_cache_for_url
-        self.middleware = HashBangMiddleware()
+        self.middleware = EscapedFragmentMiddleware()
         self.assertEqual(update_cache_for_url("http://example.com"), False)
diff --git a/django_seo_js/tests/test_middlewares.py b/django_seo_js/tests/test_middlewares.py
@@ -2,7 +2,7 @@
 from django.test import TestCase
 
 from django_seo_js.tests.utils import override_settings
-from django_seo_js.middleware import HashBangMiddleware, UserAgentMiddleware
+from django_seo_js.middleware import EscapedFragmentMiddleware, UserAgentMiddleware, HashBangMiddleware
 
 print override_settings
 
@@ -11,12 +11,12 @@ class BaseMiddlewareTest(TestCase):
     pass
 
 
-class HashBangMiddlewareTest(TestCase):
+class EscapedFragmentMiddlewareTest(TestCase):
 
     @override_settings(BACKEND='django_seo_js.backends.TestBackend')
     def setUp(self):
-        super(HashBangMiddlewareTest, self).setUp()
-        self.middleware = HashBangMiddleware()
+        super(EscapedFragmentMiddlewareTest, self).setUp()
+        self.middleware = EscapedFragmentMiddleware()
         self.request = Mock()
         self.request.path = "/"
         self.request.GET = {}
@@ -31,19 +31,19 @@ def test_does_not_have_escaped_fragment(self):
 
     @override_settings(BACKEND='django_seo_js.backends.TestBackend', ENABLED=False)
     def test_has_escaped_fragment_skips_if_disabled_via_enabled(self):
-        self.middleware = HashBangMiddleware()
+        self.middleware = EscapedFragmentMiddleware()
         self.request.GET = {}
         self.assertEqual(self.middleware.process_request(self.request), None)
 
     @override_settings(BACKEND='django_seo_js.backends.TestServiceDownBackend')
     def test_has_escaped_fragment_skips_if_service_is_down(self):
-        self.middleware = HashBangMiddleware()
+        self.middleware = EscapedFragmentMiddleware()
         self.request.GET = {"_escaped_fragment_": None}
         self.assertEqual(self.middleware.process_request(self.request), None)
 
     @override_settings(BACKEND='django_seo_js.backends.TestBackend')
     def test_overriding_skips_sitemap_xml_by_default(self):
-        self.middleware = HashBangMiddleware()
+        self.middleware = EscapedFragmentMiddleware()
         self.request.path = "/sitemap.xml"
         self.request.GET = {"_escaped_fragment_": None}
         self.assertEqual(self.middleware.process_request(self.request), None)
@@ -54,7 +54,7 @@ def test_overriding_skips_sitemap_xml_by_default(self):
         IGNORE_EXTENSIONS=[],
     )
     def test_overriding_skips_custom_overrides_xml_by_default(self):
-        self.middleware = HashBangMiddleware()
+        self.middleware = EscapedFragmentMiddleware()
         self.request.path = "/sitemap.xml"
         self.request.GET = {"_escaped_fragment_": None}
         self.assertEqual(self.middleware.process_request(self.request).content, "Test")
@@ -67,7 +67,7 @@ def test_overriding_skips_custom_overrides_xml_by_default(self):
 
     @override_settings(BACKEND='django_seo_js.backends.TestBackend')
     def test_overriding_skips_gifs_by_default(self):
-        self.middleware = HashBangMiddleware()
+        self.middleware = EscapedFragmentMiddleware()
         self.request.path = "/sitemap.xml"
         self.request.GET = {"_escaped_fragment_": None}
         self.assertEqual(self.middleware.process_request(self.request), None)
@@ -77,7 +77,7 @@ def test_overriding_skips_gifs_by_default(self):
         IGNORE_EXTENSIONS=[".html", ".txt", ]
     )
     def test_overriding_skips_custom_overrides_gifs_by_default(self):
-        self.middleware = HashBangMiddleware()
+        self.middleware = EscapedFragmentMiddleware()
         self.request.path = "/foo.gif"
         self.request.GET = {"_escaped_fragment_": None}
         self.assertEqual(self.middleware.process_request(self.request).content, "Test")
@@ -89,6 +89,17 @@ def test_overriding_skips_custom_overrides_gifs_by_default(self):
         self.assertEqual(self.middleware.process_request(self.request), None)
 
 
+class HashBangMiddlewareTest(EscapedFragmentMiddlewareTest):
+
+    @override_settings(BACKEND='django_seo_js.backends.TestBackend')
+    def setUp(self):
+        super(HashBangMiddlewareTest, self).setUp()
+        self.middleware = HashBangMiddleware()
+        self.request = Mock()
+        self.request.path = "/"
+        self.request.GET = {}
+
+
 class UserAgentMiddlewareTest(TestCase):
 
     @override_settings(BACKEND='django_seo_js.backends.TestBackend')
@@ -101,7 +112,8 @@ def setUp(self):
 
     def test_matches_one_of_the_default_user_agents(self):
         self.request.META = {
-            "HTTP_USER_AGENT": "Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)"
+            "HTTP_USER_AGENT":
+            "Mozilla/2.0 (compatible; Ask Jeeves/Teoma; +http://about.ask.com/en/docs/about/webmasters.shtml)"
         }
         self.assertEqual(self.middleware.process_request(self.request).content, "Test")
 
@@ -129,7 +141,8 @@ def test_overriding_matches(self):
     def test_overriding_does_not_match_properly(self):
         self.middleware = UserAgentMiddleware()
         self.request.META = {
-            "HTTP_USER_AGENT": "Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)"
+            "HTTP_USER_AGENT":
+            "Mozilla/2.0 (compatible; Ask Jeeves/Teoma; +http://about.ask.com/en/docs/about/webmasters.shtml)"
         }
         self.assertEqual(self.middleware.process_request(self.request), None)
 
@@ -154,7 +167,8 @@ def test_overriding_matches_skips_if_disabled_via_enabled(self):
     def test_overriding_matches_skips_if_service_is_down(self):
         self.middleware = UserAgentMiddleware()
         self.request.META = {
-            "HTTP_USER_AGENT": "Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)"
+            "HTTP_USER_AGENT":
+            "Mozilla/2.0 (compatible; Ask Jeeves/Teoma; +http://about.ask.com/en/docs/about/webmasters.shtml)"
         }
         self.assertEqual(self.middleware.process_request(self.request), None)
 
@@ -163,7 +177,8 @@ def test_overriding_skips_sitemap_xml_by_default(self):
         self.middleware = UserAgentMiddleware()
         self.request.path = "/sitemap.xml"
         self.request.META = {
-            "HTTP_USER_AGENT": "Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)"
+            "HTTP_USER_AGENT":
+            "Mozilla/2.0 (compatible; Ask Jeeves/Teoma; +http://about.ask.com/en/docs/about/webmasters.shtml)"
         }
         self.assertEqual(self.middleware.process_request(self.request), None)
 
@@ -176,7 +191,8 @@ def test_overriding_skips_custom_overrides_xml_by_default(self):
         self.middleware = UserAgentMiddleware()
         self.request.path = "/sitemap.xml"
         self.request.META = {
-            "HTTP_USER_AGENT": "Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)"
+            "HTTP_USER_AGENT":
+            "Mozilla/2.0 (compatible; Ask Jeeves/Teoma; +http://about.ask.com/en/docs/about/webmasters.shtml)"
         }
         self.assertEqual(self.middleware.process_request(self.request).content, "Test")
 
@@ -191,7 +207,8 @@ def test_overriding_skips_gifs_by_default(self):
         self.middleware = UserAgentMiddleware()
         self.request.path = "/foo.gif"
         self.request.META = {
-            "HTTP_USER_AGENT": "Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)"
+            "HTTP_USER_AGENT":
+            "Mozilla/2.0 (compatible; Ask Jeeves/Teoma; +http://about.ask.com/en/docs/about/webmasters.shtml)"
         }
         self.assertEqual(self.middleware.process_request(self.request), None)
 
@@ -203,7 +220,8 @@ def test_overriding_skips_custom_overrides_gifs_by_default(self):
         self.middleware = UserAgentMiddleware()
         self.request.path = "/foo.gif"
         self.request.META = {
-            "HTTP_USER_AGENT": "Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)"
+            "HTTP_USER_AGENT":
+            "Mozilla/2.0 (compatible; Ask Jeeves/Teoma; +http://about.ask.com/en/docs/about/webmasters.shtml)"
         }
         self.assertEqual(self.middleware.process_request(self.request).content, "Test")
 

diff --git a/settings.py b/settings.py
@@ -20,7 +20,7 @@
 )
 
 MIDDLEWARE_CLASSES = (
-    'django_seo_js.middleware.HashBangMiddleware',
+    'django_seo_js.middleware.EscapedFragmentMiddleware',
     'django_seo_js.middleware.UserAgentMiddleware',
     'django.middleware.common.CommonMiddleware',
     'django.contrib.sessions.middleware.SessionMiddleware',