launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/codeimport-git-url-per-target-rcs into lp:launchpad.

Commit message:
Allow Git-targeted code imports to share a URL with Bazaar-targeted code imports.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/codeimport-git-url-per-target-rcs/+merge/310696

I anticipate that it will be quite common for people to want to create a Git-targeted import for a repository that already has a Bazaar-targeted one, especially while switching over, and they shouldn't have to try to come up with a non-string-identical version of the same underlying network resource in order to do so.  The unique restriction on the URL is there to stop us wasting time creating several identical imports, but imports of different target types are meaningfully different.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/codeimport-git-url-per-target-rcs into lp:launchpad.

=== modified file 'lib/lp/code/browser/codeimport.py'
--- lib/lp/code/browser/codeimport.py	2016-11-03 12:23:48 +0000
+++ lib/lp/code/browser/codeimport.py	2016-11-12 02:33:42 +0000
@@ -678,7 +678,7 @@
         return (
             "You cannot create same-VCS imports for branches or repositories "
             "that are hosted by Launchpad.")
-    code_import = getUtility(ICodeImportSet).getByURL(url)
+    code_import = getUtility(ICodeImportSet).getByURL(url, target_rcs_type)
     if code_import is not None:
         if existing_import and code_import == existing_import:
             return None

=== modified file 'lib/lp/code/doc/codeimport.txt'
--- lib/lp/code/doc/codeimport.txt	2016-10-03 17:00:56 +0000
+++ lib/lp/code/doc/codeimport.txt	2016-11-12 02:33:42 +0000
@@ -159,7 +159,7 @@
 The CodeImportSet is also able to retrieve the code imports with the
 specified subversion branch url.
 
-    >>> existing_import = code_import_set.getByURL(svn_url)
+    >>> existing_import = code_import_set.getByURL(svn_url, target_bzr)
     >>> svn_import == existing_import
     True
 
@@ -187,9 +187,37 @@
 The CodeImportSet is also able to retrieve the code imports with the
 specified git repo url.
 
-    >>> existing_import = code_import_set.getByURL(git_url)
-    >>> git_import == existing_import
-    True
+    >>> existing_import = code_import_set.getByURL(git_url, target_bzr)
+    >>> git_import == existing_import
+    True
+
+Code imports from Git may target Git rather than Bazaar.
+
+    >>> from lp.code.tests.helpers import GitHostingFixture
+
+    >>> target_git = TargetRevisionControlSystems.GIT
+    >>> with GitHostingFixture():
+    ...     git_to_git_import = code_import_set.new(
+    ...         registrant=nopriv, context=context, branch_name=u'hello',
+    ...         rcs_type=git, url=git_url, target_rcs_type=target_git)
+    >>> verifyObject(ICodeImport, removeSecurityProxy(git_to_git_import))
+    True
+
+    >>> git_to_git_events = event_set.getEventsForCodeImport(
+    ...     git_to_git_import)
+    >>> [event.event_type.name for event in git_to_git_events]
+    ['CREATE']
+
+    >>> existing_import = code_import_set.getByURL(git_url, target_git)
+    >>> git_to_git_import == existing_import
+    True
+
+The previous Git-to-Bazaar import of the same URL is unaffected.
+
+    >>> existing_import = code_import_set.getByURL(git_url, target_bzr)
+    >>> git_import == existing_import
+    True
+
 
 Updating code import details
 ----------------------------

=== modified file 'lib/lp/code/interfaces/codeimport.py'
--- lib/lp/code/interfaces/codeimport.py	2016-10-13 15:52:36 +0000
+++ lib/lp/code/interfaces/codeimport.py	2016-11-12 02:33:42 +0000
@@ -265,8 +265,8 @@
     def getByCVSDetails(cvs_root, cvs_module):
         """Get the CodeImport with the specified CVS details."""
 
-    def getByURL(url):
-        """Get the CodeImport with the url."""
+    def getByURL(url, target_rcs_type):
+        """Get the CodeImport with the URL and target RCS type."""
 
     def delete(id):
         """Delete a CodeImport given its id."""

=== modified file 'lib/lp/code/model/codeimport.py'
--- lib/lp/code/model/codeimport.py	2016-10-15 02:20:53 +0000
+++ lib/lp/code/model/codeimport.py	2016-11-12 02:33:42 +0000
@@ -364,9 +364,14 @@
         return CodeImport.selectOneBy(
             cvs_root=cvs_root, cvs_module=cvs_module)
 
-    def getByURL(self, url):
+    def getByURL(self, url, target_rcs_type):
         """See `ICodeImportSet`."""
-        return CodeImport.selectOneBy(url=url)
+        clauses = [CodeImport.url == url]
+        if target_rcs_type == TargetRevisionControlSystems.BZR:
+            clauses.append(CodeImport.branch != None)
+        else:
+            clauses.append(CodeImport.git_repository != None)
+        return IStore(CodeImport).find(CodeImport, *clauses).one()
 
     def getByBranch(self, branch):
         """See `ICodeImportSet`."""

=== modified file 'lib/lp/code/stories/codeimport/xx-create-codeimport.txt'
--- lib/lp/code/stories/codeimport/xx-create-codeimport.txt	2016-10-14 17:25:51 +0000
+++ lib/lp/code/stories/codeimport/xx-create-codeimport.txt	2016-11-12 02:33:42 +0000
@@ -207,7 +207,8 @@
 ==============================
 
 The user is required to enter a project that the import is for,
-a name for the import repository, and a Git repository location.
+a name for the import repository, and a Git repository location.  The URL is
+allowed to match that of an existing Bazaar-targeted import.
 
     >>> from lp.code.interfaces.codeimport import (
     ...     CODE_IMPORT_GIT_TARGET_FEATURE_FLAG,
@@ -222,7 +223,7 @@
     ...     browser.getControl('Git', index=0).click()
     ...     browser.getControl('Git', index=1).click()
     ...     browser.getControl('Repo URL', index=0).value = (
-    ...         "git://example.com/firefox2.git")
+    ...         "git://example.com/firefox.git")
     ...     with GitHostingFixture():
     ...         browser.getControl('Request Import').click()
 
@@ -231,7 +232,7 @@
     >>> print extract_text(find_tag_by_id(browser.contents, "import-details"))
     Import Status: Reviewed
     This repository is an import of the Git repository at
-    git://example.com/firefox2.git.
+    git://example.com/firefox.git.
     The next import is scheduled to run as soon as possible.