Launchpad itself

Code review comment for lp:~michael.nelson/launchpad/594492-present-bfjs-in-builder-history

  1. 594492-present-bfjs-in-builder-history
  2. Merge into devel
Revision history for this message
Michael Nelson (michael.nelson) wrote :

Carried out an irc review with jtv yesterday:

http://irclogs.ubuntu.com/2010/06/16/%23launchpad-reviews.html#t14:53

The changes are pushed and attached from that discussion. Thanks jtv!

changes_jtv.diff
1=== modified file 'lib/lp/buildmaster/interfaces/buildfarmjob.py'
2--- lib/lp/buildmaster/interfaces/buildfarmjob.py 2010-06-15 16:17:17 +0000
3+++ lib/lp/buildmaster/interfaces/buildfarmjob.py 2010-06-16 14:12:20 +0000
4@@ -294,13 +294,14 @@
5
6
7 class IBuildFarmJobSet(Interface):
8- """A utility representing a set of package builds."""
9+ """A utility representing a set of build farm jobs."""
10
11 def getBuildsForBuilder(builder_id, status=None, user=None):
12 """Return `IBuildFarmJob` records touched by a builder.
13
14 :param builder_id: The id of the builder for which to find builds.
15- :param status: If status is provided, only builds with that status
16- will be returned.
17+ :param status: If given, limit the search to builds with this status.
18+ :param user: If given, this will be used to determine private builds
19+ that should be included.
20 :return: a `ResultSet` representing the requested builds.
21 """
22
23=== modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
24--- lib/lp/buildmaster/model/buildfarmjob.py 2010-06-16 11:25:18 +0000
25+++ lib/lp/buildmaster/model/buildfarmjob.py 2010-06-16 16:01:08 +0000
26@@ -16,7 +16,7 @@
27
28 import pytz
29
30-from storm.expr import And, Desc, Join, LeftJoin, Or, Select
31+from storm.expr import And, Coalesce, Desc, Join, LeftJoin, Select
32 from storm.locals import Bool, DateTime, Int, Reference, Storm
33 from storm.store import Store
34
35@@ -28,7 +28,7 @@
36 from canonical.database.constants import UTC_NOW
37 from canonical.database.enumcol import DBEnum
38 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
39-from canonical.launchpad.interfaces.lpstorm import IMasterStore
40+from canonical.launchpad.interfaces.lpstorm import IMasterStore, IStore
41 from canonical.launchpad.webapp.interfaces import (
42 DEFAULT_FLAVOR, IStoreSelector, MAIN_STORE)
43
44@@ -353,16 +353,19 @@
45
46 def getBuildsForBuilder(self, builder_id, status=None, user=None):
47 """See `IBuildFarmJobSet`."""
48- store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
49 # Imported here to avoid circular imports.
50 from lp.buildmaster.model.packagebuild import PackageBuild
51 from lp.soyuz.model.archive import Archive
52
53+ extra_clauses = [BuildFarmJob.builder == builder_id]
54+ if status is not None:
55+ extra_clauses.append(BuildFarmJob.status == status)
56+
57 # We need to ensure that we don't include any private builds.
58 # Currently only package builds can be private (via their
59 # related archive), but not all build farm jobs will have a
60 # related package build - hence the left join.
61- bfjs_with_optional_package_builds = LeftJoin(
62+ left_join_pkg_builds = LeftJoin(
63 BuildFarmJob,
64 Join(
65 PackageBuild,
66@@ -370,35 +373,41 @@
67 And(PackageBuild.archive == Archive.id)),
68 PackageBuild.build_farm_job == BuildFarmJob.id)
69
70- filtered_builds = store.using(bfjs_with_optional_package_builds).find(
71- BuildFarmJob,
72- BuildFarmJob.builder == builder_id)
73-
74- if status is not None:
75- filtered_builds = filtered_builds.find(
76- BuildFarmJob.status == status)
77-
78- # Anonymous users can only see public builds.
79+ filtered_builds = IStore(BuildFarmJob).using(
80+ left_join_pkg_builds).find(BuildFarmJob, *extra_clauses)
81+
82 if user is None:
83- filtered_builds = filtered_builds.find(
84- Or(Archive.private == None, Archive.private == False))
85-
86- # All other non-admins can additionally see private builds for
87- # which they are a member of the team owning the archive.
88- elif not user.inTeam(getUtility(ILaunchpadCelebrities).admin):
89+ # Anonymous requests don't get to see private builds at all.
90+ filtered_builds = filtered_builds.find(
91+ Coalesce(Archive.private, False) == False)
92+
93+ elif user.inTeam(getUtility(ILaunchpadCelebrities).admin):
94+ # Admins get to see everything.
95+ pass
96+ else:
97+ # Everyone else sees a union of all public builds and the
98+ # specific private builds to which they have access.
99+ filtered_builds = filtered_builds.find(
100+ Coalesce(Archive.private, False) == False)
101+
102 user_teams_subselect = Select(
103 TeamParticipation.teamID,
104 where=And(
105 TeamParticipation.personID == user.id,
106 TeamParticipation.teamID == Archive.ownerID))
107- filtered_builds = filtered_builds.find(
108- Or(
109- Archive.private == None,
110- Or(
111- Archive.private == False,
112- Archive.ownerID.is_in(user_teams_subselect))))
113-
114- filtered_builds.order_by(Desc(BuildFarmJob.date_finished), BuildFarmJob.id)
115+ private_builds_for_user = IStore(BuildFarmJob).find(
116+ BuildFarmJob,
117+ PackageBuild.build_farm_job == BuildFarmJob.id,
118+ PackageBuild.archive == Archive.id,
119+ Archive.private == True,
120+ Archive.ownerID.is_in(user_teams_subselect),
121+ *extra_clauses)
122+
123+ filtered_builds = filtered_builds.union(
124+ private_builds_for_user)
125+
126+ filtered_builds.order_by(
127+ Desc(BuildFarmJob.date_finished), BuildFarmJob.id)
128
129 return filtered_builds
130
131
132=== modified file 'lib/lp/buildmaster/tests/test_buildfarmjob.py'
133--- lib/lp/buildmaster/tests/test_buildfarmjob.py 2010-06-16 11:25:18 +0000
134+++ lib/lp/buildmaster/tests/test_buildfarmjob.py 2010-06-17 08:26:34 +0000
135@@ -24,28 +24,37 @@
136 BuildFarmJobType, IBuildFarmJob, IBuildFarmJobSet, IBuildFarmJobSource,
137 InconsistentBuildFarmJobError)
138 from lp.buildmaster.model.buildfarmjob import BuildFarmJob
139-from lp.testing import login, TestCaseWithFactory
140-
141-
142-class TestBuildFarmJobBase(TestCaseWithFactory):
143+from lp.testing import ANONYMOUS, login, login_person, TestCaseWithFactory
144+
145+
146+class TestBuildFarmJobMixin:
147
148 layer = DatabaseFunctionalLayer
149
150 def setUp(self):
151 """Create a build farm job with which to test."""
152- super(TestBuildFarmJobBase, self).setUp()
153+ super(TestBuildFarmJobMixin, self).setUp()
154 self.build_farm_job = self.makeBuildFarmJob()
155
156 def makeBuildFarmJob(self, builder=None,
157 job_type=BuildFarmJobType.PACKAGEBUILD,
158- status=BuildStatus.FULLYBUILT):
159+ status=BuildStatus.NEEDSBUILD,
160+ date_finished=None):
161+ """A factory method for creating PackageBuilds.
162+
163+ This is not included in the launchpad test factory because
164+ a build farm job should never be instantiated outside the
165+ context of a derived class (such as a BinaryPackageBuild
166+ or eventually a SPRecipeBuild).
167+ """
168 build_farm_job = getUtility(IBuildFarmJobSource).new(
169 job_type=job_type, status=status)
170 removeSecurityProxy(build_farm_job).builder = builder
171+ removeSecurityProxy(build_farm_job).date_finished = date_finished
172 return build_farm_job
173
174
175-class TestBuildFarmJob(TestBuildFarmJobBase):
176+class TestBuildFarmJob(TestBuildFarmJobMixin, TestCaseWithFactory):
177 """Tests for the build farm job object."""
178
179 def test_providesInterface(self):
180@@ -62,7 +71,6 @@
181 self.assertEqual(self.build_farm_job, retrieved_job)
182
183 def test_default_values(self):
184- # A build farm job defaults to the NEEDSBUILD status.
185 # We flush the database updates to ensure sql defaults
186 # are set for various attributes.
187 flush_database_updates()
188@@ -172,7 +180,7 @@
189 InconsistentBuildFarmJobError, self.build_farm_job.getSpecificJob)
190
191
192-class TestBuildFarmJobSecurity(TestBuildFarmJobBase):
193+class TestBuildFarmJobSecurity(TestBuildFarmJobMixin, TestCaseWithFactory):
194
195 def test_view_build_farm_job(self):
196 # Anonymous access can read public builds, but not edit.
197@@ -190,100 +198,102 @@
198 BuildStatus.FULLYBUILT, self.build_farm_job.status)
199
200
201-class TestBuildFarmJobSet(TestBuildFarmJobBase):
202+class TestBuildFarmJobSet(TestBuildFarmJobMixin, TestCaseWithFactory):
203
204 layer = LaunchpadFunctionalLayer
205
206 def setUp(self):
207 super(TestBuildFarmJobSet, self).setUp()
208 self.builder = self.factory.makeBuilder()
209- self.build_farm_jobs = []
210- self.build_farm_jobs.append(
211- self.makeBuildFarmJob(builder=self.builder))
212- self.build_farm_jobs.append(self.makeBuildFarmJob(
213- builder=self.builder,
214- job_type=BuildFarmJobType.RECIPEBRANCHBUILD))
215- self.build_farm_jobs.append(self.makeBuildFarmJob(
216- builder=self.builder, status=BuildStatus.BUILDING))
217-
218- # For good measure, create a different type of build that will
219- # also have an associated PackageBuild.
220- owning_team = self.factory.makeTeam()
221- archive = self.factory.makeArchive(owner=owning_team)
222- self.binary_package_build = self.factory.makeBinaryPackageBuild(
223- archive=archive, builder=self.builder)
224- self.build_farm_jobs.append(self.binary_package_build.build_farm_job)
225-
226 self.build_farm_job_set = getUtility(IBuildFarmJobSet)
227
228 def test_getBuildsForBuilder_all(self):
229 # The default call without arguments returns all builds for the
230 # builder, and not those for other builders.
231+ build1 = self.makeBuildFarmJob(builder=self.builder)
232+ build2 = self.makeBuildFarmJob(builder=self.builder)
233 self.makeBuildFarmJob(builder=self.factory.makeBuilder())
234- self.assertContentEqual(
235- self.build_farm_jobs, self.build_farm_job_set.getBuildsForBuilder(
236- self.builder))
237+ result = self.build_farm_job_set.getBuildsForBuilder(self.builder)
238+
239+ self.assertContentEqual([build1, build2], result)
240
241 def test_getBuildsForBuilder_by_status(self):
242 # If the status arg is used, the results will be filtered by
243 # status.
244- self.assertContentEqual(
245- self.build_farm_jobs[:2],
246- self.build_farm_job_set.getBuildsForBuilder(
247- self.builder, status=BuildStatus.FULLYBUILT))
248-
249- def makeBuildPrivate(self, build):
250- """Helper to privatise a package build."""
251- removeSecurityProxy(build.archive).buildd_secret = "blah"
252- removeSecurityProxy(build.archive).private = True
253+ successful_builds = [
254+ self.makeBuildFarmJob(
255+ builder=self.builder, status=BuildStatus.FULLYBUILT),
256+ self.makeBuildFarmJob(
257+ builder=self.builder, status=BuildStatus.FULLYBUILT),
258+ ]
259+ self.makeBuildFarmJob(builder=self.builder)
260+
261+ query_by_status = self.build_farm_job_set.getBuildsForBuilder(
262+ self.builder, status=BuildStatus.FULLYBUILT)
263+
264+ self.assertContentEqual(successful_builds, query_by_status)
265+
266+ def _makePrivateAndNonPrivateBuilds(self, owning_team=None):
267+ """Return a tuple of a private and non-private build farm job."""
268+ if owning_team is None:
269+ owning_team = self.factory.makeTeam()
270+ archive = self.factory.makeArchive(owner=owning_team, private=True)
271+ login_person(owning_team.teamowner)
272+ private_build = self.factory.makeBinaryPackageBuild(
273+ archive=archive, builder=self.builder)
274+ private_build = private_build.build_farm_job
275+ login(ANONYMOUS)
276+ other_build = self.makeBuildFarmJob(builder=self.builder)
277+ return (private_build, other_build)
278
279 def test_getBuildsForBuilder_hides_private_from_anon(self):
280 # If no user is passed, all private builds are filtered out.
281-
282- # When private it is not included for anon requests.
283- self.makeBuildPrivate(self.binary_package_build)
284+ private_build, other_build = self._makePrivateAndNonPrivateBuilds()
285 result = self.build_farm_job_set.getBuildsForBuilder(self.builder)
286- self.assertTrue(
287- self.binary_package_build.build_farm_job not in result)
288+ self.assertContentEqual([other_build], result)
289
290 def test_getBuildsForBuilder_hides_private_other_users(self):
291 # Private builds are not returned for users without permission
292 # to view them.
293- self.makeBuildPrivate(self.binary_package_build)
294+ private_build, other_build = self._makePrivateAndNonPrivateBuilds()
295 result = self.build_farm_job_set.getBuildsForBuilder(
296 self.builder, user=self.factory.makePerson())
297- self.assertTrue(
298- self.binary_package_build.build_farm_job not in result)
299+ self.assertContentEqual([other_build], result)
300
301 def test_getBuildsForBuilder_shows_private_to_admin(self):
302 # Admin users can see private builds.
303 admin_team = getUtility(ILaunchpadCelebrities).admin
304- self.makeBuildPrivate(self.binary_package_build)
305+ private_build, other_build = self._makePrivateAndNonPrivateBuilds()
306 result = self.build_farm_job_set.getBuildsForBuilder(
307 self.builder, user=admin_team.teamowner)
308- self.assertTrue(self.binary_package_build.build_farm_job in result)
309+ self.assertContentEqual([private_build, other_build], result)
310
311 def test_getBuildsForBuilder_shows_private_to_authorised(self):
312 # Similarly, if the user is in the owning team they can see it.
313- self.makeBuildPrivate(self.binary_package_build)
314+ owning_team = self.factory.makeTeam()
315+ private_build, other_build = self._makePrivateAndNonPrivateBuilds(
316+ owning_team=owning_team)
317 result = self.build_farm_job_set.getBuildsForBuilder(
318 self.builder,
319- user=self.binary_package_build.archive.owner.teamowner)
320- self.assertTrue(self.binary_package_build.build_farm_job in result)
321+ user=owning_team.teamowner)
322+ self.assertContentEqual([private_build, other_build], result)
323
324 def test_getBuildsForBuilder_ordered_by_date_finished(self):
325 # Results are returned with the oldest build last.
326- naked_build_0 = removeSecurityProxy(self.build_farm_jobs[0])
327- naked_build_1 = removeSecurityProxy(self.build_farm_jobs[1])
328-
329- naked_build_0.date_finished = datetime(2008, 10, 10, tzinfo=pytz.UTC)
330- naked_build_1.date_finished = datetime(2008, 11, 10, tzinfo=pytz.UTC)
331- result = self.build_farm_job_set.getBuildsForBuilder(self.builder)
332- self.assertEqual(self.build_farm_jobs[0], result[3])
333-
334- naked_build_0.date_finished = datetime(2008, 12, 10, tzinfo=pytz.UTC)
335- result = self.build_farm_job_set.getBuildsForBuilder(self.builder)
336- self.assertEqual(self.build_farm_jobs[1], result[3])
337+ build_1 = self.makeBuildFarmJob(
338+ builder=self.builder,
339+ date_finished=datetime(2008, 10, 10, tzinfo=pytz.UTC))
340+ build_2 = self.makeBuildFarmJob(
341+ builder=self.builder,
342+ date_finished=datetime(2008, 11, 10, tzinfo=pytz.UTC))
343+
344+ result = self.build_farm_job_set.getBuildsForBuilder(self.builder)
345+ self.assertEqual([build_2, build_1], list(result))
346+
347+ removeSecurityProxy(build_2).date_finished = (
348+ datetime(2008, 8, 10, tzinfo=pytz.UTC))
349+ result = self.build_farm_job_set.getBuildsForBuilder(self.builder)
350+ self.assertEqual([build_1, build_2], list(result))
351
352
353 def test_suite():

« Back to merge proposal