launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #25488
[Merge] ~cjwatson/launchpad:bugs-api-redirects into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:bugs-api-redirects into launchpad:master.
Commit message:
Send proper webservice redirects for bug attachments
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/392136
launchpadlib handles this correctly nowadays, but only if we send redirects to the correct target host (e.g. api.launchpad.net rather than bugs.launchpad.net).
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:bugs-api-redirects into launchpad:master.
diff --git a/lib/lp/app/stories/basics/notfound-traversals.txt b/lib/lp/app/stories/basics/notfound-traversals.txt
index b9676f8..4a6cc78 100644
--- a/lib/lp/app/stories/basics/notfound-traversals.txt
+++ b/lib/lp/app/stories/basics/notfound-traversals.txt
@@ -8,8 +8,9 @@ HTTP status is returned.
... output = http("GET %s HTTP/1.1\nHost: %s" % (url, host))
... status = output.getStatus()
... if status != 404:
- ... return "%s returned status %s instead of 404\n\n%s" % (
- ... url, status, str(output))
+ ... raise Exception(
+ ... "%s returned status %s instead of 404\n\n%s" % (
+ ... url, status, str(output)))
>>> def check_redirect(url, auth=False, host='launchpad.test', status=303):
... get_cmd = """
@@ -19,18 +20,26 @@ HTTP status is returned.
... if auth:
... get_cmd += ("Authorization: "
... "Basic Zm9vLmJhckBjYW5vbmljYWwuY29tOnRlc3Q=\n")
- ... rc = http(get_cmd % (url, host)).getStatus()
+ ... response = http(get_cmd % (url, host))
+ ... rc = response.getStatus()
... if rc != status:
- ... return ("%s returned status %s instead of %d" %
- ... (url, rc, status))
+ ... raise Exception(
+ ... "%s returned status %s instead of %d" % (url, rc, status))
+ ... print(response.getHeader("Location"))
>>> check_redirect("/legal", status=301)
+ https://help.launchpad.net/Legal
>>> check_redirect("/faq", status=301)
+ https://answers.launchpad.net/launchpad-project/+faqs
>>> check_redirect("/feedback", status=301)
+ https://help.launchpad.net/Feedback
>>> check_redirect("/support/", status=301)
+ http://answers.launchpad.test/launchpad
>>> check_redirect("/", host='feeds.launchpad.test', status=301)
+ https://help.launchpad.net/Feeds
>>> check_redirect("/+index", host='feeds.launchpad.test', status=301)
+ https://help.launchpad.net/Feeds
The +translate page in the main host is obsolete so it's now a redirect
to the translations site. This way, we don't break existing links to it.
@@ -38,18 +47,26 @@ Before removing this, you must be completely sure that no supported
Ubuntu release is still pointing to this old URL (see bug #138090).
>>> check_redirect("/products", status=301)
+ http://launchpad.test/projects
>>> check_redirect("/projects/firefox", status=301)
+ http://launchpad.test/firefox
>>> check_redirect("/ubuntu/+source/evolution/+editbugcontact")
+ +subscribe
>>> check_redirect("/ubuntu/hoary/+latest-full-language-pack")
+ http://localhost:.../ubuntu-hoary-translations.tar.gz
>>> check_redirect("/ubuntu/hoary/+source/mozilla-firefox/+pots")
+ http://launchpad.test/.../+pots/../+translations
Viewing a bug in the context of an upstream where the bug has already
been reported (including checking the various pages that hang off that
one.)
- >>> check_redirect("/bugs/assigned")
+ >>> check_redirect("/bugs/assigned", auth=True)
+ http://launchpad.test/~name16/+assignedbugs
>>> check_redirect("/bugs/1")
+ http://bugs.launchpad.test/firefox/+bug/1
>>> check_redirect("/firefox/+bug")
+ +bugs
Bug attachments in the context of a bugtask are all redirected to be at
+attachment/<id>. The old attachments/<id> form is deprecated.
@@ -60,15 +77,27 @@ Bug attachments in the context of a bugtask are all redirected to be at
>>> logout()
>>> check_redirect("/firefox/+bug/1/attachments/%d" % atid, status=301)
+ http://bugs.launchpad.test/firefox/+bug/1/+attachment/1
+ >>> check_redirect(
+ ... "/devel/firefox/+bug/1/attachments/%d" % atid,
+ ... host="api.launchpad.test", status=301)
+ http://api.launchpad.test/devel/firefox/+bug/1/+attachment/1
>>> check_redirect(
... "/firefox/+bug/1/attachments/%d/+edit" % atid, status=301)
+ http://bugs.launchpad.test/firefox/+bug/1/+attachment/1/+edit
>>> check_redirect("/bugs/1/attachments/%d" % atid, status=301)
+ http://bugs.launchpad.test/bugs/1/+attachment/1
+ >>> check_redirect(
+ ... "/devel/bugs/1/attachments/%d" % atid,
+ ... host="api.launchpad.test", status=301)
+ http://api.launchpad.test/devel/bugs/1/+attachment/1
>>> check_redirect("/bugs/1/attachments/%d/+edit" % atid, status=301)
+ http://bugs.launchpad.test/bugs/1/+attachment/1/+edit
Check a bug is traversable by nickname:
>>> check_redirect("/bugs/blackhole")
- >>> check_redirect("/bugs/blackhole")
+ http://bugs.launchpad.test/tomcat/+bug/2
>>> check_not_found("/bugs/invalid-nickname")
Note that you should not be able to directly file a bug on a
@@ -78,32 +107,40 @@ release. Instead, you get redirected to the appropriate distro or
distrosourcepackage filebug page.
>>> check_redirect("/ubuntu/warty/+filebug", auth=True)
+ http://launchpad.test/ubuntu/+filebug
>>> check_redirect(
... "/ubuntu/warty/+source/mozilla-firefox/+filebug", auth=True)
+ http://launchpad.test/ubuntu/+source/mozilla-firefox/+filebug
The old +filebug-advanced form now redirects to the +filebug form.
>>> check_redirect("/firefox/+filebug-advanced", auth=True, status=301)
+ http://bugs.launchpad.test/firefox/+filebug
>>> check_redirect("/ubuntu/+filebug-advanced", auth=True, status=301)
+ http://bugs.launchpad.test/ubuntu/+filebug
>>> check_redirect(
... "/ubuntu/+source/mozilla-firefox/+filebug-advanced", auth=True,
... status=301)
+ http://bugs.launchpad.test/ubuntu/+source/mozilla-firefox/+filebug
And this is for a person:
>>> check_redirect("/~name12/+branch/gnome-terminal/pushed/", status=301)
+ http://code.launchpad.test/~name12/gnome-terminal/pushed
>>> check_redirect(
... "/~name12/+branch/gnome-terminal/pushed/+edit",
... auth=True, status=301)
- >>> check_redirect("/~name12/+branch/gnome-terminal/pushed/", status=301)
- >>> check_redirect(
- ... "/~name12/+branch/gnome-terminal/pushed/+edit",
- ... auth=True, status=301)
+ http://code.launchpad.test/~name12/gnome-terminal/pushed/+edit
>>> check_redirect("/~name16/+packages", status=301)
+ http://launchpad.test/~name16/+related-packages
>>> check_redirect("/~name16/+projects", status=301)
+ http://launchpad.test/~name16/+related-projects
>>> check_redirect("/+builds", status=301)
+ /builders/
>>> check_redirect("/translations/groups/", status=301)
+ http://translations.launchpad.test/+groups
>>> check_redirect("/translations/imports/", status=301)
+ http://translations.launchpad.test/+imports
The pillar set is published through the web service, but not through the
website.
@@ -116,8 +153,11 @@ website.
Check legacy URL redirects
>>> check_redirect("/distros/ubuntu", status=301)
+ http://launchpad.test/ubuntu
>>> check_redirect("/products/ubuntu-product", status=301)
+ http://launchpad.test/projects/ubuntu-product
>>> check_redirect("/people/stub", status=301)
+ http://launchpad.test/~stub
Check redirects of Unicode URLs works
diff --git a/lib/lp/bugs/browser/bug.py b/lib/lp/bugs/browser/bug.py
index 255b9ab..61360c9 100644
--- a/lib/lp/bugs/browser/bug.py
+++ b/lib/lp/bugs/browser/bug.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""IBug related view classes."""
@@ -174,7 +174,8 @@ class BugNavigation(Navigation):
attachment = getUtility(IBugAttachmentSet)[name]
if attachment is not None and attachment.bug == self.context:
return self.redirectSubTree(
- canonical_url(attachment), status=301)
+ canonical_url(attachment, request=self.request),
+ status=301)
@stepthrough('+attachment')
def traverse_attachment(self, name):
diff --git a/lib/lp/bugs/browser/bugtask.py b/lib/lp/bugs/browser/bugtask.py
index 77e7f28..219c9e6 100644
--- a/lib/lp/bugs/browser/bugtask.py
+++ b/lib/lp/bugs/browser/bugtask.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""IBugTask-related browser views."""
@@ -370,7 +370,8 @@ class BugTaskNavigation(Navigation):
attachment = getUtility(IBugAttachmentSet)[name]
if attachment is not None and attachment.bug == self.context.bug:
return self.redirectSubTree(
- canonical_url(attachment), status=301)
+ canonical_url(attachment, request=self.request),
+ status=301)
@stepthrough('+attachment')
def traverse_attachment(self, name):
diff --git a/lib/lp/services/webservice/wadl-to-refhtml.xsl b/lib/lp/services/webservice/wadl-to-refhtml.xsl
index b3dc3e6..331f09b 100644
--- a/lib/lp/services/webservice/wadl-to-refhtml.xsl
+++ b/lib/lp/services/webservice/wadl-to-refhtml.xsl
@@ -240,7 +240,7 @@
<xsl:when test="@id = 'bug_attachment'">
<xsl:text>/bugs/</xsl:text>
<var><bug.id></var>
- <xsl:text>/attachments/</xsl:text>
+ <xsl:text>/+attachment/</xsl:text>
<var><id></var>
</xsl:when>
<xsl:when test="@id = 'bug_subscription'">
