launchpad-reviewers team mailing list archive

[Colin Watson <mp+414189@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:non-log-revision-status-artifacts into launchpad:master.

Commit message:
Add ability to attach revision status artifacts other than logs

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

This is very minimal right now (e.g. there's no way to retrieve the artifacts again), but it at least gives us something that we can use in putting together CI builds.

I tidied up a few bits of `setLog`-related code as well.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:non-log-revision-status-artifacts into launchpad:master.

diff --git a/lib/lp/code/enums.py b/lib/lp/code/enums.py
index 2dbd287..fe9f5f9 100644
--- a/lib/lp/code/enums.py
+++ b/lib/lp/code/enums.py
@@ -261,6 +261,13 @@ class RevisionStatusArtifactType(DBEnumeratedType):
         The log produced by the check job.
         """)
 
+    BINARY = DBItem(1, """
+        Binary
+
+        A binary file attached to a check job.  This artifact type is made
+        available for download, but is not otherwise interpreted in any way.
+        """)
+
 
 class RevisionStatusResult(DBEnumeratedType):
     """Revision Status Result"""
diff --git a/lib/lp/code/interfaces/revisionstatus.py b/lib/lp/code/interfaces/revisionstatus.py
index 630dec4..8e4da9f 100644
--- a/lib/lp/code/interfaces/revisionstatus.py
+++ b/lib/lp/code/interfaces/revisionstatus.py
@@ -16,7 +16,6 @@ import http.client
 
 from lazr.restful.declarations import (
     error_status,
-    export_operation_as,
     export_write_operation,
     exported,
     exported_as_webservice_entry,
@@ -120,14 +119,33 @@ class IRevisionStatusReportEdit(Interface):
                        constraint=attachment_size_constraint))
     @scoped(AccessTokenScope.REPOSITORY_BUILD_STATUS.title)
     @export_write_operation()
-    @export_operation_as(name="setLog")
     @operation_for_version("devel")
-    def api_setLog(log_data):
+    def setLog(log_data):
         """Set a new log on an existing status report.
 
         :param log_data: The contents (in bytes) of the log.
         """
 
+    # XXX cjwatson 2022-01-14: artifact_type isn't currently exported, but
+    # if RevisionStatusArtifactType gains more items (e.g. detailed test
+    # output in subunit format or similar?) then it may make sense to do so.
+    @operation_parameters(
+        name=TextLine(title=_("The name of the artifact.")),
+        data=Bytes(
+            title=_("The content of the artifact in bytes."),
+            constraint=attachment_size_constraint))
+    @scoped(AccessTokenScope.REPOSITORY_BUILD_STATUS.title)
+    @export_write_operation()
+    @operation_for_version("devel")
+    def attach(name, data, artifact_type=RevisionStatusArtifactType.BINARY):
+        """Attach a new artifact to an existing status report.
+
+        :param data: The contents (in bytes) of the artifact.
+        :param artifact_type: The type of the artifact.  This may currently
+            only be `RevisionStatusArtifactType.BINARY`, but more types may
+            be added in future.
+        """
+
     @operation_parameters(
         title=TextLine(title=_("A short title for the report."),
                        required=False),
@@ -194,12 +212,13 @@ class IRevisionStatusReportSet(Interface):
 class IRevisionStatusArtifactSet(Interface):
     """The set of all revision status artifacts."""
 
-    def new(lfa, report):
+    def new(lfa, report, artifact_type):
         """Return a new revision status artifact.
 
         :param lfa: An `ILibraryFileAlias`.
         :param report: An `IRevisionStatusReport` for which the
             artifact is being created.
+        :param artifact_type: A `RevisionStatusArtifactType`.
         """
 
     def getByID(id):
diff --git a/lib/lp/code/model/revisionstatus.py b/lib/lp/code/model/revisionstatus.py
index 78a9e1b..7fa38d3 100644
--- a/lib/lp/code/model/revisionstatus.py
+++ b/lib/lp/code/model/revisionstatus.py
@@ -77,14 +77,25 @@ class RevisionStatusReport(StormBase):
         self.date_created = UTC_NOW
         self.transitionToNewResult(result)
 
-    def api_setLog(self, log_data):
+    def setLog(self, log_data):
         filename = '%s-%s.txt' % (self.title, self.commit_sha1)
 
         lfa = getUtility(ILibraryFileAliasSet).create(
             name=filename, size=len(log_data),
-            file=io.BytesIO(log_data), contentType='text/plain')
+            file=io.BytesIO(log_data), contentType='text/plain',
+            restricted=self.git_repository.private)
 
-        getUtility(IRevisionStatusArtifactSet).new(lfa, self)
+        getUtility(IRevisionStatusArtifactSet).new(
+            lfa, self, RevisionStatusArtifactType.LOG)
+
+    def attach(self, name, data,
+               artifact_type=RevisionStatusArtifactType.BINARY):
+        """See `IRevisionStatusReport`."""
+        lfa = getUtility(ILibraryFileAliasSet).create(
+            name=name, size=len(data), file=io.BytesIO(data),
+            contentType='application/octet-stream',
+            restricted=self.git_repository.private)
+        getUtility(IRevisionStatusArtifactSet).new(lfa, self, artifact_type)
 
     def transitionToNewResult(self, result):
         if self.result == RevisionStatusResult.WAITING:
@@ -165,20 +176,20 @@ class RevisionStatusArtifact(StormBase):
     artifact_type = DBEnum(name='type', allow_none=False,
                            enum=RevisionStatusArtifactType)
 
-    def __init__(self, library_file, report):
+    def __init__(self, library_file, report, artifact_type):
         super().__init__()
         self.library_file = library_file
         self.report = report
-        self.artifact_type = RevisionStatusArtifactType.LOG
+        self.artifact_type = artifact_type
 
 
 @implementer(IRevisionStatusArtifactSet)
 class RevisionStatusArtifactSet:
 
-    def new(self, lfa, report):
+    def new(self, lfa, report, artifact_type):
         """See `IRevisionStatusArtifactSet`."""
         store = IStore(RevisionStatusArtifact)
-        artifact = RevisionStatusArtifact(lfa, report)
+        artifact = RevisionStatusArtifact(lfa, report, artifact_type)
         store.add(artifact)
         return artifact
 
diff --git a/lib/lp/code/model/tests/test_revisionstatus.py b/lib/lp/code/model/tests/test_revisionstatus.py
index 4472988..b7d9998 100644
--- a/lib/lp/code/model/tests/test_revisionstatus.py
+++ b/lib/lp/code/model/tests/test_revisionstatus.py
@@ -6,9 +6,18 @@
 import hashlib
 import io
 
+from testtools.matchers import (
+    AnyMatch,
+    Equals,
+    MatchesSetwise,
+    MatchesStructure,
+    )
 from zope.component import getUtility
 
-from lp.code.enums import RevisionStatusResult
+from lp.code.enums import (
+    RevisionStatusArtifactType,
+    RevisionStatusResult,
+    )
 from lp.code.interfaces.revisionstatus import IRevisionStatusArtifactSet
 from lp.services.auth.enums import AccessTokenScope
 from lp.testing import (
@@ -47,11 +56,8 @@ class TestRevisionStatusReportWebservice(TestCaseWithFactory):
                 scopes=[AccessTokenScope.REPOSITORY_BUILD_STATUS])
             self.header = {'Authorization': 'Token %s' % secret}
 
-    def test_setLogOnRevisionStatusReport(self):
+    def test_setLog(self):
         content = b'log_content_data'
-        filesize = len(content)
-        sha1 = hashlib.sha1(content).hexdigest()
-        md5 = hashlib.md5(content).hexdigest()
         response = self.webservice.named_post(
             self.report_url, "setLog",
             headers=self.header,
@@ -64,14 +70,39 @@ class TestRevisionStatusReportWebservice(TestCaseWithFactory):
         with person_logged_in(self.requester):
             artifacts = list(getUtility(
                 IRevisionStatusArtifactSet).findByReport(self.report))
-            lfcs = [artifact.library_file.content for artifact in artifacts]
-            sha1_of_all_artifacts = [lfc.sha1 for lfc in lfcs]
-            md5_of_all_artifacts = [lfc.md5 for lfc in lfcs]
-            filesizes_of_all_artifacts = [lfc.filesize for lfc in lfcs]
+            self.assertThat(artifacts, AnyMatch(
+                MatchesStructure(
+                    report=Equals(self.report),
+                    library_file=MatchesStructure(
+                        content=MatchesStructure.byEquality(
+                            sha256=hashlib.sha256(content).hexdigest()),
+                        filename=Equals(
+                            "%s-%s.txt" % (self.title, self.commit_sha1)),
+                        mimetype=Equals("text/plain")),
+                    artifact_type=Equals(RevisionStatusArtifactType.LOG))))
+
+    def test_attach(self):
+        filenames = ["artifact-1", "artifact-2"]
+        contents = [b"artifact 1", b"artifact 2"]
+        for filename, content in zip(filenames, contents):
+            response = self.webservice.named_post(
+                self.report_url, "attach", headers=self.header,
+                name=filename, data=io.BytesIO(content))
+            self.assertEqual(200, response.status)
 
-            self.assertIn(sha1, sha1_of_all_artifacts)
-            self.assertIn(md5, md5_of_all_artifacts)
-            self.assertIn(filesize, filesizes_of_all_artifacts)
+        with person_logged_in(self.requester):
+            artifacts = list(getUtility(
+                IRevisionStatusArtifactSet).findByReport(self.report))
+            self.assertThat(artifacts, MatchesSetwise(*(
+                MatchesStructure(
+                    report=Equals(self.report),
+                    library_file=MatchesStructure(
+                        content=MatchesStructure.byEquality(
+                            sha256=hashlib.sha256(content).hexdigest()),
+                        filename=Equals(filename),
+                        mimetype=Equals("application/octet-stream")),
+                    artifact_type=Equals(RevisionStatusArtifactType.BINARY))
+                for filename, content in zip(filenames, contents))))
 
     def test_update(self):
         response = self.webservice.named_post(
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index dff6b0a..71265f7 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -117,6 +117,7 @@ from lp.code.enums import (
     GitObjectType,
     GitRepositoryType,
     RevisionControlSystems,
+    RevisionStatusArtifactType,
     RevisionStatusResult,
     TargetRevisionControlSystems,
     )
@@ -1869,13 +1870,16 @@ class BareLaunchpadObjectFactory(ObjectFactory):
             user, title, git_repository, commit_sha1, url,
             result_summary, result)
 
-    def makeRevisionStatusArtifact(self, lfa=None, report=None):
+    def makeRevisionStatusArtifact(
+            self, lfa=None, report=None,
+            artifact_type=RevisionStatusArtifactType.LOG):
         """Create a new RevisionStatusArtifact."""
         if lfa is None:
             lfa = self.makeLibraryFileAlias()
         if report is None:
             report = self.makeRevisionStatusReport()
-        return getUtility(IRevisionStatusArtifactSet).new(lfa, report)
+        return getUtility(IRevisionStatusArtifactSet).new(
+            lfa, report, artifact_type)
 
     def makeBug(self, target=None, owner=None, bug_watch_url=None,
                 information_type=None, date_closed=None, title=None,