From 4e993a5d19143f1218ee96d0fd899bfec7674423 Mon Sep 17 00:00:00 2001 From: Steven Skoczen Date: Tue, 3 Mar 2015 13:14:39 +0700 Subject: [PATCH] Switches to EscapedFragmentMiddleware, fixes tests. --- .gitignore | 2 + README.md | 5 +- django_seo_js/middleware/__init__.py | 1 + django_seo_js/middleware/escaped_fragment.py | 36 ++++++++++++++ django_seo_js/middleware/hashbang.py | 22 +-------- django_seo_js/tests/test_helpers.py | 6 +-- django_seo_js/tests/test_middlewares.py | 52 +++++++++++++------- settings.py | 2 +- 8 files changed, 82 insertions(+), 44 deletions(-) create mode 100644 django_seo_js/middleware/escaped_fragment.py diff --git a/.gitignore b/.gitignore index 5af7173..8835198 100644 --- 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 index eb79fd7..2aa144e 100644 --- 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 index 0d7ef44..374bd39 100644 --- 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 new file mode 100644 index 0000000..155e249 --- /dev/null +++ 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 index 61f3e1d..ab01fc5 100644 --- 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 index e6a39fa..5735f6b 100644 --- a/django_seo_js/tests/test_helpers.py +++ b/django_seo_js/tests/test_helpers.py @@ -1,7 +1,7 @@ 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): @@ -9,11 +9,11 @@ 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 index c547530..2eb6f58 100644 --- 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 index 2a752e2..e7df3ae 100644 --- 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',