launchpad-reviewers team mailing list archive

[Jürgen Gmach <mp+415871@xxxxxxxxxxxxxxxxxx>]

Jürgen Gmach has proposed merging ~jugmac00/launchpad:revisionsstatusreport-attach-and-setlog-now-accept-file-object into launchpad:master.

Commit message:
RevisionStatusReport.attach/.setLog now also accept a file object

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jugmac00/launchpad/+git/launchpad/+merge/415871
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~jugmac00/launchpad:revisionsstatusreport-attach-and-setlog-now-accept-file-object into launchpad:master.

diff --git a/lib/lp/code/interfaces/revisionstatus.py b/lib/lp/code/interfaces/revisionstatus.py
index 90d4e02..8dcb2e4 100644
--- a/lib/lp/code/interfaces/revisionstatus.py
+++ b/lib/lp/code/interfaces/revisionstatus.py
@@ -149,7 +149,8 @@ class IRevisionStatusReportEdit(Interface):
     def setLog(log_data):
         """Set a new log on an existing status report.
 
-        :param log_data: The contents (in bytes) of the log.
+        :param log_data: The contents of the log, either as bytes or as a file
+            object.
         """
 
     # XXX cjwatson 2022-01-14: artifact_type isn't currently exported, but
@@ -166,7 +167,8 @@ class IRevisionStatusReportEdit(Interface):
     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 data: The contents of the artifact, either as bytes or as a file
+            object.
         :param artifact_type: The type of the artifact.  This may currently
             only be `RevisionStatusArtifactType.BINARY`, but more types may
             be added in future.
diff --git a/lib/lp/code/model/revisionstatus.py b/lib/lp/code/model/revisionstatus.py
index 8ef5cba..1f74af3 100644
--- a/lib/lp/code/model/revisionstatus.py
+++ b/lib/lp/code/model/revisionstatus.py
@@ -8,6 +8,7 @@ __all__ = [
     ]
 
 import io
+import os
 
 import pytz
 from storm.locals import (
@@ -90,9 +91,18 @@ class RevisionStatusReport(StormBase):
     def setLog(self, log_data):
         filename = '%s-%s.txt' % (self.title, self.commit_sha1)
 
+        if isinstance(log_data, bytes):
+            file = io.BytesIO(log_data)
+            size = len(log_data)
+        else:
+            file = log_data
+            file.seek(0, os.SEEK_END)
+            size = file.tell()
+            file.seek(0)
+
         lfa = getUtility(ILibraryFileAliasSet).create(
-            name=filename, size=len(log_data),
-            file=io.BytesIO(log_data), contentType='text/plain',
+            name=filename, size=size,
+            file=file, contentType='text/plain',
             restricted=self.git_repository.private)
 
         getUtility(IRevisionStatusArtifactSet).new(
@@ -101,8 +111,18 @@ class RevisionStatusReport(StormBase):
     def attach(self, name, data,
                artifact_type=RevisionStatusArtifactType.BINARY):
         """See `IRevisionStatusReport`."""
+
+        if isinstance(data, bytes):
+            file = io.BytesIO(data)
+            size = len(data)
+        else:
+            file = data
+            file.seek(0, os.SEEK_END)
+            size = file.tell()
+            file.seek(0)
+
         lfa = getUtility(ILibraryFileAliasSet).create(
-            name=name, size=len(data), file=io.BytesIO(data),
+            name=name, size=size, file=file,
             contentType='application/octet-stream',
             restricted=self.git_repository.private)
         getUtility(IRevisionStatusArtifactSet).new(lfa, self, artifact_type)
diff --git a/lib/lp/code/model/tests/test_revisionstatus.py b/lib/lp/code/model/tests/test_revisionstatus.py
index 7b5ed39..ca906f4 100644
--- a/lib/lp/code/model/tests/test_revisionstatus.py
+++ b/lib/lp/code/model/tests/test_revisionstatus.py
@@ -4,9 +4,14 @@
 """Tests for revision status reports and artifacts."""
 
 import hashlib
+from hashlib import sha1
 import io
+import os
 
-from fixtures import FakeLogger
+from fixtures import (
+    FakeLogger,
+    TempDir,
+    )
 import requests
 from storm.store import Store
 from testtools.matchers import (
@@ -30,6 +35,7 @@ from lp.code.interfaces.revisionstatus import (
     IRevisionStatusReportSet,
     )
 from lp.services.auth.enums import AccessTokenScope
+from lp.services.osutils import write_file
 from lp.services.webapp.authorization import check_permission
 from lp.testing import (
     anonymous_logged_in,
@@ -37,6 +43,7 @@ from lp.testing import (
     person_logged_in,
     TestCaseWithFactory,
     )
+from lp.testing.dbuser import switch_dbuser
 from lp.testing.layers import (
     DatabaseFunctionalLayer,
     LaunchpadFunctionalLayer,
@@ -195,6 +202,33 @@ class TestRevisionStatusReportWebservice(TestCaseWithFactory):
     def test_setLog_private(self):
         self._test_setLog(private=True)
 
+    def test_setLog_with_file_object(self):
+        switch_dbuser("launchpad_main")
+
+        # create log file
+        path = os.path.join(
+            self.useFixture(TempDir()).path, "test", "build:0.log"
+        )
+        content = "some log content"
+        write_file(path, content.encode("utf-8"))
+
+        report = self.factory.makeRevisionStatusReport(
+            title="build:0",
+            ci_build=self.factory.makeCIBuild(),
+        )
+        with person_logged_in(report.creator):
+            with open(path, "rb") as f:
+                report.setLog(f)
+
+        with person_logged_in(report.creator):
+            artifacts = list(getUtility(
+                IRevisionStatusArtifactSet).findByReport(report))
+
+        self.assertEqual(
+            artifacts[0].library_file.content.sha1,
+            sha1(content.encode()).hexdigest()
+        )
+
     def _test_attach(self, private):
         requester = self.factory.makePerson()
         with person_logged_in(requester):
@@ -234,6 +268,33 @@ class TestRevisionStatusReportWebservice(TestCaseWithFactory):
     def test_attach_private(self):
         self._test_attach(private=True)
 
+    def test_attach_with_file_object(self):
+        switch_dbuser("launchpad_main")
+
+        # create text file
+        path = os.path.join(
+            self.useFixture(TempDir()).path, "test.md"
+        )
+        content = "some content"
+        write_file(path, content.encode("utf-8"))
+
+        report = self.factory.makeRevisionStatusReport(
+            title="build:0",
+            ci_build=self.factory.makeCIBuild(),
+        )
+        with person_logged_in(report.creator):
+            with open(path, "rb") as f:
+                report.attach("text", f)
+
+        with person_logged_in(report.creator):
+            artifacts = list(getUtility(
+                IRevisionStatusArtifactSet).findByReport(report))
+
+        self.assertEqual(
+            artifacts[0].library_file.content.sha1,
+            sha1(content.encode()).hexdigest()
+        )
+
     def test_update(self):
         report = self.factory.makeRevisionStatusReport(
             result=RevisionStatusResult.FAILED)