diff --git a/.gitignore b/.gitignore index 8b7e831..f908a3f 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,10 @@ *.pot *.py[co] -htmlcov/ +.coverage +.coveralls.yml + dist/ +htmlcov/ +coverage/ django_url_tracker.egg-info/ diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 0000000..f8cce09 --- /dev/null +++ b/.travis.yml @@ -0,0 +1,18 @@ +language: python +python: + - "2.6" + - "2.7" +env: + - DJANGO_VERSION="1.3.7" + - DJANGO_VERSION="1.4.5" + - DJANGO_VERSION="1.5" +# command to install dependencies +install: + # NOTE: easy_install *will* downgrade if setup.py specifies a max version + # # This leads to seemingly passing tests + - easy_install Django==$DJANGO_VERSION +# command to run tests +script: + - make travis +after_script: + - coveralls diff --git a/AUTHORS b/AUTHORS index f086773..0b4516d 100644 --- a/AUTHORS +++ b/AUTHORS @@ -1,5 +1,6 @@ AUTHOR - * Sebastian Vetter (elbaschid) + * Sebastian Vetter (elbaschid) + * Saul Shanabrook (saulshanabrook) CONTRIBUTORS diff --git a/CHANGELOG.rst b/CHANGELOG.rst new file mode 100644 index 0000000..5365655 --- /dev/null +++ b/CHANGELOG.rst @@ -0,0 +1,29 @@ +========= +Changelog +========= + +0.1.4 +----- + +* Changed URL record fields from ``CharField`` to ``TextField`` +* Improved admin interface + +0.1.3 +----- + +* Increased the max length of the old and new URL database fields to match + Django's redirects app (200). + +0.1.2 +----- + +* The middleware now supports URLs that contain a query string and + redirect using 301 or 410 as with other URLs now. That means it + is possible to redirect ``/some/old/url/using.php?with=query&in=string`` + to a shiny new URL. + + +0.1.1 +----- + +* Initial release of the package diff --git a/MANIFEST.in b/MANIFEST.in index 8aac326..e6b34a4 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,2 +1 @@ -include README.rst LICENSE AUTHORS *.rst -recursive-include tests * +include LICENSE AUTHORS *.rst diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..2723d9b --- /dev/null +++ b/Makefile @@ -0,0 +1,16 @@ +install: + python setup.py develop + pip install -r requirements.txt --use-mirrors + +tests: install + python run_tests.py --with-specplugin + +coverage: install + python run_tests.py --with-coverage --cover-package=url_tracker \ + --with-specplugin --no-spec-color \ + --cover-html --cover-html-dir=coverage + +travis: coverage + coveralls + +.PHONY: install tests travis coverage diff --git a/README.rst b/README.rst index 797af4d..2eda1f2 100644 --- a/README.rst +++ b/README.rst @@ -1,6 +1,12 @@ django-url-tracker ================== +.. image:: https://travis-ci.org/elbaschid/django-url-tracker.png + :target: https://travis-ci.org/elbaschid/django-url-tracker + +.. image:: https://coveralls.io/repos/elbaschid/django-url-tracker/badge.png?branch=master + :target: https://coveralls.io/r/elbaschid/django-url-tracker + The ``django-url-tracker`` is meant to be a easy-to-use addition to a website to enhance its SEO. This might seem slightly pointless as `Cool URIs don't change @@ -95,13 +101,23 @@ the model:: def get_absolute_url(self): return ('project-detail', (), {'slug': self.slug}) -And now the missing link to actually start tracking URL changes is adding -the following command to the bottom of the class definition, or the file -for that matter:: +And now the missing link to actually start tracking URL changes. Add the +``URLTrackingMixin`` to your model and add the following command to the +bottom of the class definition (or the file if you prefer that):: import url_tracker + + class Project(url_tracker.URLTrackingMixin, models.Model): + ... + url_tracker.track_url_changes_for_model(Project) + +To change which functions are called to tracks urls, add a +``url_tracking_methods`` attribute to the clast, which is a list of +method names to track. By default the the list contains +``get_absolute_url``. + You are done. If you go to the admin interface, create a new project and then change its slug (which changes its URL) you will see a new ``URLChangeRecord`` reflecting the change. Opening the ``old_url`` should @@ -120,4 +136,12 @@ Create a new branch for your feature:: git commit -b feature/whatever-you-like +Then make sure all the tests past (and write new ones for any new features):: + + make tests + push the finished feature to github and open a pull request form the branch. + +If you make a change to models.py that requires a database migration, +use ``django-mini.py -p --app url_tracker --app south schemamigration +url_tracker --auto`` to create a south migration. diff --git a/requirements.txt b/requirements.txt new file mode 100644 index 0000000..c1c9981 --- /dev/null +++ b/requirements.txt @@ -0,0 +1,9 @@ +argparse==1.2.1 +coverage==3.5.2 +mock==1.0b1 +wsgiref==0.1.2 +tox==1.4.2 +django-mini==0.4 +spec==0.10.0 +django-nose==1.1 +coveralls==0.1.1 diff --git a/run_tests.py b/run_tests.py index d72da01..cefc649 100755 --- a/run_tests.py +++ b/run_tests.py @@ -4,17 +4,17 @@ import logging logging.disable(logging.CRITICAL) -from argparse import ArgumentParser +from argparse import ArgumentParser from coverage import coverage import tests.config -from django.test.simple import DjangoTestSuiteRunner +from django_nose import NoseTestSuiteRunner def run_tests(verbosity, *test_args): if not test_args: test_args = ['url_tracker'] - test_runner = DjangoTestSuiteRunner(verbosity=verbosity) + test_runner = NoseTestSuiteRunner(verbosity=verbosity) num_failures = test_runner.run_tests(test_args) if num_failures: sys.exit(num_failures) @@ -38,4 +38,5 @@ def run_tests(verbosity, *test_args): c.html_report() else: print 'Running tests' + args += ['--with-specplugin', '-s', '-x'] run_tests(options.verbosity, *args) diff --git a/setup.py b/setup.py index fc2647d..f10c010 100644 --- a/setup.py +++ b/setup.py @@ -11,7 +11,7 @@ setup( name = "django-url-tracker", - version = '0.1.1', + version = '0.1.4', url = "https://github.com/tangentlabs/django-url-tracker", author = "Sebastian Vetter", author_email = "sebastian.vetter@tangentone.com.au", @@ -22,7 +22,7 @@ packages = find_packages(exclude=["docs*", "tests*"]), include_package_data = True, install_requires=[ - 'django>=1.3.1', + 'django>=1.3.1,<1.6', 'South>=0.7.3', ], classifiers=[ @@ -38,5 +38,3 @@ ], keywords = "seo, django, framework", ) - - diff --git a/tests/functional_test.py b/tests/functional_test.py new file mode 100644 index 0000000..aed0269 --- /dev/null +++ b/tests/functional_test.py @@ -0,0 +1,40 @@ +from django.test import TestCase + +from url_tracker.models import URLChangeRecord + + +class TestUrlRecord(TestCase): + + def test_returns_404_for_invalid_url(self): + response = self.client.get('/work/an-invalid-project/') + self.assertEquals(response.status_code, 404) + + def test_returns_301_for_a_changed_url(self): + URLChangeRecord.objects.create( + old_url='/the/old-url/', + new_url='/the/new/url/', + ) + + response = self.client.get('/the/old-url/') + self.assertEquals(response.status_code, 301) + self.assertEquals(response['location'], 'http://testserver/the/new/url/') + + def test_returns_410_for_a_deleted_url(self): + URLChangeRecord.objects.create( + old_url='/the/old-url/', + new_url='', + deleted=True + ) + + response = self.client.get('/the/old-url/') + self.assertEquals(response.status_code, 410) + + def test_returns_301_for_a_changed_url_containing_querystring(self): + old_url = '/the/old-url/afile.php?q=test&another=45' + URLChangeRecord.objects.create( + old_url=old_url, + new_url='/the/new/url/', + ) + + response = self.client.get(old_url) + self.assertEquals(response.status_code, 301) diff --git a/tests/manage.py b/tests/manage.py deleted file mode 100755 index ae633bd..0000000 --- a/tests/manage.py +++ /dev/null @@ -1,16 +0,0 @@ -#!/usr/bin/env python -import sys -sys.path.prepend('..') - -from django.core.management import execute_manager -import imp -try: - imp.find_module('settings') # Assumed to be in the same directory. -except ImportError: - sys.stderr.write("Error: Can't find the file 'settings.py' in the directory containing %r. It appears you've customized things.\nYou'll have to run django-admin.py, passing it your settings module.\n" % __file__) - sys.exit(1) - -import settings - -if __name__ == "__main__": - execute_manager(settings) diff --git a/tests/settings.py b/tests/settings.py deleted file mode 100644 index f8c2191..0000000 --- a/tests/settings.py +++ /dev/null @@ -1,145 +0,0 @@ -# Django settings for tests project. - -DEBUG = True -TEMPLATE_DEBUG = DEBUG - -ADMINS = ( - # ('Your Name', 'your_email@example.com'), -) - -MANAGERS = ADMINS - -DATABASES = { - 'default': { - 'ENGINE': 'django.db.backends.', # Add 'postgresql_psycopg2', 'postgresql', 'mysql', 'sqlite3' or 'oracle'. - 'NAME': '', # Or path to database file if using sqlite3. - 'USER': '', # Not used with sqlite3. - 'PASSWORD': '', # Not used with sqlite3. - 'HOST': '', # Set to empty string for localhost. Not used with sqlite3. - 'PORT': '', # Set to empty string for default. Not used with sqlite3. - } -} - -# Local time zone for this installation. Choices can be found here: -# http://en.wikipedia.org/wiki/List_of_tz_zones_by_name -# although not all choices may be available on all operating systems. -# On Unix systems, a value of None will cause Django to use the same -# timezone as the operating system. -# If running in a Windows environment this must be set to the same as your -# system time zone. -TIME_ZONE = 'America/Chicago' - -# Language code for this installation. All choices can be found here: -# http://www.i18nguy.com/unicode/language-identifiers.html -LANGUAGE_CODE = 'en-us' - -SITE_ID = 1 - -# If you set this to False, Django will make some optimizations so as not -# to load the internationalization machinery. -USE_I18N = True - -# If you set this to False, Django will not format dates, numbers and -# calendars according to the current locale -USE_L10N = True - -# Absolute filesystem path to the directory that will hold user-uploaded files. -# Example: "/home/media/media.lawrence.com/media/" -MEDIA_ROOT = '' - -# URL that handles the media served from MEDIA_ROOT. Make sure to use a -# trailing slash. -# Examples: "http://media.lawrence.com/media/", "http://example.com/media/" -MEDIA_URL = '' - -# Absolute path to the directory static files should be collected to. -# Don't put anything in this directory yourself; store your static files -# in apps' "static/" subdirectories and in STATICFILES_DIRS. -# Example: "/home/media/media.lawrence.com/static/" -STATIC_ROOT = '' - -# URL prefix for static files. -# Example: "http://media.lawrence.com/static/" -STATIC_URL = '/static/' - -# URL prefix for admin static files -- CSS, JavaScript and images. -# Make sure to use a trailing slash. -# Examples: "http://foo.com/static/admin/", "/static/admin/". -ADMIN_MEDIA_PREFIX = '/static/admin/' - -# Additional locations of static files -STATICFILES_DIRS = ( - # Put strings here, like "/home/html/static" or "C:/www/django/static". - # Always use forward slashes, even on Windows. - # Don't forget to use absolute paths, not relative paths. -) - -# List of finder classes that know how to find static files in -# various locations. -STATICFILES_FINDERS = ( - 'django.contrib.staticfiles.finders.FileSystemFinder', - 'django.contrib.staticfiles.finders.AppDirectoriesFinder', -# 'django.contrib.staticfiles.finders.DefaultStorageFinder', -) - -# Make this unique, and don't share it with anybody. -SECRET_KEY = 'd8y$qf%goqqett8i*72newll8^rzi2u=c^1+_99vm6#ybsews%' - -# List of callables that know how to import templates from various sources. -TEMPLATE_LOADERS = ( - 'django.template.loaders.filesystem.Loader', - 'django.template.loaders.app_directories.Loader', -# 'django.template.loaders.eggs.Loader', -) - -MIDDLEWARE_CLASSES = ( - 'django.middleware.common.CommonMiddleware', - 'django.contrib.sessions.middleware.SessionMiddleware', - 'django.middleware.csrf.CsrfViewMiddleware', - 'django.contrib.auth.middleware.AuthenticationMiddleware', - 'django.contrib.messages.middleware.MessageMiddleware', -) - -ROOT_URLCONF = 'tests.urls' - -TEMPLATE_DIRS = ( - # Put strings here, like "/home/html/django_templates" or "C:/www/django/templates". - # Always use forward slashes, even on Windows. - # Don't forget to use absolute paths, not relative paths. -) - -INSTALLED_APPS = ( - 'django.contrib.auth', - 'django.contrib.contenttypes', - 'django.contrib.sessions', - 'django.contrib.sites', - 'django.contrib.messages', - 'django.contrib.staticfiles', - # Uncomment the next line to enable the admin: - # 'django.contrib.admin', - # Uncomment the next line to enable admin documentation: - # 'django.contrib.admindocs', -) - -# A sample logging configuration. The only tangible logging -# performed by this configuration is to send an email to -# the site admins on every HTTP 500 error. -# See http://docs.djangoproject.com/en/dev/topics/logging for -# more details on how to customize your logging configuration. -LOGGING = { - 'version': 1, - 'disable_existing_loggers': False, - 'handlers': { - 'mail_admins': { - 'level': 'ERROR', - 'class': 'django.utils.log.AdminEmailHandler' - } - }, - 'loggers': { - 'django.request': { - 'handlers': ['mail_admins'], - 'level': 'ERROR', - 'propagate': True, - }, - } -} diff --git a/tests/templates/404.html b/tests/templates/404.html index e69de29..205df82 100644 --- a/tests/templates/404.html +++ b/tests/templates/404.html @@ -0,0 +1,13 @@ + +{% comment %} +NOTE: This template is only needed because Django will raise a + TemplateDoesNotExist exception for a 404 (not in debug mode). +{% endcomment %} + + + Page not found! + + +

