From bc9bed573ce39c3b739a4a3b7848816d464b6bdc Mon Sep 17 00:00:00 2001 From: Johannes Maron Date: Thu, 7 May 2026 16:32:59 +0200 Subject: [PATCH] Fixed #37088 -- Included attributes in media object equality. Since in the majority of cases the `MediaAsset.attributes` will be empty or small, there's only a tiny performance penalty. However, the accidental use of the `path` property caused a 1_000x performacne degredation (N=1_000_000). --- django/forms/widgets.py | 47 +++++++++++++++++---------- tests/forms_tests/tests/test_media.py | 22 +++++++++---- 2 files changed, 44 insertions(+), 25 deletions(-) diff --git a/django/forms/widgets.py b/django/forms/widgets.py index 7e5ddddfcfcc..f401896b60e1 100644 --- a/django/forms/widgets.py +++ b/django/forms/widgets.py @@ -72,14 +72,19 @@ def __init__(self, path, **attributes): self.attributes = attributes def __eq__(self, other): - # Compare the path only, to ensure performant comparison in - # Media.merge. - return (self.__class__ is other.__class__ and self.path == other.path) or ( - isinstance(other, str) and self._path == other - ) + # Compare path and attrs to ensure performant comparison + # in Media.merge. + return ( + self.__class__ is other.__class__ + and self._path == other._path + and self.attributes == other.attributes + ) or (isinstance(other, str) and self._path == other) def __hash__(self): - # Hash the path only, to ensure performant comparison in Media.merge. + # Compare path and attrs to ensure performant comparison + # in Media.merge. + if self.attributes: + return hash(self._path) ^ hash(frozenset(self.attributes.items())) return hash(self._path) def __str__(self): @@ -142,8 +147,22 @@ def __init__(self, media=None, css=None, js=None): css = {} if js is None: js = [] - self._css_lists = [css] - self._js_lists = [js] + self._css_lists = [self._normalize_css(css)] + self._js_lists = [self._normalize_js(js)] + + @staticmethod + def _normalize_js(js): + return [Script(path) if isinstance(path, str) else path for path in js] + + @staticmethod + def _normalize_css(css): + return { + medium: [ + Stylesheet(path, media=medium) if isinstance(path, str) else path + for path in paths + ] + for medium, paths in css.items() + } def __repr__(self): return "Media(css=%r, js=%r)" % (self._css, self._js) @@ -177,11 +196,7 @@ def render_js(self, *, attrs=None): ( path.render(attrs=attrs) if isinstance(path, MediaAsset) - else ( - path.__html__() - if hasattr(path, "__html__") - else Script(path).render(attrs=attrs) - ) + else path.__html__() ) for path in self._js ] @@ -195,11 +210,7 @@ def render_css(self, *, attrs=None): ( path.render(attrs=attrs) if isinstance(path, MediaAsset) - else ( - path.__html__() - if hasattr(path, "__html__") - else Stylesheet(path, media=medium).render(attrs=attrs) - ) + else path.__html__() ) for path in self._css[medium] ] diff --git a/tests/forms_tests/tests/test_media.py b/tests/forms_tests/tests/test_media.py index 1ddf10551c47..b24ed948a955 100644 --- a/tests/forms_tests/tests/test_media.py +++ b/tests/forms_tests/tests/test_media.py @@ -17,14 +17,14 @@ def test_init(self): def test_eq(self): self.assertEqual(MediaAsset("path/to/css"), MediaAsset("path/to/css")) self.assertEqual(MediaAsset("path/to/css"), "path/to/css") - self.assertEqual( + self.assertNotEqual( MediaAsset("path/to/css", media="all"), MediaAsset("path/to/css") ) self.assertNotEqual(MediaAsset("path/to/css"), MediaAsset("path/to/other.css")) self.assertNotEqual(MediaAsset("path/to/css"), "path/to/other.css") self.assertNotEqual( - MediaAsset("path/to/css", media="all"), Stylesheet("path/to/css") + MediaAsset("path/to/css", rel="stylesheet"), Stylesheet("path/to/css") ) def test_hash(self): @@ -32,6 +32,10 @@ def test_hash(self): self.assertEqual( hash(MediaAsset("path/to/css")), hash(MediaAsset("path/to/css")) ) + self.assertNotEqual( + hash(MediaAsset("path/to/css", rel="stylesheet")), + hash(MediaAsset("path/to/css")), + ) def test_str(self): self.assertEqual( @@ -171,9 +175,10 @@ def test_construction(self): ) 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': [Stylesheet('path/to/css1')," + " Stylesheet('/path/to/css2')]}, js=[Script('/path/to/js1')," + " Script('http://media.other.com/path/to/js2')," + " Script('https://secure.other.com/path/to/js3')])", ) class Foo: @@ -807,7 +812,8 @@ def test_add_js_deduplication(self): self.assertEqual(merged._js_lists, [["a", "b", "c"], ["a", "c", "b"]]) msg = ( "Detected duplicate Media files in an opposite order: " - "['a', 'b', 'c'], ['a', 'c', 'b']" + "[Script('a'), Script('b'), Script('c')]," + " [Script('a'), Script('c'), Script('b')]" ) with self.assertWarnsMessage(RuntimeWarning, msg): merged._js @@ -840,7 +846,8 @@ def test_add_css_deduplication(self): ) msg = ( "Detected duplicate Media files in an opposite order: " - "['b.css', 'c.css'], ['c.css', 'b.css']" + "[Stylesheet('b.css'), Stylesheet('c.css')]," + " [Stylesheet('c.css'), Stylesheet('b.css')]" ) with self.assertWarnsMessage(RuntimeWarning, msg): merged._css @@ -977,6 +984,7 @@ class Media: '\n' '\n' '\n' + '\n' '\n' '",