Merge lp:~wallyworld/launchpad/recent-recipe-build-query into lp:launchpad

[Ian Booth]

[r=thumper][ui=none][bug=690035] Rework the query used to load the most recently completed daily recipe builds.

= Summary =

Quick fix to correct the query used to load most recently completed source package recipes.

= Implementation =

The Storm find query was modified to produce the correct joins and group by the correct columns. The SourcePackageRecipeBuild in the group by was replaced with SourcePackageRecipe. Tweaks also to the page template and RecipeBuildRecord named tuple to reference the changed data attribute.

= Tests =

The makeRecipeBuildRecords factory method was fixed to create recipe build records with the correct daily build archive set and enhanced to create build records which should not be displayed. These extra records have daily build set to False and/or build status set to "needs build".

bin/test -vvt test_recipebuildslisting

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/configure.zcml
  lib/lp/code/browser/tests/test_recipebuildslisting.py
  lib/lp/code/model/recipebuild.py
  lib/lp/code/templates/daily-builds-listing.pt
  lib/lp/testing/factory.py

./lib/lp/code/model/recipebuild.py
     152: E302 expected 2 blank lines, found 1

[Tim Penhey (thumper)]

review: Approve

=== modified file 'lib/lp/code/browser/tests/test_recipebuildslisting.py'
--- lib/lp/code/browser/tests/test_recipebuildslisting.py	2010-12-13 21:20:03 +0000
+++ lib/lp/code/browser/tests/test_recipebuildslisting.py	2010-12-14 05:59:55 +0000
@@ -48,14 +48,14 @@
     layer = DatabaseFunctionalLayer
 
     def _extract_view_text(self, recipe_build_record):
