Merge 999e2e1 into ed375ba

master/buildbot/process/build.py

@@ -434,8 +434,11 @@ def setupBuild(self, expectations):
         # gather owners from build requests
         owners = [r.properties['owner'] for r in self.requests
                   if r.properties.has_key('owner')]
+        user_ids = [r.properties['user_id'] for r in self.requests
+                    if r.properties.has_key('user_id')]
         if owners: self.setProperty('owners', owners, self.reason)
         self.build_status.setOwners(owners)
+        if user_ids: self.build_status.setUserID(user_ids[0])
 
         self.results = [] # list of FAILURE, SUCCESS, WARNINGS, SKIPPED
         self.result = SUCCESS # overall result, may downgrade after each step

master/buildbot/process/builder.py

@@ -773,6 +773,9 @@ def get_brs((bsid,brids)):
 
     @defer.inlineCallbacks
     def rebuildBuild(self, bs, reason="<rebuild, no reason given>", extraProperties=None, absolute=True, newOwner=''):
+        """
+        TODO: This method is probably is not used. Consider to remove it.
+        """
         if not bs.isFinished():
             return
 

master/buildbot/schedulers/forcesched.py

@@ -654,11 +654,13 @@ def force(self, owner, builderNames=None, **kwargs):
         reason = self.reason.getFromKwargs(kwargs)
         if owner is None:
             owner = self.username.getFromKwargs(kwargs)
+        user_id = kwargs.get('user_id')[0] if kwargs.get('user_id') else None
 
         properties, changeids, sourcestamps = yield self.gatherPropertiesAndChanges(**kwargs)
 
         properties.setProperty("reason", reason, "Force Build Form")
         properties.setProperty("owner", owner, "Force Build Form")
+        properties.setProperty("user_id", user_id, "Force Build Form")
 
         if 'selected_slave' in kwargs and kwargs['selected_slave'][0] != 'default':
             properties.setProperty("selected_slave", kwargs['selected_slave'][0], "Force Build Form")

master/buildbot/status/build.py

@@ -38,7 +38,7 @@
 class BuildStatus(styles.Versioned, properties.PropertiesMixin):
     implements(interfaces.IBuildStatus, interfaces.IStatusEvent)
 
-    persistenceVersion = 4
+    persistenceVersion = 5
     persistenceForgets = ( 'wasUpgraded', )
 
     sources = None
@@ -60,6 +60,7 @@ class BuildStatus(styles.Versioned, properties.PropertiesMixin):
     slavename = "???"
     foi_url = None
     artifacts = None
+    user_id = None
 
     set_runtime_properties = True
 
@@ -211,21 +212,8 @@ def waitUntilFinished(self):
     def createBuildUserStatus(self, step):
         @defer.inlineCallbacks
         def thd(build_id):
-            for owner in self.owners:
-                # TODO: Change in future to uid from session!
-                # "pyflakes pyflakes@unity3d.com" -> "pyflakes"
-                username = " ".join(owner.split()[:-1])
-                user_id = yield self.master.db.users.getUidByLdapUsername(username)
-                if not user_id:
-                    msg = "Can not find user in database: username: {user}; project: {project};" \
-                          " builder: {builder}"
-                    klog.err_json(msg.format(
-                        user=owner,
-                        project=step.builder.project,
-                        builder=step.builder.friendly_name
-                    ))
-                    continue
-                yield self.master.db.builds.createBuildUser(build_id, user_id, self.finished)
+            if self.user_id:
+                yield self.master.db.builds.createBuildUser(build_id, self.user_id, self.finished)
 
         def error_thd(err):
             msg = "There was an error in createBuildUserStatus. Project: {proj}; Builder: {builder}"
@@ -386,6 +374,27 @@ def updateBuildRequestIDs(self, brids):
     def setOwners(self, owners):
         self.owners = owners
 
+    @defer.inlineCallbacks
+    def setUserID(self, user_id):
+        self.user_id = user_id
+        if self.user_id:
+            return
+
+        # TODO: check in logz if "Found user_id in old way" is logged
+        # If yes, add generating user_id in that step
+        # Else remove below for-loop and tests for it.
+        for owner in self.owners:
+            username = " ".join(owner.split()[:-1])
+            self.user_id = yield self.master.db.users.getUidByLdapUsername(username)
+            if self.user_id:
+                klog.err_json("Found user_id in old way [user_id: %s] [project: %s] [builder: %s]" %
+                              (self.user_id, self.builder.project, self.builder.name))
+                break
+
+        if not self.user_id:
+            klog.err_json("Cannot find user_id for build [project: %s] [builder: %s]" %
+                          (self.builder.project, self.builder.name))
+
     def setReason(self, reason):
         self.reason = reason
 
