From 391c21434b7eaebf28e8ce729cd00442b27e0b94 Mon Sep 17 00:00:00 2001
From: Thomas Desveaux <thomas.desveaux@dont-nod.com>
Date: Mon, 20 May 2024 10:26:58 +0200
Subject: [PATCH 1/3] db: Add UserModel dataclass

---
 master/buildbot/db/users.py                   | 104 +++++++++++----
 master/buildbot/test/__init__.py              |   6 +
 master/buildbot/test/fakedb/users.py          |  25 ++--
 master/buildbot/test/unit/db/test_users.py    | 118 ++++++++++--------
 .../test/unit/process/test_users_manual.py    |  61 +++++----
 master/docs/developer/database/users.rst      |  20 ++-
 master/docs/spelling_wordlist.txt             |   1 +
 newsfragments/db-introduce-UserModel.change   |   1 +
 newsfragments/db-usdict.removal               |   1 +
 9 files changed, 218 insertions(+), 119 deletions(-)
 create mode 100644 newsfragments/db-introduce-UserModel.change
 create mode 100644 newsfragments/db-usdict.removal

diff --git a/master/buildbot/db/users.py b/master/buildbot/db/users.py
index f046df38aab..429f9724955 100644
--- a/master/buildbot/db/users.py
+++ b/master/buildbot/db/users.py
@@ -14,13 +14,75 @@
 # Copyright Buildbot Team Members
 
 
+from __future__ import annotations
+
+import dataclasses
+
 import sqlalchemy as sa
+from twisted.python import deprecate
+from twisted.python import versions
 
 from buildbot.db import base
 from buildbot.util import identifiers
-
-
-class UsDict(dict):
+from buildbot.warnings import warn_deprecated
+
+
+@dataclasses.dataclass
+class UserModel:
+    uid: int
+    identifier: str
+    bb_username: str | None = None
+    bb_password: str | None = None
+    attributes: dict[str, str] | None = None
+
+    # For backward compatibility
+    def __getitem__(self, key: str):
+        warn_deprecated(
+            '4.1.0',
+            (
+                'UsersConnectorComponent '
+                'getUser, getUserByUsername, and getUsers '
+                'no longer return User as dictionnaries. '
+                'Usage of [] accessor is deprecated: please access the member directly'
+            ),
+        )
+
+        if hasattr(self, key):
+            return getattr(self, key)
+
+        if self.attributes is not None and key in self.attributes:
+            return self.attributes[key]
+
+        raise KeyError(key)
+
+    def get(self, key: str, default=None):
+        try:
+            return self[key]
+        except KeyError:
+            return default
+
+    def keys(self):
+        warn_deprecated(
+            '4.1.0',
+            (
+                'UsersConnectorComponent '
+                'getUser, getUserByUsername, and getUsers '
+                'no longer return User as dictionnaries. '
+                'Usage of keys is deprecated: please access the member directly'
+            ),
+        )
+
+        keys = set()
+        if self.attributes is not None:
+            keys.update(self.attributes.keys())
+
+        keys.update(f.name for f in dataclasses.fields(self) if f.name != 'attributes')
+
+        return keys
+
+
+@deprecate.deprecated(versions.Version("buildbot", 4, 1, 0), UserModel)
+class UsDict(UserModel):
     pass
 
 
@@ -103,24 +165,21 @@ def thd(conn):
             q = tbl_info.select().where(tbl_info.c.uid == uid)
             rows = conn.execute(q).fetchall()
 
-            return self.thd_createUsDict(users_row, rows)
+            return self._model_from_row(users_row, rows)
 
         return self.db.pool.do(thd)
 
-    def thd_createUsDict(self, users_row, rows):
-        # make UsDict to return
-        usdict = UsDict()
-        for row in rows:
-            usdict[row.attr_type] = row.attr_data
-
-        # add the users_row data *after* the attributes in case attr_type
-        # matches one of these keys.
-        usdict['uid'] = users_row.uid
-        usdict['identifier'] = users_row.identifier
-        usdict['bb_username'] = users_row.bb_username
-        usdict['bb_password'] = users_row.bb_password
-
-        return usdict
+    def _model_from_row(self, users_row, attribute_rows=None):
+        attributes = None
+        if attribute_rows is not None:
+            attributes = {row.attr_type: row.attr_data for row in attribute_rows}
+        return UserModel(
+            uid=users_row.uid,
+            identifier=users_row.identifier,
+            bb_username=users_row.bb_username,
+            bb_password=users_row.bb_password,
+            attributes=attributes,
+        )
 
     # returns a Deferred that returns a value
     def getUserByUsername(self, username):
