Merge lp:~wgrant/launchpad/bug-522800-getFileByName into lp:launchpad

[William Grant]

Archive.getFileByName now returns only unexpired files.

Archive.getFileByName currently doesn't care if the file it returns is expired. That isn't excellent, since an expired file is just about useless. This branch fixes it to only return unexpired files, fixing bug #522800. It also happens to fix bug #687662, a 10.12 regression that was just discovered.

[Julian Edwards (julian-edwards)]

Turn the doctest change into a unit test and r=me

review: Approve

[Julian Edwards (julian-edwards)]

Looks good, I shall land it.

[Julian Edwards (julian-edwards)]

Except - there's a failing test!
Please fix xx-ppa-files.txt

kthxbye

review: Needs Fixing

[William Grant (wgrant)]

The test is fixed.

[Julian Edwards (julian-edwards)]

review: Approve

=== renamed file 'lib/lp/soyuz/doc/archive-files.txt' => 'lib/lp/soyuz/doc/build-files.txt'
--- lib/lp/soyuz/doc/archive-files.txt	2010-10-18 22:24:59 +0000
+++ lib/lp/soyuz/doc/build-files.txt	2010-12-10 00:12:15 +0000
@@ -1,157 +1,28 @@
-= Archive files =
+= Build files =
 
-`IArchive` provides a method that looks up files involved in the context
-archive, getFileByName()
+Create a test build.
 
     >>> from lp.registry.interfaces.person import IPersonSet
-    >>> cprov = getUtility(IPersonSet).getByName('cprov')
-
-The given filename is looked up in the context corresponding to its
-extension:
-
- * Source files: '.orig.tar.gz', 'tar.gz', '.diff.gz' and '.dsc';
- * Binary files: '.deb' and '.udeb';
- * Source changesfile: '_source.changes';
- * Package diffs: '.diff.gz';
-
-Any other extension will result in a `NotFoundError`.
-
-    >>> cprov.archive.getFileByName('biscuit.cookie')
-    Traceback (most recent call last):
-    ...
-    NotFoundError: 'biscuit.cookie'
-
-Create a test source publication.
-
     >>> from lp.soyuz.tests.test_publishing import (
     ...      SoyuzTestPublisher)
 
     >>> login('[e-mail]')
 
+    >>> cprov = getUtility(IPersonSet).getByName('cprov')
     >>> test_publisher = SoyuzTestPublisher()
     >>> test_publisher.prepareBreezyAutotest()
     >>> test_publisher.addFakeChroots()
     >>> test_source = test_publisher.getPubSource(
     ...     archive=cprov.archive, sourcename='test-pkg',
     ...     version='1.0')
-
-When the file exists, the corresponding `ILibraryFileAlias` is returned.
-
-    >>> from canonical.launchpad.interfaces.librarian import ILibraryFileAlias
-    >>> from canonical.launchpad.webapp.testing import verifyObject
-
-    >>> result = cprov.archive.getFileByName('test-pkg_1.0.dsc')
-
-    >>> verifyObject(ILibraryFileAlias, result)
-    True
-
-Otherwise a `NotFoundError` is raised.
-
-    >>> cprov.archive.getFileByName('test-pkg_1.0.orig.tar.gz')
-    Traceback (most recent call last):
-    ...
-    NotFoundError: 'test-pkg_1.0.orig.tar.gz'
-
-Adding an original source tarball.
-
-    >>> lost_orig_tar_gz = test_publisher.addMockFile(
-    ...     'test-pkg_1.0.orig.tar.gz')
-
-NotFoundError still being raised if the file exists but is not
-attached to the IArchive context.
-
-    >>> cprov.archive.getFileByName('test-pkg_1.0.orig.tar.gz')
-    Traceback (most recent call last):
-    ...
-    NotFoundError: 'test-pkg_1.0.orig.tar.gz'
-
-Attached a new file to Celso's PPA context.
-
-    >>> orig_tar_gz = test_publisher.addMockFile(
-    ...     'test-pkg_1.0.orig.tar.gz')
-    >>> unused = test_source.sourcepackagerelease.addFile(orig_tar_gz)
-
-The retrieved file is the one attached to Celso's PPA and not the old
-one.
-
-    >>> libraryfile = cprov.archive.getFileByName('test-pkg_1.0.orig.tar.gz')
-
-    >>> libraryfile == lost_orig_tar_gz
-    False
-
-    >>> libraryfile == orig_tar_gz
-    True
-
-Adding and retrieving a source diff.
-
-    >>> diff_gz = test_publisher.addMockFile('test-pkg_1.0.diff.gz')
-    >>> unused = test_source.sourcepackagerelease.addFile(diff_gz)
-
-    >>> cprov.archive.getFileByName('test-pkg_1.0.diff.gz') == diff_gz
-    True
-
-Adding and retrieving a binary 'deb'.
-
     >>> binary_pubs = test_publisher.getPubBinaries(
     ...     binaryname='test', pub_source=test_source)
     >>> deb = binary_pubs[0].binarypackagerelease.files[0].libraryfile
