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

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
1=== added file 'lib/lp/soyuz/browser/tests/test_sourcepackagerelease.py'
2--- lib/lp/soyuz/browser/tests/test_sourcepackagerelease.py 1970-01-01 00:00:00 +0000
3+++ lib/lp/soyuz/browser/tests/test_sourcepackagerelease.py 2010-05-29 04:41:26 +0000
4@@ -0,0 +1,59 @@
5+# Copyright 2010 Canonical Ltd. This software is licensed under the
6+# GNU Affero General Public License version 3 (see the file LICENSE).
7+
8+# pylint: disable-msg=F0401
9+
10+"""Unit tests for TestSourcePackageReleaseFiles."""
11+
12+__metaclass__ = type
13+__all__ = [
14+ 'TestSourcePackageReleaseFiles',
15+ 'test_suite',
16+ ]
17+
18+import unittest
19+
20+from zope.security.proxy import removeSecurityProxy
21+
22+from canonical.testing import LaunchpadFunctionalLayer
23+from lp.testing import TestCaseWithFactory
24+from lp.testing.views import create_initialized_view
25+
26+
27+class TestSourcePackageReleaseFiles(TestCaseWithFactory):
28+ """Source package release files are rendered correctly."""
29+
30+ layer = LaunchpadFunctionalLayer
31+
32+ def setUp(self):
33+ super(TestSourcePackageReleaseFiles, self).setUp()
34+ self.source_package_release = self.factory.makeSourcePackageRelease()
35+
36+ def test_spr_files_none(self):
37+ # The snippet renders appropriately when there are no files.
38+ view = create_initialized_view(self.source_package_release, "+files")
39+ html = view.__call__()
40+ self.failUnless('No files available for download.' in html)
41+
42+ def test_spr_files_one(self):
43+ # The snippet links to the file when present.
44+ library_file = self.factory.makeLibraryFileAlias(
45+ filename='test_file.dsc', content='0123456789')
46+ self.source_package_release.addFile(library_file)
47+ view = create_initialized_view(self.source_package_release, "+files")
48+ html = view.__call__()
49+ self.failUnless('test_file.dsc' in html)
50+
51+ def test_spr_files_deleted(self):
52+ # The snippet handles deleted files too.
53+ library_file = self.factory.makeLibraryFileAlias(
54+ filename='test_file.dsc', content='0123456789')
55+ self.source_package_release.addFile(library_file)
56+ removeSecurityProxy(library_file).content = None
57+ view = create_initialized_view(self.source_package_release, "+files")
58+ html = view.__call__()
59+ self.failUnless('test_file.dsc (deleted)' in html)
60+
61+
62+def test_suite():
63+ return unittest.TestLoader().loadTestsFromName(__name__)
64
65=== modified file 'lib/lp/soyuz/model/distroseriesbinarypackage.py'
66--- lib/lp/soyuz/model/distroseriesbinarypackage.py 2009-06-25 04:06:00 +0000
67+++ lib/lp/soyuz/model/distroseriesbinarypackage.py 2010-05-29 04:41:26 +0000
68@@ -61,14 +61,16 @@
69 @cachedproperty('_cache')
70 def cache(self):
71 """See IDistroSeriesBinaryPackage."""
72- return DistroSeriesPackageCache.selectOne("""
73- distroseries = %s AND
74- archive IN %s AND
75- binarypackagename = %s
76- """ % sqlvalues(
77- self.distroseries,
78- self.distroseries.distribution.all_distro_archive_ids,
79+ store = Store.of(self.distroseries)
80+ archive_ids = (
81+ self.distroseries.distribution.all_distro_archive_ids)
82+ result = store.find(
83+ DistroSeriesPackageCache,
84+ DistroSeriesPackageCache.distroseries==self.distroseries,
85+ DistroSeriesPackageCache.archiveID.is_in(archive_ids),
86+ (DistroSeriesPackageCache.binarypackagename==
87 self.binarypackagename))
88+ return result.any()
89
90 @property
91 def summary(self):
92
93=== modified file 'lib/lp/soyuz/templates/sourcepackagerelease-files.pt'
94--- lib/lp/soyuz/templates/sourcepackagerelease-files.pt 2009-09-15 16:52:05 +0000
95+++ lib/lp/soyuz/templates/sourcepackagerelease-files.pt 2010-05-29 04:41:26 +0000
96@@ -5,10 +5,15 @@
97 omit-tag="">
98 <ul tal:condition="context/files">
99 <li tal:repeat="file context/files">
100+ <tal:file_available condition="not:file/libraryfile/deleted">
101 <a class="sprite download"
102 tal:content="file/libraryfile/filename"
103 tal:attributes="href string:${context/fmt:url}/+files/${file/libraryfile/filename}" />
104 (<span tal:replace="file/libraryfile/content/filesize/fmt:bytes" />)
105+ </tal:file_available>
106+ <tal:file_unavailable condition="file/libraryfile/deleted">
107+ <span tal:replace="file/libraryfile/filename">foo.dsc</span> (deleted)
108+ </tal:file_unavailable>
109 </li>
110 </ul>
111 <p tal:condition="not:context/files">No files available for download.</p>
112
113=== added file 'lib/lp/soyuz/tests/test_distroseriesbinarypackage.py'
114--- lib/lp/soyuz/tests/test_distroseriesbinarypackage.py 1970-01-01 00:00:00 +0000
115+++ lib/lp/soyuz/tests/test_distroseriesbinarypackage.py 2010-05-29 04:41:26 +0000
116@@ -0,0 +1,69 @@
117+# Copyright 2010 Canonical Ltd. This software is licensed under the
118+# GNU Affero General Public License version 3 (see the file LICENSE).
119+
120+"""Tests for `DistroSeriesBinaryPackage`."""
121+
122+__metaclass__ = type
123+__all__ = [
124+ 'TestDistroSeriesBinaryPackage',
125+ 'test_suite',
126+ ]
127+
128+import unittest
129+import transaction
130+
131+from lp.soyuz.model.distroseriesbinarypackage import (
132+ DistroSeriesBinaryPackage)
133+from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
134+from lp.testing import TestCaseWithFactory
135+from canonical.config import config
136+from canonical.launchpad.scripts.logger import QuietFakeLogger
137+from canonical.testing import LaunchpadZopelessLayer
138+
139+
140+class TestDistroSeriesBinaryPackage(TestCaseWithFactory):
141+
142+ layer = LaunchpadZopelessLayer
143+
144+ def setUp(self):
145+ """Create a distroseriesbinarypackage to play with."""
146+ super(TestDistroSeriesBinaryPackage, self).setUp()
147+ self.publisher = SoyuzTestPublisher()
148+ self.publisher.prepareBreezyAutotest()
149+ self.distroseries = self.publisher.distroseries
150+ self.distribution = self.distroseries.distribution
151+ binaries = self.publisher.getPubBinaries(
152+ binaryname='foo-bin', summary='Foo is the best')
153+ binary_pub = binaries[0]
154+ self.binary_package_name = (
155+ binary_pub.binarypackagerelease.binarypackagename)
156+ self.distroseries_binary_package = DistroSeriesBinaryPackage(
157+ self.distroseries, self.binary_package_name)
158+
159+ def test_cache_attribute_when_two_cache_objects(self):
160+ # We have situations where there are cache objects for each
161+ # distro archive - we need to handle this situation without
162+ # OOPSing - see bug 580181.
163+ distro_archive_1 = self.distribution.main_archive
164+ distro_archive_2 = self.distribution.all_distro_archives[1]
165+
166+ # Publish the same binary in another distro archive.
167+ self.publisher.getPubBinaries(
168+ binaryname='foo-bin', summary='Foo is the best',
169+ archive=distro_archive_2)
170+
171+ logger = QuietFakeLogger()
172+ transaction.commit()
173+ LaunchpadZopelessLayer.switchDbUser(config.statistician.dbuser)
174+ self.distroseries.updatePackageCache(
175+ self.binary_package_name, distro_archive_1, logger)
176+
177+ self.distroseries.updatePackageCache(
178+ self.binary_package_name, distro_archive_2, logger)
179+
180+ self.failUnlessEqual(
181+ 'Foo is the best', self.distroseries_binary_package.summary)
182+
183+
184+def test_suite():
185+ return unittest.TestLoader().loadTestsFromName(__name__)