@@ -138,7 +197,7 @@ def thd(conn):
             q = tbl_info.select().where(tbl_info.c.uid == users_row.uid)
             rows = conn.execute(q).fetchall()
 
-            return self.thd_createUsDict(users_row, rows)
+            return self._model_from_row(users_row, rows)
 
         return self.db.pool.do(thd)
 
@@ -148,12 +207,7 @@ def thd(conn):
             tbl = self.db.model.users
             rows = conn.execute(tbl.select()).fetchall()
 
-            dicts = []
-            if rows:
-                for row in rows:
-                    ud = {"uid": row.uid, "identifier": row.identifier}
-                    dicts.append(ud)
-            return dicts
+            return [self._model_from_row(row, attribute_rows=None) for row in rows]
 
         return self.db.pool.do(thd)
 
diff --git a/master/buildbot/test/__init__.py b/master/buildbot/test/__init__.py
index 2ac66040c79..7198db8701d 100644
--- a/master/buildbot/test/__init__.py
+++ b/master/buildbot/test/__init__.py
@@ -209,3 +209,9 @@
     r"to deadlocks in the child\.",
     category=DeprecationWarning,
 )
+
+warnings.filterwarnings(
+    "ignore",
+    r".*UsersConnectorComponent getUser, getUserByUsername, and getUsers no longer return User as dictionnaries.*",
+    category=DeprecatedApiWarning,
+)
diff --git a/master/buildbot/test/fakedb/users.py b/master/buildbot/test/fakedb/users.py
index 0f3817061c5..d14f6c8c673 100644
--- a/master/buildbot/test/fakedb/users.py
+++ b/master/buildbot/test/fakedb/users.py
@@ -13,9 +13,11 @@
 #
 # Copyright Buildbot Team Members
 
+from __future__ import annotations
 
 from twisted.internet import defer
 
+from buildbot.db.users import UserModel
 from buildbot.test.fakedb.base import FakeDBComponent
 from buildbot.test.fakedb.row import Row
 
@@ -68,16 +70,25 @@ def insert_test_data(self, rows):
                         "attr_data": row.attr_data,
                     })
 
-    def _user2dict(self, uid):
-        usdict = None
+    def _model_from_uid(self, uid: int) -> UserModel | None:
+        model = None
         if uid in self.users:
             usdict = self.users[uid]
+            model = UserModel(
+                uid=uid,
+                identifier=usdict['identifier'],
+                bb_username=usdict.get('bb_username'),
+                bb_password=usdict.get('bb_password'),
+                attributes=None,
+            )
             if uid in self.users_info:
                 infos = self.users_info[uid]
+                attributes = {}
                 for attr in infos:
-                    usdict[attr['attr_type']] = attr['attr_data']
-            usdict['uid'] = uid
-        return usdict
+                    attributes[attr['attr_type']] = attr['attr_data']
+                model.attributes = attributes
+
+        return model
 
     def nextId(self):
         self.id_num += 1
@@ -99,14 +110,14 @@ def findUserByAttr(self, identifier, attr_type, attr_data):
     def getUser(self, uid):
         usdict = None
         if uid in self.users:
-            usdict = self._user2dict(uid)
+            usdict = self._model_from_uid(uid)
         return defer.succeed(usdict)
 
     def getUserByUsername(self, username):
         usdict = None
         for uid, user in self.users.items():
             if user['bb_username'] == username:
