launchpad-reviewers team mailing list archive

[Colin Watson <mp+433548@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:tighten-upload-log-navigation into launchpad:master.

Commit message:
Avoid false positives in navigation to upload logs for builds

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1997591 in Launchpad itself: "Any build artifact with filename ending "_log.txt " can not be downloaded from a launchpad build and returns 404"
  https://bugs.launchpad.net/launchpad/+bug/1997591

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

Some builds can have files attached to them whose names end with "_log.txt" and which aren't upload logs.  Fix navigation to avoid misidentifying them as upload logs so that they can be downloaded.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:tighten-upload-log-navigation into launchpad:master.

diff --git a/lib/lp/charms/model/charmrecipebuild.py b/lib/lp/charms/model/charmrecipebuild.py
index b07521e..856b535 100644
--- a/lib/lp/charms/model/charmrecipebuild.py
+++ b/lib/lp/charms/model/charmrecipebuild.py
@@ -8,6 +8,7 @@ __all__ = [
     "CharmRecipeBuild",
 ]
 
+import re
 from datetime import timedelta
 from operator import attrgetter
 
@@ -384,7 +385,7 @@ class CharmRecipeBuild(PackageBuildMixin, StormBase):
         """See `ICharmRecipeBuild`."""
         if filename.endswith(".txt.gz"):
             file_object = self.log
-        elif filename.endswith("_log.txt"):
+        elif re.match(r"^upload_[0-9]+_log\.txt$", filename) is not None:
             file_object = self.upload_log
         else:
             file_object = (
diff --git a/lib/lp/charms/tests/test_charmrecipebuild.py b/lib/lp/charms/tests/test_charmrecipebuild.py
index 6a949db..ee7c786 100644
--- a/lib/lp/charms/tests/test_charmrecipebuild.py
+++ b/lib/lp/charms/tests/test_charmrecipebuild.py
@@ -272,7 +272,7 @@ class TestCharmRecipeBuild(TestCaseWithFactory):
 
     def test_getFileByName_uploaded_files(self):
         # getFileByName returns uploaded files when requested by name.
-        filenames = ("ubuntu.squashfs", "ubuntu.manifest")
+        filenames = ("ubuntu.squashfs", "ubuntu.manifest", "foo_log.txt")
         lfas = []
         for filename in filenames:
             lfa = self.factory.makeLibraryFileAlias(filename=filename)
diff --git a/lib/lp/code/model/cibuild.py b/lib/lp/code/model/cibuild.py
index 0629fd7..b1801f0 100644
--- a/lib/lp/code/model/cibuild.py
+++ b/lib/lp/code/model/cibuild.py
@@ -7,6 +7,7 @@ __all__ = [
     "CIBuild",
 ]
 
+import re
 from datetime import timedelta
 from operator import attrgetter, itemgetter
 
@@ -455,7 +456,7 @@ class CIBuild(PackageBuildMixin, StormBase):
         """See `ICIBuild`."""
         if filename.endswith(".txt.gz"):
             file_object = self.log
-        elif filename.endswith("_log.txt"):
+        elif re.match(r"^upload_[0-9]+_log\.txt$", filename) is not None:
             file_object = self.upload_log
         else:
             file_object = None
diff --git a/lib/lp/snappy/model/snapbuild.py b/lib/lp/snappy/model/snapbuild.py
index 05eddc9..96a02b4 100644
--- a/lib/lp/snappy/model/snapbuild.py
+++ b/lib/lp/snappy/model/snapbuild.py
@@ -6,6 +6,7 @@ __all__ = [
     "SnapFile",
 ]
 
+import re
 from datetime import timedelta
 from operator import attrgetter
 
@@ -342,7 +343,7 @@ class SnapBuild(PackageBuildMixin, Storm):
         """See `ISnapBuild`."""
         if filename.endswith(".txt.gz"):
             file_object = self.log
-        elif filename.endswith("_log.txt"):
+        elif re.match(r"^upload_[0-9]+_log\.txt$", filename) is not None:
             file_object = self.upload_log
         else:
             file_object = (
diff --git a/lib/lp/snappy/tests/test_snapbuild.py b/lib/lp/snappy/tests/test_snapbuild.py
index 33a1d1d..fc38dca 100644
--- a/lib/lp/snappy/tests/test_snapbuild.py
+++ b/lib/lp/snappy/tests/test_snapbuild.py
@@ -315,7 +315,7 @@ class TestSnapBuild(TestCaseWithFactory):
 
     def test_getFileByName_uploaded_files(self):
         # getFileByName returns uploaded files when requested by name.
-        filenames = ("ubuntu.squashfs", "ubuntu.manifest")
+        filenames = ("ubuntu.squashfs", "ubuntu.manifest", "foo_log.txt")
         lfas = []
         for filename in filenames:
             lfa = self.factory.makeLibraryFileAlias(filename=filename)
diff --git a/lib/lp/soyuz/model/binarypackagebuild.py b/lib/lp/soyuz/model/binarypackagebuild.py
index dea601c..2f6dbd3 100644
--- a/lib/lp/soyuz/model/binarypackagebuild.py
+++ b/lib/lp/soyuz/model/binarypackagebuild.py
@@ -12,6 +12,7 @@ __all__ = [
 ]
 
 import datetime
+import re
 import warnings
 from operator import attrgetter, itemgetter
 
@@ -774,7 +775,7 @@ class BinaryPackageBuild(PackageBuildMixin, SQLBase):
             file_object = self.upload_changesfile
         elif filename.endswith(".txt.gz"):
             file_object = self.log
-        elif filename.endswith("_log.txt"):
+        elif re.match(r"^upload_[0-9]+_log\.txt$", filename) is not None:
             file_object = self.upload_log
         elif filename.endswith("deb"):
             file_object = self._getDebByFileName(filename)
diff --git a/lib/lp/soyuz/model/livefsbuild.py b/lib/lp/soyuz/model/livefsbuild.py
index f912b5d..a179dd9 100644
--- a/lib/lp/soyuz/model/livefsbuild.py
+++ b/lib/lp/soyuz/model/livefsbuild.py
@@ -6,6 +6,7 @@ __all__ = [
     "LiveFSFile",
 ]
 
+import re
 from datetime import timedelta
 
 import pytz
@@ -285,7 +286,7 @@ class LiveFSBuild(PackageBuildMixin, Storm):
         """See `ILiveFSBuild`."""
         if filename.endswith(".txt.gz"):
             file_object = self.log
-        elif filename.endswith("_log.txt"):
+        elif re.match(r"^upload_[0-9]+_log\.txt$", filename) is not None:
             file_object = self.upload_log
         else:
             file_object = (
diff --git a/lib/lp/soyuz/tests/test_livefsbuild.py b/lib/lp/soyuz/tests/test_livefsbuild.py
index 83110e0..a36c986 100644
--- a/lib/lp/soyuz/tests/test_livefsbuild.py
+++ b/lib/lp/soyuz/tests/test_livefsbuild.py
@@ -318,7 +318,7 @@ class TestLiveFSBuild(TestCaseWithFactory):
 
     def test_getFileByName_uploaded_files(self):
         # getFileByName returns uploaded files when requested by name.
-        filenames = ("ubuntu.squashfs", "ubuntu.manifest")
+        filenames = ("ubuntu.squashfs", "ubuntu.manifest", "foo_log.txt")
         lfas = []
         for filename in filenames:
             lfa = self.factory.makeLibraryFileAlias(filename=filename)