launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #24263
[Merge] ~pappacena/launchpad:pkg-upload-log-api into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:pkg-upload-log-api into launchpad:master with ~pappacena/launchpad:archive-queue-audit-trail as a prerequisite.
Commit message:
API for Package Upload logs
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/378124
Reopening MP (https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/377897) with proper prerequisite repo and branch.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:pkg-upload-log-api into launchpad:master.
diff --git a/lib/lp/_schema_circular_imports.py b/lib/lp/_schema_circular_imports.py
index 20db323..49b37ec 100644
--- a/lib/lp/_schema_circular_imports.py
+++ b/lib/lp/_schema_circular_imports.py
@@ -206,7 +206,7 @@ from lp.soyuz.interfaces.publishing import (
ISourcePackagePublishingHistoryEdit,
ISourcePackagePublishingHistoryPublic,
)
-from lp.soyuz.interfaces.queue import IPackageUpload
+from lp.soyuz.interfaces.queue import IPackageUpload, IPackageUploadLog
from lp.soyuz.interfaces.sourcepackagerelease import ISourcePackageRelease
from lp.translations.interfaces.hastranslationimports import (
IHasTranslationImports,
@@ -363,6 +363,8 @@ patch_reference_property(
ISourcePackagePublishingHistory)
patch_reference_property(
ISourcePackagePublishingHistory, 'packageupload', IPackageUpload)
+patch_reference_property(
+ IPackageUploadLog, 'package_upload', IPackageUpload)
patch_entry_return_type(
ISourcePackagePublishingHistoryEdit, 'changeOverride',
ISourcePackagePublishingHistory)
diff --git a/lib/lp/soyuz/browser/configure.zcml b/lib/lp/soyuz/browser/configure.zcml
index 1ce6b53..4ab2e41 100644
--- a/lib/lp/soyuz/browser/configure.zcml
+++ b/lib/lp/soyuz/browser/configure.zcml
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2019 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).
-->
@@ -661,6 +661,11 @@
path_expression="string:+upload/${id}"
attribute_to_parent="distroseries"
/>
+ <browser:url
+ for="lp.soyuz.interfaces.queue.IPackageUploadLog"
+ path_expression="string:+log/${id}"
+ attribute_to_parent="package_upload"
+ />
<browser:navigation
module="lp.soyuz.browser.queue"
classes="PackageUploadNavigation"
diff --git a/lib/lp/soyuz/browser/queue.py b/lib/lp/soyuz/browser/queue.py
index 39177bd..02d5f54 100644
--- a/lib/lp/soyuz/browser/queue.py
+++ b/lib/lp/soyuz/browser/queue.py
@@ -69,7 +69,6 @@ from lp.soyuz.model.files import (
from lp.soyuz.model.packagecopyjob import PackageCopyJob
from lp.soyuz.model.queue import (
PackageUploadBuild,
- PackageUploadLog,
PackageUploadSource,
)
from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
@@ -211,26 +210,6 @@ class QueueItemsView(LaunchpadView):
list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
person_ids, need_validity=True))
- def _getPreloadedLogs(self, uploads):
- """Returns a dict of preloaded PackageUploadLog
-
- The keys from the returning dict are the package_upload_id, and the
- values are lists of log entries
- """
- logs = load_referencing(
- PackageUploadLog, uploads, ['package_upload_id'])
-
- # Preload users from log entries
- # Not using `need_icon` since the log's reviewers are always persons,
- # and fetching icons should be only needed for teams
- list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
- [log.reviewer_id for log in logs],
- need_validity=True))
- logs_dict = defaultdict(list)
- for log in logs:
- logs_dict[log.package_upload_id].append(log)
- return logs_dict
-
def decoratedQueueBatch(self):
"""Return the current batch, converted to decorated objects.
@@ -244,12 +223,8 @@ class QueueItemsView(LaunchpadView):
return None
upload_ids = [upload.id for upload in uploads]
- puses = load_referencing(
- PackageUploadSource, uploads, ['packageuploadID'])
- pubs = load_referencing(
- PackageUploadBuild, uploads, ['packageuploadID'])
-
- logs_dict = self._getPreloadedLogs(uploads)
+ puses = sum([u.sources for u in uploads], [])
+ pubs = sum([u.builds for u in uploads], [])
source_sprs = load_related(
SourcePackageRelease, puses, ['sourcepackagereleaseID'])
@@ -288,9 +263,7 @@ class QueueItemsView(LaunchpadView):
return [
CompletePackageUpload(
- item, build_upload_files, source_upload_files, package_sets,
- sorted(logs_dict[item.id], key=attrgetter("date_created"),
- reverse=True))
+ item, build_upload_files, source_upload_files, package_sets)
for item in uploads]
def is_new(self, binarypackagerelease):
@@ -517,24 +490,18 @@ class CompletePackageUpload:
# (i.e. no proxying of __set__).
pocket = None
date_created = None
- sources = None
- builds = None
- logs = None
customfiles = None
contains_source = None
contains_build = None
sourcepackagerelease = None
def __init__(self, packageupload, build_upload_files,
- source_upload_files, package_sets, logs=None):
+ source_upload_files, package_sets):
self.pocket = packageupload.pocket
self.date_created = packageupload.date_created
self.context = packageupload
- self.sources = list(packageupload.sources)
self.contains_source = len(self.sources) > 0
- self.builds = list(packageupload.builds)
self.contains_build = len(self.builds) > 0
- self.logs = list(logs) if logs is not None else []
self.customfiles = list(packageupload.customfiles)
# Create a dictionary of binary files keyed by
diff --git a/lib/lp/soyuz/browser/tests/test_queue.py b/lib/lp/soyuz/browser/tests/test_queue.py
index 242e920..0e08739 100644
--- a/lib/lp/soyuz/browser/tests/test_queue.py
+++ b/lib/lp/soyuz/browser/tests/test_queue.py
@@ -438,7 +438,7 @@ class TestQueueItemsView(TestCaseWithFactory):
with StormStatementRecorder() as recorder:
view = self.makeView(distroseries, queue_admin)
view()
- self.assertThat(recorder, HasQueryCount(Equals(57)))
+ self.assertThat(recorder, HasQueryCount(Equals(55)))
def test_package_upload_with_logs_query_count(self):
login(ADMIN_EMAIL)
diff --git a/lib/lp/soyuz/interfaces/queue.py b/lib/lp/soyuz/interfaces/queue.py
index 929dc45..3a02689 100644
--- a/lib/lp/soyuz/interfaces/queue.py
+++ b/lib/lp/soyuz/interfaces/queue.py
@@ -26,6 +26,7 @@ __all__ = [
import httplib
+from lazr.lifecycle.snapshot import doNotSnapshot
from lazr.restful.declarations import (
call_with,
error_status,
@@ -37,7 +38,10 @@ from lazr.restful.declarations import (
operation_parameters,
REQUEST_USER,
)
-from lazr.restful.fields import Reference
+from lazr.restful.fields import (
+ CollectionField,
+ Reference,
+ )
from zope.interface import (
Attribute,
Interface,
@@ -113,6 +117,44 @@ class IPackageUploadQueue(Interface):
"""
+class IPackageUploadLog(Interface):
+ """A log entry recording a change in a package upload's status."""
+
+ export_as_webservice_entry(publish_web_link=True, as_of="devel")
+
+ id = Int(title=_('ID'), required=True, readonly=True)
+
+ package_upload = exported(
+ Reference(
+ Interface, title=_("The package upload that generated this log"),
+ required=True, readonly=True))
+
+ date_created = exported(
+ Datetime(
+ title=_("When this action happened."), required=True,
+ readonly=True))
+
+ reviewer = exported(
+ Reference(
+ IPerson, title=_("Who did this action."),
+ required=True, readonly=True))
+
+ old_status = exported(
+ Choice(
+ vocabulary=PackageUploadStatus, description=_("Old status."),
+ required=True, readonly=True))
+
+ new_status = exported(
+ Choice(
+ vocabulary=PackageUploadStatus, description=_("New status."),
+ required=True, readonly=True))
+
+ comment = exported(
+ TextLine(
+ title=_("User's comment about this change."),
+ required=False, readonly=True))
+
+
class IPackageUpload(Interface):
"""A Queue item for the archive uploader."""
@@ -151,8 +193,6 @@ class IPackageUpload(Interface):
title=_('Date created'),
description=_("The date this package upload was done.")))
- logs = Attribute(_("The change log of this PackageUpload."))
-
changesfile = Attribute("The librarian alias for the changes file "
"associated with this upload")
changes_file_url = exported(
@@ -185,6 +225,14 @@ class IPackageUpload(Interface):
sources = Attribute("The queue sources associated with this queue item")
builds = Attribute("The queue builds associated with the queue item")
+ logs = exported(
+ doNotSnapshot(
+ CollectionField(
+ title=_("The package upload logs"),
+ value_type=Reference(schema=IPackageUploadLog),
+ readonly=True)),
+ as_of="devel")
+
customfiles = Attribute("Custom upload files associated with this "
"queue item")
custom_file_urls = exported(
@@ -507,9 +555,11 @@ class IPackageUploadBuild(Interface):
readonly=False,
)
- build = Int(
- title=_("The related build"), required=True, readonly=False,
- )
+ build = Int(title=_("The related build"), required=True, readonly=False)
+
+ buildID = Int(
+ title=_("The Related build ID"), required=True,
+ readonly=True)
def binaries():
"""Returns the properties of the binaries in this build.
@@ -552,8 +602,11 @@ class IPackageUploadSource(Interface):
sourcepackagerelease = Int(
title=_("The related source package release"), required=True,
- readonly=False,
- )
+ readonly=False)
+
+ sourcepackagereleaseID = Int(
+ title=_("The related source package release ID"), required=True,
+ readonly=True)
def getSourceAncestryForDiffs():
"""Return a suitable ancestry publication for this context.
@@ -715,35 +768,6 @@ class IPackageUploadCustom(Interface):
"""
-class IPackageUploadLog(Interface):
- """Entries of package upload status change"""
-
- id = Int(title=_('ID'), required=True, readonly=True)
-
- package_upload = Reference(
- IPackageUpload,
- title=_("Original package upload."), required=True, readonly=True)
-
- date_created = Datetime(
- title=_("When this action happened."), required=True, readonly=True)
-
- reviewer = Reference(
- IPerson, title=_("Who did this action."),
- required=True, readonly=True)
-
- old_status = Choice(
- vocabulary=PackageUploadStatus, description=_("Old status."),
- required=True, readonly=True)
-
- new_status = Choice(
- vocabulary=PackageUploadStatus, description=_("New status."),
- required=True, readonly=True)
-
- comment = TextLine(
- title=_("User's comment about this change."),
- required=False, readonly=True)
-
-
class IPackageUploadSet(Interface):
"""Represents a set of IPackageUploads"""
diff --git a/lib/lp/soyuz/interfaces/webservice.py b/lib/lp/soyuz/interfaces/webservice.py
index 3620e81..90ef0e1 100644
--- a/lib/lp/soyuz/interfaces/webservice.py
+++ b/lib/lp/soyuz/interfaces/webservice.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2019 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""All the interfaces that are exposed through the webservice.
@@ -35,6 +35,7 @@ __all__ = [
'ILiveFSBuild',
'ILiveFSSet',
'IPackageUpload',
+ 'IPackageUploadLog',
'IPackageset',
'IPackagesetSet',
'ISourcePackagePublishingHistory',
@@ -104,7 +105,10 @@ from lp.soyuz.interfaces.publishing import (
IBinaryPackagePublishingHistory,
ISourcePackagePublishingHistory,
)
-from lp.soyuz.interfaces.queue import IPackageUpload
+from lp.soyuz.interfaces.queue import (
+ IPackageUpload,
+ IPackageUploadLog,
+ )
_schema_circular_imports
diff --git a/lib/lp/soyuz/model/queue.py b/lib/lp/soyuz/model/queue.py
index 6a55fa7..91e92cd 100644
--- a/lib/lp/soyuz/model/queue.py
+++ b/lib/lp/soyuz/model/queue.py
@@ -12,7 +12,9 @@ __all__ = [
'PackageUploadSource',
]
+from collections import defaultdict
from itertools import chain
+from operator import attrgetter
import pytz
from sqlobject import (
@@ -42,6 +44,7 @@ from zope.interface import implementer
from lp.app.errors import NotFoundError
from lp.archiveuploader.tagfiles import parse_tagfile_content
from lp.registry.interfaces.gpg import IGPGKeySet
+from lp.registry.interfaces.person import IPersonSet
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.model.sourcepackagename import SourcePackageName
from lp.services.auditor.client import AuditorClient
@@ -1600,8 +1603,10 @@ class PackageUploadSet:
PackageUploadBuild, rows, ["packageuploadID"])
pucs = load_referencing(
PackageUploadCustom, rows, ["packageuploadID"])
+ logs = load_referencing(
+ PackageUploadLog, rows, ["package_upload_id"])
- prefill_packageupload_caches(rows, puses, pubs, pucs)
+ prefill_packageupload_caches(rows, puses, pubs, pucs, logs)
return DecoratedResultSet(query, pre_iter_hook=preload_hook)
@@ -1622,18 +1627,32 @@ class PackageUploadSet:
PackageUpload.package_copy_job_id.is_in(pcj_ids))
-def prefill_packageupload_caches(uploads, puses, pubs, pucs):
+def prefill_packageupload_caches(uploads, puses, pubs, pucs, logs):
# Circular imports.
from lp.soyuz.model.archive import Archive
from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild
from lp.soyuz.model.publishing import SourcePackagePublishingHistory
from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
+ logs_per_pu = defaultdict(list)
+ reviewer_ids = set()
+ for log in logs:
+ reviewer_ids.add(log.reviewer_id)
+ logs_per_pu[log.package_upload_id].append(log)
+
+ # Preload reviewers of the logs.
+ # We are not using `need_icon` here because reviewers are persons,
+ # and icons are only available for teams.
+ list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
+ reviewer_ids, need_validity=True))
+
for pu in uploads:
cache = get_property_cache(pu)
cache.sources = []
cache.builds = []
cache.customfiles = []
+ cache.logs = sorted(
+ logs_per_pu[pu.id], key=attrgetter("date_created"), reverse=True)
for pus in puses:
get_property_cache(pus.packageupload).sources.append(pus)
diff --git a/lib/lp/soyuz/tests/test_packageupload.py b/lib/lp/soyuz/tests/test_packageupload.py
index 0c83f33..1e7e655 100644
--- a/lib/lp/soyuz/tests/test_packageupload.py
+++ b/lib/lp/soyuz/tests/test_packageupload.py
@@ -1483,3 +1483,23 @@ class TestPackageUploadWebservice(TestCaseWithFactory):
person, component=self.universe),
5)
self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
+
+ def test_api_package_upload_log(self):
+ # API clients can see upload logs of a source uploads.
+ admin = self.makeQueueAdmin([self.universe])
+ upload, ws_upload = self.makeSourcePackageUpload(
+ admin, sourcepackagename="hello", component=self.universe)
+ with person_logged_in(admin):
+ upload.rejectFromQueue(admin, 'not a good change')
+ upload.acceptFromQueue(admin)
+
+ logs = removeSecurityProxy(upload).logs
+ ws_logs = ws_upload.logs
+ self.assertEqual(len(ws_logs), len(logs))
+ for log, ws_log in zip(logs, ws_logs):
+ self.assertEqual(log.comment, ws_log.comment)
+ self.assertEqual(log.date_created, ws_log.date_created)
+ self.assertEqual(log.new_status.title, ws_log.new_status)
+ self.assertEqual(log.old_status.title, ws_log.old_status)
+ self.assertEqual(log.reviewer.name, ws_log.reviewer.name)
+ self.assertEqual(ws_log.package_upload.id, ws_upload.id)