-                usdict = self._user2dict(uid)
+                usdict = self._model_from_uid(uid)
         return defer.succeed(usdict)
 
     def updateUser(
diff --git a/master/buildbot/test/unit/db/test_users.py b/master/buildbot/test/unit/db/test_users.py
index 585f8c18954..39cfc00ee3f 100644
--- a/master/buildbot/test/unit/db/test_users.py
+++ b/master/buildbot/test/unit/db/test_users.py
@@ -56,29 +56,34 @@ def tearDown(self):
 
     user3_rows = [fakedb.User(uid=3, identifier='marla', bb_username='marla', bb_password='cancer')]
 
-    user1_dict = {
-        'uid': 1,
-        'identifier': 'soap',
-        'bb_username': None,
-        'bb_password': None,
-        'IPv9': '0578cc6.8db024',
-    }
-
-    user2_dict = {
-        'uid': 2,
-        'identifier': 'lye',
-        'bb_username': None,
-        'bb_password': None,
-        'irc': 'durden',
-        'git': 'Tyler Durden <tyler@mayhem.net>',
-    }
-
-    user3_dict = {
-        'uid': 3,
-        'identifier': 'marla',
-        'bb_username': 'marla',
-        'bb_password': 'cancer',
-    }
+    user1_model = users.UserModel(
+        uid=1,
+        identifier='soap',
+        bb_username=None,
+        bb_password=None,
+        attributes={
+            'IPv9': '0578cc6.8db024',
+        },
+    )
+
+    user2_model = users.UserModel(
+        uid=2,
+        identifier='lye',
+        bb_username=None,
+        bb_password=None,
+        attributes={
+            'irc': 'durden',
+            'git': 'Tyler Durden <tyler@mayhem.net>',
+        },
+    )
+
+    user3_model = users.UserModel(
+        uid=3,
+        identifier='marla',
+        bb_username='marla',
+        bb_password='cancer',
+        attributes={},
+    )
 
     # tests
 
@@ -234,7 +239,7 @@ def test_getUser(self):
 
         usdict = yield self.db.users.getUser(1)
 
-        self.assertEqual(usdict, self.user1_dict)
+        self.assertEqual(usdict, self.user1_model)
 
     @defer.inlineCallbacks
     def test_getUser_bb(self):
@@ -242,7 +247,7 @@ def test_getUser_bb(self):
 
         usdict = yield self.db.users.getUser(3)
 
-        self.assertEqual(usdict, self.user3_dict)
+        self.assertEqual(usdict, self.user3_model)
 
     @defer.inlineCallbacks
     def test_getUser_multi_attr(self):
@@ -250,7 +255,7 @@ def test_getUser_multi_attr(self):
 
         usdict = yield self.db.users.getUser(2)
 
-        self.assertEqual(usdict, self.user2_dict)
+        self.assertEqual(usdict, self.user2_model)
 
     @defer.inlineCallbacks
     def test_getUser_no_match(self):
@@ -272,7 +277,7 @@ def test_getUsers(self):
 
         res = yield self.db.users.getUsers()
 
-        self.assertEqual(res, [{"uid": 1, "identifier": 'soap'}])
+        self.assertEqual(res, [users.UserModel(uid=1, identifier='soap')])
 
     @defer.inlineCallbacks
     def test_getUsers_multiple(self):
@@ -280,7 +285,10 @@ def test_getUsers_multiple(self):
 
         res = yield self.db.users.getUsers()
 
-        self.assertEqual(res, [{"uid": 1, "identifier": 'soap'}, {"uid": 2, "identifier": 'lye'}])
+        self.assertEqual(
+            res,
+            [users.UserModel(uid=1, identifier='soap'), users.UserModel(uid=2, identifier='lye')],
+        )
 
     @defer.inlineCallbacks
     def test_getUserByUsername(self):
@@ -288,7 +296,7 @@ def test_getUserByUsername(self):
 
         res = yield self.db.users.getUserByUsername("marla")
 
-        self.assertEqual(res, self.user3_dict)
+        self.assertEqual(res, self.user3_model)
 
     @defer.inlineCallbacks
     def test_getUserByUsername_no_match(self):
@@ -306,8 +314,8 @@ def test_updateUser_existing_type(self):
 
         usdict = yield self.db.users.getUser(1)
 
-        self.assertEqual(usdict['IPv9'], 'abcd.1234')
-        self.assertEqual(usdict['identifier'], 'soap')  # no change
+        self.assertEqual(usdict.attributes['IPv9'], 'abcd.1234')
+        self.assertEqual(usdict.identifier, 'soap')  # no change
 
     @defer.inlineCallbacks
     def test_updateUser_new_type(self):
@@ -317,9 +325,9 @@ def test_updateUser_new_type(self):
 
         usdict = yield self.db.users.getUser(1)
 
-        self.assertEqual(usdict['IPv4'], '123.134.156.167')
-        self.assertEqual(usdict['IPv9'], '0578cc6.8db024')  # no change
-        self.assertEqual(usdict['identifier'], 'soap')  # no change
+        self.assertEqual(usdict.attributes['IPv4'], '123.134.156.167')
+        self.assertEqual(usdict.attributes['IPv9'], '0578cc6.8db024')  # no change
+        self.assertEqual(usdict.identifier, 'soap')  # no change
 
     @defer.inlineCallbacks
     def test_updateUser_identifier(self):
@@ -329,8 +337,8 @@ def test_updateUser_identifier(self):
 
         usdict = yield self.db.users.getUser(1)
 
-        self.assertEqual(usdict['identifier'], 'lye')
-        self.assertEqual(usdict['IPv9'], '0578cc6.8db024')  # no change
+        self.assertEqual(usdict.identifier, 'lye')
+        self.assertEqual(usdict.attributes['IPv9'], '0578cc6.8db024')  # no change
 
     @defer.inlineCallbacks
     def test_updateUser_bb(self):
@@ -340,9 +348,9 @@ def test_updateUser_bb(self):
 
         usdict = yield self.db.users.getUser(3)
 
-        self.assertEqual(usdict['bb_username'], 'boss')
-        self.assertEqual(usdict['bb_password'], 'fired')
-        self.assertEqual(usdict['identifier'], 'marla')  # no change
+        self.assertEqual(usdict.bb_username, 'boss')
+        self.assertEqual(usdict.bb_password, 'fired')
+        self.assertEqual(usdict.identifier, 'marla')  # no change
 
     @defer.inlineCallbacks
     def test_updateUser_all(self):
@@ -359,11 +367,19 @@ def test_updateUser_all(self):
 
         usdict = yield self.db.users.getUser(1)
 
-        self.assertEqual(usdict['identifier'], 'lye')
-        self.assertEqual(usdict['bb_username'], 'marla')
-        self.assertEqual(usdict['bb_password'], 'cancer')
-        self.assertEqual(usdict['IPv4'], '123.134.156.167')
-        self.assertEqual(usdict['IPv9'], '0578cc6.8db024')  # no change
+        self.assertEqual(
+            usdict,
+            users.UserModel(
+                uid=1,
+                identifier='lye',
+                bb_username='marla',
+                bb_password='cancer',
+                attributes={
+                    'IPv4': '123.134.156.167',
+                    'IPv9': '0578cc6.8db024',  # no change
+                },
+            ),
+        )
 
     @defer.inlineCallbacks
     def test_updateUser_race(self):
@@ -404,12 +420,12 @@ def race_thd(conn):
 
         usdict = yield self.db.users.getUser(1)
 
-        self.assertEqual(usdict['identifier'], 'soap')
+        self.assertEqual(usdict.identifier, 'soap')
         if transaction_wins:
-            self.assertEqual(usdict['IPv4'], '123.134.156.167')
+            self.assertEqual(usdict.attributes['IPv4'], '123.134.156.167')
         else:
-            self.assertEqual(usdict['IPv4'], '8.8.8.8')
-        self.assertEqual(usdict['IPv9'], '0578cc6.8db024')  # no change
+            self.assertEqual(usdict.attributes['IPv4'], '8.8.8.8')
+        self.assertEqual(usdict.attributes['IPv9'], '0578cc6.8db024')  # no change
 
     @defer.inlineCallbacks
     def test_update_NoMatch_identifier(self):
@@ -419,7 +435,7 @@ def test_update_NoMatch_identifier(self):
 
         usdict = yield self.db.users.getUser(1)
 
-        self.assertEqual(usdict['identifier'], 'soap')  # no change
+        self.assertEqual(usdict.identifier, 'soap')  # no change
 
     @defer.inlineCallbacks
     def test_update_NoMatch_attribute(self):
@@ -429,7 +445,7 @@ def test_update_NoMatch_attribute(self):
 
         usdict = yield self.db.users.getUser(1)
 
-        self.assertEqual(usdict['IPv9'], '0578cc6.8db024')  # no change
+        self.assertEqual(usdict.attributes['IPv9'], '0578cc6.8db024')  # no change
 
     @defer.inlineCallbacks
     def test_update_NoMatch_bb(self):
@@ -439,7 +455,7 @@ def test_update_NoMatch_bb(self):
 
         usdict = yield self.db.users.getUser(1)
 
-        self.assertEqual(usdict['IPv9'], '0578cc6.8db024')  # no change
+        self.assertEqual(usdict.attributes['IPv9'], '0578cc6.8db024')  # no change
 
     @defer.inlineCallbacks
     def test_removeUser_uid(self):
diff --git a/master/buildbot/test/unit/process/test_users_manual.py b/master/buildbot/test/unit/process/test_users_manual.py
index 374aaa89653..fcbf7fd6e13 100644
--- a/master/buildbot/test/unit/process/test_users_manual.py
+++ b/master/buildbot/test/unit/process/test_users_manual.py
@@ -21,6 +21,7 @@
 from twisted.internet import defer
 from twisted.trial import unittest
 
+from buildbot.db.users import UserModel
 from buildbot.process.users import manual
 from buildbot.test.fake import fakemaster
 from buildbot.test.reactor import TestReactorMixin
@@ -62,7 +63,9 @@ def test_perspective_commandline_add(self):
 
         self.assertEqual(
             usdict,
-            {"uid": 1, "identifier": 'x', "bb_username": None, "bb_password": None, "git": 'x'},
+            UserModel(
+                uid=1, identifier='x', bb_username=None, bb_password=None, attributes={"git": 'x'}
+            ),
         )
 
     @defer.inlineCallbacks
@@ -78,7 +81,9 @@ def test_perspective_commandline_update(self):
 
         self.assertEqual(
             usdict,
-            {"uid": 1, "identifier": 'x', "bb_username": None, "bb_password": None, "svn": 'y'},
+            UserModel(
+                uid=1, identifier='x', bb_username=None, bb_password=None, attributes={"svn": 'y'}
+            ),
         )
 
     @defer.inlineCallbacks
@@ -94,13 +99,13 @@ def test_perspective_commandline_update_bb(self):
 
         self.assertEqual(
             usdict,
-            {
-                "uid": 1,
-                "identifier": 'x',
-                "bb_username": 'bb_user',
-                "bb_password": 'hashed_bb_pass',
-                "svn": 'x',
-            },
+            UserModel(
+                uid=1,
+                identifier='x',
+                bb_username='bb_user',
+                bb_password='hashed_bb_pass',
+                attributes={"svn": 'x'},
+            ),
         )
 
     @defer.inlineCallbacks
@@ -115,13 +120,13 @@ def test_perspective_commandline_update_both(self):
         usdict = yield self.master.db.users.getUser(1)
         self.assertEqual(
             usdict,
-            {
-                "uid": 1,
-                "identifier": 'x',
-                "bb_username": 'bb_user',
-                "bb_password": 'hashed_bb_pass',
-                "svn": 'y',
-            },
+            UserModel(
+                uid=1,
+                identifier='x',
+                bb_username='bb_user',
+                bb_password='hashed_bb_pass',
+                attributes={"svn": 'y'},
+            ),
         )
 
     @defer.inlineCallbacks
@@ -143,7 +148,14 @@ def test_perspective_commandline_get(self):
 
         res = yield self.master.db.users.getUser(1)
         self.assertEqual(
-            res, {"uid": 1, "identifier": 'x', "bb_username": None, "bb_password": None, "svn": 'x'}
+            res,
+            UserModel(
+                uid=1,
+                identifier='x',
+                bb_username=None,
+                bb_password=None,
+                attributes={"svn": 'x'},
+            ),
         )
 
     @defer.inlineCallbacks
@@ -156,14 +168,13 @@ def test_perspective_commandline_get_multiple_attrs(self):
         res = yield self.master.db.users.getUser(1)
         self.assertEqual(
             res,
-            {
-                "uid": 1,
-                "identifier": 'x',
-                "bb_username": None,
-                "bb_password": None,
-                "svn": 'x',
-                "git": 'x@c',
-            },
+            UserModel(
+                uid=1,
+                identifier='x',
+                bb_username=None,
+                bb_password=None,
+                attributes={"svn": 'x', 'git': 'x@c'},
+            ),
         )
 
     @defer.inlineCallbacks
diff --git a/master/docs/developer/database/users.rst b/master/docs/developer/database/users.rst
index 294646f80b1..3497ba306e5 100644
--- a/master/docs/developer/database/users.rst
+++ b/master/docs/developer/database/users.rst
@@ -17,16 +17,14 @@ Users connector
     type.  The :py:meth:`findUserByAttr` method uses these attributes to match users,
     adding a new user if no matching user is found.
 
-    Users are identified canonically by *uid*, and are represented by *usdicts* (user
-    dictionaries) with the following keys:
+    Users are identified canonically by *uid*, and are represented by a :class:`UserModel`
+    dataclass with the following fields:
 
     * ``uid``
     * ``identifier`` (display name for the user)
     * ``bb_username`` (buildbot login username)
     * ``bb_password`` (hashed login password)
-
-    All attributes are also included in the dictionary, keyed by type.  Types
-    colliding with the keys above are ignored.
+    * ``attributes`` (a dictionary of attributes, keyed by type)
 
     .. py:method:: findUserByAttr(identifier, attr_type, attr_data)
 
@@ -54,26 +52,26 @@ Users connector
         :type key: int
         :param no_cache: bypass cache and always fetch from database
         :type no_cache: boolean
-        :returns: usdict via Deferred
+        :returns: :class:`UserModel` or ``None`` via Deferred
 
-        Get a usdict for the given user, or ``None`` if no matching user is
+        Get a :class:`UserModel` for the given user, or ``None`` if no matching user is
         found.
 
     .. py:method:: getUserByUsername(username)
 
         :param username: username portion of user credentials
         :type username: string
-        :returns: usdict or None via deferred
+        :returns: :class:`UserModel` or None via deferred
 
-        Looks up the user with the bb_username, returning the usdict or
+        Looks up the user with the bb_username, returning a :class:`UserModel` or
         ``None`` if no matching user is found.
 
     .. py:method:: getUsers()
 
-        :returns: list of partial usdicts via Deferred
+        :returns: list of :class:`UserModel` without ``attributes`` via Deferred
 
         Get the entire list of users.  User attributes are not included, so the
-        results are not full usdicts.
+        ``attributes`` field of the resulting :class:`UserModel` are ``None``.
 
     .. py:method:: updateUser(uid=None, identifier=None, bb_username=None, bb_password=None, attr_type=None, attr_data=None)
 
diff --git a/master/docs/spelling_wordlist.txt b/master/docs/spelling_wordlist.txt
index ca6c92dfd47..dd6632640b5 100644
--- a/master/docs/spelling_wordlist.txt
+++ b/master/docs/spelling_wordlist.txt
@@ -951,6 +951,7 @@ userid
 userids
 userInfoProvider
 userlist
+UserModel
 username
 Username
 usernames
diff --git a/newsfragments/db-introduce-UserModel.change b/newsfragments/db-introduce-UserModel.change
new file mode 100644
index 00000000000..dc2d65fcd40
--- /dev/null
+++ b/newsfragments/db-introduce-UserModel.change
@@ -0,0 +1 @@
+:class:`UsersConnectorComponent` `getUser`, `getUserByUsername`, and `getUsers` now return a :class`UserModel` instead of a dictionary of User data
\ No newline at end of file
diff --git a/newsfragments/db-usdict.removal b/newsfragments/db-usdict.removal
new file mode 100644
index 00000000000..24e1d480e3e
--- /dev/null
+++ b/newsfragments/db-usdict.removal
@@ -0,0 +1 @@
+:class:`buildbot.db.users.UsDict` is deprecated in favor of :class:`buildbot.db.users.UserModel`
\ No newline at end of file

From c06fbcd523dac7b23fd0f5fa8ef1abd7f968ae44 Mon Sep 17 00:00:00 2001
From: Thomas Desveaux <thomas.desveaux@dont-nod.com>
Date: Tue, 21 May 2024 10:19:27 +0200
Subject: [PATCH 2/3] Update usage of UserModel get and keys

---
 master/buildbot/db/users.py                   | 25 -------------------
 master/buildbot/process/users/manual.py       | 15 ++++++++---
 master/buildbot/process/users/users.py        | 11 +++++---
 master/buildbot/test/__init__.py              |  6 -----
 .../test/unit/process/test_users_manual.py    | 10 +++++++-
 5 files changed, 28 insertions(+), 39 deletions(-)

diff --git a/master/buildbot/db/users.py b/master/buildbot/db/users.py
index 429f9724955..fcb7e1cd92e 100644
--- a/master/buildbot/db/users.py
+++ b/master/buildbot/db/users.py
@@ -55,31 +55,6 @@ def __getitem__(self, key: str):
 
         raise KeyError(key)
 
-    def get(self, key: str, default=None):
-        try:
-            return self[key]
-        except KeyError:
-            return default
-
-    def keys(self):
-        warn_deprecated(
-            '4.1.0',
-            (
-                'UsersConnectorComponent '
-                'getUser, getUserByUsername, and getUsers '
-                'no longer return User as dictionnaries. '
-                'Usage of keys is deprecated: please access the member directly'
-            ),
-        )
-
-        keys = set()
-        if self.attributes is not None:
-            keys.update(self.attributes.keys())
-
-        keys.update(f.name for f in dataclasses.fields(self) if f.name != 'attributes')
-
-        return keys
-
 
 @deprecate.deprecated(versions.Version("buildbot", 4, 1, 0), UserModel)
 class UsDict(UserModel):
diff --git a/master/buildbot/process/users/manual.py b/master/buildbot/process/users/manual.py
index 0814203b43d..642610d5e33 100644
--- a/master/buildbot/process/users/manual.py
+++ b/master/buildbot/process/users/manual.py
@@ -73,10 +73,17 @@ def formatResults(self, op, results):
             formatted_results += "user(s) found:\n"
             for user in results:
                 if user:
-                    for key in sorted(user.keys()):
-                        if key != 'bb_password':
-                            formatted_results += f"{key}: {user[key]}\n"
-                    formatted_results += "\n"
+                    formatted_results += (
+                        f"uid: {user.uid}\n"
+                        f"identifier: {user.identifier}\n"
+                        f"bb_username: {user.bb_username}\n"
+                    )
+                    if user.attributes:
+                        formatted_results += "attributes:\n"
+                        formatted_results += (
+                            ''.join(f"\t{key}: {value}\n" for key, value in user.attributes.items())
+                            + '\n'
+                        )
                 else:
                     formatted_results += "no match found\n"
         return formatted_results
diff --git a/master/buildbot/process/users/users.py b/master/buildbot/process/users/users.py
index 896d4e4f258..d804e66ef7e 100644
--- a/master/buildbot/process/users/users.py
+++ b/master/buildbot/process/users/users.py
@@ -13,10 +13,12 @@
 #
 # Copyright Buildbot Team Members
 
+from __future__ import annotations
 
 import os
 from binascii import hexlify
 from hashlib import sha1
+from typing import TYPE_CHECKING
 
 from twisted.internet import defer
 from twisted.python import log
@@ -24,6 +26,9 @@
 from buildbot.util import bytes2unicode
 from buildbot.util import unicode2bytes
 
+if TYPE_CHECKING:
+    from buildbot.db.users import UserModel
+
 # TODO: fossil comes from a plugin. We should have an API that plugins could use to
 # register allowed user types.
 srcs = ['git', 'svn', 'hg', 'cvs', 'darcs', 'bzr', 'fossil']
@@ -63,10 +68,10 @@ def createUserObject(master, author, src=None):
     )
 
 