Page could not be found (404)

+ + diff --git a/url_tracker/tests.py b/tests/unit_tests.py similarity index 73% rename from url_tracker/tests.py rename to tests/unit_tests.py index 2782b74..a7d130d 100644 --- a/url_tracker/tests.py +++ b/tests/unit_tests.py @@ -6,11 +6,15 @@ """ from mock import Mock -from django.http import Http404 from django.test import TestCase import url_tracker from url_tracker.models import URLChangeRecord +from url_tracker.mixins import URLTrackingMixin + + +class TrackedModelMock(URLTrackingMixin, Mock): + pass class TestTracking(TestCase): @@ -19,11 +23,11 @@ def setUp(self): class DoesNotExist(BaseException): pass - self.model_mock = Mock - self.model_mock.get_absolute_url = Mock(method='get_absolute_url') + self.model_mock = TrackedModelMock + self.model_mock.get_absolute_url = Mock() self.tracked_model = self.model_mock(name='TrackedModel') - self.tracked_model._get_tracked_url = lambda: u'/the/new/one/' + self.tracked_model._get_tracked_url = lambda: u'/the/new/one/' def raise_exception(*args, **kwargs): raise self.tracked_model.__class__.DoesNotExist @@ -38,22 +42,14 @@ def raise_exception(*args, **kwargs): self.tracked_db_model._get_tracked_url = lambda: u'/the/old/one/' def test_tracking_model_without_url_method(self): - - class EmptyModel(object): - pass - + instance = TrackedModelMock() + instance.url_tracking_methods = [] self.assertRaises( url_tracker.URLTrackingError, url_tracker.track_url_changes_for_model, - EmptyModel(), + instance, ) - def test__lookup_url_with_new_instance(self): - url_tracker.track_url_changes_for_model(Mock) - url_tracker.lookup_previous_url(self.tracked_model) - - self.assertEquals(self.tracked_model._old_url, None) - def test_lookup_url_with_existing_instance(self): def return_instance(pk): return self.tracked_db_model @@ -61,29 +57,44 @@ def return_instance(pk): class_objects = Mock(name='MockModelManager') class_objects.get = return_instance self.tracked_model.__class__.objects = class_objects + self.tracked_model.get_absolute_url.return_value = u'/the/old/one/' - url_tracker.track_url_changes_for_model(Mock) + url_tracker.track_url_changes_for_model(TrackedModelMock) url_tracker.lookup_previous_url(self.tracked_model) - self.assertEquals(self.tracked_model._old_url, u'/the/old/one/') + expected_dict = {'get_absolute_url': u'/the/old/one/'} + self.assertEquals(self.tracked_model._old_urls, expected_dict) + + def test_create_delete_record_for_url_method_returning_none(self): + instance = self.tracked_model + instance.get_absolute_url.return_value = None + instance._old_urls = {'get_absolute_url': u'/the/old/one/'} + + url_tracker.track_changed_url(instance) + self.assertEquals(URLChangeRecord.objects.count(), 1) + record = URLChangeRecord.objects.all()[0] + self.assertEquals(record.deleted, True) def test_track_changed_url_with_new_instance(self): instance = self.tracked_model - instance._old_url = None + instance.get_absolute_url.return_value = u'/the/new/one/' + instance._old_urls = {'get_absolute_url': None} url_tracker.track_changed_url(instance) self.assertEquals(URLChangeRecord.objects.count(), 0) def test_track_changed_url_with_unchanged_url(self): instance = self.tracked_model - instance._old_url = '/the/new/one/' + instance.get_absolute_url.return_value = u'/the/old/one/' + instance._old_urls = {'get_absolute_url': u'/the/old/one/'} url_tracker.track_changed_url(instance) self.assertEquals(URLChangeRecord.objects.count(), 0) def test_track_changed_url_without_existing_records(self): instance = self.tracked_model - instance._old_url = '/the/old/one/' + instance.get_absolute_url.return_value = u'/the/new/one/' + instance._old_urls = {'get_absolute_url': u'/the/old/one/'} url_tracker.track_changed_url(instance) self.assertEquals(URLChangeRecord.objects.count(), 1) @@ -97,7 +108,8 @@ def test_track_changed_url_with_existing_records(self): URLChangeRecord.objects.create(old_url='/one/', new_url='/the/') instance = self.tracked_model - instance._old_url = '/the/old/one/' + instance.get_absolute_url.return_value = u'/the/new/one/' + instance._old_urls = {'get_absolute_url': u'/the/old/one/'} url_tracker.track_changed_url(instance) self.assertEquals(URLChangeRecord.objects.count(), 3) @@ -115,7 +127,8 @@ def test_track_changed_url_with_existing_records_and_old_url(self): URLChangeRecord.objects.create(old_url='/the/old/one/', new_url='/the/') instance = self.tracked_model - instance._old_url = '/the/old/one/' + instance.get_absolute_url.return_value = u'/the/new/one/' + instance._old_urls = {'get_absolute_url': u'/the/old/one/'} url_tracker.track_changed_url(instance) self.assertEquals(URLChangeRecord.objects.count(), 2) @@ -135,7 +148,8 @@ def test_track_changed_url_with_existing_deleted_record(self): URLChangeRecord.objects.create(old_url='/one/', new_url='/the/') instance = self.tracked_model - instance._old_url = '/the/old/one/' + instance.get_absolute_url.return_value = u'/the/new/one/' + instance._old_urls = {'get_absolute_url': u'/the/old/one/'} url_tracker.track_changed_url(instance) @@ -146,7 +160,8 @@ def test_track_changed_url_with_existing_deleted_record(self): def test_track_deleted_url_without_existing_records(self): instance = self.tracked_model - instance._old_url = '/the/old/one/' + instance.get_absolute_url.return_value = u'/the/new/one/' + instance._old_urls = {'get_absolute_url': u'/the/old/one/'} url_tracker.track_deleted_url(instance) @@ -160,7 +175,8 @@ def test_track_changed_url_deleting_exsiting_record_with_new_url(self): URLChangeRecord.objects.create(old_url='/the/new/one/', new_url='/the/') instance = self.tracked_model - instance._old_url = '/the/old/one/' + instance.get_absolute_url.return_value = u'/the/new/one/' + instance._old_urls = {'get_absolute_url': '/the/old/one/'} url_tracker.track_changed_url(instance) self.assertEquals(URLChangeRecord.objects.count(), 1) @@ -168,30 +184,3 @@ def test_track_changed_url_deleting_exsiting_record_with_new_url(self): self.assertEquals(record.old_url, u'/the/old/one/') self.assertEquals(record.new_url, u'/the/new/one/') self.assertEquals(record.deleted, False) - - -class TestUrlChanges(TestCase): - - def test_invalid_url(self): - response = self.client.get('/work/an-invalid-project/') - self.assertEquals(response.status_code, 404) - - def test_changed_url(self): - url_change = URLChangeRecord.objects.create( - old_url='/the/old-url/', - new_url='/the/new/url/', - ) - - response = self.client.get('/the/old-url/') - self.assertEquals(response.status_code, 301) - self.assertEquals(response['location'], 'http://testserver/the/new/url/') - - def test_deleted_url(self): - url_change = URLChangeRecord.objects.create( - old_url='/the/old-url/', - new_url='', - deleted=True - ) - - response = self.client.get('/the/old-url/') - self.assertEquals(response.status_code, 410) diff --git a/tests/urls.py b/tests/urls.py index 6ffa8ab..16a2e18 100644 --- a/tests/urls.py +++ b/tests/urls.py @@ -1,17 +1,5 @@ -from django.conf.urls.defaults import patterns, include, url +from django.conf.urls.defaults import patterns -# Uncomment the next two lines to enable the admin: -# from django.contrib import admin -# admin.autodiscover() - -urlpatterns = patterns('', - # Examples: - # url(r'^$', 'tests.views.home', name='home'), - # url(r'^tests/', include('tests.foo.urls')), - - # Uncomment the admin/doc line below to enable admin documentation: - # url(r'^admin/doc/', include('django.contrib.admindocs.urls')), - - # Uncomment the next line to enable the admin: - # url(r'^admin/', include(admin.site.urls)), -) +# this is only in here so that Django is not complaining about +# missing URLs +urlpatterns = patterns('') diff --git a/tox.ini b/tox.ini new file mode 100644 index 0000000..f54caa4 --- /dev/null +++ b/tox.ini @@ -0,0 +1,6 @@ +#content of: tox.ini , put in same dir as setup.py +[tox] +envlist = py26,py27 +[testenv] +commands= + - make test diff --git a/url_tracker/__init__.py b/url_tracker/__init__.py index cb158c4..623f307 100644 --- a/url_tracker/__init__.py +++ b/url_tracker/__init__.py @@ -1,9 +1,14 @@ import logging -logger = logging.getLogger(__file__) +import warnings from django.db.models import signals +from django.core.exceptions import ImproperlyConfigured +from django.core.urlresolvers import NoReverseMatch + +from .models import URLChangeRecord +from .mixins import URLTrackingMixin -from url_tracker.models import URLChangeRecord +logger = logging.getLogger(__file__) class URLTrackingError(Exception): @@ -16,133 +21,145 @@ class URLTrackingError(Exception): def lookup_previous_url(instance, **kwargs): """ Look up the absolute URL of *instance* from the database while it is - in a ``pre_save`` state. The previous url is saved in the instance as - *_old_url* so that it can be used after the instance was saved. + in a ``pre_save`` state. The previous URLs are saved in the instance's + *_old_urls* attribute as dictionary. The method name for the given URLs + are used as the dictionary keys. - If the instance has not been saved to the database (i.e. is new) the - *_old_url* will be stored as ``None`` which will prevent further tracking - after saving the instance. + If the instance has not been saved to the database (i.e. is new) + the ``_old_urls`` dictionary is set to ``{}`` which will prevent a record + to be created. + """ + instance._old_urls = {} + for method_name in instance.get_url_tracking_methods(): + try: + method = getattr(instance, method_name) + except AttributeError: + raise ImproperlyConfigured( + "model instance '%s' does not have a method '%s'" % ( + instance.__class__.__name__, + method_name + ) + ) + try: + old_url = method() + except NoReverseMatch: + logger.debug("Method's URL doesn't resolve") + old_url = None + instance._old_urls[method_name] = old_url + + +def _create_delete_record(url): """ - try: - db_instance = instance.__class__.objects.get(pk=instance.pk) - logger.debug("saving old URL for instance '%s' URL", instance.__class__.__name__) - instance._old_url = db_instance._get_tracked_url() - except instance.__class__.DoesNotExist: - logger.debug("new instance, no URL tracking required") - instance._old_url = None + Create a delete record for the given *url*. This updates all records + where *url* is the ``new_url`` (previous redirects). It also creates + a new record with *url* being the ``old_url`` and no ``new_url`` and + marked as deleted. This marks an endpoint in the chain of URL + redirects. + """ + # updated existing records with the old URL being the new_url + # of this record. Changed the *deleted* flag to be ``False`` + URLChangeRecord.objects.filter(new_url=url).update( + new_url='', + deleted=True + ) + + record, __ = URLChangeRecord.objects.get_or_create(old_url=url) + record.deleted = True + record.save() def track_changed_url(instance, **kwargs): """ - Track a URL change for *instance* after a new instance was saved. If - the old URL is ``None`` (i.e. *instance* is new) or the new URL and - the old one are equal (i.e. URL is unchanged), nothing will be changed - in the database. - - For URL changes, the database will be checked for existing records that - have a *new_url* entry equal to the old URL of *instance* and updates - these records. Then, a new ``URLChangeRecord`` is created for the - *instance*. + Track a URL changes for *instance* after a new instance was saved. If + no old URLs are available (i.e. *instance* is new) or if a new and old URL + are the same (i.e. URL is unchanged), nothing will be changed in the + database for this URL. + + For URLs that have changed, the database will be checked for existing + records that have a *new_url* entry equal to the old URL of *instance* and + updates these records. Then, a new ``URLChangeRecord`` is created for this + URL. """ - old_url = getattr(instance, '_old_url', None) - - if old_url is None: - return - - new_url = instance._get_tracked_url() - # we don't want to store URL changes for unchanged URL - if old_url == new_url: - return - - logger.debug( - "tracking URL change for instance '%s' URL", - instance.__class__.__name__ - ) - - # check if the new URL is already in the table and - # remove these entries - for record in URLChangeRecord.objects.filter(old_url=new_url): - record.delete() - - # updated existing records with the old URL being - # the new URL in the record - url_records = URLChangeRecord.objects.filter(new_url=old_url) - for record in url_records: - record.new_url = new_url - record.deleted = False - record.save() - - # create a new/updated record for this combination of old and - # new URL. If the record already exists, it is assumed that the - # current change is to be used and the existing new_url will be - # detached from the old_url. - try: - logger.info( - "a record for '%s' already exists and will be updated", - instance._old_url, + for method_name, old_url in getattr(instance, '_old_urls', {}).items(): + try: + new_url = getattr(instance, method_name)() + except NoReverseMatch: + new_url = None + + # we don't want to store URL changes for unchanged URL + if not old_url or (old_url == new_url): + continue + + # if the new URL is None we assume that it has been deleted and + # create a delete record for the old URL. + if not new_url: + _create_delete_record(old_url) + continue + + logger.debug( + "tracking URL change for instance '%s' URL", + instance.__class__.__name__ ) - record = URLChangeRecord.objects.get(old_url=instance._old_url) - record.new_url = new_url - record.deleted = False - record.save() - except URLChangeRecord.DoesNotExist: - URLChangeRecord.objects.create( - old_url=instance._old_url, + + # check if the new URL is already in the table and + # remove these entries + URLChangeRecord.objects.filter(old_url=new_url).delete() + + # updated existing records with the old URL being + # the new URL in the record + url_records = URLChangeRecord.objects.filter(new_url=old_url).update( new_url=new_url, deleted=False ) + # create a new/updated record for this combination of old and + # new URL. If the record already exists, it is assumed that the + # current change is to be used and the existing new_url will be + # detached from the old_url. + + record, __ = URLChangeRecord.objects.get_or_create(old_url=old_url) + record.new_url = new_url + record.deleted = False + record.save() def track_deleted_url(instance, **kwargs): """ - Track the URL of a deleted *instance*. It updates all existing + Track the URL of a deleted *instance*. It updates all existing records with ``new_url`` being set to the *instance*'s old URL and - marks this record as deleted URL. - + marks this record as deleted URL. + A new ``URLChangeRecord`` is created for the old URL of *instance* that is marked as deleted. """ - logger.debug("tracking deleted instance '%s' URL", instance.__class__.__name__) - old_url = getattr(instance, '_old_url', None) - if old_url is None: - return - - # updated existing records with the old URL being the new_url - # of this record. Changed the *deleted* flag to be ``False`` - url_records = URLChangeRecord.objects.filter(new_url=old_url) - for record in url_records: - record.new_url = '' - record.deleted = True - record.save() - - try: - url_change = URLChangeRecord.objects.get(old_url=old_url) - url_change.deleted = True - url_change.save() - except URLChangeRecord.DoesNotExist: - urlchange = URLChangeRecord.objects.create( - old_url=old_url, - deleted=True - ) + logger.debug("tracking deleted instance '%s' URL", + instance.__class__.__name__) + for old_url in getattr(instance, '_old_urls', {}).values(): + _create_delete_record(old_url) def track_url_changes_for_model(model, absolute_url_method='get_absolute_url'): """ - Keep track of URL changes of the specified *model*. This will connect the - *model*'s ``pre_save``, ``post_save`` and ``post_delete`` signals to the - tracking methods ``lookup_previous_url``, ``track_changed_url`` - and ``track_deleted_url`` respectively. URL changes will be logged in the - ``URLChangeRecord`` table and are handled by the tracking middleware when - a changed URL is called. + Register the *model* for URL tracking. It requires the *model* to provide + an attribute ``url_tracking_methods`` and/or a ``get_url_tracking_methods`` + method to return a list of methods to retrieve trackable URLs. + The default setup provides ``url_tracking_methods = ['get_absolute_url']``. + + The ``pre_save``, ``post_save`` and ``post_delete`` methods are connected + to different tracking methods for *model* and create/update + ``URLChangeRecord``s as required. """ - try: - model._get_tracked_url = getattr(model, absolute_url_method) - except AttributeError: - raise URLTrackingError( - "cannot track instance %s without method %s", - model.__class__.__name__, - absolute_url_method, + if not hasattr(model, 'get_url_tracking_methods'): + warnings.warn( + "the 'absolute_url_method' is deprecated, use the " + "'UrlTrackingMixin' instead", + PendingDeprecationWarning ) + model.url_tracking_methods = [absolute_url_method] + model.get_url_tracking_methods = URLTrackingMixin.get_url_tracking_methods + + # make sure that URL method names are specified for the given model + if not getattr(model, 'url_tracking_methods', None): + raise URLTrackingError("no URLs specified for model '%s'" % model) signals.pre_save.connect(lookup_previous_url, sender=model, weak=False) signals.post_save.connect(track_changed_url, sender=model, weak=False) diff --git a/url_tracker/admin.py b/url_tracker/admin.py index 866efcb..dc71ddd 100644 --- a/url_tracker/admin.py +++ b/url_tracker/admin.py @@ -2,4 +2,9 @@ from url_tracker.models import URLChangeRecord -admin.site.register(URLChangeRecord) + +class URLChangeRecordAdmin(admin.ModelAdmin): + list_display = ('__unicode__', 'deleted', 'date_changed') + list_filter = ('deleted', 'date_changed') + +admin.site.register(URLChangeRecord, URLChangeRecordAdmin) diff --git a/url_tracker/middleware.py b/url_tracker/middleware.py index 7a32f79..32d9401 100644 --- a/url_tracker/middleware.py +++ b/url_tracker/middleware.py @@ -11,6 +11,11 @@ def process_response(self, request, response): return response # No need to check for non-404 responses. try: requested_url = request.path_info + query_string = request.META['QUERY_STRING'] + + if query_string: + requested_url = "%s?%s" % (requested_url, query_string) + redirect_url = URLChangeRecord.objects.get(old_url__exact=requested_url) if redirect_url.deleted: return http.HttpResponseGone() diff --git a/url_tracker/migrations/0004_auto__chg_field_urlchangerecord_old_url__chg_field_urlchangerecord_new.py b/url_tracker/migrations/0004_auto__chg_field_urlchangerecord_old_url__chg_field_urlchangerecord_new.py new file mode 100644 index 0000000..8329fd7 --- /dev/null +++ b/url_tracker/migrations/0004_auto__chg_field_urlchangerecord_old_url__chg_field_urlchangerecord_new.py @@ -0,0 +1,35 @@ +# -*- coding: utf-8 -*- +from south.db import db +from south.v2 import SchemaMigration + + +class Migration(SchemaMigration): + + def forwards(self, orm): + + # Changing field 'URLChangeRecord.old_url' + db.alter_column('url_tracker_urlchangerecord', 'old_url', self.gf('django.db.models.fields.CharField')(unique=True, max_length=200)) + + # Changing field 'URLChangeRecord.new_url' + db.alter_column('url_tracker_urlchangerecord', 'new_url', self.gf('django.db.models.fields.CharField')(max_length=200, null=True)) + + def backwards(self, orm): + + # Changing field 'URLChangeRecord.old_url' + db.alter_column('url_tracker_urlchangerecord', 'old_url', self.gf('django.db.models.fields.CharField')(max_length=100, unique=True)) + + # Changing field 'URLChangeRecord.new_url' + db.alter_column('url_tracker_urlchangerecord', 'new_url', self.gf('django.db.models.fields.CharField')(max_length=100, null=True)) + + models = { + 'url_tracker.urlchangerecord': { + 'Meta': {'object_name': 'URLChangeRecord'}, + 'date_changed': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}), + 'deleted': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'new_url': ('django.db.models.fields.CharField', [], {'max_length': '200', 'null': 'True', 'blank': 'True'}), + 'old_url': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '200'}) + } + } + + complete_apps = ['url_tracker'] diff --git a/url_tracker/migrations/0005_auto__chg_field_urlchangerecord_old_url__chg_field_urlchangerecord_new.py b/url_tracker/migrations/0005_auto__chg_field_urlchangerecord_old_url__chg_field_urlchangerecord_new.py new file mode 100644 index 0000000..c587a85 --- /dev/null +++ b/url_tracker/migrations/0005_auto__chg_field_urlchangerecord_old_url__chg_field_urlchangerecord_new.py @@ -0,0 +1,37 @@ +# -*- coding: utf-8 -*- +import datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + + # Changing field 'URLChangeRecord.old_url' + db.alter_column('url_tracker_urlchangerecord', 'old_url', self.gf('django.db.models.fields.TextField')(unique=True)) + + # Changing field 'URLChangeRecord.new_url' + db.alter_column('url_tracker_urlchangerecord', 'new_url', self.gf('django.db.models.fields.TextField')(null=True)) + + def backwards(self, orm): + + # Changing field 'URLChangeRecord.old_url' + db.alter_column('url_tracker_urlchangerecord', 'old_url', self.gf('django.db.models.fields.CharField')(max_length=200, unique=True)) + + # Changing field 'URLChangeRecord.new_url' + db.alter_column('url_tracker_urlchangerecord', 'new_url', self.gf('django.db.models.fields.CharField')(max_length=200, null=True)) + + models = { + 'url_tracker.urlchangerecord': { + 'Meta': {'object_name': 'URLChangeRecord'}, + 'date_changed': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}), + 'deleted': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'new_url': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'old_url': ('django.db.models.fields.TextField', [], {'unique': 'True'}) + } + } + + complete_apps = ['url_tracker'] \ No newline at end of file diff --git a/url_tracker/mixins.py b/url_tracker/mixins.py new file mode 100644 index 0000000..d4044bf --- /dev/null +++ b/url_tracker/mixins.py @@ -0,0 +1,8 @@ +class URLTrackingMixin(object): + url_tracking_methods = [ + 'get_absolute_url', + ] + _old_urls = {} + + def get_url_tracking_methods(self): + return self.url_tracking_methods diff --git a/url_tracker/models.py b/url_tracker/models.py index c9094eb..9442c2a 100644 --- a/url_tracker/models.py +++ b/url_tracker/models.py @@ -2,10 +2,12 @@ class URLChangeRecord(models.Model): - old_url = models.CharField(max_length=100, unique=True) - new_url = models.CharField(max_length=100, blank=True, null=True) + old_url = models.TextField(unique=True) + new_url = models.TextField(blank=True, null=True) deleted = models.BooleanField(default=False) date_changed = models.DateTimeField(auto_now_add=True) def __unicode__(self): - return "URL change for '%s'" % self.old_url + if self.new_url and not self.deleted: + return "%s ---> %s" % (self.old_url, self.new_url) + return self.old_url diff --git a/url_tracker/views.py b/url_tracker/views.py deleted file mode 100644 index e69de29..0000000