launchpad-reviewers team mailing list archive

[Colin Watson <mp+422618@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:gitref-deep-recursion into launchpad:master.

Commit message:
Refactor GitRef.findByReposAndPaths to avoid deep recursion

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1973480 in Launchpad itself: "Repository scan failed (Git)"
  https://bugs.launchpad.net/launchpad/+bug/1973480

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

The previous code could cause us to run out of stack space if many refs were involved.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:gitref-deep-recursion into launchpad:master.

diff --git a/lib/lp/code/model/gitref.py b/lib/lp/code/model/gitref.py
index c49c94a..335881f 100644
--- a/lib/lp/code/model/gitref.py
+++ b/lib/lp/code/model/gitref.py
@@ -20,7 +20,10 @@ from lazr.lifecycle.event import ObjectCreatedEvent
 import pytz
 import requests
 import six
-from storm.expr import And
+from storm.expr import (
+    And,
+    Or,
+    )
 from storm.locals import (
     DateTime,
     Int,
@@ -643,15 +646,16 @@ class GitRef(GitRefMixin, StormBase):
                 return path
             return "refs/heads/%s" % path
 
-        condition = None
+        clauses = []
         for repo, path in repos_and_paths:
-            clause = And(
+            clauses.append(And(
                 GitRef.path.is_in([path, full_path(path)]),
-                (GitRef.repository_id == repo.id))
-            condition = clause if condition is None else condition | clause
-        result = {}
+                (GitRef.repository_id == repo.id)))
+        if not clauses:
+            return {}
 
-        for row in IStore(GitRef).find(GitRef, condition):
+        result = {}
+        for row in IStore(GitRef).find(GitRef, Or(*clauses)):
             result[(row.repository_id, row.path)] = row
 
         return_value = {}