launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-do-not-snapshot-refs into lp:launchpad.

Commit message:
Don't snapshot GitRepository.refs or GitRepository.branches.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1511838 in Launchpad itself: "Editing git repository with >1000 refs results in oops"
  https://bugs.launchpad.net/launchpad/+bug/1511838

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-do-not-snapshot-refs/+merge/276404

Don't snapshot GitRepository.refs or GitRepository.branches.  They may be very large.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-do-not-snapshot-refs into lp:launchpad.

=== modified file 'lib/lp/code/interfaces/gitrepository.py'
--- lib/lp/code/interfaces/gitrepository.py	2015-09-29 15:54:05 +0000
+++ lib/lp/code/interfaces/gitrepository.py	2015-11-02 15:37:27 +0000
@@ -17,6 +17,7 @@
 
 import re
 
+from lazr.lifecycle.snapshot import doNotSnapshot
 from lazr.restful.declarations import (
     call_with,
     collection_default_content,
@@ -224,17 +225,17 @@
         title=_("SSH URL"), readonly=True,
         description=_("A git+ssh:// URL for this repository.")))
 
-    refs = exported(CollectionField(
+    refs = exported(doNotSnapshot(CollectionField(
         title=_("The references present in this repository."),
         readonly=True,
         # Really IGitRef, patched in _schema_circular_imports.py.
-        value_type=Reference(Interface)))
+        value_type=Reference(Interface))))
 
-    branches = exported(CollectionField(
+    branches = exported(doNotSnapshot(CollectionField(
         title=_("The branch references present in this repository."),
         readonly=True,
         # Really IGitRef, patched in _schema_circular_imports.py.
-        value_type=Reference(Interface)))
+        value_type=Reference(Interface))))
 
     branches_by_date = Attribute(
         "The branch references present in this repository, ordered by last "

=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py	2015-10-05 17:02:57 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py	2015-11-02 15:37:27 +0000
@@ -69,6 +69,7 @@
 from lp.code.interfaces.gitrepository import (
     IGitRepository,
     IGitRepositorySet,
+    IGitRepositoryView,
     )
 from lp.code.interfaces.revision import IRevisionSet
 from lp.code.model.branchmergeproposal import BranchMergeProposal
@@ -136,6 +137,7 @@
     ZopelessDatabaseLayer,
     )
 from lp.testing.mail_helpers import pop_notifications
+from lp.testing.matchers import DoesNotSnapshot
 from lp.testing.pages import webservice_for_person
 
 
@@ -148,6 +150,12 @@
         repository = self.factory.makeGitRepository()
         verifyObject(IGitRepository, repository)
 
+    def test_avoids_large_snapshots(self):
+        large_properties = ['refs', 'branches']
+        self.assertThat(
+            self.factory.makeGitRepository(),
+            DoesNotSnapshot(large_properties, IGitRepositoryView))
+
     def test_unique_name_project(self):
         project = self.factory.makeProduct()
         repository = self.factory.makeGitRepository(target=project)