builders: use DB API from data API

master/buildbot/data/builders.py

@@ -21,14 +21,20 @@ class BuilderEndpoint(base.Endpoint):
     pathPatterns = [ ( 'builder', 'i:builderid' ),
                      ( 'master', 'i:masterid', 'builder', 'i:builderid' ) ]
 
+    @defer.inlineCallbacks
     def get(self, options, kwargs):
-        rtype = self.master.data.rtypes['builder']
         builderid = kwargs['builderid']
-        if builderid not in rtype.builderIds:
-            return defer.succeed(None)
-        return defer.succeed(
+        bdict = yield self.master.db.builders.getBuilder(builderid)
+        if not bdict:
+            yield defer.returnValue(None)
+            return
+        if 'masterid' in kwargs:
+            if kwargs['masterid'] not in bdict['masterids']:
+                yield defer.returnValue(None)
+                return
+        yield defer.returnValue(
             dict(builderid=builderid,
-                 name=rtype.builderIds[builderid],
+                 name=bdict['name'],
                  link=base.Link(('builder', str(kwargs['builderid'])))))
 
 
@@ -38,17 +44,15 @@ class BuildersEndpoint(base.Endpoint):
     pathPatterns = [ ( 'builder', ),
                      ( 'master', 'i:masterid', 'builder' ) ]
 
+    @defer.inlineCallbacks
     def get(self, options, kwargs):
-        rtype = self.master.data.rtypes['builder']
-        names = set(rtype.builders)
-        with_ids = [ (id, name) for id, name in rtype.builderIds.iteritems()
-                     if name in names ]
-        with_ids.sort()
-        return defer.succeed([
-            dict(builderid=id,
-                 name=name,
-                 link=base.Link(('builder', str(id))))
-            for id, name in with_ids ])
+        bdicts = yield self.master.db.builders.getBuilders(
+                masterid=kwargs.get('masterid', None))
+        yield defer.returnValue([
+            dict(builderid=bd['id'],
+                 name=bd['name'],
+                 link=base.Link(('builder', str(bd['id']))))
+            for bd in bdicts ])
 
     def startConsuming(self, callback, options, kwargs):
         return self.master.mq.startConsuming(callback,
@@ -65,20 +69,26 @@ class BuildersResourceType(base.ResourceType):
 
     def __init__(self, master):
         base.ResourceType.__init__(self, master)
-        self.builderIds = {} # name : id
-        self.builders = [] # list of names
-        self.masterid = None
 
     @base.updateMethod
     @defer.inlineCallbacks
     def updateBuilderList(self, masterid, builderNames):
-        self.masterid = masterid
-        for name in builderNames:
-            if name not in self.builderIds:
-                builderid = len(self.builderIds)+1
-                self.builderIds[builderid] = name
-                self.produceEvent(
-                        dict(builderid=builderid, name=name),
-                        'new')
-        self.builders = builderNames
-        yield defer.returnValue(None)
+        # get the "current" list of builders for this master, so we know what
+        # changes to make.  Race conditions here aren't a great worry, as this
+        # is the only master inserting or deleting these records.
+        builders = yield self.master.db.builders.getBuilders(masterid=masterid)
+
+        # figure out what to remove and remove it
+        builderNames_set = set(builderNames)
+        for bldr in builders:
+            if bldr['name'] not in builderNames_set:
+                yield self.master.db.builders.removeBuilderMaster(
+                        masterid=masterid, builderid=bldr['id'])
+            else:
+                builderNames_set.remove(bldr['name'])
+
+        # now whatever's left in builderNames_set is new
+        for name in builderNames_set:
+            builderid = yield self.master.db.builders.findBuilderId(name)
+            yield self.master.db.builders.addBuilderMaster(
+                        masterid=masterid, builderid=builderid)

master/buildbot/test/unit/test_data_builders.py

@@ -17,18 +17,20 @@
 from twisted.internet import defer
 from buildbot.data import builders
 from buildbot.test.util import types, endpoint
-from buildbot.test.fake import fakemaster
+from buildbot.test.fake import fakemaster, fakedb
 
 class Builder(endpoint.EndpointMixin, unittest.TestCase):
 
     endpointClass = builders.BuilderEndpoint
-    resourceTypeClass = builders.BuildersResourceType
 
     def setUp(self):
         self.setUpEndpoint()
-        # TODO: use insertTestData instead
-        self.rtype.builderIds = { 1 : u'buildera', 2 : u'builderb' }
-        self.rtype.builders = self.rtype.builderIds.keys()
+        return self.db.insertTestData([
+            fakedb.Builder(id=1, name=u'buildera'),
+            fakedb.Builder(id=2, name=u'builderb'),
+            fakedb.Master(id=13),
+            fakedb.BuilderMaster(id=1, builderid=2, masterid=13),
+        ])
 
     def tearDown(self):
         self.tearDownEndpoint()
@@ -41,6 +43,8 @@ def check(builder):
             self.assertEqual(builder['name'], u'builderb')
         return d
 
+    # TODO: test get with masterid that exists or doesn't exist
+
     def test_get_missing(self):
         d = self.callGet(dict(), dict(builderid=99))
         @d.addCallback
@@ -67,12 +71,15 @@ def check(builder):
 class Builders(endpoint.EndpointMixin, unittest.TestCase):
 
     endpointClass = builders.BuildersEndpoint
-    resourceTypeClass = builders.BuildersResourceType
 
     def setUp(self):
         self.setUpEndpoint()
-        self.rtype.builderIds = { 1 : u'buildera', 2 : u'builderb' }
-        self.rtype.builders = self.rtype.builderIds.values()
+        return self.db.insertTestData([
+            fakedb.Builder(id=1, name=u'buildera'),
+            fakedb.Builder(id=2, name=u'builderb'),
+            fakedb.Master(id=13),
+            fakedb.BuilderMaster(id=1, builderid=2, masterid=13),
+        ])
 
 
     def tearDown(self):
@@ -88,6 +95,23 @@ def check(builders):
                              [1, 2])
         return d
 
+    def test_get_masterid(self):
+        d = self.callGet(dict(), dict(masterid=13))
+        @d.addCallback
+        def check(builders):
+            [ types.verifyData(self, 'builder', {}, b) for b in builders ]
+            self.assertEqual(sorted([b['builderid'] for b in builders]),
+                             [2])
+        return d
+
+    def test_get_masterid_missing(self):
+        d = self.callGet(dict(), dict(masterid=14))
+        @d.addCallback
+        def check(builders):
+            self.assertEqual(sorted([b['builderid'] for b in builders]),
+                             [])
+        return d
+
     def test_startConsuming(self):
         self.callStartConsuming({}, {},
                 expected_filter=('builder', None, 'new'))
@@ -99,8 +123,37 @@ def setUp(self):
         self.master = fakemaster.make_master(wantMq=True, wantDb=True,
                                                 testcase=self)
         self.rtype = builders.BuildersResourceType(self.master)
+        return self.master.db.insertTestData([
+            fakedb.Master(id=13),
+            fakedb.Master(id=14),
+        ])
 
     @defer.inlineCallbacks
     def test_updateBuilderList(self):
-        # TODO: this method doesn't do anything yet, so very little to test..
-        yield self.rtype.updateBuilderList(13, [ u'somebuidler' ])
+        # add one builder master
+        yield self.rtype.updateBuilderList(13, [ u'somebuilder' ])
+        self.assertEqual(sorted((yield self.master.db.builders.getBuilders())),
+        sorted([
+            dict(id=1, masterids=[13], name='somebuilder'),
+        ]))
+        # add another
+        yield self.rtype.updateBuilderList(13, [ u'somebuilder', u'another' ])
+        self.assertEqual(sorted((yield self.master.db.builders.getBuilders())),
+        sorted([
+            dict(id=1, masterids=[13], name='somebuilder'),
+            dict(id=2, masterids=[13], name='another'),
+        ]))
+        # add one for another master
+        yield self.rtype.updateBuilderList(14, [ u'another' ])
+        self.assertEqual(sorted((yield self.master.db.builders.getBuilders())),
+        sorted([
+            dict(id=1, masterids=[13], name='somebuilder'),
+            dict(id=2, masterids=[13, 14], name='another'),
+        ]))
+        # remove both for the first master
+        yield self.rtype.updateBuilderList(13, [ ])
+        self.assertEqual(sorted((yield self.master.db.builders.getBuilders())),
+        sorted([
+            dict(id=1, masterids=[], name='somebuilder'),
+            dict(id=2, masterids=[14], name='another'),
+        ]))

master/docs/developer/rtype-builder.rst

@@ -38,4 +38,4 @@ Builders
         :pathkey integer builderid: the ID of the builder
 
         This path selects a specific builder, identified by ID.
-        The ``:masterid`` field is ignored, since ``:builderid`` uniquely identifies the builder.
+        If the given builder is not running on the given master, this path returns nothing.