@@ -557,6 +566,18 @@ def upgradeToVersion4(self):
             del self.source
         self.wasUpgraded = True
 
+    @defer.inlineCallbacks
+    def upgradeToVersion5(self):
+        self.user_id = None
+        self.wasUpgraded = True
+
+        if not self.owners:
+            return
+        # search user_id 'in old way'
+        for owner in self.owners:
+            username = " ".join(owner.split()[:-1])
+            self.user_id = yield self.master.db.users.getUidByLdapUsername(username)
+
     def checkLogfiles(self):
         # check that all logfiles exist, and remove references to any that
         # have been deleted (e.g., by purge())

master/buildbot/status/web/builder.py

@@ -41,10 +41,14 @@ class ForceAction(ActionResource):
     @defer.inlineCallbacks
     def force(self, req, builderNames):
         master = self.getBuildmaster(req)
-        owner = self.getAuthz(req).getUsernameFull(req)
+        authz = self.getAuthz(req)
+        owner = authz.getUsernameFull(req)
+        username = authz.getUsername(req)
+        user_id = authz.getUserInfo(username).get('uid')
         scheduler_name = req.args.get("forcescheduler", ["<unknown>"])[0]
 
         args = self.decode_request_arguments(req)
+        args['user_id'] = user_id
 
         if scheduler_name == "<unknown>":
             scheduler = master.scheduler_manager.findSchedulerByBuilderName(

master/buildbot/test/unit/test_schedulers_forcesched.py

@@ -135,7 +135,8 @@ def test_basicForce(self):
                         property1_name='p1',property1_value='e',
                         property2_name='p2',property2_value='f',
                         property3_name='p3',property3_value='g',
-                        property4_name='p4',property4_value='h'
+                        property4_name='p4',property4_value='h',
+                        user_id=42,
                         )
         bsid,brids = res
 
@@ -155,6 +156,7 @@ def test_basicForce(self):
                                ('p4', ('h', 'Force Build Form')),
                                ('reason', ('because', 'Force Build Form')),
                                ('scheduler', ('testsched', 'Scheduler')),
+                               ('user_id', (42, 'Force Build Form')),
                                ],
                   sourcestampsetid=100),
              {'':
@@ -167,7 +169,7 @@ def test_force_allBuilders(self):
         sched = self.makeScheduler()
 
         res = yield sched.force('user', branch='a', reason='because',revision='c',
-                        repository='d', project='p',
+                        repository='d', project='p', user_id=42,
                         )
         bsid,brids = res
 
@@ -183,6 +185,7 @@ def test_force_allBuilders(self):
                                ('owner', ('user', 'Force Build Form')),
                                ('reason', ('because', 'Force Build Form')),
                                ('scheduler', ('testsched', 'Scheduler')),
+                               ('user_id', (42, 'Force Build Form')),
                                ],
                   sourcestampsetid=100),
              {'':
@@ -196,7 +199,7 @@ def test_force_someBuilders(self):
 
         res = yield sched.force('user', builderNames=['a','b'],
                         branch='a', reason='because',revision='c',
-                        repository='d', project='p',
+                        repository='d', project='p', user_id=42,
                         )
         bsid,brids = res
 
@@ -212,6 +215,7 @@ def test_force_someBuilders(self):
                                ('owner', ('user', 'Force Build Form')),
                                ('reason', ('because', 'Force Build Form')),
                                ('scheduler', ('testsched', 'Scheduler')),
+                               ('user_id', (42, 'Force Build Form')),
                                ],
                   sourcestampsetid=100),
              {'':
@@ -257,7 +261,8 @@ def test_good_codebases(self):
                         property1_name='p1',property1_value='e',
                         property2_name='p2',property2_value='f',
                         property3_name='p3',property3_value='g',
-                        property4_name='p4',property4_value='h'
+                        property4_name='p4',property4_value='h',
+                        user_id=42,
                         )
 
         bsid,brids = res
@@ -274,6 +279,7 @@ def test_good_codebases(self):
                                ('p4', ('h', 'Force Build Form')),
                                ('reason', ('because', 'Force Build Form')),
                                ('scheduler', ('testsched', 'Scheduler')),
