Data API: add filter options to buildrequest collection endpoints

Updates:
- adding filters to the BuildRequestsEndpoint.get() method
- replacing claimed='mine' option in buildrequest DB API, with
claimed=<master id>

Signed-off-by: Olivier Monnier <olivier.monnier@intel.com>

master/buildbot/data/buildrequests.py

@@ -90,15 +90,25 @@ def get(self, resultSpec, kwargs):
                 defer.returnValue([])
         else:
             buildername = None
+        complete = resultSpec.popBooleanFilter('complete')
+        claimed_by_masterid = resultSpec.popBooleanFilter('claimed_by_masterid')
+        if claimed_by_masterid:
+            # claimed_by_masterid takes precedence over 'claimed' filter
+            # (no need to check consistency with 'claimed' filter even if
+            # 'claimed'=False with 'claimed_by_masterid' set, doesn't make sense)
+            claimed = claimed_by_masterid
+        else:
+            claimed = resultSpec.popBooleanFilter('claimed')
+        bsid = resultSpec.popBooleanFilter('bsid')
+        branch = resultSpec.popBooleanFilter('branch')
+        repository = resultSpec.popBooleanFilter('repository')
         buildrequests = yield self.master.db.buildrequests.getBuildRequests(
             buildername=buildername,
-            # TODO: support other filters in this endpoint
-            # complete=None,
-            # claimed=None,
-            # bsid=None,
-            # branch=None,
-            # repository=None
-            )
+            complete=complete,
+            claimed=claimed,
+            bsid=bsid,
+            branch=branch,
+            repository=repository)
         if buildrequests:
 
             @defer.inlineCallbacks

master/buildbot/db/buildrequests.py

@@ -79,16 +79,17 @@ def thd(conn):
 
             q = sa.select([reqs_tbl, claims_tbl]).select_from(from_clause)
             if claimed is not None:
-                if not claimed:
-                    q = q.where(
-                        (claims_tbl.c.claimed_at == None) &
-                        (reqs_tbl.c.complete == 0))
-                elif claimed == "mine":
-                    q = q.where(
-                        (claims_tbl.c.masterid == self.db.master.masterid))
+                if isinstance(claimed, bool):
+                    if not claimed:
+                        q = q.where(
+                            (claims_tbl.c.claimed_at == None) &
+                            (reqs_tbl.c.complete == 0))
+                    else:
+                        q = q.where(
+                            (claims_tbl.c.claimed_at != None))
                 else:
                     q = q.where(
-                        (claims_tbl.c.claimed_at != None))
+                        (claims_tbl.c.masterid == claimed))
             if buildername is not None:
                 q = q.where(reqs_tbl.c.buildername == buildername)
             if complete is not None:

master/buildbot/test/fake/fakedb.py

@@ -1474,14 +1474,15 @@ def getBuildRequests(self, buildername=None, complete=None, claimed=None,
             else:
                 br.claimed_at = None
             if claimed is not None:
-                if claimed == "mine":
-                    if not claim_row or claim_row.masterid != self.MASTER_ID:
-                        continue
-                elif claimed:
-                    if not claim_row:
-                        continue
+                if isinstance(claimed, bool):
+                    if claimed:
+                        if not claim_row:
+                            continue
+                    else:
+                        if br.complete or claim_row:
+                            continue
                 else:
-                    if br.complete or claim_row:
+                    if not claim_row or claim_row.masterid != claimed:
                         continue
             if bsid is not None:
                 if br.buildsetid != bsid:

master/buildbot/test/unit/test_data_buildrequests.py

@@ -17,6 +17,7 @@
 import mock
 
 from buildbot.data import buildrequests
+from buildbot.data import resultspec
 from buildbot.data.base import Link
 from buildbot.test.fake import fakedb
 from buildbot.test.fake import fakemaster
@@ -146,6 +147,57 @@ def testGetUnknownBuilderid(self):
         buildrequests = yield self.callGet(('builder', 79, 'buildrequest'))
         self.assertEqual(buildrequests, [])
 
+    @defer.inlineCallbacks
+    def testGetNoFilters(self):
+        getBuildRequestsMock = mock.Mock(return_value={})
+        self.patch(self.master.db.buildrequests, 'getBuildRequests', getBuildRequestsMock)
+        yield self.callGet(('buildrequest',))
+        getBuildRequestsMock.assert_called_with(
+            buildername=None,
+            complete=None,
+            claimed=None,
+            bsid=None,
+            branch=None,
+            repository=None)
+
+    @defer.inlineCallbacks
+    def testGetFilters(self):
+        getBuildRequestsMock = mock.Mock(return_value={})
+        self.patch(self.master.db.buildrequests, 'getBuildRequests', getBuildRequestsMock)
+        f1 = resultspec.Filter('complete', 'eq', [False])
+        f2 = resultspec.Filter('claimed', 'eq', [True])
+        f3 = resultspec.Filter('bsid', 'eq', [55])
+        f4 = resultspec.Filter('branch', 'eq', ['mybranch'])
+        f5 = resultspec.Filter('repository', 'eq', ['myrepo'])
+        yield self.callGet(
+            ('buildrequest',),
+            resultSpec=resultspec.ResultSpec(filters=[f1, f2, f3, f4, f5]))
+        getBuildRequestsMock.assert_called_with(
+            buildername=None,
+            complete=False,
+            claimed=True,
+            bsid=55,
+            branch='mybranch',
+            repository='myrepo')
+
+    @defer.inlineCallbacks
+    def testGetClaimedByMasterIdFilters(self):
+        getBuildRequestsMock = mock.Mock(return_value={})
+        self.patch(self.master.db.buildrequests, 'getBuildRequests', getBuildRequestsMock)
+        f1 = resultspec.Filter('claimed', 'eq', [True])
+        f2 = resultspec.Filter('claimed_by_masterid', 'eq',
+                               [fakedb.FakeBuildRequestsComponent.MASTER_ID])
+        yield self.callGet(
+            ('buildrequest',),
+            resultSpec=resultspec.ResultSpec(filters=[f1, f2]))
+        getBuildRequestsMock.assert_called_with(
+            buildername=None,
+            complete=None,
+            claimed=fakedb.FakeBuildRequestsComponent.MASTER_ID,
+            bsid=None,
+            branch=None,
+            repository=None)
+
 
 class TestBuildRequest(interfaces.InterfaceTests, unittest.TestCase):
 

master/buildbot/test/unit/test_db_buildrequests.py

@@ -124,7 +124,7 @@ def test_getBuildRequests_no_claimed_arg(self):
 
     def test_getBuildRequests_claimed_mine(self):
         return self.do_test_getBuildRequests_claim_args(
-            claimed="mine",
+            claimed=self.MASTER_ID,
             expected=[50])
 
     def test_getBuildRequests_claimed_true(self):
@@ -278,7 +278,8 @@ def test_getBuildRequests_combo(self):
         ])
         d.addCallback(lambda _:
                       self.db.buildrequests.getBuildRequests(buildername="bbb",
-                                                             claimed="mine", complete=True, bsid=self.BSID))
+                                                             claimed=self.MASTER_ID,
+                                                             complete=True, bsid=self.BSID))
 
         def check(brlist):
             self.assertEqual([br['brid'] for br in brlist], [44])
@@ -349,7 +350,8 @@ def do_test_claimBuildRequests(self, rows, now, brids, expected=None,
         d = self.insertTestData(rows)
         d.addCallback(lambda _:
                       self.db.buildrequests.claimBuildRequests(brids=brids,
-                                                               claimed_at=claimed_at, _reactor=clock))
+                                                               claimed_at=claimed_at,
+                                                               _reactor=clock))
         d.addCallback(lambda _:
                       self.db.buildrequests.getBuildRequests())
 
@@ -541,7 +543,8 @@ def do_test_completeBuildRequests(self, rows, now, expected=None,
         d = self.insertTestData(rows)
         d.addCallback(lambda _:
                       self.db.buildrequests.completeBuildRequests(brids=brids,
-                                                                  results=7, complete_at=complete_at,
+                                                                  results=7,
+                                                                  complete_at=complete_at,
                                                                   _reactor=clock))
         d.addCallback(lambda _:
                       self.db.buildrequests.getBuildRequests())

master/docs/developer/db.rst

@@ -128,8 +128,8 @@ buildrequests
 
         The ``claimed`` parameter can be ``None`` (the default) to ignore the
         claimed status of requests; ``True`` to return only claimed builds,
-        ``False`` to return only unclaimed builds, or ``"mine"`` to return only
-        builds claimed by this master instance.  A request is considered
+        ``False`` to return only unclaimed builds, or a ``master ID`` to return only
+        builds claimed by a particular master instance.  A request is considered
         unclaimed if its ``claimed_at`` column is either NULL or 0, and it is
         not complete.  If ``bsid`` is specified, then only build requests for
         that buildset will be returned.

master/docs/developer/rtype-buildrequest.rst

@@ -39,6 +39,15 @@ Endpoints
 
     This path lists buildrequests, sorted by ID.
 
+        :opt complete: if true, only returns completed buildsets;
+                       if false, only returns incomplete buildsets
+        :opt claimed: if true, only returns the claimed buildrequests;
+                      if false, only returns unclaimed builds
+        :opt claimed_by_masterid: only returns buildrequests claimed by this master instance
+        :opt bsid: only returns buildrequests contained by this buildset ID
+        :opt branch: only returns buildrequests on this branch
+        :opt repository: only returns buildrequests on this repository
+
 .. bb:rpath:: /buildrequest/:buildrequestid
 
     :pathkey integer buildrequestid: the ID of the buildrequest
@@ -57,12 +66,16 @@ Endpoints
 
     This path lists buildrequests performed for the identified builder, sorted by ID.
 
+        :opt this endpoint supports same options as /buildrequest
+
 .. bb:rpath:: /builder/:builderid/buildrequest
 
     :pathkey integer builderid: the id of the builder
 
     This path lists buildrequests performed for the identified builder, sorted by ID.
 
+        :opt this endpoint supports same options as /buildrequest
+
 .. todo::
     May need to define additional useful collection endpoints like e.g:
         * /buildset/:buildsetid/buildrequest