From 6eda8d784a128309a67540fb0a62373667958daa Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Mon, 24 Sep 2012 16:03:12 +0200 Subject: [PATCH 1/7] Enlarged exception catching when testing for GDAL presence Other import errors than ImportError can happen during import of GDAL files (e.g. OGRException). Some further auditing may be needed if we want to restrict the catched exceptions at a later stage. Thanks Ramiro Morales for raising the issue. --- django/contrib/gis/gdal/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/django/contrib/gis/gdal/__init__.py b/django/contrib/gis/gdal/__init__.py index f477f05982..de41df90ff 100644 --- a/django/contrib/gis/gdal/__init__.py +++ b/django/contrib/gis/gdal/__init__.py @@ -41,7 +41,7 @@ try: from django.contrib.gis.gdal.srs import SpatialReference, CoordTransform from django.contrib.gis.gdal.geometries import OGRGeometry HAS_GDAL = True -except ImportError: +except Exception: HAS_GDAL = False try: From e72e22e518a730cd28cd68c9374fa79a45e27a9c Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Mon, 24 Sep 2012 16:07:58 +0200 Subject: [PATCH 2/7] Replaced a deprecated assertEquals --- tests/regressiontests/utils/html.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/regressiontests/utils/html.py b/tests/regressiontests/utils/html.py index 98df80a5e2..6a93dff85e 100644 --- a/tests/regressiontests/utils/html.py +++ b/tests/regressiontests/utils/html.py @@ -154,4 +154,4 @@ class TestUtilsHtml(unittest.TestCase): ("x

y

", "a b", "x

y

"), ) for value, tags, output in items: - self.assertEquals(f(value, tags), output) + self.assertEqual(f(value, tags), output) From fc69fff9ab5bba8a6cff3eaf51f02a3204b1c015 Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Mon, 24 Sep 2012 22:11:42 +0200 Subject: [PATCH 3/7] Fixed #14861 -- Moved logging config outside of Settings.__init__ Thanks donspaulding for the report and simonpercivall for the initial patch. --- django/conf/__init__.py | 30 ++++++++++--------- docs/topics/logging.txt | 30 ------------------- .../logging_tests/logconfig.py | 7 +++++ tests/regressiontests/logging_tests/tests.py | 29 ++++++++++++++++++ 4 files changed, 52 insertions(+), 44 deletions(-) create mode 100644 tests/regressiontests/logging_tests/logconfig.py diff --git a/django/conf/__init__.py b/django/conf/__init__.py index 6272f4ed5d..d636ff0b6c 100644 --- a/django/conf/__init__.py +++ b/django/conf/__init__.py @@ -43,13 +43,28 @@ class LazySettings(LazyObject): % (name, ENVIRONMENT_VARIABLE)) self._wrapped = Settings(settings_module) - + self._configure_logging() def __getattr__(self, name): if self._wrapped is empty: self._setup(name) return getattr(self._wrapped, name) + def _configure_logging(self): + """ + Setup logging from LOGGING_CONFIG and LOGGING settings. + """ + if self.LOGGING_CONFIG: + # First find the logging configuration function ... + logging_config_path, logging_config_func_name = self.LOGGING_CONFIG.rsplit('.', 1) + logging_config_module = importlib.import_module(logging_config_path) + logging_config_func = getattr(logging_config_module, logging_config_func_name) + + # Backwards-compatibility shim for #16288 fix + compat_patch_logging_config(self.LOGGING) + + # ... then invoke it with the logging settings + logging_config_func(self.LOGGING) def configure(self, default_settings=global_settings, **options): """ @@ -133,19 +148,6 @@ class Settings(BaseSettings): os.environ['TZ'] = self.TIME_ZONE time.tzset() - # Settings are configured, so we can set up the logger if required - if self.LOGGING_CONFIG: - # First find the logging configuration function ... - logging_config_path, logging_config_func_name = self.LOGGING_CONFIG.rsplit('.', 1) - logging_config_module = importlib.import_module(logging_config_path) - logging_config_func = getattr(logging_config_module, logging_config_func_name) - - # Backwards-compatibility shim for #16288 fix - compat_patch_logging_config(self.LOGGING) - - # ... then invoke it with the logging settings - logging_config_func(self.LOGGING) - class UserSettingsHolder(BaseSettings): """ diff --git a/docs/topics/logging.txt b/docs/topics/logging.txt index 94236babd6..a4aae0bc02 100644 --- a/docs/topics/logging.txt +++ b/docs/topics/logging.txt @@ -345,36 +345,6 @@ This logging configuration does the following things: printed to the console; ``ERROR`` and ``CRITICAL`` messages will also be output via email. -.. admonition:: Custom handlers and circular imports - - If your ``settings.py`` specifies a custom handler class and the file - defining that class also imports ``settings.py`` a circular import will - occur. - - For example, if ``settings.py`` contains the following config for - :setting:`LOGGING`:: - - LOGGING = { - 'version': 1, - 'handlers': { - 'custom_handler': { - 'level': 'INFO', - 'class': 'myproject.logconfig.MyHandler', - } - } - } - - and ``myproject/logconfig.py`` has the following line before the - ``MyHandler`` definition:: - - from django.conf import settings - - then the ``dictconfig`` module will raise an exception like the following:: - - ValueError: Unable to configure handler 'custom_handler': - Unable to configure handler 'custom_handler': - 'module' object has no attribute 'logconfig' - .. _formatter documentation: http://docs.python.org/library/logging.html#formatter-objects Custom logging configuration diff --git a/tests/regressiontests/logging_tests/logconfig.py b/tests/regressiontests/logging_tests/logconfig.py new file mode 100644 index 0000000000..8524aa2c24 --- /dev/null +++ b/tests/regressiontests/logging_tests/logconfig.py @@ -0,0 +1,7 @@ +import logging + +from django.conf import settings + +class MyHandler(logging.Handler): + def __init__(self, *args, **kwargs): + self.config = settings.LOGGING diff --git a/tests/regressiontests/logging_tests/tests.py b/tests/regressiontests/logging_tests/tests.py index f444e0ff46..a54b425f67 100644 --- a/tests/regressiontests/logging_tests/tests.py +++ b/tests/regressiontests/logging_tests/tests.py @@ -10,6 +10,8 @@ from django.test import TestCase, RequestFactory from django.test.utils import override_settings from django.utils.log import CallbackFilter, RequireDebugFalse +from ..admin_scripts.tests import AdminScriptTestCase + # logging config prior to using filter with mail_admins OLD_LOGGING = { @@ -253,3 +255,30 @@ class AdminEmailHandlerTest(TestCase): self.assertEqual(len(mail.outbox), 1) self.assertEqual(mail.outbox[0].subject, expected_subject) + + +class SettingsConfigTest(AdminScriptTestCase): + """ + Test that accessing settings in a custom logging handler does not trigger + a circular import error. + """ + def setUp(self): + log_config = """{ + 'version': 1, + 'handlers': { + 'custom_handler': { + 'level': 'INFO', + 'class': 'logging_tests.logconfig.MyHandler', + } + } +}""" + self.write_settings('settings.py', sdict={'LOGGING': log_config}) + + def tearDown(self): + self.remove_settings('settings.py') + + def test_circular_dependency(self): + # validate is just an example command to trigger settings configuration + out, err = self.run_manage(['validate']) + self.assertNoOutput(err) + self.assertOutput(out, "0 errors found") From 515fd6a5de2f1bf817fac9ced07d0485d594b510 Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Mon, 24 Sep 2012 22:42:58 +0200 Subject: [PATCH 4/7] Called parent __init__ in test logging handler --- tests/regressiontests/logging_tests/logconfig.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/regressiontests/logging_tests/logconfig.py b/tests/regressiontests/logging_tests/logconfig.py index 8524aa2c24..fc5ea1a0bd 100644 --- a/tests/regressiontests/logging_tests/logconfig.py +++ b/tests/regressiontests/logging_tests/logconfig.py @@ -3,5 +3,6 @@ import logging from django.conf import settings class MyHandler(logging.Handler): - def __init__(self, *args, **kwargs): + def __init__(self): + logging.Handler.__init__(self) self.config = settings.LOGGING From f51eab796d087439eedcb768cdd312517780940e Mon Sep 17 00:00:00 2001 From: Ramiro Morales Date: Mon, 24 Sep 2012 22:02:59 -0300 Subject: [PATCH 5/7] Fixed #18072 -- Made more admin links use reverse() instead of hard-coded relative URLs. Thanks kmike for the report and initial patch for the changelist->edit object view link URL. Other affected links include the delete object one and object history one (in this case the change had been implemented in commit 5a9e127, this commit adds admin-quoting of the object PK in a way similar to a222d6e.) Refs #15294. --- .../admin/templates/admin/change_form.html | 2 +- .../admin/templates/admin/submit_line.html | 6 +- .../admin/templatetags/admin_modify.py | 6 +- django/contrib/admin/util.py | 6 +- django/contrib/admin/views/main.py | 7 +- .../regressiontests/admin_changelist/tests.py | 10 ++- .../admin_custom_urls/fixtures/actions.json | 7 -- .../admin_custom_urls/tests.py | 16 +--- tests/regressiontests/admin_views/tests.py | 75 +++++++++++++------ 9 files changed, 79 insertions(+), 56 deletions(-) diff --git a/django/contrib/admin/templates/admin/change_form.html b/django/contrib/admin/templates/admin/change_form.html index e27875cdad..4962e732a2 100644 --- a/django/contrib/admin/templates/admin/change_form.html +++ b/django/contrib/admin/templates/admin/change_form.html @@ -29,7 +29,7 @@ {% if change %}{% if not is_popup %} diff --git a/django/contrib/admin/templates/admin/submit_line.html b/django/contrib/admin/templates/admin/submit_line.html index d6f854a233..8c9d22752d 100644 --- a/django/contrib/admin/templates/admin/submit_line.html +++ b/django/contrib/admin/templates/admin/submit_line.html @@ -1,8 +1,8 @@ -{% load i18n %} +{% load i18n admin_urls %}
{% if show_save %}{% endif %} -{% if show_delete_link %}{% endif %} +{% if show_delete_link %}{% endif %} {% if show_save_as_new %}{%endif%} -{% if show_save_and_add_another %}{% endif %} +{% if show_save_and_add_another %}{% endif %} {% if show_save_and_continue %}{% endif %}
diff --git a/django/contrib/admin/templatetags/admin_modify.py b/django/contrib/admin/templatetags/admin_modify.py index c190533f95..f6ac59635a 100644 --- a/django/contrib/admin/templatetags/admin_modify.py +++ b/django/contrib/admin/templatetags/admin_modify.py @@ -28,7 +28,8 @@ def submit_row(context): change = context['change'] is_popup = context['is_popup'] save_as = context['save_as'] - return { + ctx = { + 'opts': opts, 'onclick_attrib': (opts.get_ordered_objects() and change and 'onclick="submitOrderForm();"' or ''), 'show_delete_link': (not is_popup and context['has_delete_permission'] @@ -40,6 +41,9 @@ def submit_row(context): 'is_popup': is_popup, 'show_save': True } + if context.get('original') is not None: + ctx['original'] = context['original'] + return ctx @register.filter def cell_count(inline_admin_form): diff --git a/django/contrib/admin/util.py b/django/contrib/admin/util.py index f95fe53de1..74eef2e733 100644 --- a/django/contrib/admin/util.py +++ b/django/contrib/admin/util.py @@ -48,9 +48,9 @@ def prepare_lookup_value(key, value): def quote(s): """ Ensure that primary key values do not confuse the admin URLs by escaping - any '/', '_' and ':' characters. Similar to urllib.quote, except that the - quoting is slightly different so that it doesn't get automatically - unquoted by the Web browser. + any '/', '_' and ':' and similarly problematic characters. + Similar to urllib.quote, except that the quoting is slightly different so + that it doesn't get automatically unquoted by the Web browser. """ if not isinstance(s, six.string_types): return s diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index 74ef095b4b..5033ba98bc 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -3,6 +3,7 @@ from functools import reduce from django.core.exceptions import SuspiciousOperation, ImproperlyConfigured from django.core.paginator import InvalidPage +from django.core.urlresolvers import reverse from django.db import models from django.db.models.fields import FieldDoesNotExist from django.utils.datastructures import SortedDict @@ -376,4 +377,8 @@ class ChangeList(object): return qs def url_for_result(self, result): - return "%s/" % quote(getattr(result, self.pk_attname)) + pk = getattr(result, self.pk_attname) + return reverse('admin:%s_%s_change' % (self.opts.app_label, + self.opts.module_name), + args=(quote(pk),), + current_app=self.model_admin.admin_site.name) diff --git a/tests/regressiontests/admin_changelist/tests.py b/tests/regressiontests/admin_changelist/tests.py index be88c9a161..2b1c1a9bcf 100644 --- a/tests/regressiontests/admin_changelist/tests.py +++ b/tests/regressiontests/admin_changelist/tests.py @@ -6,6 +6,7 @@ from django.contrib import admin from django.contrib.admin.options import IncorrectLookupParameters from django.contrib.admin.views.main import ChangeList, SEARCH_VAR, ALL_VAR from django.contrib.auth.models import User +from django.core.urlresolvers import reverse from django.template import Context, Template from django.test import TestCase from django.test.client import RequestFactory @@ -65,7 +66,8 @@ class ChangeListTests(TestCase): template = Template('{% load admin_list %}{% spaceless %}{% result_list cl %}{% endspaceless %}') context = Context({'cl': cl}) table_output = template.render(context) - row_html = 'name(None)' % new_child.id + link = reverse('admin:admin_changelist_child_change', args=(new_child.id,)) + row_html = 'name(None)' % link self.assertFalse(table_output.find(row_html) == -1, 'Failed to find expected row element: %s' % table_output) @@ -87,7 +89,8 @@ class ChangeListTests(TestCase): template = Template('{% load admin_list %}{% spaceless %}{% result_list cl %}{% endspaceless %}') context = Context({'cl': cl}) table_output = template.render(context) - row_html = 'nameParent object' % new_child.id + link = reverse('admin:admin_changelist_child_change', args=(new_child.id,)) + row_html = 'nameParent object' % link self.assertFalse(table_output.find(row_html) == -1, 'Failed to find expected row element: %s' % table_output) @@ -425,7 +428,8 @@ class ChangeListTests(TestCase): request = self._mocked_authenticated_request('/child/', superuser) response = m.changelist_view(request) for i in range(1, 10): - self.assertContains(response, '%s' % (i, i)) + link = reverse('admin:admin_changelist_child_change', args=(i,)) + self.assertContains(response, '%s' % (link, i)) list_display = m.get_list_display(request) list_display_links = m.get_list_display_links(request, list_display) diff --git a/tests/regressiontests/admin_custom_urls/fixtures/actions.json b/tests/regressiontests/admin_custom_urls/fixtures/actions.json index a63cf8135c..7c6341d71d 100644 --- a/tests/regressiontests/admin_custom_urls/fixtures/actions.json +++ b/tests/regressiontests/admin_custom_urls/fixtures/actions.json @@ -40,12 +40,5 @@ "fields": { "description": "An action with a name suspected of being a XSS attempt" } - }, - { - "pk": "The name of an action", - "model": "admin_custom_urls.action", - "fields": { - "description": "A generic action" - } } ] diff --git a/tests/regressiontests/admin_custom_urls/tests.py b/tests/regressiontests/admin_custom_urls/tests.py index 64ff9f6692..3e9cf28965 100644 --- a/tests/regressiontests/admin_custom_urls/tests.py +++ b/tests/regressiontests/admin_custom_urls/tests.py @@ -1,5 +1,6 @@ from __future__ import absolute_import, unicode_literals +from django.contrib.admin.util import quote from django.core.urlresolvers import reverse from django.template.response import TemplateResponse from django.test import TestCase @@ -67,7 +68,7 @@ class AdminCustomUrlsTest(TestCase): # Ditto, but use reverse() to build the URL url = reverse('admin:%s_action_change' % Action._meta.app_label, - args=('add',)) + args=(quote('add'),)) response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertContains(response, 'Change action') @@ -75,19 +76,8 @@ class AdminCustomUrlsTest(TestCase): # Should correctly get the change_view for the model instance with the # funny-looking PK (the one wth a 'path/to/html/document.html' value) url = reverse('admin:%s_action_change' % Action._meta.app_label, - args=("path/to/html/document.html",)) + args=(quote("path/to/html/document.html"),)) response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertContains(response, 'Change action') self.assertContains(response, 'value="path/to/html/document.html"') - - def testChangeViewHistoryButton(self): - url = reverse('admin:%s_action_change' % Action._meta.app_label, - args=('The name of an action',)) - response = self.client.get(url) - self.assertEqual(response.status_code, 200) - expected_link = reverse('admin:%s_action_history' % - Action._meta.app_label, - args=('The name of an action',)) - self.assertContains(response, 'Horizontal', msg_prefix=fail_msg, html=True) - self.assertContains(response, 'Vertical', msg_prefix=fail_msg, html=True) + self.assertContains(response, 'Horizontal' % link1, msg_prefix=fail_msg, html=True) + self.assertContains(response, 'Vertical' % link2, msg_prefix=fail_msg, html=True) def testNamedGroupFieldChoicesFilter(self): """ @@ -1371,9 +1381,12 @@ class AdminViewStringPrimaryKeyTest(TestCase): self.assertEqual(response.status_code, 200) def test_changelist_to_changeform_link(self): - "The link from the changelist referring to the changeform of the object should be quoted" - response = self.client.get('/test_admin/admin/admin_views/modelwithstringprimarykey/') - should_contain = """%s""" % (escape(quote(self.pk)), escape(self.pk)) + "Link to the changeform of the object in changelist should use reverse() and be quoted -- #18072" + prefix = '/test_admin/admin/admin_views/modelwithstringprimarykey/' + response = self.client.get(prefix) + # this URL now comes through reverse(), thus iri_to_uri encoding + pk_final_url = escape(iri_to_uri(quote(self.pk))) + should_contain = """%s""" % (prefix, pk_final_url, escape(self.pk)) self.assertContains(response, should_contain) def test_recentactions_link(self): @@ -1441,6 +1454,18 @@ class AdminViewStringPrimaryKeyTest(TestCase): should_contain = '/%s/" class="viewsitelink">' % model.pk self.assertContains(response, should_contain) + def test_change_view_history_link(self): + """Object history button link should work and contain the pk value quoted.""" + url = reverse('admin:%s_modelwithstringprimarykey_change' % + ModelWithStringPrimaryKey._meta.app_label, + args=(quote(self.pk),)) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + expected_link = reverse('admin:%s_modelwithstringprimarykey_history' % + ModelWithStringPrimaryKey._meta.app_label, + args=(quote(self.pk),)) + self.assertContains(response, '%d' % (story1.id, story1.id), 1) - self.assertContains(response, '%d' % (story2.id, story2.id), 1) + self.assertContains(response, '%d' % (link1, story1.id), 1) + self.assertContains(response, '%d' % (link2, story2.id), 1) @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',)) From 5a1bf7eccb100ea4f4f61ceba9381a11e3e0afc5 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Tue, 25 Sep 2012 17:08:07 +0200 Subject: [PATCH 6/7] Fix little typo in cache documentation --- docs/topics/cache.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/cache.txt b/docs/topics/cache.txt index e80ac85bd8..2f95c33dd5 100644 --- a/docs/topics/cache.txt +++ b/docs/topics/cache.txt @@ -286,7 +286,7 @@ cache is multi-process and thread-safe. To use it, set The cache :setting:`LOCATION ` is used to identify individual memory stores. If you only have one locmem cache, you can omit the -:setting:`LOCATION `; however, if you have more that one local +:setting:`LOCATION `; however, if you have more than one local memory cache, you will need to assign a name to at least one of them in order to keep them separate. From 1f84b042f1c3fab0f806de814d668917dd86236a Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Tue, 25 Sep 2012 18:38:47 +0200 Subject: [PATCH 7/7] Fixed #19020 -- Do not depend on dict order in formtools tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thanks metzen for the report and initial patch, and Ɓukasz Rekucki for an inspirational patch on his randomhash_fixes branch. --- django/contrib/formtools/tests/__init__.py | 12 +++++------- .../contrib/formtools/tests/wizard/cookiestorage.py | 5 ++++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/django/contrib/formtools/tests/__init__.py b/django/contrib/formtools/tests/__init__.py index 15941332ed..a21ffde533 100644 --- a/django/contrib/formtools/tests/__init__.py +++ b/django/contrib/formtools/tests/__init__.py @@ -12,6 +12,7 @@ from django.conf import settings from django.contrib.formtools import preview, utils from django.contrib.formtools.wizard import FormWizard from django.test import TestCase +from django.test.html import parse_html from django.test.utils import override_settings from django.utils import unittest @@ -218,7 +219,6 @@ class DummyRequest(http.HttpRequest): ) class WizardTests(TestCase): urls = 'django.contrib.formtools.tests.urls' - input_re = re.compile('name="([^"]+)" value="([^"]+)"') wizard_step_data = ( { '0-name': 'Pony', @@ -409,14 +409,13 @@ class WizardTests(TestCase): """ Pull the appropriate field data from the context to pass to the next wizard step """ - previous_fields = response.context['previous_fields'] + previous_fields = parse_html(response.context['previous_fields']) fields = {'wizard_step': response.context['step0']} - def grab(m): - fields[m.group(1)] = m.group(2) - return '' + for input_field in previous_fields: + input_attrs = dict(input_field.attributes) + fields[input_attrs["name"]] = input_attrs["value"] - self.input_re.sub(grab, previous_fields) return fields def check_wizard_step(self, response, step_no): @@ -428,7 +427,6 @@ class WizardTests(TestCase): """ step_count = len(self.wizard_step_data) - self.assertEqual(response.status_code, 200) self.assertContains(response, 'Step %d of %d' % (step_no, step_count)) data = self.grab_field_data(response) diff --git a/django/contrib/formtools/tests/wizard/cookiestorage.py b/django/contrib/formtools/tests/wizard/cookiestorage.py index 495d3afd03..d450f47861 100644 --- a/django/contrib/formtools/tests/wizard/cookiestorage.py +++ b/django/contrib/formtools/tests/wizard/cookiestorage.py @@ -1,3 +1,5 @@ +import json + from django.test import TestCase from django.core import signing from django.core.exceptions import SuspiciousOperation @@ -41,4 +43,5 @@ class TestCookieStorage(TestStorage, TestCase): storage.init_data() storage.update_response(response) unsigned_cookie_data = cookie_signer.unsign(response.cookies[storage.prefix].value) - self.assertEqual(unsigned_cookie_data, '{"step_files":{},"step":null,"extra_data":{},"step_data":{}}') + self.assertEqual(json.loads(unsigned_cookie_data), + {"step_files": {}, "step": None, "extra_data": {}, "step_data": {}})