-
-    >>> deb == cprov.archive.getFileByName('test_1.0_all.deb')
-    True
-
-Adding and retrieving a binary 'udeb'.
-
     >>> [build] = test_source.getBuilds()
-    >>> binary = test_publisher.uploadBinaryForBuild(
-    ...     build, binaryname='micro-test')
-    >>> unused = test_publisher.publishBinaryInArchive(binary, cprov.archive)
-
-    >>> from zope.security.proxy import removeSecurityProxy
-    >>> bin_file = removeSecurityProxy(binary.files[0].libraryfile)
-    >>> bin_file.filename = 'micro-test_1.0_all.udeb'
-
-    >>> bin_file == cprov.archive.getFileByName('micro-test_1.0_all.udeb')
-    True
-
-Adding and retrieving a source changesfile.
-
-    >>> from lp.registry.interfaces.pocket import (
-    ...     PackagePublishingPocket)
-    >>> source_upload = test_source.distroseries.createQueueEntry(
-    ...     pocket=PackagePublishingPocket.RELEASE,
-    ...     changesfilename='test-pkg_1.0_source.changes',
-    ...     changesfilecontent='Bogus',
-    ...     archive=cprov.archive)
-    >>> unused = source_upload.addSource(test_source.sourcepackagerelease)
-    >>> source_upload.setDone()
-
-    >>> cprov.archive.getFileByName('test-pkg_1.0_source.changes')
-    <LibraryFileAlias ...>
-
-Adding and retrieving a package-diff.
-
-    >>> another_test_source = test_publisher.getPubSource(
-    ...     archive=cprov.archive, sourcename='test-pkg',
-    ...     version='1.1')
-
-    >>> print another_test_source.sourcepackagerelease.package_diffs.count()
-    0
-
-    >>> package_diff  = test_source.sourcepackagerelease.requestDiffTo(
-    ...     cprov, another_test_source.sourcepackagerelease)
-
-    >>> print another_test_source.sourcepackagerelease.package_diffs.count()
-    1
-
-    >>> diff_name = 'test-pkg_1.0_1.1.diff.gz'
-    >>> diff = test_publisher.addMockFile(diff_name)
-    >>> package_diff.diff_content = diff
-
-    >>> diff == cprov.archive.getFileByName(diff_name)
-    True
-
-Similarly, IBuild provide a getFileByName() method, which retuns one
-of the following file type in its context.
+
+
+IBuild provide a getFileByName() method, which retuns one of the
+following file type in its context.
 
  * Binary changesfile: '.changes';
  * Build logs: '.txt.gz';

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2010-12-03 17:35:33 +0000
+++ lib/lp/soyuz/model/archive.py	2010-12-10 00:12:15 +0000
@@ -1311,6 +1311,7 @@
 
         base_clauses = (
             LibraryFileAlias.filename == filename,
+            LibraryFileAlias.content != None,
             )
 
         if re_issource.match(filename):

=== modified file 'lib/lp/soyuz/stories/ppa/xx-ppa-files.txt'
--- lib/lp/soyuz/stories/ppa/xx-ppa-files.txt	2010-11-06 12:50:22 +0000
+++ lib/lp/soyuz/stories/ppa/xx-ppa-files.txt	2010-12-10 00:12:15 +0000
@@ -465,7 +465,7 @@
     ...
     Include the error ID OOPS-...
     ...
-    DeletedProxiedLibraryFileAlias:
-    Object: u'test-pkg_1.0.dsc', name: &lt;Archive at
+    NotFound:
+    Object: &lt;Archive at ...&gt;, name: u'test-pkg_1.0.dsc'...
     ...
 

=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py	2010-12-03 17:35:33 +0000
+++ lib/lp/soyuz/tests/test_archive.py	2010-12-10 00:12:15 +0000
@@ -21,6 +21,7 @@
     DatabaseFunctionalLayer,
     LaunchpadZopelessLayer,
     )
+from lp.app.errors import NotFoundError
 from lp.buildmaster.enums import BuildStatus
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.series import SeriesStatus
@@ -1505,3 +1506,102 @@
         archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
         self.assertEqual(
             [PackagePublishingPocket.RELEASE], archive.getPockets())
+
+
+class TestGetFileByName(TestCaseWithFactory):
+    """Tests for Archive.getFileByName."""
+
+    layer = LaunchpadZopelessLayer
+
+    def setUp(self):
+        super(TestGetFileByName, self).setUp()
+        self.archive = self.factory.makeArchive()
+
+    def test_unknown_file_is_not_found(self):
+        # A file with an unsupported extension is not found.
+        self.assertRaises(NotFoundError, self.archive.getFileByName, 'a.bar')
+
+    def test_source_file_is_found(self):
+        # A file from a published source package can be retrieved.
+        pub = self.factory.makeSourcePackagePublishingHistory(
+            archive=self.archive)
+        dsc = self.factory.makeLibraryFileAlias(filename='foo_1.0.dsc')
+        self.assertRaises(
+            NotFoundError, self.archive.getFileByName, dsc.filename)
+        pub.sourcepackagerelease.addFile(dsc)
+        self.assertEquals(dsc, self.archive.getFileByName(dsc.filename))
+
+    def test_nonexistent_source_file_is_not_found(self):
+        # Something that looks like a source file but isn't is not
+        # found.
+        self.assertRaises(
+            NotFoundError, self.archive.getFileByName, 'foo_1.0.dsc')
+
+    def test_binary_file_is_found(self):
+        # A file from a published binary package can be retrieved.
+        pub = self.factory.makeBinaryPackagePublishingHistory(
+            archive=self.archive)
+        deb = self.factory.makeLibraryFileAlias(filename='foo_1.0_all.deb')
+        self.assertRaises(
+            NotFoundError, self.archive.getFileByName, deb.filename)
+        pub.binarypackagerelease.addFile(deb)
+        self.assertEquals(deb, self.archive.getFileByName(deb.filename))
+
+    def test_nonexistent_binary_file_is_not_found(self):
+        # Something that looks like a binary file but isn't is not
+        # found.
+        self.assertRaises(
+            NotFoundError, self.archive.getFileByName, 'foo_1.0_all.deb')
+
+    def test_source_changes_file_is_found(self):
+        # A .changes file from a published source can be retrieved.
+        pub = self.factory.makeSourcePackagePublishingHistory(
+            archive=self.archive)
+        pu = self.factory.makePackageUpload(
+            changes_filename='foo_1.0_source.changes')
+        pu.setDone()
+        self.assertRaises(
+            NotFoundError, self.archive.getFileByName,
+            pu.changesfile.filename)
+        pu.addSource(pub.sourcepackagerelease)
+        self.assertEquals(
+            pu.changesfile,
+            self.archive.getFileByName(pu.changesfile.filename))
+
+    def test_nonexistent_source_changes_file_is_not_found(self):
+        # Something that looks like a source .changes file but isn't is not
+        # found.
+        self.assertRaises(
+            NotFoundError, self.archive.getFileByName,
+            'foo_1.0_source.changes')
+
+    def test_package_diff_is_found(self):
+        # A .diff.gz from a package diff can be retrieved.
+        pub = self.factory.makeSourcePackagePublishingHistory(
+            archive=self.archive)
+        diff = self.factory.makePackageDiff(
+            to_source=pub.sourcepackagerelease,
+            diff_filename='foo_1.0.diff.gz')
+        self.assertEquals(
+            diff.diff_content,
+            self.archive.getFileByName(diff.diff_content.filename))
+
+    def test_expired_files_are_skipped(self):
+        # Expired files are ignored.
+        pub = self.factory.makeSourcePackagePublishingHistory(
+            archive=self.archive)
+        dsc = self.factory.makeLibraryFileAlias(filename='foo_1.0.dsc')
+        pub.sourcepackagerelease.addFile(dsc)
+
+        # The file is initially found without trouble.
+        self.assertEquals(dsc, self.archive.getFileByName(dsc.filename))
+
+        # But after expiry it is not.
+        removeSecurityProxy(dsc).content = None
+        self.assertRaises(
+            NotFoundError, self.archive.getFileByName, dsc.filename)
+
+        # It reappears if we create a new one.
+        new_dsc = self.factory.makeLibraryFileAlias(filename=dsc.filename)
+        pub.sourcepackagerelease.addFile(new_dsc)
+        self.assertEquals(new_dsc, self.archive.getFileByName(dsc.filename))

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-12-08 19:03:17 +0000
+++ lib/lp/testing/factory.py	2010-12-10 00:12:15 +0000
@@ -3270,7 +3270,7 @@
 
     def makePackageDiff(self, from_source=None, to_source=None,
                         requester=None, status=None, date_fulfilled=None,
-                        diff_content=None):
+                        diff_content=None, diff_filename=None):
         """Create a new `PackageDiff`."""
         if from_source is None:
             from_source = self.makeSourcePackageRelease()
@@ -3284,7 +3284,8 @@
             date_fulfilled = UTC_NOW
         if diff_content is None:
             diff_content = self.getUniqueString("packagediff")
-        lfa = self.makeLibraryFileAlias(content=diff_content)
+        lfa = self.makeLibraryFileAlias(
+            filename=diff_filename, content=diff_content)
         return ProxyFactory(
             PackageDiff(
                 requester=requester, from_source=from_source,