From 8ea3f363aa136798b128f75dac0432d36bbb8aa5 Mon Sep 17 00:00:00 2001 From: Daniel Nouri Date: Tue, 17 Jan 2012 18:13:01 +0100 Subject: Fix bug with registration of identical views with same 'accept'. --- pyramid/config/views.py | 9 +++++++-- pyramid/tests/test_config/test_views.py | 29 +++++++++++++++++++++++++---- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 3cca1c7c5..d9507136f 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -513,8 +513,13 @@ class MultiView(object): self.views.sort(key=operator.itemgetter(0)) else: subset = self.media_views.setdefault(accept, []) - subset.append((order, view, phash)) - subset.sort() + for i, (s, v, h) in enumerate(list(subset)): + if phash == h and order == s: + subset[i] = (order, view, phash) + return + else: + subset.append((order, view, phash)) + subset.sort() accepts = set(self.accepts) accepts.add(accept) self.accepts = list(accepts) # dedupe diff --git a/pyramid/tests/test_config/test_views.py b/pyramid/tests/test_config/test_views.py index aa4c03db6..5ec63beab 100644 --- a/pyramid/tests/test_config/test_views.py +++ b/pyramid/tests/test_config/test_views.py @@ -621,6 +621,24 @@ class TestViewsConfigurationMixin(unittest.TestCase): request.accept = DummyAccept('text/html', 'text/html') self.assertEqual(wrapper(None, request), 'OK2') + def test_add_views_with_accept_multiview_replaces_existing(self): + from pyramid.renderers import null_renderer + def view(context, request): + return 'OK' + def view2(context, request): + return 'OK2' + def view3(context, request): + return 'OK3' + config = self._makeOne(autocommit=True) + config.add_view(view=view, renderer=null_renderer) + config.add_view(view=view2, accept='text/html', renderer=null_renderer) + config.add_view(view=view3, accept='text/html', renderer=null_renderer) + wrapper = self._getViewCallable(config) + self.assertEqual(wrapper(None, None), 'OK') + request = DummyRequest() + request.accept = DummyAccept('text/html', 'text/html') + self.assertEqual(wrapper(None, request), 'OK3') + def test_add_view_exc_with_accept_multiview_replaces_existing_view(self): from pyramid.renderers import null_renderer from zope.interface import implementedBy @@ -1963,12 +1981,15 @@ class TestMultiView(unittest.TestCase): mv.add('view4', 99, 'text/html') self.assertEqual(mv.media_views['text/html'], [(99, 'view4', None), (100, 'view3', None)]) - mv.add('view5', 100, 'text/xml') - self.assertEqual(mv.media_views['text/xml'], [(100, 'view5', None)]) + mv.add('view5', 100, 'text/html') + self.assertEqual(mv.media_views['text/html'], + [(99, 'view4', None), (100, 'view5', None)]) + mv.add('view6', 100, 'text/xml') + self.assertEqual(mv.media_views['text/xml'], [(100, 'view6', None)]) self.assertEqual(set(mv.accepts), set(['text/xml', 'text/html'])) self.assertEqual(mv.views, [(99, 'view2', None), (100, 'view', None)]) - mv.add('view6', 98, 'text/*') - self.assertEqual(mv.views, [(98, 'view6', None), + mv.add('view7', 98, 'text/*') + self.assertEqual(mv.views, [(98, 'view7', None), (99, 'view2', None), (100, 'view', None)]) -- cgit v1.2.3 From f4b7fd8db3c3c729d625f037cd48f1839332519d Mon Sep 17 00:00:00 2001 From: Daniel Nouri Date: Tue, 17 Jan 2012 21:16:53 +0100 Subject: For #404: Remove the 'order == s' part of the overwriting condition. Also, enhance tests. --- pyramid/config/views.py | 2 +- pyramid/tests/test_config/test_views.py | 24 +++++++++++++++--------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/pyramid/config/views.py b/pyramid/config/views.py index d9507136f..89a0d77c5 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -514,7 +514,7 @@ class MultiView(object): else: subset = self.media_views.setdefault(accept, []) for i, (s, v, h) in enumerate(list(subset)): - if phash == h and order == s: + if phash == h: subset[i] = (order, view, phash) return else: diff --git a/pyramid/tests/test_config/test_views.py b/pyramid/tests/test_config/test_views.py index 5ec63beab..f1fe03c25 100644 --- a/pyramid/tests/test_config/test_views.py +++ b/pyramid/tests/test_config/test_views.py @@ -634,6 +634,7 @@ class TestViewsConfigurationMixin(unittest.TestCase): config.add_view(view=view2, accept='text/html', renderer=null_renderer) config.add_view(view=view3, accept='text/html', renderer=null_renderer) wrapper = self._getViewCallable(config) + self.assertEqual(len(wrapper.media_views['text/html']), 1) self.assertEqual(wrapper(None, None), 'OK') request = DummyRequest() request.accept = DummyAccept('text/html', 'text/html') @@ -1978,18 +1979,15 @@ class TestMultiView(unittest.TestCase): self.assertEqual(mv.views, [(99, 'view2', None), (100, 'view', None)]) mv.add('view3', 100, 'text/html') self.assertEqual(mv.media_views['text/html'], [(100, 'view3', None)]) - mv.add('view4', 99, 'text/html') + mv.add('view4', 99, 'text/html', 'abc') self.assertEqual(mv.media_views['text/html'], - [(99, 'view4', None), (100, 'view3', None)]) - mv.add('view5', 100, 'text/html') - self.assertEqual(mv.media_views['text/html'], - [(99, 'view4', None), (100, 'view5', None)]) - mv.add('view6', 100, 'text/xml') - self.assertEqual(mv.media_views['text/xml'], [(100, 'view6', None)]) + [(99, 'view4', 'abc'), (100, 'view3', None)]) + mv.add('view5', 100, 'text/xml') + self.assertEqual(mv.media_views['text/xml'], [(100, 'view5', None)]) self.assertEqual(set(mv.accepts), set(['text/xml', 'text/html'])) self.assertEqual(mv.views, [(99, 'view2', None), (100, 'view', None)]) - mv.add('view7', 98, 'text/*') - self.assertEqual(mv.views, [(98, 'view7', None), + mv.add('view6', 98, 'text/*') + self.assertEqual(mv.views, [(98, 'view6', None), (99, 'view2', None), (100, 'view', None)]) @@ -2006,6 +2004,14 @@ class TestMultiView(unittest.TestCase): self.assertEqual(mv.views, [(100, 'view', 'abc'), (100, 'view', 'def')]) + def test_add_with_phash_override_accept(self): + mv = self._makeOne() + mv.add('view2', 100, accept='text/html', phash='abc') + mv.add('view3', 100, accept='text/html', phash='abc') + mv.add('view4', 99, accept='text/html', phash='def') + self.assertEqual(mv.media_views['text/html'], + [(99, 'view4', 'def'), (100, 'view3', 'abc')]) + def test_multiple_with_functions_as_views(self): # this failed on py3 at one point, because functions aren't orderable # and we were sorting the views via a plain sort() rather than -- cgit v1.2.3