launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:oci-recipe-build-navigation into launchpad:master.

Commit message:
Add OCI recipe build file navigation

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1847444 in Launchpad itself: "Support OCI image building"
  https://bugs.launchpad.net/launchpad/+bug/1847444

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/382242
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:oci-recipe-build-navigation into launchpad:master.

diff --git a/lib/lp/oci/browser/ocirecipebuild.py b/lib/lp/oci/browser/ocirecipebuild.py
index dfed462..5d58f5e 100644
--- a/lib/lp/oci/browser/ocirecipebuild.py
+++ b/lib/lp/oci/browser/ocirecipebuild.py
@@ -21,6 +21,11 @@ from lp.app.browser.launchpadform import (
     LaunchpadFormView,
     )
 from lp.oci.interfaces.ocirecipebuild import IOCIRecipeBuild
+from lp.services.librarian.browser import (
+    FileNavigationMixin,
+    ProxiedLibraryFileAlias,
+    )
+from lp.services.propertycache import cachedproperty
 from lp.services.webapp import (
     canonical_url,
     ContextMenu,
@@ -31,7 +36,7 @@ from lp.services.webapp import (
 from lp.soyuz.interfaces.binarypackagebuild import IBuildRescoreForm
 
 
-class OCIRecipeBuildNavigation(Navigation):
+class OCIRecipeBuildNavigation(Navigation, FileNavigationMixin):
 
     usedfor = IOCIRecipeBuild
 
@@ -70,6 +75,20 @@ class OCIRecipeBuildView(LaunchpadFormView):
 
     page_title = label
 
+    @cachedproperty
+    def files(self):
+        """Return `LibraryFileAlias`es for files produced by this build."""
+        if not self.context.was_built:
+            return None
+
+        return [
+            ProxiedLibraryFileAlias(alias, self.context)
+            for _, alias, _ in self.context.getFiles() if not alias.deleted]
+
+    @cachedproperty
+    def has_files(self):
+        return bool(self.files)
+
     @property
     def next_url(self):
         return canonical_url(self.context)
diff --git a/lib/lp/oci/browser/tests/test_ocirecipebuild.py b/lib/lp/oci/browser/tests/test_ocirecipebuild.py
index 9c6eae3..c366055 100644
--- a/lib/lp/oci/browser/tests/test_ocirecipebuild.py
+++ b/lib/lp/oci/browser/tests/test_ocirecipebuild.py
@@ -16,6 +16,7 @@ from testtools.matchers import StartsWith
 import transaction
 from zope.component import getUtility
 from zope.security.interfaces import Unauthorized
+from zope.security.proxy import removeSecurityProxy
 from zope.testbrowser.browser import LinkNotFoundError
 
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
@@ -39,6 +40,7 @@ from lp.testing.pages import (
     find_main_content,
     find_tags_by_class,
     )
+from lp.testing.views import create_initialized_view
 
 
 class TestCanonicalUrlForOCIRecipeBuild(TestCaseWithFactory):
@@ -90,6 +92,21 @@ class TestOCIRecipeBuildView(BrowserTestCase):
                 recipe.owner.display_name),
             self.getMainText(build))
 
+    def test_files(self):
+        # OCIRecipeBuildView.files returns all the associated files.
+        build = self.factory.makeOCIRecipeBuild(status=BuildStatus.FULLYBUILT)
+        oci_file = self.factory.makeOCIFile(build=build)
+        build_view = create_initialized_view(build, "+index")
+        self.assertEqual(
+            [oci_file.library_file.filename],
+            [lfa.filename for lfa in build_view.files])
+        # Deleted files won't be included.
+        self.assertFalse(oci_file.library_file.deleted)
+        removeSecurityProxy(oci_file.library_file).content = None
+        self.assertTrue(oci_file.library_file.deleted)
+        build_view = create_initialized_view(build, "+index")
+        self.assertEqual([], build_view.files)
+
 
 class TestOCIRecipeBuildOperations(BrowserTestCase):
 
diff --git a/lib/lp/oci/interfaces/ocirecipebuild.py b/lib/lp/oci/interfaces/ocirecipebuild.py
index 752e61d..b97d1dc 100644
--- a/lib/lp/oci/interfaces/ocirecipebuild.py
+++ b/lib/lp/oci/interfaces/ocirecipebuild.py
@@ -57,13 +57,29 @@ class IOCIRecipeBuildView(IPackageBuild):
             "The date when the build completed or is estimated to complete."),
         readonly=True)
 
-    def getByFileName():
-        """Retrieve a file by filename
+    def getFiles():
+        """Retrieve the build's `IOCIFile` records.
 
         :return: A result set of (`IOCIFile`, `ILibraryFileAlias`,
             `ILibraryFileContent`).
         """
 
+    def getFileByName():
+        """Return the corresponding `ILibraryFileAlias` in this context.
+
+        The following file types (and extension) can be looked up:
+
+         * Build log: '.txt.gz'
+         * Upload log: '_log.txt'
+
+        Any filename not matching one of these extensions is looked up as an
+        OCI recipe output file.
+
+        :param filename: The filename to look up.
+        :raises NotFoundError: if no file exists with the given name.
+        :return: The corresponding `ILibraryFileAlias`.
+        """
+
     def getLayerFileByDigest(layer_file_digest):
         """Retrieve a layer file by the digest.
 