-def _extractContact(usdict, contact_types, uid):
-    if usdict:
+def _extractContact(user: UserModel | None, contact_types, uid):
+    if user is not None and user.attributes is not None:
         for type in contact_types:
-            contact = usdict.get(type)
+            contact = user.attributes.get(type)
             if contact:
                 break
     else:
diff --git a/master/buildbot/test/__init__.py b/master/buildbot/test/__init__.py
index 7198db8701d..2ac66040c79 100644
--- a/master/buildbot/test/__init__.py
+++ b/master/buildbot/test/__init__.py
@@ -209,9 +209,3 @@
     r"to deadlocks in the child\.",
     category=DeprecationWarning,
 )
-
-warnings.filterwarnings(
-    "ignore",
-    r".*UsersConnectorComponent getUser, getUserByUsername, and getUsers no longer return User as dictionnaries.*",
-    category=DeprecatedApiWarning,
-)
diff --git a/master/buildbot/test/unit/process/test_users_manual.py b/master/buildbot/test/unit/process/test_users_manual.py
index fcbf7fd6e13..bcb0d02da4b 100644
--- a/master/buildbot/test/unit/process/test_users_manual.py
+++ b/master/buildbot/test/unit/process/test_users_manual.py
@@ -216,7 +216,15 @@ def test_perspective_commandline_get_format(self):
 
         result = yield self.call_perspective_commandline('get', None, None, ['x@y'], None)
 