+                               ('user_id', (42, 'Force Build Form')),
                                ],
                   sourcestampsetid=100),
              {'foo': dict(codebase='foo', sourcestampsetid=100,
@@ -309,7 +315,7 @@ def do_ParameterTest(self,
         sched = self.makeScheduler(properties=[prop])
 
         if not req:
-            req = {name:value, 'reason':'because'}
+            req = {name:value, 'reason':'because', 'user_id': 42}
         try:
             bsid, brids = yield sched.force(owner, builderNames=['a'], **req)
         except Exception,e:
@@ -326,6 +332,7 @@ def do_ParameterTest(self,
             ('owner', ('user', 'Force Build Form')),
             ('reason', ('because', 'Force Build Form')),
             ('scheduler', ('testsched', 'Scheduler')),
+            ('user_id', (42, 'Force Build Form')),
         ]
 
         if expectKind is None:
@@ -362,22 +369,22 @@ def test_FixedParameter(self):
 
 
     def test_BooleanParameter_True(self):
-        req = dict(p1=True,reason='because')
+        req = dict(p1=True,reason='because', user_id=42)
         self.do_ParameterTest(expect=True, klass=BooleanParameter, req=req)
 
 
     def test_WhenBooleanParameterIsStringTrue_ThenPropertyIsSetToTrue(self):
-        req = dict(p1='True',reason='because')
+        req = dict(p1='True',reason='because', user_id=42)
         self.do_ParameterTest(expect=True, klass=BooleanParameter, req=req)
 
 
     def test_WhenBooleanParameterIsRandomString_ThenPropertyIsSetToFalse(self):
-        req = dict(p1='Foobar',reason='because')
+        req = dict(p1='Foobar',reason='because', user_id=42)
         self.do_ParameterTest(expect=False, klass=BooleanParameter, req=req)
 
 
     def test_BooleanParameter_False(self):
-        req = dict(p2=True,reason='because')
+        req = dict(p2=True,reason='because', user_id=42)
         self.do_ParameterTest(expect=False, klass=BooleanParameter, req=req)
 
 
@@ -428,7 +435,7 @@ def test_ChoiceParameterMultipleError(self):
                 multiple=True, debug=False)
 
     def test_WhenRequestDoesNotContainValueAndItIsReadonly_ThenPropertyIsNotAssigned(self):
-        self.do_ParameterTest(req=dict(reason='because'),
+        self.do_ParameterTest(req=dict(reason='because', user_id=42),
                 expect={},
                 expectKind=dict,
                 klass=IntParameter,
@@ -444,7 +451,7 @@ def test_NestedParameter(self):
         fields = [
             IntParameter(name="foo")
         ]
-        self.do_ParameterTest(req=dict(p1_foo='123', reason="because"),
+        self.do_ParameterTest(req=dict(p1_foo='123', reason="because", user_id=42),
                               expect=dict(foo=123),
                               klass=NestedParameter, fields=fields)
 
@@ -460,7 +467,8 @@ def test_NestedNestedParameter(self):
                                        p1_inner_str="bar",
                                        p1_inner_any_name="hello",
                                        p1_inner_any_value="world",
-                                       reason="because"),
+                                       reason="because",
+                                       user_id=42),
                               expect=dict(foo=123, inner=dict(str="bar", hello="world")),
                               klass=NestedParameter, fields=fields)
 
@@ -485,7 +493,8 @@ def test_NestedParameter_nullname(self):
                                        bar_a_name="a",
                                        bar_a_value="7",
                                        bar_b_name="b",
-                                       bar_b_value="8"),
+                                       bar_b_value="8",
+                                       user_id=42),
                               expect=dict(foo=123,
                                           inner=dict(str="bar", hello="world"),
                                           bar={'a':'7', 'b':'8'}),
@@ -496,7 +505,7 @@ def test_NestedParameter_nullname(self):
     def test_specifySlave(self):
         sched = self.makeScheduler()
         res = yield sched.force('user', 'a', branch='a', repository='http://repo', reason='because',
-                                selected_slave='slave1')
+                                selected_slave='slave1', user_id=42)
         bsid, brids = res
         self.db.buildsets.assertBuildset(bsid, dict(
             reason="A build was forced by 'user': because",
@@ -506,7 +515,8 @@ def test_specifySlave(self):
                               ('owner', ('user', 'Force Build Form')),
                               ('reason', ('because', 'Force Build Form')),
                               ('scheduler', ('testsched', 'Scheduler')),
-                              ('selected_slave', ('slave1', 'Force Build Form'))],
+                              ('selected_slave', ('slave1', 'Force Build Form')),
+                              ('user_id', (42, 'Force Build Form'))],
             sourcestampsetid=100),
             {'': {'branch': 'a', 'codebase': '', 'project': '', 'repository': 'http://repo', 'revision': '',
                   'sourcestampsetid': 100}})
