launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging ~twom/launchpad:git-branch-picker-oops-with-no-repository into launchpad:master.

Commit message:
Fix OOPS with gitref widget form validation and missing repositories

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~twom/launchpad/+git/launchpad/+merge/396678

On entering a repository that doesn't exist, the validation on gitref widget will fail to set the repository details on the vocabulary.
This seems like reasonable behaviour, given that the repository doesn't exist.

In this case, we should return no results from the vocabulary, rather than an AssertionError.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~twom/launchpad:git-branch-picker-oops-with-no-repository into launchpad:master.

diff --git a/lib/lp/code/vocabularies/gitref.py b/lib/lp/code/vocabularies/gitref.py
index 7d98208..3c2bf4a 100644
--- a/lib/lp/code/vocabularies/gitref.py
+++ b/lib/lp/code/vocabularies/gitref.py
@@ -86,11 +86,8 @@ class GitRefVocabulary(StormVocabularyBase):
         self.repository = None
         self.repository_url = repository_url
 
-    def _assertHasRepository(self):
-        if self.repository is None and self.repository_url is None:
-            raise AssertionError(
-                "GitRefVocabulary cannot be used without setting a "
-                "repository or a repository URL.")
+    def _checkHasRepository(self):
+        return not (self.repository is None and self.repository_url is None)
 
     @property
     def _order_by(self):
@@ -108,7 +105,8 @@ class GitRefVocabulary(StormVocabularyBase):
 
     def getTermByToken(self, token):
         """See `IVocabularyTokenized`."""
-        self._assertHasRepository()
+        if not self._checkHasRepository():
+            raise LookupError(token)
         if self.repository is not None:
             ref = self.repository.getRefByPath(token)
             if ref is None:
@@ -125,7 +123,8 @@ class GitRefVocabulary(StormVocabularyBase):
 
     def searchForTerms(self, query=None, vocab_filter=None):
         """See `IHugeVocabulary."""
-        self._assertHasRepository()
+        if not self._checkHasRepository():
+            return CountableIterator(0, [], self.toTerm)
         if self.repository is not None:
             pattern = self._makePattern(query=query)
             results = IStore(self._table).find(
diff --git a/lib/lp/code/vocabularies/tests/test_gitref_vocabularies.py b/lib/lp/code/vocabularies/tests/test_gitref_vocabularies.py
index 41f5f83..9edbff9 100644
--- a/lib/lp/code/vocabularies/tests/test_gitref_vocabularies.py
+++ b/lib/lp/code/vocabularies/tests/test_gitref_vocabularies.py
@@ -118,6 +118,24 @@ class TestGitRefVocabulary(TestCaseWithFactory):
             "refs/heads/next-squared", "refs/tags/next-1.0"])
         self.assertEqual(4, len(self.vocabulary_class(ref_master.repository)))
 
+    def test_no_term_with_no_repository(self):
+        # Form validation can mean that we don't always have a repository
+        # available before a widget is rendered, which needs to not error
+        vocab = self.vocabulary_class(
+            self.factory.makeSnap(branch=self.factory.makeAnyBranch()))
+        [ref] = self.factory.makeGitRefs()
+        term = SimpleTerm(ref, ref.name, ref.name)
+        self.assertRaises(
+            LookupError,
+            vocab.getTermByToken,
+            term.token)
+
+    def test_no_results_with_no_repository(self):
+        vocab = self.vocabulary_class(
+            self.factory.makeSnap(branch=self.factory.makeAnyBranch()))
+        results = vocab.searchForTerms("master")
+        self.assertEqual(results.count(), 0)
+
 
 class TestGitBranchVocabulary(TestCaseWithFactory):