-        exp_format = 'user(s) found:\nbb_username: None\ngit: x <x@y>\nidentifier: x@y\nuid: 1\n\n'
+        exp_format = (
+            'user(s) found:\n'
+            'uid: 1\n'
+            'identifier: x@y\n'
+            'bb_username: None\n'
+            'attributes:\n'
+            '\tgit: x <x@y>\n'
+            '\n'
+        )
         self.assertEqual(result, exp_format)
 
     @defer.inlineCallbacks

From 9a82ee5450fbe32dac4c7564936def002338ac4d Mon Sep 17 00:00:00 2001
From: Thomas Desveaux <thomas.desveaux@dont-nod.com>
Date: Tue, 21 May 2024 09:43:12 +0200
Subject: [PATCH 3/3] db: type-hint UsersConnectorComponent

---
 master/buildbot/db/users.py          | 41 +++++++++++++++-------------
 master/buildbot/test/fakedb/users.py |  4 +--
 2 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/master/buildbot/db/users.py b/master/buildbot/db/users.py
index fcb7e1cd92e..23e13cacf52 100644
--- a/master/buildbot/db/users.py
+++ b/master/buildbot/db/users.py
@@ -17,6 +17,7 @@
 from __future__ import annotations
 
 import dataclasses
+from typing import TYPE_CHECKING
 
 import sqlalchemy as sa
 from twisted.python import deprecate