-        naked_recipebuild = removeSecurityProxy(
-            recipe_build_record.recipebuild)
+        naked_archive = removeSecurityProxy(
+            recipe_build_record.archive)
         naked_distribution = removeSecurityProxy(
-            naked_recipebuild.distribution)
+            naked_archive.distribution)
         text = '\n'.join(str(item) for item in (
                 naked_distribution.displayname,
                 recipe_build_record.sourcepackagename.name,
-                naked_recipebuild.recipe.name,
+                recipe_build_record.recipe.name,
                 recipe_build_record.recipeowner.displayname,
                 recipe_build_record.archive.displayname,
                 recipe_build_record.most_recent_build_time.strftime(

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2010-11-26 03:16:53 +0000
+++ lib/lp/code/configure.zcml	2010-12-14 05:59:55 +0000
@@ -1012,7 +1012,7 @@
   </securedutility>
 
   <class class="lp.code.model.recipebuild.RecipeBuildRecord">
-    <allow attributes="sourcepackagename recipeowner recipebuild archive
+    <allow attributes="sourcepackagename recipeowner recipe archive
                         most_recent_build_time distro_source_package"/>
   </class>
 

=== modified file 'lib/lp/code/model/recipebuild.py'
--- lib/lp/code/model/recipebuild.py	2010-12-01 23:42:55 +0000
+++ lib/lp/code/model/recipebuild.py	2010-12-14 05:59:55 +0000
@@ -48,7 +48,7 @@
 
 class RecipeBuildRecord(namedtuple(
     'RecipeBuildRecord',
-    """sourcepackagename, recipeowner, archive, recipebuild,
+    """sourcepackagename, recipeowner, archive, recipe,
         most_recent_build_time""")):
     # We need to implement our own equality check since __eq__ is broken on
     # SourcePackageRecipe. It's broken there because __eq__ is broken,
@@ -56,7 +56,7 @@
     def __eq__(self, other):
         return (self.sourcepackagename == other.sourcepackagename
             and self.recipeowner == other.recipeowner
-            and self.recipebuild.recipe.name == other.recipebuild.recipe.name
+            and self.recipe.name == other.recipe.name
             and self.archive == other.archive
             and self.most_recent_build_time == other.most_recent_build_time)
 
@@ -64,13 +64,13 @@
         return (
             hash(self.sourcepackagename.name) ^
             hash(self.recipeowner.name) ^
-            hash(self.recipebuild.recipe.name) ^
+            hash(self.recipe.name) ^
             hash(self.archive.name) ^
             hash(self.most_recent_build_time))
 
     @property
     def distro_source_package(self):
-        return self.recipebuild.distribution.getSourcePackage(
+        return self.archive.distribution.getSourcePackage(
             self.sourcepackagename)
 
 
@@ -104,7 +104,7 @@
                  BinaryPackageBuild.package_build_id),
             Join(Archive,
                  Archive.id ==
-                 PackageBuild.archive_id),
+                 SourcePackageRecipe.daily_build_archive_id),
             Join(BuildFarmJob,
                  BuildFarmJob.id ==
                  PackageBuild.build_farm_job_id),
@@ -119,7 +119,7 @@
         result_set = store.using(*tables).find(
                 (SourcePackageName,
                     Person,
-                    SourcePackageRecipeBuild,
+                    SourcePackageRecipe,
                     Archive,
                     Max(BuildFarmJob.date_finished),
                     ),
@@ -127,7 +127,7 @@
             ).group_by(
                 SourcePackageName,
                 Person,
-                SourcePackageRecipeBuild,
+                SourcePackageRecipe,
                 Archive,
             ).order_by(
                 SourcePackageName.name,
@@ -136,11 +136,11 @@
                 )
 
         def _makeRecipeBuildRecord(values):
-            (sourcepackagename, recipeowner, recipebuild, archive,
+            (sourcepackagename, recipeowner, recipe, archive,
                 date_finished) = values
             return RecipeBuildRecord(
                 sourcepackagename, recipeowner,
-                archive, recipebuild,
+                archive, recipe,
                 date_finished)
 
         return RecipeBuildRecordResultSet(

=== modified file 'lib/lp/code/templates/daily-builds-listing.pt'
--- lib/lp/code/templates/daily-builds-listing.pt	2010-11-26 03:14:03 +0000
+++ lib/lp/code/templates/daily-builds-listing.pt	2010-12-14 05:59:55 +0000
@@ -69,14 +69,14 @@
           <tbody>
             <tr tal:repeat="dailybuild dailybuilds">
               <td>
-                <a tal:replace="structure dailybuild/recipebuild/distribution/fmt:link">distribution</a>
+                <a tal:replace="structure dailybuild/archive/distribution/fmt:link">distribution</a>
                 <a tal:attributes="href dailybuild/distro_source_package/fmt:url"
                   tal:content="dailybuild/sourcepackagename/name">source package name</a>
               </td>
 
               <td>
-                <a href="recipe" tal:attributes="href dailybuild/recipebuild/recipe/fmt:url">
-                <span tal:replace="dailybuild/recipebuild/recipe/name">
+                <a href="recipe" tal:attributes="href dailybuild/recipe/fmt:url">
+                <span tal:replace="dailybuild/recipe/name">
                   recipe
                 </span>
                 </a>

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-12-14 02:02:38 +0000
+++ lib/lp/testing/factory.py	2010-12-14 05:59:55 +0000
@@ -2295,7 +2295,10 @@
 
         A RecipeBuildRecord is a named tuple. Some records will be created
         with archive of type ArchivePurpose.PRIMARY, others with type
-        ArchivePurpose.PPA.
+        ArchivePurpose.PPA. Some build records with be created with a build
+        status of fully built and some will be created with the daily build
+        option set to True and False. Only those records with daily build set
+        to True and build status to fully built are returned.
         :param num_recent_records: the number of records within the specified
          time window.
         :param num_records_outside_epoch: the number of records outside the
@@ -2308,12 +2311,6 @@
         sourcepackage = self.makeSourcePackage(
             sourcepackagename=sourcepackagename,
             distroseries=distroseries)
-        recipeowner = self.makePerson()
-        recipe = self.makeSourcePackageRecipe(
-            build_daily=True,
-            owner=recipeowner,
-            name="Recipe_"+sourcepackagename.name,
-            distroseries=distroseries)
 
         records = []
         records_outside_epoch = []
@@ -2323,44 +2320,60 @@
                 purpose = ArchivePurpose.PPA
             else:
                 purpose = ArchivePurpose.PRIMARY
-            archive = self.makeArchive(purpose=purpose)
-            sprb = self.makeSourcePackageRecipeBuild(
-                requester=recipeowner,
-                recipe=recipe,
-                archive=archive,
-                sourcepackage=sourcepackage,
-                distroseries=distroseries)
-            spr = self.makeSourcePackageRelease(
-                source_package_recipe_build=sprb,
-                archive=archive,
-                sourcepackagename=sourcepackagename,
-                distroseries=distroseries)
-            binary_build = self.makeBinaryPackageBuild(
-                    source_package_release=spr)
-            naked_build = removeSecurityProxy(binary_build)
-            naked_build.queueBuild()
-            naked_build.status = BuildStatus.FULLYBUILT
-
-            from random import randrange
-            offset = randrange(0, epoch_days)
-            now = datetime.now(UTC)
-            if x >= num_recent_records:
-                offset = epoch_days + 1 + offset
-            naked_build.date_finished = (
-                now - timedelta(days=offset))
-            naked_build.date_started = (
-                naked_build.date_finished - timedelta(minutes=5))
-            rbr = RecipeBuildRecord(
-                removeSecurityProxy(sourcepackagename),
-                removeSecurityProxy(recipeowner),
-                removeSecurityProxy(archive),
-                removeSecurityProxy(sprb),
-                naked_build.date_finished.replace(tzinfo=None))
-
-            if x < num_recent_records:
-                records.append(rbr)
-            else:
-                records_outside_epoch.append(rbr)
+            archive = self.makeArchive(
+                purpose=purpose, distribution=distroseries.distribution)
+            # Make some daily and non-daily recipe builds.
+            for daily in (True, False):
+                recipeowner = self.makePerson()
+                recipe = self.makeSourcePackageRecipe(
+                    build_daily=daily,
+                    owner=recipeowner,
+                    name="Recipe_%s_%d" % (sourcepackagename.name, x),
+                    daily_build_archive=archive,
+                    distroseries=distroseries)
+                sprb = self.makeSourcePackageRecipeBuild(
+                    requester=recipeowner,
+                    recipe=recipe,
+                    sourcepackage=sourcepackage,
+                    distroseries=distroseries)
+                spr = self.makeSourcePackageRelease(
+                    source_package_recipe_build=sprb,
+                    sourcepackagename=sourcepackagename,
+                    distroseries=distroseries)
+
+                # Make some complete and incomplete builds.
+                for build_status in (
+                    BuildStatus.FULLYBUILT,
+                    BuildStatus.NEEDSBUILD,
+                    ):
+                    binary_build = self.makeBinaryPackageBuild(
+                            source_package_release=spr)
+                    naked_build = removeSecurityProxy(binary_build)
+                    naked_build.queueBuild()
+                    naked_build.status = build_status
+
+                    from random import randrange
+                    offset = randrange(0, epoch_days)
+                    now = datetime.now(UTC)
+                    if x >= num_recent_records:
+                        offset = epoch_days + 1 + offset
+                    naked_build.date_finished = (
+                        now - timedelta(days=offset))
+                    naked_build.date_started = (
+                        naked_build.date_finished - timedelta(minutes=5))
+                    rbr = RecipeBuildRecord(
+                        removeSecurityProxy(sourcepackagename),
+                        removeSecurityProxy(recipeowner),
+                        removeSecurityProxy(archive),
+                        removeSecurityProxy(recipe),
+                        naked_build.date_finished.replace(tzinfo=None))
+
+                    # Only return fully completed daily builds.
+                    if daily and build_status == BuildStatus.FULLYBUILT:
+                        if x < num_recent_records:
+                            records.append(rbr)
+                        else:
+                            records_outside_epoch.append(rbr)
         # We need to explicitly commit because if don't, the records don't
         # appear in the slave datastore.
         transaction.commit()