♻️ Refactor UsersRepo to use instance methods for user creation and linking

Author: pcrespov

packages/postgres-database/src/simcore_postgres_database/utils_users.py

@@ -55,10 +55,11 @@ def generate_alternative_username(username: str) -> str:
 
 
 class UsersRepo:
+    def __init__(self, engine: AsyncEngine):
+        self._engine = engine
 
-    @staticmethod
     async def new_user(
-        engine: AsyncEngine,
+        self,
         connection: AsyncConnection | None = None,
         *,
         email: str,
@@ -78,14 +79,14 @@ async def new_user(
         user_id = None
         while user_id is None:
             try:
-                async with transaction_context(engine, connection) as conn:
+                async with transaction_context(self._engine, connection) as conn:
                     user_id = await conn.scalar(
                         users.insert().values(**data).returning(users.c.id)
                     )
             except IntegrityError:
                 data["name"] = generate_alternative_username(data["name"])
 
-        async with pass_or_acquire_connection(engine, connection) as conn:
+        async with pass_or_acquire_connection(self._engine, connection) as conn:
             result = await conn.execute(
                 sa.select(
                     users.c.id,
@@ -97,9 +98,9 @@ async def new_user(
             )
             return result.one()
 
-    @staticmethod
     async def link_and_update_user_from_pre_registration(
-        connection: AsyncConnection,
+        self,
+        connection: AsyncConnection | None = None,
         *,
         new_user_id: int,
         new_user_email: str,
@@ -113,48 +114,51 @@ async def link_and_update_user_from_pre_registration(
         assert new_user_email  # nosec
         assert new_user_id > 0  # nosec
 
-        # link both tables first
-        result = await connection.execute(
-            users_pre_registration_details.update()
-            .where(users_pre_registration_details.c.pre_email == new_user_email)
-            .values(user_id=new_user_id)
-        )
-
-        if update_user:
-            # COPIES some pre-registration details to the users table
-            pre_columns = (
-                users_pre_registration_details.c.pre_first_name,
-                users_pre_registration_details.c.pre_last_name,
-                # NOTE: pre_phone is not copied since it has to be validated. Otherwise, if
-                # phone is wrong, currently user won't be able to login!
+        async with transaction_context(self._engine, connection) as conn:
+            # link both tables first
+            result = await conn.execute(
+                users_pre_registration_details.update()
+                .where(users_pre_registration_details.c.pre_email == new_user_email)
+                .values(user_id=new_user_id)
             )
 
-            assert {c.name for c in pre_columns} == {  # nosec
-                c.name
-                for c in users_pre_registration_details.columns
-                if c
-                not in (
-                    users_pre_registration_details.c.pre_email,
-                    users_pre_registration_details.c.pre_phone,
+            if update_user:
+                # COPIES some pre-registration details to the users table
+                pre_columns = (
+                    users_pre_registration_details.c.pre_first_name,
+                    users_pre_registration_details.c.pre_last_name,
+                    # NOTE: pre_phone is not copied since it has to be validated. Otherwise, if
+                    # phone is wrong, currently user won't be able to login!
                 )
-                and c.name.startswith("pre_")
-            }, "Different pre-cols detected. This code might need an update update"
 
-            result = await connection.execute(
-                sa.select(*pre_columns).where(
-                    users_pre_registration_details.c.pre_email == new_user_email
-                )
-            )
-            if pre_registration_details_data := result.first():
-                # NOTE: could have many products! which to use?
-                await connection.execute(
-                    users.update()
-                    .where(users.c.id == new_user_id)
-                    .values(
-                        first_name=pre_registration_details_data.pre_first_name,  # type: ignore[union-attr]
-                        last_name=pre_registration_details_data.pre_last_name,  # type: ignore[union-attr]
+                assert {c.name for c in pre_columns} == {  # nosec
+                    c.name
+                    for c in users_pre_registration_details.columns
+                    if c
+                    not in (
+                        users_pre_registration_details.c.pre_email,
+                        users_pre_registration_details.c.pre_phone,
+                    )
+                    and c.name.startswith("pre_")
+                }, "Different pre-cols detected. This code might need an update update"
+
+                result = await conn.execute(
+                    sa.select(*pre_columns).where(
+                        users_pre_registration_details.c.pre_email
+                        == new_user_email
+                        # FIXME: product_name!
                     )
                 )
+                if pre_registration_details_data := result.first():
+                    # NOTE: could have many products! which to use?
+                    await conn.execute(
+                        users.update()
+                        .where(users.c.id == new_user_id)
+                        .values(
+                            first_name=pre_registration_details_data.pre_first_name,  # type: ignore[union-attr]
+                            last_name=pre_registration_details_data.pre_last_name,  # type: ignore[union-attr]
+                        )
+                    )
 
     @staticmethod
     def get_billing_details_query(user_id: int):

packages/postgres-database/tests/test_users.py

@@ -143,7 +143,8 @@ async def test_new_user(
         "status": UserStatus.ACTIVE,
         "expires_at": datetime.utcnow(),
     }
-    new_user = await UsersRepo.new_user(asyncpg_engine, **data)
+    repo = UsersRepo(asyncpg_engine)
+    new_user = await repo.new_user(**data)
 
     assert new_user.email == data["email"]
     assert new_user.status == data["status"]
@@ -153,7 +154,7 @@ async def test_new_user(
     assert _generate_username_from_email(other_email) == new_user.name
     other_data = {**data, "email": other_email}
 
-    other_user = await UsersRepo.new_user(asyncpg_engine, **other_data)
+    other_user = await repo.new_user(**other_data)
     assert other_user.email != new_user.email
     assert other_user.name != new_user.name
 

packages/postgres-database/tests/test_users_details.py

@@ -257,16 +257,18 @@ async def test_create_and_link_user_from_pre_registration(
     # Invitation link is clicked and the user is created and linked to the pre-registration
     async with transaction_context(asyncpg_engine) as connection:
         # user gets created
-        new_user = await UsersRepo.new_user(
-            asyncpg_engine,
+        repo = UsersRepo(asyncpg_engine)
+        new_user = await repo.new_user(
             connection,
             email=pre_email,
             password_hash="123456",  # noqa: S106
             status=UserStatus.ACTIVE,
             expires_at=None,
         )
-        await UsersRepo.link_and_update_user_from_pre_registration(
-            connection, new_user_id=new_user.id, new_user_email=new_user.email
+        await repo.link_and_update_user_from_pre_registration(
+            connection,
+            new_user_id=new_user.id,
+            new_user_email=new_user.email,
         )
 
     # Verify the user was created and linked
@@ -292,16 +294,18 @@ async def test_get_billing_details_from_pre_registration(
 
     # Create the user
     async with transaction_context(asyncpg_engine) as connection:
-        new_user = await UsersRepo.new_user(
-            asyncpg_engine,
+        repo = UsersRepo(asyncpg_engine)
+        new_user = await repo.new_user(
             connection,
             email=pre_email,
             password_hash="123456",  # noqa: S106
             status=UserStatus.ACTIVE,
             expires_at=None,
         )
-        await UsersRepo.link_and_update_user_from_pre_registration(
-            connection, new_user_id=new_user.id, new_user_email=new_user.email
+        await repo.link_and_update_user_from_pre_registration(
+            connection,
+            new_user_id=new_user.id,
+            new_user_email=new_user.email,
         )
 
     # Get billing details
@@ -333,16 +337,18 @@ async def test_update_user_from_pre_registration(
 
     # Create the user and link to pre-registration
     async with transaction_context(asyncpg_engine) as connection:
-        new_user = await UsersRepo.new_user(
-            asyncpg_engine,
+        repo = UsersRepo(asyncpg_engine)
+        new_user = await repo.new_user(
             connection,
             email=pre_email,
             password_hash="123456",  # noqa: S106
             status=UserStatus.ACTIVE,
             expires_at=None,
         )
-        await UsersRepo.link_and_update_user_from_pre_registration(
-            connection, new_user_id=new_user.id, new_user_email=new_user.email
+        await repo.link_and_update_user_from_pre_registration(
+            connection,
+            new_user_id=new_user.id,
+            new_user_email=new_user.email,
         )
 
     # Update the user manually
@@ -361,8 +367,11 @@ async def test_update_user_from_pre_registration(
 
     # Re-link the user to pre-registration, which should override manual updates
     async with transaction_context(asyncpg_engine) as connection:
-        await UsersRepo.link_and_update_user_from_pre_registration(
-            connection, new_user_id=new_user.id, new_user_email=new_user.email
+        repo = UsersRepo(asyncpg_engine)
+        await repo.link_and_update_user_from_pre_registration(
+            connection,
+            new_user_id=new_user.id,
+            new_user_email=new_user.email,
         )
 
         result = await connection.execute(
@@ -492,16 +501,18 @@ async def test_user_preregisters_for_multiple_products_with_different_outcomes(
 
     # 3.Now create a user account with the approved pre-registration
     async with transaction_context(asyncpg_engine) as connection:
-        new_user = await UsersRepo.new_user(
-            asyncpg_engine,
+        repo = UsersRepo(asyncpg_engine)
+        new_user = await repo.new_user(
             connection,
             email=user_email,
             password_hash="123456",  # noqa: S106
             status=UserStatus.ACTIVE,
             expires_at=None,
         )
-        await UsersRepo.link_and_update_user_from_pre_registration(
-            connection, new_user_id=new_user.id, new_user_email=new_user.email
+        await repo.link_and_update_user_from_pre_registration(
+            connection,
+            new_user_id=new_user.id,
+            new_user_email=new_user.email,
         )
 
     # Verify both pre-registrations are linked to the new user

packages/postgres-database/tests/test_utils_users.py

@@ -41,7 +41,7 @@ async def user(
 
 
 async def test_users_repo_get(asyncpg_engine: AsyncEngine, user: dict[str, Any]):
-    repo = UsersRepo()
+    repo = UsersRepo(asyncpg_engine)
 
     async with pass_or_acquire_connection(asyncpg_engine) as connection:
         assert await repo.get_email(connection, user_id=user["id"]) == user["email"]

services/web/server/src/simcore_service_webserver/login/_auth_service.py

@@ -32,16 +32,16 @@ async def create_user(
 ) -> dict[str, Any]:
 
     asyncpg_engine = get_asyncpg_engine(app)
+    repo = UsersRepo(asyncpg_engine)
     async with transaction_context(asyncpg_engine) as conn:
-        user = await UsersRepo.new_user(
-            asyncpg_engine,
+        user = await repo.new_user(
             conn,
             email=email,
             password_hash=security_service.encrypt_password(password),
             status=status_upon_creation,
             expires_at=expires_at,
         )
-        await UsersRepo.link_and_update_user_from_pre_registration(
+        await repo.link_and_update_user_from_pre_registration(
             conn, new_user_id=user.id, new_user_email=user.email
         )
     return dict(user._mapping)  # pylint: disable=protected-access # noqa: SLF001