@@ -26,6 +27,9 @@
 from buildbot.util import identifiers
 from buildbot.warnings import warn_deprecated
 
+if TYPE_CHECKING:
+    from twisted.internet import defer
+
 
 @dataclasses.dataclass
 class UserModel:
@@ -62,11 +66,12 @@ class UsDict(UserModel):
 
 
 class UsersConnectorComponent(base.DBConnectorComponent):
-    # returns a Deferred that returns a value
-    def findUserByAttr(self, identifier, attr_type, attr_data, _race_hook=None):
+    def findUserByAttr(
+        self, identifier: str, attr_type: str, attr_data: str, _race_hook=None
+    ) -> defer.Deferred[int]:
         # note that since this involves two tables, self.findSomethingId is not
         # helpful
-        def thd(conn, no_recurse=False, identifier=identifier):
+        def thd(conn, no_recurse=False, identifier=identifier) -> int:
             tbl = self.db.model.users
             tbl_info = self.db.model.users_info
 
@@ -123,10 +128,9 @@ def thd(conn, no_recurse=False, identifier=identifier):
 
         return self.db.pool.do(thd)
 
-    # returns a Deferred that returns a value
     @base.cached("usdicts")
-    def getUser(self, uid):
-        def thd(conn):
+    def getUser(self, uid: int) -> defer.Deferred[UserModel | None]:
+        def thd(conn) -> UserModel | None:
             tbl = self.db.model.users
             tbl_info = self.db.model.users_info
 
