launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #24452
[Merge] ~cjwatson/launchpad:date-ordered-repositories into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:date-ordered-repositories into launchpad:master.
Commit message:
Add API to get branches/repositories ordered by modification date
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/380391
This is useful to allow external source-code-archiving services to do so efficiently.
For the most part, I've tried to ensure that the API looks similar between branches and git_repositories; the exception is that branches.getBranches doesn't currently take a target, since IHasBranches isn't exported as a webservice entry type and since IHasBranches.getBranches also exists.
Beware that the modification date is (of course) mutable, so client code that tries to iterate over successive batches of the returned collection may currently accidentally miss items between batches if branches/repositories are modified between iterations. It ought to be possible for us to fix this by using StormRangeFactory, but in the meantime, clients can work around this by calling getBranches/getRepositories again with an appropriate modified_since_date rather than traversing to the next batch of the collection in the normal way.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:date-ordered-repositories into launchpad:master.
diff --git a/lib/lp/code/interfaces/branch.py b/lib/lp/code/interfaces/branch.py
index f0e9393..ccb4084 100644
--- a/lib/lp/code/interfaces/branch.py
+++ b/lib/lp/code/interfaces/branch.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Branch interfaces."""
@@ -1427,10 +1427,36 @@ class IBranchSet(Interface):
Return None if no match was found.
"""
- @collection_default_content()
- def getBranches(limit=50, eager_load=True):
+ @call_with(user=REQUEST_USER)
+ @operation_parameters(
+ order_by_modified_date=Bool(
+ title=_("Order by modified date"),
+ # order_by_modified_date=False currently returns branches in the
+ # same order as True (descending date_last_modified followed by
+ # descending ID), but we don't want to commit to this.
+ description=_(
+ "Return most-recently-modified branches first. If False, "
+ "the order of results is unspecified."),
+ required=False),
+ modified_since_date=Datetime(
+ title=_("Modified since date"),
+ description=_(
+ "Return only branches whose `date_last_modified` is "
+ "greater than or equal to this date.")))
+ @operation_returns_collection_of(IBranch)
+ @export_read_operation()
+ @operation_for_version("devel")
+ @collection_default_content(user=None, limit=50)
+ def getBranches(user, order_by_modified_date=False,
+ modified_since_date=None, limit=None, eager_load=True):
"""Return a collection of branches.
+ :param user: An `IPerson`. Only branches visible by this user will
+ be returned.
+ :param order_by_modified_date: If True, order results by descending
+ `date_last_modified`; if False, the order is unspecified.
+ :param modified_since_date: If not None, return only branches whose
+ `date_last_modified` is greater than this date.
:param eager_load: If True (the default because this is used in the
web service and it needs the related objects to create links)
eager load related objects (products, code imports etc).
diff --git a/lib/lp/code/interfaces/gitcollection.py b/lib/lp/code/interfaces/gitcollection.py
index 90ab25e..406be51 100644
--- a/lib/lp/code/interfaces/gitcollection.py
+++ b/lib/lp/code/interfaces/gitcollection.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""A collection of Git repositories.
@@ -151,6 +151,9 @@ class IGitCollection(Interface):
"""Restrict the collection to repositories owned by exclusive
people."""
+ def modifiedSince(date):
+ """Restrict the collection to repositories modified since `date`."""
+
def ownedBy(person):
"""Restrict the collection to repositories owned by 'person'."""
diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py
index eeac433..124ebaa 100644
--- a/lib/lp/code/interfaces/gitrepository.py
+++ b/lib/lp/code/interfaces/gitrepository.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2019 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Git repository interfaces."""
@@ -989,16 +989,35 @@ class IGitRepositorySet(Interface):
@call_with(user=REQUEST_USER)
@operation_parameters(
target=Reference(
- title=_("Target"), required=True, schema=IHasGitRepositories))
+ title=_("Target"), required=False, schema=IHasGitRepositories),
+ order_by_modified_date=Bool(
+ title=_("Order by modified date"),
+ # order_by_modified_date=False currently returns repositories
+ # ordered by ID, but we don't want to commit to this.
+ description=_(
+ "Return most-recently-modified repositories first. If False, "
+ "the order of results is unspecified."),
+ required=False),
+ modified_since_date=Datetime(
+ title=_("Modified since date"),
+ description=_(
+ "Return only repositories whose `date_last_modified` is "
+ "greater than or equal to this date.")))
@operation_returns_collection_of(IGitRepository)
@export_read_operation()
@operation_for_version("devel")
- def getRepositories(user, target):
+ def getRepositories(user, target=None, order_by_modified_date=False,
+ modified_since_date=None):
"""Get all repositories for a target.
:param user: An `IPerson`. Only repositories visible by this user
will be returned.
- :param target: An `IHasGitRepositories`.
+ :param target: An `IHasGitRepositories`, or None to get repositories
+ for all targets.
+ :param order_by_modified_date: If True, order results by descending
+ `date_last_modified`; if False, the order is unspecified.
+ :param modified_since_date: If not None, return only repositories
+ whose `date_last_modified` is greater than this date.
:return: A collection of `IGitRepository` objects.
"""
diff --git a/lib/lp/code/model/branch.py b/lib/lp/code/model/branch.py
index c95247a..e322649 100644
--- a/lib/lp/code/model/branch.py
+++ b/lib/lp/code/model/branch.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -110,7 +110,10 @@ from lp.code.interfaces.branch import (
user_has_special_branch_access,
WrongNumberOfReviewTypeArguments,
)
-from lp.code.interfaces.branchcollection import IAllBranches
+from lp.code.interfaces.branchcollection import (
+ IAllBranches,
+ IBranchCollection,
+ )
from lp.code.interfaces.branchhosting import IBranchHostingClient
from lp.code.interfaces.branchlookup import IBranchLookup
from lp.code.interfaces.branchmergeproposal import (
@@ -1712,13 +1715,21 @@ class BranchSet:
"""See `IBranchSet`."""
return getUtility(IBranchLookup).getByPath(path)
- def getBranches(self, limit=50, eager_load=True):
+ def getBranches(self, user, target=None, order_by_modified_date=False,
+ modified_since_date=None, limit=None, eager_load=True):
"""See `IBranchSet`."""
- anon_branches = getUtility(IAllBranches).visibleByUser(None)
- branches = anon_branches.scanned().getBranches(eager_load=eager_load)
+ if target is not None:
+ collection = IBranchCollection(target)
+ else:
+ collection = getUtility(IAllBranches)
+ collection = collection.visibleByUser(user)
+ if modified_since_date is not None:
+ collection = collection.modifiedSince(modified_since_date)
+ branches = collection.scanned().getBranches(eager_load=eager_load)
branches.order_by(
Desc(Branch.date_last_modified), Desc(Branch.id))
- branches.config(limit=limit)
+ if limit is not None:
+ branches.config(limit=limit)
return branches
def getBranchVisibilityInfo(self, user, person, branch_names):
diff --git a/lib/lp/code/model/gitcollection.py b/lib/lp/code/model/gitcollection.py
index 552ea24..fc3778b 100644
--- a/lib/lp/code/model/gitcollection.py
+++ b/lib/lp/code/model/gitcollection.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Implementations of `IGitCollection`."""
@@ -222,7 +222,8 @@ class GenericGitCollection:
resultset = self.store.using(*tables).find(find_expr, *expressions)
assert not order_by_date or not order_by_id
if order_by_date:
- resultset.order_by(Desc(GitRepository.date_last_modified))
+ resultset.order_by(
+ Desc(GitRepository.date_last_modified), Desc(GitRepository.id))
elif order_by_id:
resultset.order_by(GitRepository.id)
@@ -489,6 +490,11 @@ class GenericGitCollection:
table=Person,
join=Join(Person, GitRepository.owner_id == Person.id))
+ def modifiedSince(self, date):
+ """See `IGitCollection`."""
+ return self._filterBy(
+ [GitRepository.date_last_modified > date], symmetric=False)
+
def ownedBy(self, person):
"""See `IGitCollection`."""
return self._filterBy([GitRepository.owner == person], symmetric=False)
diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
index 6260c16..f35025a 100644
--- a/lib/lp/code/model/gitrepository.py
+++ b/lib/lp/code/model/gitrepository.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2019 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -1701,10 +1701,22 @@ class GitRepositorySet:
return repository
return None
- def getRepositories(self, user, target):
+ def getRepositories(self, user, target=None, order_by_modified_date=False,
+ modified_since_date=None):
"""See `IGitRepositorySet`."""
- collection = IGitCollection(target).visibleByUser(user)
- return collection.getRepositories(eager_load=True, order_by_id=True)
+ if target is not None:
+ collection = IGitCollection(target)
+ else:
+ collection = getUtility(IAllGitRepositories)
+ collection = collection.visibleByUser(user)
+ if modified_since_date is not None:
+ collection = collection.modifiedSince(modified_since_date)
+ kwargs = {}
+ if order_by_modified_date:
+ kwargs["order_by_date"] = True
+ else:
+ kwargs["order_by_id"] = True
+ return collection.getRepositories(eager_load=True, **kwargs)
def getRepositoryVisibilityInfo(self, user, person, repository_names):
"""See `IGitRepositorySet`."""
diff --git a/lib/lp/code/model/tests/test_branchset.py b/lib/lp/code/model/tests/test_branchset.py
index 2ba714f..a5e4a68 100644
--- a/lib/lp/code/model/tests/test_branchset.py
+++ b/lib/lp/code/model/tests/test_branchset.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for BranchSet."""
@@ -7,11 +7,16 @@ from __future__ import absolute_import, print_function, unicode_literals
__metaclass__ = type
+from datetime import datetime
+
+import pytz
from testtools.matchers import LessThan
+from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
from lp.app.enums import InformationType
from lp.code.interfaces.branch import IBranchSet
+from lp.code.interfaces.branchcollection import IAllBranches
from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
from lp.code.model.branch import BranchSet
from lp.services.propertycache import clear_property_cache
@@ -60,6 +65,39 @@ class TestBranchSet(TestCaseWithFactory):
self.assertEqual(branch, BranchSet().getByPath(branch.shortened_path))
self.assertIsNone(BranchSet().getByPath('nonexistent'))
+ def test_getBranches_order_by_modified_date(self):
+ # We can get a collection of all branches ordered by modification
+ # date.
+ # Unscan branches from sampledata so that they don't get in our way.
+ for branch in getUtility(IAllBranches).scanned().getBranches():
+ removeSecurityProxy(branch).unscan(rescan=False)
+ branches = [self.factory.makeProductBranch() for _ in range(5)]
+ modified_dates = [
+ datetime(2010, 1, 1, tzinfo=pytz.UTC),
+ datetime(2015, 1, 1, tzinfo=pytz.UTC),
+ datetime(2014, 1, 1, tzinfo=pytz.UTC),
+ datetime(2020, 1, 1, tzinfo=pytz.UTC),
+ datetime(2019, 1, 1, tzinfo=pytz.UTC),
+ ]
+ for branch, modified_date in zip(branches, modified_dates):
+ self.factory.makeRevisionsForBranch(branch)
+ branch.date_last_modified = modified_date
+ removeSecurityProxy(branches[0]).transitionToInformationType(
+ InformationType.PRIVATESECURITY, branches[0].registrant)
+ self.assertEqual(
+ [branches[3], branches[4], branches[1], branches[2], branches[0]],
+ list(getUtility(IBranchSet).getBranches(
+ branches[0].owner, order_by_modified_date=True)))
+ self.assertEqual(
+ [branches[3], branches[4], branches[1]],
+ list(getUtility(IBranchSet).getBranches(
+ branches[0].owner, order_by_modified_date=True,
+ modified_since_date=datetime(2014, 12, 1, tzinfo=pytz.UTC))))
+ self.assertEqual(
+ [branches[3], branches[4], branches[1], branches[2]],
+ list(getUtility(IBranchSet).getBranches(
+ None, order_by_modified_date=True)))
+
def test_api_branches_query_count(self):
webservice = LaunchpadWebServiceCaller()
collector = RequestTimelineCollector()
diff --git a/lib/lp/code/model/tests/test_gitcollection.py b/lib/lp/code/model/tests/test_gitcollection.py
index f96e8e8..3ae0d04 100644
--- a/lib/lp/code/model/tests/test_gitcollection.py
+++ b/lib/lp/code/model/tests/test_gitcollection.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for Git repository collections."""
@@ -231,6 +231,20 @@ class TestGitCollectionFilters(TestCaseWithFactory):
self.assertEqual(1, len(list(collection.getRepositories())))
self.assertEqual(1, collection.count())
+ def test_modifiedSince(self):
+ # Only repositories modified since the time specified will be
+ # returned.
+ old_repository = self.factory.makeGitRepository()
+ removeSecurityProxy(old_repository).date_last_modified = datetime(
+ 2008, 1, 1, tzinfo=pytz.UTC)
+ new_repository = self.factory.makeGitRepository()
+ removeSecurityProxy(new_repository).date_last_modified = datetime(
+ 2009, 1, 1, tzinfo=pytz.UTC)
+ repositories = self.all_repositories.modifiedSince(
+ datetime(2008, 6, 1, tzinfo=pytz.UTC))
+ self.assertEqual(
+ [new_repository], list(repositories.getRepositories()))
+
def test_ownedBy(self):
# 'ownedBy' returns a new collection restricted to repositories
# owned by the given person.
diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
index 287fd9e..510ad16 100644
--- a/lib/lp/code/model/tests/test_gitrepository.py
+++ b/lib/lp/code/model/tests/test_gitrepository.py
@@ -2,7 +2,7 @@
# NOTE: The first line above must stay first; do not move the copyright
# notice to the top. See http://www.python.org/dev/peps/pep-0263/.
#
-# Copyright 2015-2019 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for Git repositories."""
@@ -3269,6 +3269,37 @@ class TestGitRepositorySet(TestCaseWithFactory):
public_repositories + [private_repository],
self.repository_set.getRepositories(other_person, project))
+ def test_getRepositories_order_by_modified_date(self):
+ # We can get a collection of all repositories ordered by
+ # modification date.
+ repositories = [self.factory.makeGitRepository() for _ in range(5)]
+ modified_dates = [
+ datetime(2010, 1, 1, tzinfo=pytz.UTC),
+ datetime(2015, 1, 1, tzinfo=pytz.UTC),
+ datetime(2014, 1, 1, tzinfo=pytz.UTC),
+ datetime(2020, 1, 1, tzinfo=pytz.UTC),
+ datetime(2019, 1, 1, tzinfo=pytz.UTC),
+ ]
+ for repository, modified_date in zip(repositories, modified_dates):
+ removeSecurityProxy(repository).date_last_modified = modified_date
+ removeSecurityProxy(repositories[0]).transitionToInformationType(
+ InformationType.PRIVATESECURITY, repositories[0].registrant)
+ self.assertEqual(
+ [repositories[3], repositories[4], repositories[1],
+ repositories[2], repositories[0]],
+ list(self.repository_set.getRepositories(
+ repositories[0].owner, order_by_modified_date=True)))
+ self.assertEqual(
+ [repositories[3], repositories[4], repositories[1]],
+ list(self.repository_set.getRepositories(
+ repositories[0].owner, order_by_modified_date=True,
+ modified_since_date=datetime(2014, 12, 1, tzinfo=pytz.UTC))))
+ self.assertEqual(
+ [repositories[3], repositories[4], repositories[1],
+ repositories[2]],
+ list(self.repository_set.getRepositories(
+ None, order_by_modified_date=True)))
+
def test_getRepositoryVisibilityInfo_empty_repository_names(self):
# If repository_names is empty, getRepositoryVisibilityInfo returns
# an empty visible_repositories list.
Follow ups