@@ -535,7 +545,7 @@ def makeFakeSlave(name, paused=False):
             makeFakeSlave('slave-03', paused=True)]
 
         res = yield sched.force('user', 'a', branch='a', repository='http://repo', reason='because',
-                                selected_slave='allCompatible')
+                                selected_slave='allCompatible', user_id=42)
 
         self.assertTrue(len(res) == 3)
 
@@ -549,7 +559,8 @@ def checkBuildset(res, slavename, ssid):
                                   ('owner', ('user', 'Force Build Form')),
                                   ('reason', ('because', 'Force Build Form')),
                                   ('scheduler', ('testsched', 'Scheduler')),
-                                  ('selected_slave', (slavename, 'Scheduler'))],
+                                  ('selected_slave', (slavename, 'Scheduler')),
+                                  ('user_id', (42, 'Force Build Form'))],
                 sourcestampsetid=ssid),
                 {'': {'branch': 'a', 'codebase': '', 'project': '', 'repository': 'http://repo', 'revision': '',
                       'sourcestampsetid': ssid}})

master/buildbot/test/unit/test_status_build.py

@@ -161,6 +161,14 @@ def test_getSourceStamps_with_codebases_less_gotrevisions_absolute(self):
 
 
 class TestBuildStatusUtils(unittest.TestCase):
+    BUILD_NUMBER = 33
+
+    def setUp(self):
+        self.builder_status = FakeBuilderStatus()
+        self.master = fakemaster.make_master()
+        self.build_status = build.BuildStatus(self.builder_status, self.master,
+                                              self.BUILD_NUMBER)
+
     @mock.patch('buildbot.status.web.base.path_to_build_by_params')
     def test_get_url_and_name_build_in_chain_with_selected_build_in_chain(self, path_mock):
         path_mock.return_value = "http://example.com/example/url"
@@ -202,6 +210,50 @@ def test_get_url_and_name_build_in_chain_with_selected_build_not_in_chain(self):
         self.assertEqual(build_url, None)
         self.assertEqual(build_name, None)
 
+    def test_setUserID(self):
+        self.build_status.setUserID(5)
+
+        self.assertEqual(self.build_status.user_id, 5)
+
+    @mock.patch("klog.err_json")
+    def test_setUserID_from_owners(self, err_json):
+        self.build_status.owners = ['pyflakes <pyflakes@unity3d.com>']
+        self.build_status.master.db = mock.Mock()
+        self.build_status.master.db.users.getUidByLdapUsername = mock.Mock(return_value=6)
+        self.build_status.builder.project = "Test Project"
+        self.build_status.builder.name = "Test Builder"
+
+        self.build_status.setUserID(None)
+
+        self.assertEqual(self.build_status.user_id, 6)
+        self.assertEqual(err_json.called, True)
+
+    @mock.patch("klog.err_json")
+    def test_setUserID_unknown_owner(self, err_json):
+        self.build_status.owners = ['pyflakes <pyflakes@unity3d.com>']
+        self.build_status.master.db = mock.Mock()
+        self.build_status.master.db.users.getUidByLdapUsername = mock.Mock(return_value=None)
+        self.build_status.builder.project = "Test Project"
+        self.build_status.builder.name = "Test Builder"
+
+        self.build_status.setUserID(None)
+
+        self.assertEqual(self.build_status.user_id, None)
+        self.assertEqual(err_json.called, True)
+
+    @mock.patch("klog.err_json")
+    def test_setUserID_empty_owners(self, err_json):
+        self.build_status.owners = []
+        self.build_status.master.db = mock.Mock()
+        self.build_status.master.db.users.getUidByLdapUsername = mock.Mock(return_value=7)
+        self.build_status.builder.project = "Test Project"
+        self.build_status.builder.name = "Test Builder"
+
+        self.build_status.setUserID(None)
+
+        self.assertEqual(self.build_status.user_id, None)
+        self.assertEqual(err_json.called, True)
+
 
 class BuildStepStub:
     implements(interfaces.IBuildStepStatus)