@@ -157,8 +161,8 @@ def _model_from_row(self, users_row, attribute_rows=None):
         )
 
     # returns a Deferred that returns a value
-    def getUserByUsername(self, username):
-        def thd(conn):
+    def getUserByUsername(self, username: str | None) -> defer.Deferred[UserModel | None]:
+        def thd(conn) -> UserModel | None:
             tbl = self.db.model.users
             tbl_info = self.db.model.users_info
 
@@ -176,9 +180,8 @@ def thd(conn):
 
         return self.db.pool.do(thd)
 
-    # returns a Deferred that returns a value
-    def getUsers(self):
-        def thd(conn):
+    def getUsers(self) -> defer.Deferred[list[UserModel]]:
+        def thd(conn) -> list[UserModel]:
             tbl = self.db.model.users
             rows = conn.execute(tbl.select()).fetchall()
 
@@ -189,12 +192,12 @@ def thd(conn):
     # returns a Deferred that returns None
     def updateUser(
         self,
-        uid=None,
-        identifier=None,
-        bb_username=None,
-        bb_password=None,
-        attr_type=None,
-        attr_data=None,
+        uid: int | None = None,
+        identifier: str | None = None,
+        bb_username: str | None = None,
+        bb_password: str | None = None,
+        attr_type: str | None = None,
+        attr_data: str | None = None,
         _race_hook=None,
     ):
         def thd(conn):
