From 0c3c84e5fd51c9b3279e93983afa3d72099a87e9 Mon Sep 17 00:00:00 2001 From: Adrian Tangochin Date: Sat, 25 Aug 2018 00:25:14 +0500 Subject: [PATCH 01/12] Added Django 1.11 to travis.yml --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 606b69f..97b5b60 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,6 +4,7 @@ python: - "3.5" - "3.6" env: + - DJANGO=1.11 DB=sqlite - DJANGO=2.0 DB=sqlite install: - pip install -q Django==$DJANGO From 3360c84eaea08b6424d3acc9510a5fd502ef7ec8 Mon Sep 17 00:00:00 2001 From: Adrian Tangochin Date: Sat, 25 Aug 2018 00:39:57 +0500 Subject: [PATCH 02/12] Rollback --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 97b5b60..606b69f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,7 +4,6 @@ python: - "3.5" - "3.6" env: - - DJANGO=1.11 DB=sqlite - DJANGO=2.0 DB=sqlite install: - pip install -q Django==$DJANGO From 33806c3069fbc201667bc1ef973bb81d62989c40 Mon Sep 17 00:00:00 2001 From: Adrian Tangochin Date: Wed, 24 Oct 2018 18:30:17 +0500 Subject: [PATCH 03/12] Removed django.contrib.redirects for seo-redirects --- README.rst | 3 ++- djangoseo/utils.py | 20 +++++++------------- setup.py | 2 +- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/README.rst b/README.rst index b79fd34..e0cdcfd 100644 --- a/README.rst +++ b/README.rst @@ -244,7 +244,6 @@ Redirects --------- Currently supported are two types of redirects: when an occurs error 404 and when model changes its URL on the site. -For each type of redirects used functional of `django.contrib.redirects `_. You must configure it before use redirects from ``django-seo``. If you need a redirection when an error occurs 404, enable ``SEO_USE_REDIRECTS`` and setup URL patterns for redirection in admin interface. It's like a standard URL patterns, but instead of finding a suitable view it creates a redirect in case of an error 404 for a given pattern. @@ -259,6 +258,8 @@ If you need a redirection when model changes its URL list the full path to the m 'your_app.models.Bar' ) +The default class for redirection is ``django.http.response.HttpResponsePermanentRedirect``, but if you want to change this behavior, you can change the HttpResponse classes used by the middleware by creating a subclass of RedirectsMiddleware and overriding response_redirect_class. + Attention: each path to model must be direct and model must have a method ``get_absolute_url``. Work such redirection follows: when path to model on site changed, it create redirection to old path. For example: diff --git a/djangoseo/utils.py b/djangoseo/utils.py index 0eee434..1954947 100644 --- a/djangoseo/utils.py +++ b/djangoseo/utils.py @@ -3,6 +3,7 @@ import re import importlib +from django import http from django.contrib.sites.shortcuts import get_current_site from django.utils.functional import lazy from django.utils.safestring import mark_safe @@ -183,13 +184,13 @@ def import_tracked_models(): return models -def handle_seo_redirects(request): +def handle_seo_redirects(request, response_redirect_class=http.HttpResponsePermanentRedirect): """ Handle SEO redirects. Create django.contrib.redirects.models.Redirect if exists redirect pattern. :param request: Django request + :param response_redirect_class: Django HTTP class for redirect """ from .models import RedirectPattern - from django.contrib.redirects.models import Redirect if not getattr(settings, 'SEO_USE_REDIRECTS', False): return @@ -204,14 +205,7 @@ def handle_seo_redirects(request): ).order_by('all_subdomains') for redirect_pattern in redirect_patterns: - if re.match(redirect_pattern.url_pattern, full_path): - kwargs = { - 'site': current_site, - 'old_path': full_path, - 'new_path': redirect_pattern.redirect_path - } - try: - Redirect.objects.get_or_create(**kwargs) - except Exception: - logger.warning('Failed to create redirection', exc_info=True, extra=kwargs) - break + if re.match(redirect_pattern.url_pattern, full_path) and ( + redirect_pattern.subdomain == subdomain or redirect_pattern.all_subdomains + ): + return response_redirect_class(redirect_pattern.redirect_path) diff --git a/setup.py b/setup.py index e4d855c..52d53fa 100755 --- a/setup.py +++ b/setup.py @@ -18,7 +18,7 @@ def read_version(): name="django-seo", version=read_version(), packages=find_packages(exclude=["docs*", "tests*"]), - install_requires=['Django>=1.11'], + install_requires=['Django>=2.0'], author="Will Hardy", author_email="djangoseo@willhardy.com.au", description="A framework for managing SEO metadata in Django.", From 922b7c55d5e516c946471de1246519bad050ce76 Mon Sep 17 00:00:00 2001 From: Adrian Tangochin Date: Thu, 25 Oct 2018 00:26:02 +0500 Subject: [PATCH 04/12] New redirect model and middleware --- djangoseo/middleware.py | 53 +++++++++++++++++++++++++++++++++--- djangoseo/models.py | 59 +++++++++++++++++++++++++++++++++++++++-- djangoseo/utils.py | 34 ++++++++++++++---------- tests/settings.py | 1 + 4 files changed, 128 insertions(+), 19 deletions(-) diff --git a/djangoseo/middleware.py b/djangoseo/middleware.py index c5b7cf0..475dbc2 100644 --- a/djangoseo/middleware.py +++ b/djangoseo/middleware.py @@ -1,10 +1,13 @@ -import re from logging import getLogger +from django import http +from django.apps import apps from django.conf import settings -from django.http import Http404 +from django.contrib.sites.shortcuts import get_current_site +from django.core.exceptions import ImproperlyConfigured from django.utils.deprecation import MiddlewareMixin +from .models import Redirect from .utils import handle_seo_redirects @@ -17,6 +20,50 @@ def process_exception(self, request, exception): if not getattr(settings, 'SEO_USE_REDIRECTS', False): return - if request.method == 'GET' and isinstance(exception, Http404): + if request.method == 'GET' and isinstance(exception, http.Http404): handle_seo_redirects(request) return + + +class RedirectFallbackMiddleware(MiddlewareMixin): + # Defined as class-level attributes to be subclassing-friendly. + response_gone_class = http.HttpResponseGone + response_redirect_class = http.HttpResponsePermanentRedirect + + def __init__(self, get_response=None): + if not apps.is_installed('django.contrib.sites'): + raise ImproperlyConfigured( + "You cannot use RedirectFallbackMiddleware when " + "django.contrib.sites is not installed." + ) + super(RedirectFallbackMiddleware, self).__init__(get_response) + + def process_response(self, request, response): + # No need to check for a redirect for non-404 responses. + if response.status_code != 404: + return response + + subdomain = getattr('subdomain', request, '') + full_path = request.get_full_path() + current_site = get_current_site(request) + + r = None + try: + r = Redirect.objects.get(site=current_site, old_path=full_path) + except Redirect.DoesNotExist: + pass + if r is None and settings.APPEND_SLASH and not request.path.endswith('/'): + try: + r = Redirect.objects.get( + site=current_site, + old_path=request.get_full_path(force_append_slash=True), + ) + except Redirect.DoesNotExist: + pass + if r is not None: + if r.new_path == '': + return self.response_gone_class() + return self.response_redirect_class(r.new_path) + + # No redirect was found. Return the response. + return response diff --git a/djangoseo/models.py b/djangoseo/models.py index fd42593..f6b86fa 100644 --- a/djangoseo/models.py +++ b/djangoseo/models.py @@ -6,15 +6,18 @@ from django.conf import settings from django.utils.translation import ugettext_lazy as _ from django.contrib import admin +from django.contrib.sites.models import Site from .utils import create_dynamic_model, register_model_in_admin RedirectPattern = None +Redirect = None def setup(): global RedirectPattern + global Redirect is_loaded = False # Look for Metadata subclasses in appname/seo.py files for app in settings.INSTALLED_APPS: @@ -34,7 +37,7 @@ def setup(): def magic_str_method(self): return self.redirect_path - class Meta: + class RedirectPatternMeta: verbose_name = _('Redirect pattern') verbose_name_plural = _('Redirect patterns') @@ -65,7 +68,7 @@ class Meta: help_text=_('Pattern works for all subdomains') ), '__str__': magic_str_method, - 'Meta': Meta + 'Meta': RedirectPatternMeta }) RedirectPatternAdmin = type('RedirectPatternAdmin', (admin.ModelAdmin,), { @@ -75,7 +78,59 @@ class Meta: 'search_fields': ['redirect_path'], }) + class RedirectMeta: + verbose_name = _('redirect') + verbose_name_plural = _('redirects') + db_table = 'django_redirect' + unique_together = (('site', 'old_path'),) + ordering = ('old_path',) + + def redirect_str_method(self): + return '%s ---> %s' % (self.old_path, self.new_path) + + Redirect = create_dynamic_model('RedirectPattern', **{ + 'site': models.ForeignKey( + to=Site, + on_delete=models.CASCADE, + verbose_name=_('site') + ), + 'old_path': models.CharField( + verbose_name=_('redirect from'), + max_length=200, + db_index=True, + help_text=_("This should be an absolute path, excluding the domain name. Example: '/events/search/'."), + ), + 'new_path': models.CharField( + verbose_name=_('redirect to'), + max_length=200, + blank=True, + help_text=_("This can be either an absolute path (as above) or a full URL starting with 'http://'."), + ), + 'subdomain': models.CharField( + verbose_name=_('subdomain'), + max_length=250, + blank=True, + null=True, + default='' + ), + 'all_subdomains': models.BooleanField( + verbose_name=_('all subdomains'), + default=False, + help_text=_('Pattern works for all subdomains') + ), + '__str__': redirect_str_method, + 'Meta': RedirectMeta + }) + + RedirectAdmin = type('RedirectAdmin', (admin.ModelAdmin,), { + 'list_display': ('old_path', 'new_path'), + 'list_filter': ('site',), + 'search_fields': ('old_path', 'new_path'), + 'radio_fields': {'site': admin.VERTICAL} + }) + register_model_in_admin(RedirectPattern, RedirectPatternAdmin) + register_model_in_admin(Redirect, RedirectAdmin) from djangoseo.base import register_signals register_signals() diff --git a/djangoseo/utils.py b/djangoseo/utils.py index 1954947..c887625 100644 --- a/djangoseo/utils.py +++ b/djangoseo/utils.py @@ -3,7 +3,6 @@ import re import importlib -from django import http from django.contrib.sites.shortcuts import get_current_site from django.utils.functional import lazy from django.utils.safestring import mark_safe @@ -16,12 +15,12 @@ from django.db.models import Q from django.utils import six - logger = logging.getLogger(__name__) class NotSet(object): """ A singleton to identify unset values (where None would have meaning) """ + def __str__(self): return "NotSet" @@ -34,6 +33,7 @@ def __repr__(self): class Literal(object): """ Wrap literal values so that the system knows to treat them that way """ + def __init__(self, value): self.value = value @@ -85,12 +85,10 @@ def _replace_quot(match): def escape_tags(value, valid_tags): """ Strips text from the given html string, leaving only tags. - This functionality requires BeautifulSoup, nothing will be + This functionality requires BeautifulSoup, nothing will be done otherwise. - This isn't perfect. Someone could put javascript in here: test - So if you use valid_tags, you still need to trust your data entry. Or we could try: - only escape the non matching bits @@ -112,7 +110,7 @@ def escape_tags(value, valid_tags): # Allow comments to be hidden value = value.replace("<!--", "") - + return mark_safe(value) @@ -123,7 +121,7 @@ def _get_seo_content_types(seo_models): from django.contrib.contenttypes.models import ContentType try: return [ContentType.objects.get_for_model(m).id for m in seo_models] - except: # previously caught DatabaseError + except: # previously caught DatabaseError # Return an empty list if this is called too early return [] @@ -184,13 +182,12 @@ def import_tracked_models(): return models -def handle_seo_redirects(request, response_redirect_class=http.HttpResponsePermanentRedirect): +def handle_seo_redirects(request): """ Handle SEO redirects. Create django.contrib.redirects.models.Redirect if exists redirect pattern. :param request: Django request - :param response_redirect_class: Django HTTP class for redirect """ - from .models import RedirectPattern + from .models import RedirectPattern, Redirect if not getattr(settings, 'SEO_USE_REDIRECTS', False): return @@ -205,7 +202,16 @@ def handle_seo_redirects(request, response_redirect_class=http.HttpResponsePerma ).order_by('all_subdomains') for redirect_pattern in redirect_patterns: - if re.match(redirect_pattern.url_pattern, full_path) and ( - redirect_pattern.subdomain == subdomain or redirect_pattern.all_subdomains - ): - return response_redirect_class(redirect_pattern.redirect_path) + if re.match(redirect_pattern.url_pattern, full_path): + kwargs = { + 'site': current_site, + 'old_path': full_path, + 'new_path': redirect_pattern.redirect_path, + 'subdomain': subdomain, + 'all_subdomains': redirect_pattern.all_subdomains + } + try: + Redirect.objects.get_or_create(**kwargs) + except Exception: + logger.warning('Failed to create redirection', exc_info=True, extra=kwargs) + break diff --git a/tests/settings.py b/tests/settings.py index 03029d1..cbcc4d5 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -37,6 +37,7 @@ 'django.contrib.auth.middleware.AuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'django.contrib.redirects.middleware.RedirectFallbackMiddleware', + 'djangoseo.middleware.RedirectsMiddleware' ) From 4526f289f5c8ae30df2c41e69c97b5f5cc892268 Mon Sep 17 00:00:00 2001 From: Adrian Tangochin Date: Thu, 25 Oct 2018 18:26:50 +0500 Subject: [PATCH 05/12] Create and select redirect data with subdomains --- djangoseo/base.py | 5 +++-- djangoseo/middleware.py | 36 +++++++++++++++---------------- djangoseo/models.py | 3 ++- djangoseo/utils.py | 2 +- tests/settings.py | 30 +++----------------------- tests/userapp/__init__.py | 1 + tests/userapp/apps.py | 10 +++++++++ tests/userapp/tests.py | 45 ++++++++++++++++++++------------------- 8 files changed, 61 insertions(+), 71 deletions(-) create mode 100644 tests/userapp/apps.py diff --git a/djangoseo/base.py b/djangoseo/base.py index 705001a..7edd74b 100644 --- a/djangoseo/base.py +++ b/djangoseo/base.py @@ -369,7 +369,7 @@ def _handle_redirects_callback(model_class, sender, instance, **kwargs): create instances of redirects for changed URLs. """ # avoid RuntimeError for apps without enabled redirects - from django.contrib.redirects.models import Redirect + from .models import Redirect if not instance.pk: return @@ -380,7 +380,8 @@ def _handle_redirects_callback(model_class, sender, instance, **kwargs): Redirect.objects.get_or_create( old_path=before, new_path=after, - site=Site.objects.get_current() + site=Site.objects.get_current(), + all_subdomains=True ) except Exception as e: logger.exception('Failed to create new redirect') diff --git a/djangoseo/middleware.py b/djangoseo/middleware.py index 475dbc2..23089ff 100644 --- a/djangoseo/middleware.py +++ b/djangoseo/middleware.py @@ -2,6 +2,7 @@ from django import http from django.apps import apps +from django.db.models import Q from django.conf import settings from django.contrib.sites.shortcuts import get_current_site from django.core.exceptions import ImproperlyConfigured @@ -33,8 +34,8 @@ class RedirectFallbackMiddleware(MiddlewareMixin): def __init__(self, get_response=None): if not apps.is_installed('django.contrib.sites'): raise ImproperlyConfigured( - "You cannot use RedirectFallbackMiddleware when " - "django.contrib.sites is not installed." + 'You cannot use RedirectFallbackMiddleware when ' + 'django.contrib.sites is not installed.' ) super(RedirectFallbackMiddleware, self).__init__(get_response) @@ -47,23 +48,22 @@ def process_response(self, request, response): full_path = request.get_full_path() current_site = get_current_site(request) - r = None - try: - r = Redirect.objects.get(site=current_site, old_path=full_path) - except Redirect.DoesNotExist: - pass - if r is None and settings.APPEND_SLASH and not request.path.endswith('/'): - try: - r = Redirect.objects.get( - site=current_site, - old_path=request.get_full_path(force_append_slash=True), - ) - except Redirect.DoesNotExist: - pass - if r is not None: - if r.new_path == '': + redirect = Redirect.objects.filter( + Q(site=current_site), + Q(old_path=full_path), + Q(subdomain=subdomain) | Q(all_subdomains=True) + ).order_by('all_subdomains').first() + + if redirect is None and settings.APPEND_SLASH and not request.path.endswith('/'): + redirect = Redirect.objects.filter( + Q(site=current_site), + Q(old_path=request.get_full_path(force_append_slash=True)), + Q(subdomain=subdomain) | Q(all_subdomains=True) + ).order_by('all_subdomains').first() + if redirect is not None: + if redirect.new_path == '': return self.response_gone_class() - return self.response_redirect_class(r.new_path) + return self.response_redirect_class(redirect.new_path) # No redirect was found. Return the response. return response diff --git a/djangoseo/models.py b/djangoseo/models.py index f6b86fa..bb5a0fe 100644 --- a/djangoseo/models.py +++ b/djangoseo/models.py @@ -6,7 +6,6 @@ from django.conf import settings from django.utils.translation import ugettext_lazy as _ from django.contrib import admin -from django.contrib.sites.models import Site from .utils import create_dynamic_model, register_model_in_admin @@ -88,6 +87,8 @@ class RedirectMeta: def redirect_str_method(self): return '%s ---> %s' % (self.old_path, self.new_path) + from django.contrib.sites.models import Site + Redirect = create_dynamic_model('RedirectPattern', **{ 'site': models.ForeignKey( to=Site, diff --git a/djangoseo/utils.py b/djangoseo/utils.py index c887625..16ceb65 100644 --- a/djangoseo/utils.py +++ b/djangoseo/utils.py @@ -184,7 +184,7 @@ def import_tracked_models(): def handle_seo_redirects(request): """ - Handle SEO redirects. Create django.contrib.redirects.models.Redirect if exists redirect pattern. + Handle SEO redirects. Create Redirect instance if exists redirect pattern. :param request: Django request """ from .models import RedirectPattern, Redirect diff --git a/tests/settings.py b/tests/settings.py index cbcc4d5..cead919 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -1,29 +1,20 @@ from __future__ import unicode_literals +import os """ Django settings for tests project. """ -# Build paths inside the project like this: os.path.join(BASE_DIR, ...) -import os BASE_DIR = os.path.dirname(os.path.dirname(__file__)) -# Quick-start development settings - unsuitable for production -# See https://docs.djangoproject.com/en/1.7/howto/deployment/checklist/ - -# SECURITY WARNING: keep the secret key used in production secret! SECRET_KEY = '=)c(th7-3@w*n9mf9_b+2qg685lc6qgfars@yu1g516xu5&is)' -# SECURITY WARNING: don't run with debug turned on in production! DEBUG = True -# Application definition - INSTALLED_APPS = ( 'django.contrib.auth', 'django.contrib.contenttypes', 'django.contrib.sessions', 'django.contrib.sites', - 'django.contrib.redirects', 'django.contrib.admin', 'django.contrib.flatpages', 'djangoseo', @@ -33,11 +24,9 @@ MIDDLEWARE = ( 'django.middleware.common.CommonMiddleware', 'django.contrib.sessions.middleware.SessionMiddleware', - #'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', - 'django.contrib.redirects.middleware.RedirectFallbackMiddleware', - + 'djangoseo.middleware.RedirectFallbackMiddleware', 'djangoseo.middleware.RedirectsMiddleware' ) @@ -60,12 +49,6 @@ ROOT_URLCONF = 'tests.urls' -#WSGI_APPLICATION = 'tests.wsgi.application' - - -# Database -# https://docs.djangoproject.com/en/1.7/ref/settings/#databases - DATABASES = { 'default': { 'ENGINE': 'django.db.backends.sqlite3', @@ -73,9 +56,6 @@ } } -# Internationalization -# https://docs.djangoproject.com/en/1.7/topics/i18n/ - LANGUAGE_CODE = 'ru-RU' TIME_ZONE = 'Europe/Moscow' @@ -86,10 +66,6 @@ USE_TZ = True - -# Static files (CSS, JavaScript, Images) -# https://docs.djangoproject.com/en/1.7/howto/static-files/ - STATIC_URL = '/static/' MEDIA_URL = '/media/' @@ -97,7 +73,7 @@ CACHE_BACKEND = 'dummy://' # Enable when testing cache -#CACHE_BACKEND = "locmem://?timeout=30&max_entries=400" +# CACHE_BACKEND = "locmem://?timeout=30&max_entries=400" SEO_MODELS = ('userapp',) diff --git a/tests/userapp/__init__.py b/tests/userapp/__init__.py index e69de29..c63034a 100644 --- a/tests/userapp/__init__.py +++ b/tests/userapp/__init__.py @@ -0,0 +1 @@ +default_app_config = 'tests.userapp.apps.UserAppConfig' \ No newline at end of file diff --git a/tests/userapp/apps.py b/tests/userapp/apps.py new file mode 100644 index 0000000..046eb92 --- /dev/null +++ b/tests/userapp/apps.py @@ -0,0 +1,10 @@ +from django.apps.config import AppConfig + +from djangoseo.models import setup + + +class UserAppConfig(AppConfig): + name = 'userapp' + + def ready(self): + setup() diff --git a/tests/userapp/tests.py b/tests/userapp/tests.py index b001e0b..39b5fc9 100644 --- a/tests/userapp/tests.py +++ b/tests/userapp/tests.py @@ -14,7 +14,6 @@ from django.test.client import FakePayload, RequestFactory from django.contrib.contenttypes.models import ContentType from django.contrib.sites.models import Site -from django.contrib.redirects.models import Redirect from django.contrib.auth.models import User from django.conf import settings from django.db import models, IntegrityError, transaction @@ -29,7 +28,7 @@ from djangoseo.utils import create_dynamic_model, register_model_in_admin, import_tracked_models from djangoseo.seo import get_metadata as seo_get_metadata from djangoseo.base import registry -from djangoseo.models import RedirectPattern +from djangoseo.models import RedirectPattern, Redirect from djangoseo.middleware import RedirectsMiddleware from .views import product_detail from .models import Page, Product, NoPath, Tag, Category @@ -1214,41 +1213,43 @@ def test_import(self): class RedirectsMiddlewareTest(TestCase): + @classmethod + def setUpTestData(cls): + cls.current_site = Site.objects.get_current() + cls.redirect_path = reverse('userapp_page_detail', args=('product',)) + cls.redirect_pattern1 = RedirectPattern.objects.create( + url_pattern='/products/(\d+)/', + redirect_path=cls.redirect_path, + site=cls.current_site + ) + cls.redirect_pattern2 = RedirectPattern.objects.create( + url_pattern='/products/(\d+)/', + redirect_path=cls.redirect_path, + site=cls.current_site, + all_subdomains=True + ) + def test_create_redirect(self): middleware = RedirectsMiddleware() request_factory = RequestFactory() - current_site = Site.objects.get_current() product_id = '123' product_path = reverse('userapp_product_detail', args=(product_id,)) response = self.client.get(product_path) self.assertEqual(response.status_code, 404) - redirect_path = reverse('userapp_page_detail', args=('product',)) - redirect_pattern1 = RedirectPattern.objects.create( - url_pattern='/products/(\d+)/', - redirect_path=redirect_path, - site=current_site - ) - redirect_pattern2 = RedirectPattern.objects.create( - url_pattern='/products/(\d+)/', - redirect_path=redirect_path, - site=current_site, - all_subdomains=True, - ) - # main scenario response = self.client.get(product_path) self.assertEqual(response.status_code, 301) self.assertEqual(Redirect.objects.count(), 1) redirect = Redirect.objects.first() self.assertEqual(redirect.old_path, product_path) - self.assertEqual(redirect.new_path, redirect_path) + self.assertEqual(redirect.new_path, self.redirect_path) # with subdomain redirect.delete() - redirect_pattern1.subdomain = 'msk' - redirect_pattern1.save() + self.redirect_pattern1.subdomain = 'msk' + self.redirect_pattern1.save() request = request_factory.get(product_path) request.subdomain = 'msk' try: @@ -1262,7 +1263,7 @@ def test_create_redirect(self): response = self.client.get(product_path) self.assertEqual(response.status_code, 301) self.assertEqual(Redirect.objects.count(), 1) - redirect_pattern2.delete() + self.redirect_pattern2.delete() Redirect.objects.first().delete() # with another site @@ -1270,8 +1271,8 @@ def test_create_redirect(self): domain='example.net', name='example.net' ) - redirect_pattern1.site = another_site - redirect_pattern1.save() + self.redirect_pattern1.site = another_site + self.redirect_pattern1.save() response = self.client.get(product_path) self.assertEqual(response.status_code, 404) self.assertEqual(Redirect.objects.count(), 0) From 124d23a905246215cbb7eb03acb91035e05be8d4 Mon Sep 17 00:00:00 2001 From: Adrian Tangochin Date: Thu, 25 Oct 2018 22:35:43 +0500 Subject: [PATCH 06/12] Fixed database error, added more tests --- djangoseo/middleware.py | 2 +- djangoseo/models.py | 10 ++++---- djangoseo/utils.py | 2 +- tests/userapp/tests.py | 52 +++++++++++++++++++++++------------------ 4 files changed, 36 insertions(+), 30 deletions(-) diff --git a/djangoseo/middleware.py b/djangoseo/middleware.py index 23089ff..d8858a4 100644 --- a/djangoseo/middleware.py +++ b/djangoseo/middleware.py @@ -44,7 +44,7 @@ def process_response(self, request, response): if response.status_code != 404: return response - subdomain = getattr('subdomain', request, '') + subdomain = getattr(request, 'subdomain', '') full_path = request.get_full_path() current_site = get_current_site(request) diff --git a/djangoseo/models.py b/djangoseo/models.py index bb5a0fe..193f21d 100644 --- a/djangoseo/models.py +++ b/djangoseo/models.py @@ -33,8 +33,8 @@ def setup(): # if SEO_USE_REDIRECTS is enabled, add model for redirect and register it in admin if getattr(settings, 'SEO_USE_REDIRECTS', False): - def magic_str_method(self): - return self.redirect_path + def redirect_pattern_str_method(self): + return '%s -> %s' % (self.url_pattern, self.redirect_path) class RedirectPatternMeta: verbose_name = _('Redirect pattern') @@ -66,7 +66,7 @@ class RedirectPatternMeta: default=False, help_text=_('Pattern works for all subdomains') ), - '__str__': magic_str_method, + '__str__': redirect_pattern_str_method, 'Meta': RedirectPatternMeta }) @@ -85,11 +85,11 @@ class RedirectMeta: ordering = ('old_path',) def redirect_str_method(self): - return '%s ---> %s' % (self.old_path, self.new_path) + return '%s -> %s' % (self.old_path, self.new_path) from django.contrib.sites.models import Site - Redirect = create_dynamic_model('RedirectPattern', **{ + Redirect = create_dynamic_model('Redirect', **{ 'site': models.ForeignKey( to=Site, on_delete=models.CASCADE, diff --git a/djangoseo/utils.py b/djangoseo/utils.py index 16ceb65..1803c8a 100644 --- a/djangoseo/utils.py +++ b/djangoseo/utils.py @@ -207,7 +207,7 @@ def handle_seo_redirects(request): 'site': current_site, 'old_path': full_path, 'new_path': redirect_pattern.redirect_path, - 'subdomain': subdomain, + 'subdomain': redirect_pattern.subdomain, 'all_subdomains': redirect_pattern.all_subdomains } try: diff --git a/tests/userapp/tests.py b/tests/userapp/tests.py index 39b5fc9..9d05971 100644 --- a/tests/userapp/tests.py +++ b/tests/userapp/tests.py @@ -1213,22 +1213,6 @@ def test_import(self): class RedirectsMiddlewareTest(TestCase): - @classmethod - def setUpTestData(cls): - cls.current_site = Site.objects.get_current() - cls.redirect_path = reverse('userapp_page_detail', args=('product',)) - cls.redirect_pattern1 = RedirectPattern.objects.create( - url_pattern='/products/(\d+)/', - redirect_path=cls.redirect_path, - site=cls.current_site - ) - cls.redirect_pattern2 = RedirectPattern.objects.create( - url_pattern='/products/(\d+)/', - redirect_path=cls.redirect_path, - site=cls.current_site, - all_subdomains=True - ) - def test_create_redirect(self): middleware = RedirectsMiddleware() request_factory = RequestFactory() @@ -1238,18 +1222,34 @@ def test_create_redirect(self): response = self.client.get(product_path) self.assertEqual(response.status_code, 404) + current_site = Site.objects.get_current() + redirect_path = reverse('userapp_page_detail', args=('product',)) + redirect_pattern1 = RedirectPattern.objects.create( + url_pattern='/products/(\d+)/', + redirect_path=redirect_path, + site=current_site + ) + redirect_pattern2 = RedirectPattern.objects.create( + url_pattern='/products/(\d+)/', + redirect_path=redirect_path, + site=current_site, + all_subdomains=True, + ) + # main scenario response = self.client.get(product_path) self.assertEqual(response.status_code, 301) self.assertEqual(Redirect.objects.count(), 1) redirect = Redirect.objects.first() self.assertEqual(redirect.old_path, product_path) - self.assertEqual(redirect.new_path, self.redirect_path) + self.assertEqual(redirect.new_path, redirect_path) + self.assertEqual(redirect.subdomain, redirect_pattern1.subdomain) + self.assertEqual(redirect.all_subdomains, redirect_pattern1.all_subdomains) # with subdomain redirect.delete() - self.redirect_pattern1.subdomain = 'msk' - self.redirect_pattern1.save() + redirect_pattern1.subdomain = 'msk' + redirect_pattern1.save() request = request_factory.get(product_path) request.subdomain = 'msk' try: @@ -1257,22 +1257,28 @@ def test_create_redirect(self): except Http404 as e: middleware.process_exception(request, e) self.assertEqual(Redirect.objects.count(), 1) + redirect = Redirect.objects.first() + self.assertEqual(redirect.subdomain, redirect_pattern1.subdomain) + self.assertEqual(redirect.all_subdomains, redirect_pattern1.all_subdomains) # all subdomain flag Redirect.objects.first().delete() response = self.client.get(product_path) self.assertEqual(response.status_code, 301) self.assertEqual(Redirect.objects.count(), 1) - self.redirect_pattern2.delete() - Redirect.objects.first().delete() + redirect = Redirect.objects.first() + self.assertEqual(redirect.subdomain, redirect_pattern2.subdomain) + self.assertEqual(redirect.all_subdomains, redirect_pattern2.all_subdomains) + redirect_pattern2.delete() + redirect.delete() # with another site another_site = Site.objects.create( domain='example.net', name='example.net' ) - self.redirect_pattern1.site = another_site - self.redirect_pattern1.save() + redirect_pattern1.site = another_site + redirect_pattern1.save() response = self.client.get(product_path) self.assertEqual(response.status_code, 404) self.assertEqual(Redirect.objects.count(), 0) From ca4ab01b814e8762743ca255f3872a7e537b59c6 Mon Sep 17 00:00:00 2001 From: Adrian Tangochin Date: Fri, 26 Oct 2018 17:19:06 +0500 Subject: [PATCH 07/12] Revert support for Django 1.10+ --- djangoseo/middleware.py | 22 +++++++++++++++++++++- djangoseo/utils.py | 29 +++++++++++++++++++++++------ setup.py | 2 +- tests/settings.py | 10 +++++----- tests/urls.py | 8 ++++++-- tests/userapp/models.py | 6 +++++- tests/userapp/tests.py | 20 ++++++++++++++++---- 7 files changed, 77 insertions(+), 20 deletions(-) diff --git a/djangoseo/middleware.py b/djangoseo/middleware.py index d8858a4..3f66e93 100644 --- a/djangoseo/middleware.py +++ b/djangoseo/middleware.py @@ -6,7 +6,6 @@ from django.conf import settings from django.contrib.sites.shortcuts import get_current_site from django.core.exceptions import ImproperlyConfigured -from django.utils.deprecation import MiddlewareMixin from .models import Redirect from .utils import handle_seo_redirects @@ -15,6 +14,27 @@ logger = getLogger(__name__) +# TODO: replace after removing support for old versions of Django +class MiddlewareMixin(object): + """ + This mixin a full copy of Django 1.10 django.utils.deprecation.MiddlewareMixin. + Needed for compatibility reasons. + """ + def __init__(self, get_response=None): + self.get_response = get_response + super(MiddlewareMixin, self).__init__() + + def __call__(self, request): + response = None + if hasattr(self, 'process_request'): + response = self.process_request(request) + if not response: + response = self.get_response(request) + if hasattr(self, 'process_response'): + response = self.process_response(request, response) + return response + + class RedirectsMiddleware(MiddlewareMixin): def process_exception(self, request, exception): diff --git a/djangoseo/utils.py b/djangoseo/utils.py index 1803c8a..82d79c4 100644 --- a/djangoseo/utils.py +++ b/djangoseo/utils.py @@ -3,17 +3,21 @@ import re import importlib +import django from django.contrib.sites.shortcuts import get_current_site from django.utils.functional import lazy from django.utils.safestring import mark_safe from django.utils.module_loading import import_string from django.utils.html import conditional_escape -from django.urls import (URLResolver as RegexURLResolver, URLPattern as RegexURLPattern, Resolver404, get_resolver, - clear_url_caches) from django.conf import settings from django.db import models from django.db.models import Q from django.utils import six +if django.VERSION < (2, 0): + from django.core.urlresolvers import RegexURLResolver, RegexURLPattern, Resolver404, get_resolver, clear_url_caches +else: + from django.urls import (URLResolver as RegexURLResolver, URLPattern as RegexURLPattern, Resolver404, get_resolver, + clear_url_caches) logger = logging.getLogger(__name__) @@ -39,7 +43,10 @@ def __init__(self, value): def _pattern_resolve_to_name(pattern, path): - match = pattern.pattern.regex.search(path) + if django.VERSION < (2, 0): + match = pattern.regex.search(path) + else: + match = pattern.pattern.regex.search(path) if match: name = "" if pattern.name: @@ -53,7 +60,11 @@ def _pattern_resolve_to_name(pattern, path): def _resolver_resolve_to_name(resolver, path): tried = [] - match = resolver.pattern.regex.search(path) + django1 = django.VERSION < (2, 0) + if django1: + match = resolver.regex.search(path) + else: + match = resolver.pattern.regex.search(path) if match: new_path = path[match.end():] for pattern in resolver.url_patterns: @@ -63,11 +74,17 @@ def _resolver_resolve_to_name(resolver, path): elif isinstance(pattern, RegexURLResolver): name = _resolver_resolve_to_name(pattern, new_path) except Resolver404 as e: - tried.extend([(pattern.pattern.regex.pattern + ' ' + t) for t in e.args[0]['tried']]) + if django1: + tried.extend([(pattern.regex.pattern + ' ' + t) for t in e.args[0]['tried']]) + else: + tried.extend([(pattern.pattern.regex.pattern + ' ' + t) for t in e.args[0]['tried']]) else: if name: return name - tried.append(pattern.pattern.regex.pattern) + if django1: + tried.append(pattern.regex.pattern) + else: + tried.append(pattern.pattern.regex.pattern) raise Resolver404({'tried': tried, 'path': new_path}) diff --git a/setup.py b/setup.py index 52d53fa..1558f50 100755 --- a/setup.py +++ b/setup.py @@ -18,7 +18,7 @@ def read_version(): name="django-seo", version=read_version(), packages=find_packages(exclude=["docs*", "tests*"]), - install_requires=['Django>=2.0'], + install_requires=['Django>=1.10'], author="Will Hardy", author_email="djangoseo@willhardy.com.au", description="A framework for managing SEO metadata in Django.", diff --git a/tests/settings.py b/tests/settings.py index cead919..a330f85 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -21,7 +21,7 @@ 'userapp', ) -MIDDLEWARE = ( +MIDDLEWARE_CLASSES = ( 'django.middleware.common.CommonMiddleware', 'django.contrib.sessions.middleware.SessionMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', @@ -37,10 +37,10 @@ 'OPTIONS': { 'debug': DEBUG, 'context_processors': [ - "django.contrib.auth.context_processors.auth", - "django.template.context_processors.debug", - "django.template.context_processors.i18n", - "django.template.context_processors.media", + 'django.contrib.auth.context_processors.auth', + 'django.template.context_processors.debug', + 'django.template.context_processors.i18n', + 'django.template.context_processors.media', 'django.template.context_processors.request', ] } diff --git a/tests/urls.py b/tests/urls.py index 41d8b8a..684ff94 100644 --- a/tests/urls.py +++ b/tests/urls.py @@ -1,10 +1,14 @@ # -*- coding: UTF-8 -*- from __future__ import unicode_literals -from django.conf.urls import url -from django.urls import include +import django from django.contrib import admin from userapp.admin import alternative_site +if django.VERSION < (2, 0): + from django.conf.urls import include, url +else: + from django.conf.urls import url + from django.urls import include urlpatterns = [ diff --git a/tests/userapp/models.py b/tests/userapp/models.py index 01ae592..47f68b6 100644 --- a/tests/userapp/models.py +++ b/tests/userapp/models.py @@ -1,9 +1,13 @@ # -*- coding: utf-8 -*- from __future__ import unicode_literals +import django from django.db import models -from django.urls import reverse from django.utils.encoding import python_2_unicode_compatible +if django.VERSION < (2, 0): + from django.core.urlresolvers import reverse +else: + from django.urls import reverse @python_2_unicode_compatible diff --git a/tests/userapp/tests.py b/tests/userapp/tests.py index 9d05971..85c1025 100644 --- a/tests/userapp/tests.py +++ b/tests/userapp/tests.py @@ -2,11 +2,10 @@ from __future__ import unicode_literals import hashlib +import django from django.utils import six -from django.urls import reverse from django.test import TestCase, override_settings from django.http import Http404 - try: from django.test import TransactionTestCase except ImportError: @@ -34,6 +33,11 @@ from .models import Page, Product, NoPath, Tag, Category from .seo import Coverage, WithSites, WithI18n, WithBackends, WithSubdomains +if django.VERSION < (2, 0): + from django.core.urlresolvers import reverse +else: + from django.urls import reverse + """ Test suite for SEO framework. It is divided into 7 sections: @@ -278,7 +282,10 @@ def test_useful_error_messages(self): """ Tests that the system gracefully handles a developer error (eg exception in get_absolute_url). """ - from django.urls import NoReverseMatch + if django.VERSION < (2, 0): + from django.core.urlresolvers import NoReverseMatch + else: + from django.urls import NoReverseMatch with self.assertRaises(NoReverseMatch): self.page.type = "a type with spaces!" # this causes get_absolute_url() to fail self.page.save() @@ -1164,7 +1171,11 @@ def test_model_name(self): self.assertNotEquals(self.model._meta.model_name.lower(), self.model_name) def test_model_fields(self): - received_fields = [f.name for f in self.model._meta.get_fields()] + if django.VERSION < (2, 0): + # received_fields = self.model._meta.get_all_field_names() + received_fields = [f.name for f in self.model._meta.get_fields()] + else: + received_fields = [f.name for f in self.model._meta.get_fields()] expected_fields = ['id'] + list(self.attrs.keys()) self.assertSetEqual(set(received_fields), set(expected_fields)) @@ -1294,6 +1305,7 @@ def test_redirects_models(self): self.page.save() redirect = Redirect.objects.first() self.assertTrue(redirect) + self.assertTrue(redirect.all_subdomains) self.assertTrue(redirect.old_path == reverse('userapp_page_detail', args=['asd'])) self.assertTrue(redirect.new_path == reverse('userapp_page_detail', args=['dsa'])) self.assertTrue(redirect.site == Site.objects.get_current()) From e4acb6b70ac1a252d800a950d0a34850c4a217b3 Mon Sep 17 00:00:00 2001 From: Adrian Tangochin Date: Mon, 29 Oct 2018 19:06:52 +0500 Subject: [PATCH 08/12] Removed excess code, update middleware settings, readme, update version --- README.rst | 2 +- djangoseo/version.py | 2 +- tests/settings.py | 2 +- tests/userapp/__init__.py | 1 - tests/userapp/apps.py | 10 ---------- 5 files changed, 3 insertions(+), 14 deletions(-) delete mode 100644 tests/userapp/apps.py diff --git a/README.rst b/README.rst index e0cdcfd..d20a9d9 100644 --- a/README.rst +++ b/README.rst @@ -258,7 +258,7 @@ If you need a redirection when model changes its URL list the full path to the m 'your_app.models.Bar' ) -The default class for redirection is ``django.http.response.HttpResponsePermanentRedirect``, but if you want to change this behavior, you can change the HttpResponse classes used by the middleware by creating a subclass of RedirectsMiddleware and overriding response_redirect_class. +The default class for redirection is ``django.http.response.HttpResponsePermanentRedirect``, but if you want to change this behavior, you can change the HttpResponse classes used by the middleware by creating a subclass of RedirectFallbackMiddleware and overriding response_redirect_class. Attention: each path to model must be direct and model must have a method ``get_absolute_url``. Work such redirection follows: when path to model on site changed, it create redirection to old path. diff --git a/djangoseo/version.py b/djangoseo/version.py index 8d14ea9..1ef0b1f 100644 --- a/djangoseo/version.py +++ b/djangoseo/version.py @@ -1,5 +1,5 @@ # -*- coding:utf-8 -*- -VERSION = (2, 5, '0wf', 'final', 0) +VERSION = (2, 6, '0wf', 'final', 0) def get_version(): diff --git a/tests/settings.py b/tests/settings.py index a330f85..85ef980 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -21,7 +21,7 @@ 'userapp', ) -MIDDLEWARE_CLASSES = ( +MIDDLEWARE = ( 'django.middleware.common.CommonMiddleware', 'django.contrib.sessions.middleware.SessionMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', diff --git a/tests/userapp/__init__.py b/tests/userapp/__init__.py index c63034a..e69de29 100644 --- a/tests/userapp/__init__.py +++ b/tests/userapp/__init__.py @@ -1 +0,0 @@ -default_app_config = 'tests.userapp.apps.UserAppConfig' \ No newline at end of file diff --git a/tests/userapp/apps.py b/tests/userapp/apps.py deleted file mode 100644 index 046eb92..0000000 --- a/tests/userapp/apps.py +++ /dev/null @@ -1,10 +0,0 @@ -from django.apps.config import AppConfig - -from djangoseo.models import setup - - -class UserAppConfig(AppConfig): - name = 'userapp' - - def ready(self): - setup() From a1e6b2f9fce8de94b767ad80f10d5116a862f482 Mon Sep 17 00:00:00 2001 From: Adrian Tangochin Date: Mon, 29 Oct 2018 22:19:19 +0500 Subject: [PATCH 09/12] Update travis config --- .travis.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.travis.yml b/.travis.yml index 606b69f..0bd3884 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,7 +4,16 @@ python: - "3.5" - "3.6" env: + - DJANGO=1.10 DB=sqlite + - DJANGO=1.11 DB=sqlite - DJANGO=2.0 DB=sqlite + - DJANGO=2.1 DB=sqlite +matrix: + exclude: + - python: "3.4" + env: DJANGO=2.1 DB=sqlite + - python: "3.5" + env: DJANGO=2.1 DB=sqlite install: - pip install -q Django==$DJANGO - pip install coveralls coverage django-discover-runner From ba34ee5aee69e4c0d2dcff494cec8ed9638dd149 Mon Sep 17 00:00:00 2001 From: Adrian Tangochin Date: Mon, 29 Oct 2018 23:34:57 +0500 Subject: [PATCH 10/12] Added Python 3.4 and Python 3.5 to travis config --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 9479286..0bd3884 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,7 @@ language: python python: + - "3.4" + - "3.5" - "3.6" env: - DJANGO=1.10 DB=sqlite From 260e0b5d3a11ca8f8da9d62757d92f7d5859eadf Mon Sep 17 00:00:00 2001 From: Adrian Tangochin Date: Tue, 30 Oct 2018 00:56:14 +0500 Subject: [PATCH 11/12] Fixed Redirect model --- djangoseo/models.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/djangoseo/models.py b/djangoseo/models.py index 193f21d..755a9e9 100644 --- a/djangoseo/models.py +++ b/djangoseo/models.py @@ -80,7 +80,6 @@ class RedirectPatternMeta: class RedirectMeta: verbose_name = _('redirect') verbose_name_plural = _('redirects') - db_table = 'django_redirect' unique_together = (('site', 'old_path'),) ordering = ('old_path',) @@ -93,7 +92,8 @@ def redirect_str_method(self): 'site': models.ForeignKey( to=Site, on_delete=models.CASCADE, - verbose_name=_('site') + verbose_name=_('site'), + related_name='seo_redirect' ), 'old_path': models.CharField( verbose_name=_('redirect from'), @@ -117,7 +117,7 @@ def redirect_str_method(self): 'all_subdomains': models.BooleanField( verbose_name=_('all subdomains'), default=False, - help_text=_('Pattern works for all subdomains') + help_text=_('Will works for all subdomains') ), '__str__': redirect_str_method, 'Meta': RedirectMeta From 9df77a05f0a75171169654fe2d6780c6d244c34c Mon Sep 17 00:00:00 2001 From: Adrian Tangochin Date: Tue, 30 Oct 2018 20:44:49 +0500 Subject: [PATCH 12/12] pep 440 for version --- djangoseo/version.py | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/djangoseo/version.py b/djangoseo/version.py index 1ef0b1f..d26ca3a 100644 --- a/djangoseo/version.py +++ b/djangoseo/version.py @@ -1,17 +1,2 @@ # -*- coding:utf-8 -*- -VERSION = (2, 6, '0wf', 'final', 0) - - -def get_version(): - global VERSION - version = '%s.%s' % (VERSION[0], VERSION[1]) - if VERSION[2]: - version = '%s.%s' % (version, VERSION[2]) - if VERSION[3:] == ('alpha', 0): - version = '%s pre-alpha' % version - elif VERSION[3] != 'final': - version = '%s %s %s' % (version, VERSION[3], VERSION[4]) - return version - - -__version__ = get_version() +__version__ = '2.6.0+whyfly.1'