Merge lp:~jtv/maas/1.2-bug-1075597 into lp:maas/1.2

[Jeroen T. Vermeulen]

Backport trunk r1334: don't include the script_name prefix in build_absolute_uri; it was already in the path it appends.

Unmodified backport from https://code.launchpad.net/~jtv/maas/bug-1075597/+merge/133328

Jeroen

[Jeroen T. Vermeulen (jtv)]

Unmodified backport; self-approving.

review: Approve

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-11-08 06:40:03 +0000
+++ src/maasserver/api.py	2012-11-08 09:34:41 +0000
@@ -1867,7 +1867,11 @@
 def describe(request):
     """Return a description of the whole MAAS API.
 
-    Returns a JSON object describing the whole MAAS API.
+    :param request: The http request for this document.  This is used to
+        derive the URL where the client expects to see the MAAS API.
+    :return: A JSON object describing the whole MAAS API.  Links to the API
+        will use the same scheme and hostname that the client used in
+        `request`.
     """
     from maasserver import urls_api as urlconf
     resources = [

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-11-08 06:40:03 +0000
+++ src/maasserver/tests/test_api.py	2012-11-08 09:34:41 +0000
@@ -38,7 +38,11 @@
 from celery.app import app_or_default
 from django.conf import settings
 from django.contrib.auth.models import AnonymousUser
-from django.core.urlresolvers import reverse
+import django.core.urlresolvers
+from django.core.urlresolvers import (
+    reverse,
+    get_script_prefix,
+    )
 from django.http import QueryDict
 from django.test.client import RequestFactory
 from fixtures import Fixture
@@ -4292,19 +4296,53 @@
     scenarios = multiply_scenarios(
         scenarios_schemes, scenarios_paths)
 
-    def test_handler_uris_are_absolute(self):
-        server = factory.make_name("server").lower()
-        extra = {
+    def make_params(self):
+        """Create parameters for http request, based on current scenario."""
+        return {
             "PATH_INFO": self.path_info,
             "SCRIPT_NAME": self.script_name,
-            "SERVER_NAME": server,
+            "SERVER_NAME": factory.make_name('server').lower(),
             "wsgi.url_scheme": self.scheme,
-            }
-        request = RequestFactory().get(
-            "/%s/describe" % factory.make_name("path"), **extra)
+        }
+
+    def get_description(self, params):
+        """GET the API description (at a random API path), as JSON."""
+        path = '/%s/describe' % factory.make_name('path')
+        request = RequestFactory().get(path, **params)
         response = describe(request)
-        self.assertEqual(httplib.OK, response.status_code, response.content)
-        description = json.loads(response.content)
+        self.assertEqual(
+            httplib.OK, response.status_code,
+            "API description failed with code %s:\n%s"
+            % (response.status_code, response.content))
+        return json.loads(response.content)
+
+    def patch_script_prefix(self, script_name):
+        """Patch up Django's and Piston's notion of the script_name prefix.
+
+        This manipulates how Piston gets Django's version of script_name
+        which it needs so that it can prefix script_name to URL paths.
+        """
+        # Patching up get_script_prefix doesn't seem to do the trick,
+        # and patching it in the right module requires unwarranted
+        # intimacy with Piston.  So just go through the proper call and
+        # set the prefix.  But clean this up after the test or it will
+        # break other tests!
+        original_prefix = get_script_prefix()
+        self.addCleanup(
+            django.core.urlresolvers.set_script_prefix, original_prefix)
+        django.core.urlresolvers.set_script_prefix(script_name)
+
+    def test_handler_uris_are_absolute(self):
+        params = self.make_params()
+        server = params['SERVER_NAME']
+
+        # Without this, the test wouldn't be able to detect accidental
+        # duplication of the script_name portion of the URL path:
+        # /MAAS/MAAS/api/...
+        self.patch_script_prefix(self.script_name)
+
+        description = self.get_description(params)
+
         expected_uri = AfterPreprocessing(
             urlparse, MatchesStructure(
                 scheme=Equals(self.scheme), hostname=Equals(server),

=== modified file 'src/maasserver/utils/__init__.py'
--- src/maasserver/utils/__init__.py	2012-11-02 09:48:02 +0000
+++ src/maasserver/utils/__init__.py	2012-11-08 09:34:41 +0000
@@ -87,20 +87,17 @@
 
 
 def build_absolute_uri(request, path):
-    """Given a full app-relative path, returns an absolute URI.
-
-    The path ordinarily starts with a forward-slash... imagine that you've
-    magically chroot'ed to your Django application; the path is the absolute
-    path to the view within that environment.
-
-    The URI returned uses the request to figure out how to make an absolute
-    URL. This means that the URI returned will use the same IP address or
-    alias that the request came in on.
+    """Return absolute URI corresponding to given absolute path.
+
+    :param request: An http request to the API.  This is needed in order to
+        figure out how the client is used to addressing
+        the API on the network.
+    :param path: The absolute http path to a given resource.
+    :return: Full, absolute URI to the resource, taking its networking
+        portion from `request` but the rest from `path`.
     """
-    script_name = request.META["SCRIPT_NAME"]
-    return "%s://%s%s%s" % (
-        "https" if request.is_secure() else "http",
-        request.get_host(), script_name, path)
+    scheme = "https" if request.is_secure() else "http"
+    return "%s://%s%s" % (scheme, request.get_host(), path)
 
 
 def strip_domain(hostname):

=== modified file 'src/maasserver/utils/tests/test_utils.py'
--- src/maasserver/utils/tests/test_utils.py	2012-11-02 09:48:02 +0000
+++ src/maasserver/utils/tests/test_utils.py	2012-11-08 09:34:41 +0000
@@ -134,12 +134,13 @@
             "http://example.com:1234/fred",
             build_absolute_uri(request, "/fred"))
 
-    def test_script_name_is_prefixed(self):
-        # The script name is always prefixed to the given path.
+    def test_script_name_is_ignored(self):
+        # The given path already includes the script_name, so the
+        # script_name passed in the request is not included again.
         request = self.make_request(script_name="/foo/bar")
         self.assertEqual(
             "http://example.com/foo/bar/fred",
-            build_absolute_uri(request, "/fred"))
+            build_absolute_uri(request, "/foo/bar/fred"))
 
     def test_secure(self):
         request = self.make_request(port=443, is_secure=True)
@@ -153,21 +154,13 @@
             "https://example.com:9443/fred",
             build_absolute_uri(request, "/fred"))
 
-    def test_no_leading_forward_slash(self):
-        # No attempt is made to ensure that the given path is separated from
-        # the to-be-prefixed path.
-        request = self.make_request(script_name="/foo")
-        self.assertEqual(
-            "http://example.com/foobar",
-            build_absolute_uri(request, "bar"))
-
     def test_preserve_two_leading_slashes(self):
         # Whilst this shouldn't ordinarily happen, two leading slashes in the
         # path should be preserved, and not treated specially.
-        request = self.make_request(script_name="//foo")
+        request = self.make_request()
         self.assertEqual(
-            "http://example.com//foo/fred",
-            build_absolute_uri(request, "/fred"))
+            "http://example.com//foo",
+            build_absolute_uri(request, "//foo"))
 
 
 class TestStripDomain(TestCase):