diff --git a/lib/lp/oci/model/ocirecipebuild.py b/lib/lp/oci/model/ocirecipebuild.py
index 21c1cce..ab9d1c1 100644
--- a/lib/lp/oci/model/ocirecipebuild.py
+++ b/lib/lp/oci/model/ocirecipebuild.py
@@ -230,15 +230,31 @@ class OCIRecipeBuild(PackageBuildMixin, Storm):
         durations.sort()
         return durations[len(durations) // 2]
 
-    def getByFileName(self, filename):
+    def getFiles(self):
+        """See `IOCIRecipeBuild`."""
         result = Store.of(self).find(
             (OCIFile, LibraryFileAlias, LibraryFileContent),
             OCIFile.build == self.id,
             LibraryFileAlias.id == OCIFile.library_file_id,
-            LibraryFileContent.id == LibraryFileAlias.contentID,
-            LibraryFileAlias.filename == filename).one()
-        if result is not None:
-            return result
+            LibraryFileContent.id == LibraryFileAlias.contentID)
+        return result.order_by([LibraryFileAlias.filename, OCIFile.id])
+
+    def getFileByName(self, filename):
+        """See `IOCIRecipeBuild`."""
+        if filename.endswith(".txt.gz"):
+            file_object = self.log
+        elif filename.endswith("_log.txt"):
+            file_object = self.upload_log
+        else:
+            file_object = Store.of(self).find(
+                LibraryFileAlias,
+                OCIFile.build == self.id,
+                LibraryFileAlias.id == OCIFile.library_file_id,
+                LibraryFileAlias.filename == filename).one()
+
+        if file_object is not None and file_object.filename == filename:
+            return file_object
+
         raise NotFoundError(filename)
 
     @cachedproperty
diff --git a/lib/lp/oci/templates/ocirecipebuild-index.pt b/lib/lp/oci/templates/ocirecipebuild-index.pt
index b9ff8c1..2f88103 100644
--- a/lib/lp/oci/templates/ocirecipebuild-index.pt
+++ b/lib/lp/oci/templates/ocirecipebuild-index.pt
@@ -30,6 +30,10 @@
         </div>
       </div> <!-- yui-g  -->
 
+      <div id="files" class="portlet" tal:condition="view/has_files">
+        <div metal:use-macro="template/macros/files"/>
+      </div>
+
       <div id="buildlog" class="portlet"
            tal:condition="context/status/enumvalue:BUILDING">
         <div metal:use-macro="template/macros/buildlog"/>
@@ -143,6 +147,22 @@
     </div>
   </metal:macro>
 
+  <metal:macro define-macro="files">
+    <tal:comment replace="nothing">
+      Files section.
+    </tal:comment>
+    <h2>Built files</h2>
+    <p>Files resulting from this build:</p>
+    <ul>
+      <li tal:repeat="file view/files">
+        <a class="sprite download"
+           tal:content="file/filename"
+           tal:attributes="href file/http_url"/>
+        (<span tal:replace="file/content/filesize/fmt:bytes"/>)
+      </li>
+    </ul>
+  </metal:macro>
+
   <metal:macro define-macro="buildlog">
     <tal:comment replace="nothing">
       Buildlog section.
diff --git a/lib/lp/oci/tests/test_ocirecipebuild.py b/lib/lp/oci/tests/test_ocirecipebuild.py
index 5ae8b78..400d78d 100644
--- a/lib/lp/oci/tests/test_ocirecipebuild.py
+++ b/lib/lp/oci/tests/test_ocirecipebuild.py
@@ -68,21 +68,32 @@ class TestOCIRecipeBuild(TestCaseWithFactory):
     def test_addFile(self):
         lfa = self.factory.makeLibraryFileAlias()
         self.build.addFile(lfa)
-        _, result_lfa, _ = self.build.getByFileName(lfa.filename)
+        result_lfa = self.build.getFileByName(lfa.filename)
         self.assertEqual(result_lfa, lfa)
 
-    def test_getByFileName(self):
+    def test_getFileByName(self):
         files = [self.factory.makeOCIFile(build=self.build) for x in range(3)]
-        result, _, _ = self.build.getByFileName(
-            files[0].library_file.filename)
-        self.assertEqual(result, files[0])
+        result = self.build.getFileByName(files[0].library_file.filename)
+        self.assertEqual(files[0].library_file, result)
 
-    def test_getByFileName_missing(self):
+    def test_getFileByName_missing(self):
         self.assertRaises(
             NotFoundError,
-            self.build.getByFileName,
+            self.build.getFileByName,
             "missing")
 
+    def test_getFileByName_logs(self):
+        # getFileByName returns the logs when requested by name.
+        self.build.setLog(
+            self.factory.makeLibraryFileAlias(filename="buildlog.txt.gz"))
+        self.assertEqual(
+            self.build.log, self.build.getFileByName("buildlog.txt.gz"))
+        self.assertRaises(NotFoundError, self.build.getFileByName, "foo")
+        self.build.storeUploadLog("uploaded")
+        self.assertEqual(
+            self.build.upload_log,
+            self.build.getFileByName(self.build.upload_log.filename))
+
     def test_getLayerFileByDigest(self):
         files = [self.factory.makeOCIFile(
                     build=self.build, layer_file_digest=six.text_type(x))