diff --git a/django/forms/widgets.py b/django/forms/widgets.py index 02aa32b207..e944091f0d 100644 --- a/django/forms/widgets.py +++ b/django/forms/widgets.py @@ -6,16 +6,21 @@ import copy import datetime import re import warnings +from collections import defaultdict from itertools import chain from django.conf import settings from django.forms.utils import to_current_timezone from django.templatetags.static import static from django.utils import datetime_safe, formats +from django.utils.datastructures import OrderedSet from django.utils.dates import MONTHS from django.utils.formats import get_format from django.utils.html import format_html, html_safe from django.utils.safestring import mark_safe +from django.utils.topological_sort import ( + CyclicDependencyError, stable_topological_sort, +) from django.utils.translation import gettext_lazy as _ from .renderers import get_default_renderer @@ -59,22 +64,15 @@ class Media: @property def _css(self): - css = self._css_lists[0] - # filter(None, ...) avoids calling merge with empty dicts. - for obj in filter(None, self._css_lists[1:]): - css = { - medium: self.merge(css.get(medium, []), obj.get(medium, [])) - for medium in css.keys() | obj.keys() - } - return css + css = defaultdict(list) + for css_list in self._css_lists: + for medium, sublist in css_list.items(): + css[medium].append(sublist) + return {medium: self.merge(*lists) for medium, lists in css.items()} @property def _js(self): - js = self._js_lists[0] - # filter(None, ...) avoids calling merge() with empty lists. - for obj in filter(None, self._js_lists[1:]): - js = self.merge(js, obj) - return js + return self.merge(*self._js_lists) def render(self): return mark_safe('\n'.join(chain.from_iterable(getattr(self, 'render_' + name)() for name in MEDIA_TYPES))) @@ -115,39 +113,37 @@ class Media: raise KeyError('Unknown media type "%s"' % name) @staticmethod - def merge(list_1, list_2): + def merge(*lists): """ - Merge two lists while trying to keep the relative order of the elements. - Warn if the lists have the same two elements in a different relative - order. + Merge lists while trying to keep the relative order of the elements. + Warn if the lists have the same elements in a different relative order. For static assets it can be important to have them included in the DOM in a certain order. In JavaScript you may not be able to reference a global or in CSS you might want to override a style. """ - # Start with a copy of list_1. - combined_list = list(list_1) - last_insert_index = len(list_1) - # Walk list_2 in reverse, inserting each element into combined_list if - # it doesn't already exist. - for path in reversed(list_2): - try: - # Does path already exist in the list? - index = combined_list.index(path) - except ValueError: - # Add path to combined_list since it doesn't exist. - combined_list.insert(last_insert_index, path) - else: - if index > last_insert_index: - warnings.warn( - 'Detected duplicate Media files in an opposite order:\n' - '%s\n%s' % (combined_list[last_insert_index], combined_list[index]), - MediaOrderConflictWarning, - ) - # path already exists in the list. Update last_insert_index so - # that the following elements are inserted in front of this one. - last_insert_index = index - return combined_list + dependency_graph = defaultdict(set) + all_items = OrderedSet() + for list_ in filter(None, lists): + head = list_[0] + # The first items depend on nothing but have to be part of the + # dependency graph to be included in the result. + dependency_graph.setdefault(head, set()) + for item in list_: + all_items.add(item) + # No self dependencies + if head != item: + dependency_graph[item].add(head) + head = item + try: + return stable_topological_sort(all_items, dependency_graph) + except CyclicDependencyError: + warnings.warn( + 'Detected duplicate Media files in an opposite order: {}'.format( + ', '.join(repr(l) for l in lists) + ), MediaOrderConflictWarning, + ) + return list(all_items) def __add__(self, other): combined = Media() diff --git a/tests/admin_inlines/tests.py b/tests/admin_inlines/tests.py index 0a1ab5acbb..5272d44f35 100644 --- a/tests/admin_inlines/tests.py +++ b/tests/admin_inlines/tests.py @@ -497,10 +497,10 @@ class TestInlineMedia(TestDataMixin, TestCase): response.context['inline_admin_formsets'][0].media._js, [ 'admin/js/vendor/jquery/jquery.min.js', - 'admin/js/jquery.init.js', - 'admin/js/inlines.min.js', 'my_awesome_inline_scripts.js', 'custom_number.js', + 'admin/js/jquery.init.js', + 'admin/js/inlines.min.js', ] ) self.assertContains(response, 'my_awesome_inline_scripts.js') diff --git a/tests/admin_widgets/test_autocomplete_widget.py b/tests/admin_widgets/test_autocomplete_widget.py index cd9d58e347..601929fd37 100644 --- a/tests/admin_widgets/test_autocomplete_widget.py +++ b/tests/admin_widgets/test_autocomplete_widget.py @@ -139,4 +139,4 @@ class AutocompleteMixinTests(TestCase): else: expected_files = base_files with translation.override(lang): - self.assertEqual(AutocompleteSelect(rel, admin.site).media._js, expected_files) + self.assertEqual(AutocompleteSelect(rel, admin.site).media._js, list(expected_files)) diff --git a/tests/forms_tests/tests/test_media.py b/tests/forms_tests/tests/test_media.py index 8cb484a15e..192d78f331 100644 --- a/tests/forms_tests/tests/test_media.py +++ b/tests/forms_tests/tests/test_media.py @@ -25,8 +25,8 @@ class FormsMediaTestCase(SimpleTestCase): ) self.assertEqual( repr(m), - "Media(css={'all': ('path/to/css1', '/path/to/css2')}, " - "js=('/path/to/js1', 'http://media.other.com/path/to/js2', 'https://secure.other.com/path/to/js3'))" + "Media(css={'all': ['path/to/css1', '/path/to/css2']}, " + "js=['/path/to/js1', 'http://media.other.com/path/to/js2', 'https://secure.other.com/path/to/js3'])" ) class Foo: @@ -125,8 +125,8 @@ class FormsMediaTestCase(SimpleTestCase): - -""" + +""" ) # media addition hasn't affected the original objects @@ -151,6 +151,17 @@ class FormsMediaTestCase(SimpleTestCase): self.assertEqual(str(w4.media), """ """) + def test_media_deduplication(self): + # A deduplication test applied directly to a Media object, to confirm + # that the deduplication doesn't only happen at the point of merging + # two or more media objects. + media = Media( + css={'all': ('/path/to/css1', '/path/to/css1')}, + js=('/path/to/js1', '/path/to/js1'), + ) + self.assertEqual(str(media), """ +""") + def test_media_property(self): ############################################################### # Property-based media definitions @@ -197,12 +208,12 @@ class FormsMediaTestCase(SimpleTestCase): self.assertEqual( str(w6.media), """ - + + - -""" +""" ) def test_media_inheritance(self): @@ -247,8 +258,8 @@ class FormsMediaTestCase(SimpleTestCase): - -""" + +""" ) def test_media_inheritance_from_property(self): @@ -322,8 +333,8 @@ class FormsMediaTestCase(SimpleTestCase): - -""" + +""" ) def test_media_inheritance_single_type(self): @@ -420,8 +431,8 @@ class FormsMediaTestCase(SimpleTestCase): - -""" + +""" ) def test_form_media(self): @@ -462,8 +473,8 @@ class FormsMediaTestCase(SimpleTestCase): - -""" + +""" ) # Form media can be combined to produce a single media definition. @@ -477,8 +488,8 @@ class FormsMediaTestCase(SimpleTestCase): - -""" + +""" ) # Forms can also define media, following the same rules as widgets. @@ -495,28 +506,28 @@ class FormsMediaTestCase(SimpleTestCase): self.assertEqual( str(f3.media), """ + - + - -""" +""" ) # Media works in templates self.assertEqual( Template("{{ form.media.js }}{{ form.media.css }}").render(Context({'form': f3})), """ + - -""" +""" """ + - -""" +""" ) def test_html_safe(self): @@ -526,19 +537,23 @@ class FormsMediaTestCase(SimpleTestCase): def test_merge(self): test_values = ( - (([1, 2], [3, 4]), [1, 2, 3, 4]), + (([1, 2], [3, 4]), [1, 3, 2, 4]), (([1, 2], [2, 3]), [1, 2, 3]), (([2, 3], [1, 2]), [1, 2, 3]), (([1, 3], [2, 3]), [1, 2, 3]), (([1, 2], [1, 3]), [1, 2, 3]), (([1, 2], [3, 2]), [1, 3, 2]), + (([1, 2], [1, 2]), [1, 2]), + ([[1, 2], [1, 3], [2, 3], [5, 7], [5, 6], [6, 7, 9], [8, 9]], [1, 5, 8, 2, 6, 3, 7, 9]), + ((), []), + (([1, 2],), [1, 2]), ) - for (list1, list2), expected in test_values: - with self.subTest(list1=list1, list2=list2): - self.assertEqual(Media.merge(list1, list2), expected) + for lists, expected in test_values: + with self.subTest(lists=lists): + self.assertEqual(Media.merge(*lists), expected) def test_merge_warning(self): - msg = 'Detected duplicate Media files in an opposite order:\n1\n2' + msg = 'Detected duplicate Media files in an opposite order: [1, 2], [2, 1]' with self.assertWarnsMessage(RuntimeWarning, msg): self.assertEqual(Media.merge([1, 2], [2, 1]), [1, 2]) @@ -546,28 +561,30 @@ class FormsMediaTestCase(SimpleTestCase): """ The relative order of scripts is preserved in a three-way merge. """ - # custom_widget.js doesn't depend on jquery.js. - widget1 = Media(js=['custom_widget.js']) - widget2 = Media(js=['jquery.js', 'uses_jquery.js']) - form_media = widget1 + widget2 - # The relative ordering of custom_widget.js and jquery.js has been - # established (but without a real need to). - self.assertEqual(form_media._js, ['custom_widget.js', 'jquery.js', 'uses_jquery.js']) - # The inline also uses custom_widget.js. This time, it's at the end. - inline_media = Media(js=['jquery.js', 'also_jquery.js']) + Media(js=['custom_widget.js']) - merged = form_media + inline_media - self.assertEqual(merged._js, ['custom_widget.js', 'jquery.js', 'uses_jquery.js', 'also_jquery.js']) + widget1 = Media(js=['color-picker.js']) + widget2 = Media(js=['text-editor.js']) + widget3 = Media(js=['text-editor.js', 'text-editor-extras.js', 'color-picker.js']) + merged = widget1 + widget2 + widget3 + self.assertEqual(merged._js, ['text-editor.js', 'text-editor-extras.js', 'color-picker.js']) + + def test_merge_js_three_way2(self): + # The merge prefers to place 'c' before 'b' and 'g' before 'h' to + # preserve the original order. The preference 'c'->'b' is overridden by + # widget3's media, but 'g'->'h' survives in the final ordering. + widget1 = Media(js=['a', 'c', 'f', 'g', 'k']) + widget2 = Media(js=['a', 'b', 'f', 'h', 'k']) + widget3 = Media(js=['b', 'c', 'f', 'k']) + merged = widget1 + widget2 + widget3 + self.assertEqual(merged._js, ['a', 'b', 'c', 'f', 'g', 'h', 'k']) def test_merge_css_three_way(self): - widget1 = Media(css={'screen': ['a.css']}) - widget2 = Media(css={'screen': ['b.css']}) - widget3 = Media(css={'all': ['c.css']}) - form1 = widget1 + widget2 - form2 = widget2 + widget1 - # form1 and form2 have a.css and b.css in different order... - self.assertEqual(form1._css, {'screen': ['a.css', 'b.css']}) - self.assertEqual(form2._css, {'screen': ['b.css', 'a.css']}) - # ...but merging succeeds as the relative ordering of a.css and b.css - # was never specified. - merged = widget3 + form1 + form2 - self.assertEqual(merged._css, {'screen': ['a.css', 'b.css'], 'all': ['c.css']}) + widget1 = Media(css={'screen': ['c.css'], 'all': ['d.css', 'e.css']}) + widget2 = Media(css={'screen': ['a.css']}) + widget3 = Media(css={'screen': ['a.css', 'b.css', 'c.css'], 'all': ['e.css']}) + merged = widget1 + widget2 + # c.css comes before a.css because widget1 + widget2 establishes this + # order. + self.assertEqual(merged._css, {'screen': ['c.css', 'a.css'], 'all': ['d.css', 'e.css']}) + merged = merged + widget3 + # widget3 contains an explicit ordering of c.css and a.css. + self.assertEqual(merged._css, {'screen': ['a.css', 'b.css', 'c.css'], 'all': ['d.css', 'e.css']})