handle missing account properties for GerritChangeSource

* create a separate function to convert account properties
* make use of the function in all cases where the situation might take place
* update test cases

Fixes ticket:2918

master/buildbot/changes/gerritchangesource.py

@@ -42,6 +42,21 @@ def __init__(self,
             del self.checks["branch"]
 
 
+def _gerrit_user_to_author(props, username=u"unknown"):
+    """
+    Convert Gerrit account properties to Buildbot format
+
+    Take into account missing values
+    """
+    username = props.get("username", username)
+
+    result = [props.get("name", username)]
+    if "email" in props:
+        result.append(u"<%s>" % props["email"])
+
+    return u" ".join(result)
+
+
 class GerritChangeSource(base.ChangeSource):
 
     """This source will maintain a connection to gerrit ssh server
@@ -187,11 +202,8 @@ def addChangeFromEvent(self, properties, event):
 
         if "change" in event and "patchSet" in event:
             event_change = event["change"]
-            username = event_change["owner"].get("username", u"unknown")
             return self.addChange({
-                'author': "%s <%s>" % (
-                    event_change["owner"].get("name", username),
-                    event_change["owner"].get("email", u'unknown@example.com')),
+                'author': _gerrit_user_to_author(event_change["owner"]),
                 'project': util.ascii2unicode(event_change["project"]),
                 'repository': u"ssh://%s@%s:%s/%s" % (
                     self.username, self.gerritserver,
@@ -206,11 +218,10 @@ def addChangeFromEvent(self, properties, event):
 
     def eventReceived_ref_updated(self, properties, event):
         ref = event["refUpdate"]
-        author = "gerrit"
+        author = u"gerrit"
 
         if "submitter" in event:
-            author = "%s <%s>" % (
-                event["submitter"]["name"], event["submitter"]["email"])
+            author = _gerrit_user_to_author(event["submitter"], author)
 
         return self.addChange(dict(
             author=author,

master/buildbot/test/unit/test_changes_gerritchangesource.py

@@ -21,6 +21,59 @@
 from twisted.trial import unittest
 
 
+class TestGerritHelpers(unittest.TestCase):
+    def test_proper_json(self):
+        self.assertEqual(u"Justin Case <justin.case@example.com>",
+                         gerritchangesource._gerrit_user_to_author({
+                            "username": "justincase",
+                            "name": "Justin Case",
+                            "email": "justin.case@example.com"
+                         }))
+
+    def test_missing_username(self):
+        self.assertEqual(u"Justin Case <justin.case@example.com>",
+                         gerritchangesource._gerrit_user_to_author({
+                            "name": "Justin Case",
+                            "email": "justin.case@example.com"
+                         }))
+
+    def test_missing_name(self):
+        self.assertEqual(u"unknown <justin.case@example.com>",
+                         gerritchangesource._gerrit_user_to_author({
+                            "email": "justin.case@example.com"
+                         }))
+        self.assertEqual(u"gerrit <justin.case@example.com>",
+                         gerritchangesource._gerrit_user_to_author({
+                            "email": "justin.case@example.com"
+                         }, u"gerrit"))
+        self.assertEqual(u"justincase <justin.case@example.com>",
+                         gerritchangesource._gerrit_user_to_author({
+                            "username": "justincase",
+                            "email": "justin.case@example.com"
+                         }, u"gerrit"))
+
+    def test_missing_email(self):
+        self.assertEqual(u"Justin Case",
+                         gerritchangesource._gerrit_user_to_author({
+                            "username": "justincase",
+                            "name": "Justin Case"
+                         }))
+        self.assertEqual(u"Justin Case",
+                         gerritchangesource._gerrit_user_to_author({
+                            "name": "Justin Case"
+                         }))
+        self.assertEqual(u"justincase",
+                         gerritchangesource._gerrit_user_to_author({
+                            "username": "justincase"
+                         }))
+        self.assertEqual(u"unknown",
+                         gerritchangesource._gerrit_user_to_author({
+                         }))
+        self.assertEqual(u"gerrit",
+                         gerritchangesource._gerrit_user_to_author({
+                         }, u"gerrit"))
+
+
 class TestGerritChangeSource(changesource.ChangeSourceMixin,
                              unittest.TestCase):