Merge lp:~cjwatson/launchpad/improve-savane-url-parsing into lp:launchpad

[Colin Watson]

Parse a few more possible Savane URL formats.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/bugs/model/bugwatch.py'
--- lib/lp/bugs/model/bugwatch.py	2016-07-04 17:11:29 +0000
+++ lib/lp/bugs/model/bugwatch.py	2018-11-11 23:14:12 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -613,12 +613,20 @@
 
     def parseSavaneURL(self, scheme, host, path, query):
         """Extract Savane base URL and bug ID."""
-        # Savane bugs URLs are in the form /bugs/?<bug-id>, so we
-        # exclude any path that isn't '/bugs/'. We also exclude query
-        # string that have a length of more or less than one, since in
-        # such cases we'd be taking a guess at the bug ID, which would
-        # probably be wrong.
-        if path != '/bugs/' or len(query) != 1:
+        # We're only interested in URLs that look like they come from a
+        # Savane bugtracker. We currently accept URL paths /bugs/ or
+        # /bugs/index.php, and accept query strings that are just the bug ID
+        # or that have an item_id parameter containing the bug ID.
+        if path not in ('/bugs/', '/bugs/index.php'):
+            return None
+        if len(query) == 1 and query.values()[0] is None:
+            # The query string is just a bare ID.
+            remote_bug = query.keys()[0]
+        elif 'item_id' in query:
+            remote_bug = query['item_id']
+        else:
+            return None
+        if not remote_bug.isdigit():
             return None
 
         # There's only one global Savannah bugtracker registered with
@@ -628,13 +636,6 @@
             urlsplit(alias)[1] for alias in savannah_tracker.aliases]
         savannah_hosts.append(urlsplit(savannah_tracker.baseurl)[1])
 
-        # The remote bug is actually a key in the query dict rather than
-        # a value, so we simply use the first and only key we come
-        # across as a best-effort guess.
-        remote_bug = query.popitem()[0]
-        if remote_bug is None or not remote_bug.isdigit():
-            return None
-
         if host in savannah_hosts:
             return savannah_tracker.baseurl, remote_bug
         else:
 
=== modified file 'lib/lp/bugs/tests/test_bugwatch.py'
--- lib/lp/bugs/tests/test_bugwatch.py	2018-01-02 10:54:31 +0000
+++ lib/lp/bugs/tests/test_bugwatch.py	2018-11-11 23:14:12 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for BugWatchSet."""
@@ -143,6 +143,15 @@
             'bug_id': '22003',
             'already_registered': True,
             }),
+        ('SavannahWithParameters', {
+            'bugtracker_type': BugTrackerType.SAVANE,
+            'bug_url': (
+                'http://savannah.gnu.org/bugs/index.php'
+                '?func=detailitem&item_id=22003'),
+            'base_url': 'http://savannah.gnu.org/',
+            'bug_id': '22003',
+            'already_registered': True,
+            }),
         ('Savane', {
             'bugtracker_type': BugTrackerType.SAVANE,
             'bug_url': 'http://savane.example.com/bugs/?12345',