fix: added suggestions from @seratch

galuszkak · galuszkak · commit 4be23e76b865 · 2025-01-14T17:17:50.000+01:00
diff --git a/slack_sdk/oauth/installation_store/sqlalchemy/__init__.py b/slack_sdk/oauth/installation_store/sqlalchemy/__init__.py
@@ -222,21 +222,7 @@ def find_bot(
         with self.engine.connect() as conn:
             result: object = conn.execute(query)
             for row in result.mappings():  # type: ignore[attr-defined]
-                return Bot(
-                    app_id=row["app_id"],
-                    enterprise_id=row["enterprise_id"],
-                    enterprise_name=row["enterprise_name"],
-                    team_id=row["team_id"],
-                    team_name=row["team_name"],
-                    bot_token=row["bot_token"],
-                    bot_id=row["bot_id"],
-                    bot_user_id=row["bot_user_id"],
-                    bot_scopes=row["bot_scopes"],
-                    bot_refresh_token=row["bot_refresh_token"],
-                    bot_token_expires_at=row["bot_token_expires_at"],
-                    is_enterprise_install=row["is_enterprise_install"],
-                    installed_at=row["installed_at"],
-                )
+                return self.build_bot_entity(row)
             return None
 
     def find_installation(
@@ -270,33 +256,8 @@ def find_installation(
         with self.engine.connect() as conn:
             result: object = conn.execute(query)
             for row in result.mappings():  # type: ignore[attr-defined]
-                installation = Installation(
-                    app_id=row["app_id"],
-                    enterprise_id=row["enterprise_id"],
-                    enterprise_name=row["enterprise_name"],
-                    enterprise_url=row["enterprise_url"],
-                    team_id=row["team_id"],
-                    team_name=row["team_name"],
-                    bot_token=row["bot_token"],
-                    bot_id=row["bot_id"],
-                    bot_user_id=row["bot_user_id"],
-                    bot_scopes=row["bot_scopes"],
-                    bot_refresh_token=row["bot_refresh_token"],
-                    bot_token_expires_at=row["bot_token_expires_at"],
-                    user_id=row["user_id"],
-                    user_token=row["user_token"],
-                    user_scopes=row["user_scopes"],
-                    user_refresh_token=row["user_refresh_token"],
-                    user_token_expires_at=row["user_token_expires_at"],
-                    # Only the incoming webhook issued in the latest installation is set in this logic
-                    incoming_webhook_url=row["incoming_webhook_url"],
-                    incoming_webhook_channel=row["incoming_webhook_channel"],
-                    incoming_webhook_channel_id=row["incoming_webhook_channel_id"],
-                    incoming_webhook_configuration_url=row["incoming_webhook_configuration_url"],
-                    is_enterprise_install=row["is_enterprise_install"],
-                    token_type=row["token_type"],
-                    installed_at=row["installed_at"],
-                )
+                (row)
+                installation = self.build_installation_entity(row)
 
         has_user_installation = user_id is not None and installation is not None
         no_bot_token_installation = installation is not None and installation.bot_token is None
@@ -366,6 +327,54 @@ def delete_installation(
                 )
                 conn.execute(deletion)
 
+    @classmethod
+    def build_installation_entity(row) -> Installation:
+        return Installation(
+            app_id=row["app_id"],
+            enterprise_id=row["enterprise_id"],
+            enterprise_name=row["enterprise_name"],
+            enterprise_url=row["enterprise_url"],
+            team_id=row["team_id"],
+            team_name=row["team_name"],
+            bot_token=row["bot_token"],
+            bot_id=row["bot_id"],
+            bot_user_id=row["bot_user_id"],
+            bot_scopes=row["bot_scopes"],
+            bot_refresh_token=row["bot_refresh_token"],
+            bot_token_expires_at=row["bot_token_expires_at"],
+            user_id=row["user_id"],
+            user_token=row["user_token"],
+            user_scopes=row["user_scopes"],
+            user_refresh_token=row["user_refresh_token"],
+            user_token_expires_at=row["user_token_expires_at"],
+            # Only the incoming webhook issued in the latest installation is set in this logic
+            incoming_webhook_url=row["incoming_webhook_url"],
+            incoming_webhook_channel=row["incoming_webhook_channel"],
+            incoming_webhook_channel_id=row["incoming_webhook_channel_id"],
+            incoming_webhook_configuration_url=row["incoming_webhook_configuration_url"],
+            is_enterprise_install=row["is_enterprise_install"],
+            token_type=row["token_type"],
+            installed_at=row["installed_at"],
+        )
+
+    @classmethod
+    def build_bot_entity(row) -> Bot:
+        return Bot(
+            app_id=row["app_id"],
+            enterprise_id=row["enterprise_id"],
+            enterprise_name=row["enterprise_name"],
+            team_id=row["team_id"],
+            team_name=row["team_name"],
+            bot_token=row["bot_token"],
+            bot_id=row["bot_id"],
+            bot_user_id=row["bot_user_id"],
+            bot_scopes=row["bot_scopes"],
+            bot_refresh_token=row["bot_refresh_token"],
+            bot_token_expires_at=row["bot_token_expires_at"],
+            is_enterprise_install=row["is_enterprise_install"],
+            installed_at=row["installed_at"],
+        )
+
 
 class AsyncSQLAlchemyInstallationStore(AsyncInstallationStore):
     default_bots_table_name: str = "slack_bots"
@@ -493,21 +502,7 @@ async def async_find_bot(
         async with self.engine.connect() as conn:
             result: object = await conn.execute(query)
             for row in result.mappings():  # type: ignore[attr-defined]
-                return Bot(
-                    app_id=row["app_id"],
-                    enterprise_id=row["enterprise_id"],
-                    enterprise_name=row["enterprise_name"],
-                    team_id=row["team_id"],
-                    team_name=row["team_name"],
-                    bot_token=row["bot_token"],
-                    bot_id=row["bot_id"],
-                    bot_user_id=row["bot_user_id"],
-                    bot_scopes=row["bot_scopes"],
-                    bot_refresh_token=row["bot_refresh_token"],
-                    bot_token_expires_at=row["bot_token_expires_at"],
-                    is_enterprise_install=row["is_enterprise_install"],
-                    installed_at=row["installed_at"],
-                )
+                return SQLAlchemyInstallationStore.build_bot_entity(row)
             return None
 
     async def async_find_installation(
@@ -541,33 +536,7 @@ async def async_find_installation(
         async with self.engine.connect() as conn:
             result: object = await conn.execute(query)
             for row in result.mappings():  # type: ignore[attr-defined]
-                installation = Installation(
-                    app_id=row["app_id"],
-                    enterprise_id=row["enterprise_id"],
-                    enterprise_name=row["enterprise_name"],
-                    enterprise_url=row["enterprise_url"],
-                    team_id=row["team_id"],
-                    team_name=row["team_name"],
-                    bot_token=row["bot_token"],
-                    bot_id=row["bot_id"],
-                    bot_user_id=row["bot_user_id"],
-                    bot_scopes=row["bot_scopes"],
-                    bot_refresh_token=row["bot_refresh_token"],
-                    bot_token_expires_at=row["bot_token_expires_at"],
-                    user_id=row["user_id"],
-                    user_token=row["user_token"],
-                    user_scopes=row["user_scopes"],
-                    user_refresh_token=row["user_refresh_token"],
-                    user_token_expires_at=row["user_token_expires_at"],
-                    # Only the incoming webhook issued in the latest installation is set in this logic
-                    incoming_webhook_url=row["incoming_webhook_url"],
-                    incoming_webhook_channel=row["incoming_webhook_channel"],
-                    incoming_webhook_channel_id=row["incoming_webhook_channel_id"],
-                    incoming_webhook_configuration_url=row["incoming_webhook_configuration_url"],
-                    is_enterprise_install=row["is_enterprise_install"],
-                    token_type=row["token_type"],
-                    installed_at=row["installed_at"],
-                )
+                installation = SQLAlchemyInstallationStore.build_installation_entity(row)
 
         has_user_installation = user_id is not None and installation is not None
         no_bot_token_installation = installation is not None and installation.bot_token is None
diff --git a/tests/slack_sdk/oauth/state_store/test_async_sqlalchemy.py b/tests/slack_sdk/oauth/state_store/test_async_sqlalchemy.py
@@ -1,4 +1,4 @@
-import time
+import asyncio
 import unittest
 
 from sqlalchemy.ext.asyncio import AsyncEngine, create_async_engine
@@ -29,6 +29,6 @@ async def test_issue_and_consume(self):
 
     async def test_expiration(self):
         state = await self.store.async_issue()
-        time.sleep(3)
+        await asyncio.sleep(3)  
         result = await self.store.async_consume(state)
         self.assertFalse(result)
