launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/bug-1062607 into lp:maas.

Commit message:
Cope with doubles slashes being removed by apache when editing an interface.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1062607 in MAAS: "Broken link in edit cluster controller page"
  https://bugs.launchpad.net/maas/+bug/1062607

For more details, see:
https://code.launchpad.net/~rvb/maas/bug-1062607/+merge/128543

This branch fixes the url machinery so that it will cope with the fact that apache transforms multiple slashes in a url into a single one.  That pattern appears in the url that maas generates when one edits an interface with an empty name.

= Notes =

Maybe one can find a better way to express the fact that the new url matchers should match an empty interface "(?P<interface>[\w\-]{0})"…?
-- 
https://code.launchpad.net/~rvb/maas/bug-1062607/+merge/128543
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/bug-1062607 into lp:maas.

=== modified file 'src/maasserver/tests/test_views_settings_clusters.py'
--- src/maasserver/tests/test_views_settings_clusters.py	2012-10-03 07:57:55 +0000
+++ src/maasserver/tests/test_views_settings_clusters.py	2012-10-08 16:34:29 +0000
@@ -29,11 +29,14 @@
     reload_object,
     )
 from maasserver.testing.factory import factory
-from maasserver.testing.testcase import (
-    AdminLoggedInTestCase,
-    )
+from maasserver.testing.testcase import AdminLoggedInTestCase
 from maastesting.matchers import ContainsAll
-from testtools.matchers import MatchesStructure
+from testtools.matchers import (
+    AllMatch,
+    Contains,
+    Equals,
+    MatchesStructure,
+    )
 
 
 class ClusterListingTest(AdminLoggedInTestCase):
@@ -142,3 +145,32 @@
         self.assertThat(
             reload_object(interface),
             MatchesStructure.byEquality(**data))
+
+
+# XXX: rvb 2012-10-08 bug=1063881: apache transforms '//' into '/' in
+# the urls it passes around and this happens when an interface has an empty
+# name.
+class ClusterInterfaceDoubleSlashBugTest(AdminLoggedInTestCase):
+
+    def test_edit_delete_empty_cluster_interface_when_slash_removed(self):
+        nodegroup = factory.make_node_group()
+        interface = factory.make_node_group_interface(
+            nodegroup=nodegroup, interface='',
+            management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED)
+        edit_link = reverse(
+            'cluster-interface-edit',
+            args=[nodegroup.uuid, interface.interface])
+        delete_link = reverse(
+            'cluster-interface-delete',
+            args=[nodegroup.uuid, interface.interface])
+        links = [edit_link, delete_link]
+        # Just make sure that the urls contains '//'.  If this is not
+        # true anymore, because we've refactored the urls, this test can
+        # problably be removed.
+        self.assertThat(links, AllMatch(Contains('//')))
+        # Simulate what apache (when used as a frontend) does to the
+        # urls.
+        new_links = [link.replace('//', '/') for link in links]
+        response_statuses = [
+            self.client.get(link).status_code for link in new_links]
+        self.assertThat(response_statuses, AllMatch(Equals(httplib.OK)))

=== modified file 'src/maasserver/urls.py'
--- src/maasserver/urls.py	2012-10-08 08:28:23 +0000
+++ src/maasserver/urls.py	2012-10-08 16:34:29 +0000
@@ -141,6 +141,17 @@
         r'^clusters/(?P<uuid>[\w\-]+)/interfaces/(?P<interface>[\w\-]*)/'
         'delete/$',
         ClusterInterfaceDelete.as_view(), name='cluster-interface-delete'),
+    # XXX: rvb 2012-10-08 bug=1063881:
+    # These two urls are only here to cope with the fact that an interface
+    # can have an empty name, thus leading to urls containing the
+    # pattern '//' that is then reduced by apache into '/'.
+    adminurl(
+        r'^clusters/(?P<uuid>[\w\-]+)/interfaces/(?P<interface>[\w\-]{0})'
+        'edit/$', ClusterInterfaceEdit.as_view()),
+    adminurl(
+        r'^clusters/(?P<uuid>[\w\-]+)/interfaces/(?P<interface>[\w\-]{0})'
+        'delete/$', ClusterInterfaceDelete.as_view()),
+    # /XXX
     adminurl(r'^settings/$', settings, name='settings'),
     adminurl(
         r'^settings/archives/add/$', settings_add_archive,