@@ -269,8 +272,8 @@ def thd(conn):
         return self.db.pool.do(thd)
 
     # returns a Deferred that returns a value
-    def identifierToUid(self, identifier):
-        def thd(conn):
+    def identifierToUid(self, identifier) -> defer.Deferred[int | None]:
+        def thd(conn) -> int | None:
             tbl = self.db.model.users
 
             q = tbl.select().where(tbl.c.identifier == identifier)
diff --git a/master/buildbot/test/fakedb/users.py b/master/buildbot/test/fakedb/users.py
index d14f6c8c673..97f5a89a00b 100644
--- a/master/buildbot/test/fakedb/users.py
+++ b/master/buildbot/test/fakedb/users.py
@@ -107,13 +107,13 @@ def findUserByAttr(self, identifier, attr_type, attr_data):
         self.db.insert_test_data([UserInfo(uid=uid, attr_type=attr_type, attr_data=attr_data)])
         return defer.succeed(uid)
 
-    def getUser(self, uid):
+    def getUser(self, uid) -> defer.Deferred[UserModel | None]:
         usdict = None
         if uid in self.users:
             usdict = self._model_from_uid(uid)
         return defer.succeed(usdict)
 
-    def getUserByUsername(self, username):
+    def getUserByUsername(self, username) -> defer.Deferred[UserModel | None]:
         usdict = None
         for uid, user in self.users.items():
             if user['bb_username'] == username: