Launchpad itself

Merge lp:~michael.nelson/launchpad/580181-only-one-ds-cache into lp:launchpad

  1. 580181-only-one-ds-cache
  2. Merge into devel
Proposed by Michael Nelson
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merge reported by: Michael Nelson
Merged at revision: not available
Proposed branch: lp:~michael.nelson/launchpad/580181-only-one-ds-cache
Merge into: lp:launchpad
Diff against target: 185 lines (+142/-7)
4 files modified
lib/lp/soyuz/browser/tests/test_sourcepackagerelease.py (+59/-0)
lib/lp/soyuz/model/distroseriesbinarypackage.py (+9/-7)
lib/lp/soyuz/templates/sourcepackagerelease-files.pt (+5/-0)
lib/lp/soyuz/tests/test_distroseriesbinarypackage.py (+69/-0)
To merge this branch: bzr merge lp:~michael.nelson/launchpad/580181-only-one-ds-cache
Reviewer Review Type Date Requested Status
Curtis Hovey (community) release-critical code Approve
Canonical Launchpad Engineering Pending
Review via email: mp+26329@code.launchpad.net

Commit message

Deleted files for a source package release do not cause oops when rendering template.

Description of the change

This is a fix for bug 580181 which is causing ~700 or so oopses per day currently.

It tests and fixes both the NotOneError: one() and LocationError: (None, 'filesize') oopses mentioned on the bug report.

To test:
bin/test -vv -m browser.tests.test_sourcepackagerelease -m test_distroseriesbinarypackage

I assume this will be an RC candidate due to the frequency of the oopses.

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

I am so happy to see this fix. You are awarded a gold start, and Approve, and an RC.

review: Approve (release-critical code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== added file 'lib/lp/soyuz/browser/tests/test_sourcepackagerelease.py'
--- lib/lp/soyuz/browser/tests/test_sourcepackagerelease.py 1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/browser/tests/test_sourcepackagerelease.py 2010-05-29 04:41:26 +0000
@@ -0,0 +1,59 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4# pylint: disable-msg=F0401
5
6"""Unit tests for TestSourcePackageReleaseFiles."""
7
8__metaclass__ = type
9__all__ = [
10 'TestSourcePackageReleaseFiles',
11 'test_suite',
12 ]
13
14import unittest
15
16from zope.security.proxy import removeSecurityProxy
17
18from canonical.testing import LaunchpadFunctionalLayer
19from lp.testing import TestCaseWithFactory
20from lp.testing.views import create_initialized_view
21
22
23class TestSourcePackageReleaseFiles(TestCaseWithFactory):
24 """Source package release files are rendered correctly."""
25
26 layer = LaunchpadFunctionalLayer
27
28 def setUp(self):
29 super(TestSourcePackageReleaseFiles, self).setUp()
30 self.source_package_release = self.factory.makeSourcePackageRelease()
31
32 def test_spr_files_none(self):
33 # The snippet renders appropriately when there are no files.
34 view = create_initialized_view(self.source_package_release, "+files")
35 html = view.__call__()
36 self.failUnless('No files available for download.' in html)
37
38 def test_spr_files_one(self):
39 # The snippet links to the file when present.
40 library_file = self.factory.makeLibraryFileAlias(
41 filename='test_file.dsc', content='0123456789')
42 self.source_package_release.addFile(library_file)
43 view = create_initialized_view(self.source_package_release, "+files")
44 html = view.__call__()
45 self.failUnless('test_file.dsc' in html)
46
47 def test_spr_files_deleted(self):
48 # The snippet handles deleted files too.
49 library_file = self.factory.makeLibraryFileAlias(
50 filename='test_file.dsc', content='0123456789')
51 self.source_package_release.addFile(library_file)
52 removeSecurityProxy(library_file).content = None
53 view = create_initialized_view(self.source_package_release, "+files")
54 html = view.__call__()
55 self.failUnless('test_file.dsc (deleted)' in html)
56
57
58def test_suite():
59 return unittest.TestLoader().loadTestsFromName(__name__)
060
=== modified file 'lib/lp/soyuz/model/distroseriesbinarypackage.py'
--- lib/lp/soyuz/model/distroseriesbinarypackage.py 2009-06-25 04:06:00 +0000
+++ lib/lp/soyuz/model/distroseriesbinarypackage.py 2010-05-29 04:41:26 +0000
@@ -61,14 +61,16 @@
61 @cachedproperty('_cache')61 @cachedproperty('_cache')
62 def cache(self):62 def cache(self):
63 """See IDistroSeriesBinaryPackage."""63 """See IDistroSeriesBinaryPackage."""
64 return DistroSeriesPackageCache.selectOne("""64 store = Store.of(self.distroseries)
65 distroseries = %s AND65 archive_ids = (
66 archive IN %s AND66 self.distroseries.distribution.all_distro_archive_ids)
67 binarypackagename = %s67 result = store.find(
68 """ % sqlvalues(68 DistroSeriesPackageCache,
69 self.distroseries,69 DistroSeriesPackageCache.distroseries==self.distroseries,
70 self.distroseries.distribution.all_distro_archive_ids,70 DistroSeriesPackageCache.archiveID.is_in(archive_ids),
71 (DistroSeriesPackageCache.binarypackagename==
71 self.binarypackagename))72 self.binarypackagename))
73 return result.any()
7274
73 @property75 @property
74 def summary(self):76 def summary(self):
7577
=== modified file 'lib/lp/soyuz/templates/sourcepackagerelease-files.pt'
--- lib/lp/soyuz/templates/sourcepackagerelease-files.pt 2009-09-15 16:52:05 +0000
+++ lib/lp/soyuz/templates/sourcepackagerelease-files.pt 2010-05-29 04:41:26 +0000
@@ -5,10 +5,15 @@
5 omit-tag="">5 omit-tag="">
6 <ul tal:condition="context/files">6 <ul tal:condition="context/files">
7 <li tal:repeat="file context/files">7 <li tal:repeat="file context/files">
8 <tal:file_available condition="not:file/libraryfile/deleted">
8 <a class="sprite download"9 <a class="sprite download"
9 tal:content="file/libraryfile/filename"10 tal:content="file/libraryfile/filename"
10 tal:attributes="href string:${context/fmt:url}/+files/${file/libraryfile/filename}" />11 tal:attributes="href string:${context/fmt:url}/+files/${file/libraryfile/filename}" />
11 (<span tal:replace="file/libraryfile/content/filesize/fmt:bytes" />)12 (<span tal:replace="file/libraryfile/content/filesize/fmt:bytes" />)
13 </tal:file_available>
14 <tal:file_unavailable condition="file/libraryfile/deleted">
15 <span tal:replace="file/libraryfile/filename">foo.dsc</span> (deleted)
16 </tal:file_unavailable>
12 </li>17 </li>
13 </ul>18 </ul>
14 <p tal:condition="not:context/files">No files available for download.</p>19 <p tal:condition="not:context/files">No files available for download.</p>
1520
=== added file 'lib/lp/soyuz/tests/test_distroseriesbinarypackage.py'
--- lib/lp/soyuz/tests/test_distroseriesbinarypackage.py 1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/tests/test_distroseriesbinarypackage.py 2010-05-29 04:41:26 +0000
@@ -0,0 +1,69 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Tests for `DistroSeriesBinaryPackage`."""
5
6__metaclass__ = type
7__all__ = [
8 'TestDistroSeriesBinaryPackage',
9 'test_suite',
10 ]
11
12import unittest
13import transaction
14
15from lp.soyuz.model.distroseriesbinarypackage import (
16 DistroSeriesBinaryPackage)
17from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
18from lp.testing import TestCaseWithFactory
19from canonical.config import config
20from canonical.launchpad.scripts.logger import QuietFakeLogger
21from canonical.testing import LaunchpadZopelessLayer
22
23
24class TestDistroSeriesBinaryPackage(TestCaseWithFactory):
25
26 layer = LaunchpadZopelessLayer
27
28 def setUp(self):
29 """Create a distroseriesbinarypackage to play with."""
30 super(TestDistroSeriesBinaryPackage, self).setUp()
31 self.publisher = SoyuzTestPublisher()
32 self.publisher.prepareBreezyAutotest()
33 self.distroseries = self.publisher.distroseries
34 self.distribution = self.distroseries.distribution
35 binaries = self.publisher.getPubBinaries(
36 binaryname='foo-bin', summary='Foo is the best')
37 binary_pub = binaries[0]
38 self.binary_package_name = (
39 binary_pub.binarypackagerelease.binarypackagename)
40 self.distroseries_binary_package = DistroSeriesBinaryPackage(
41 self.distroseries, self.binary_package_name)
42
43 def test_cache_attribute_when_two_cache_objects(self):
44 # We have situations where there are cache objects for each
45 # distro archive - we need to handle this situation without
46 # OOPSing - see bug 580181.
47 distro_archive_1 = self.distribution.main_archive
48 distro_archive_2 = self.distribution.all_distro_archives[1]
49
50 # Publish the same binary in another distro archive.
51 self.publisher.getPubBinaries(
52 binaryname='foo-bin', summary='Foo is the best',
53 archive=distro_archive_2)
54
55 logger = QuietFakeLogger()
56 transaction.commit()
57 LaunchpadZopelessLayer.switchDbUser(config.statistician.dbuser)
58 self.distroseries.updatePackageCache(
59 self.binary_package_name, distro_archive_1, logger)
60
61 self.distroseries.updatePackageCache(
62 self.binary_package_name, distro_archive_2, logger)
63
64 self.failUnlessEqual(
65 'Foo is the best', self.distroseries_binary_package.summary)
66
67
68def test_suite():
69 return unittest.TestLoader().loadTestsFromName(__name__)