fix: rely on naive datetime for sqlalchemy (#1833)

Author: WilliamBergamin

.github/workflows/ci-build.yml

@@ -114,13 +114,63 @@ jobs:
           token: ${{ secrets.CODECOV_TOKEN }}
           verbose: true
 
+  databases:
+    # TODO: Add MySQL and other database testing when possible
+    name: Database Unit Tests
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    permissions:
+      contents: read
+    services:
+      postgres:
+        image: postgres@sha256:e4842c8a99ca99339e1693e6fe5fe62c7becb31991f066f989047dfb2fbf47af # 16
+        env:
+          POSTGRES_USER: test_user
+          POSTGRES_PASSWORD: password
+          POSTGRES_DB: test
+        ports:
+          - 5432:5432
+        options: >-
+          --health-cmd pg_isready
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
+    steps:
+      - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+        with:
+          persist-credentials: false
+      - name: Set up Python ${{ env.LATEST_SUPPORTED_PY }}
+        uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0
+        with:
+          python-version: ${{ env.LATEST_SUPPORTED_PY }}
+          cache: pip
+      - name: Install dependencies
+        run: |
+          pip install -U pip
+          pip install -r requirements/testing.txt
+          pip install -r requirements/optional.txt
+          pip install -r requirements/databases.txt
+      - name: Run sync tests (PostgreSQL)
+        env:
+          TEST_DATABASE_URL: postgresql://test_user:password@localhost/test
+        run: |
+          PYTHONPATH=$PWD:$PYTHONPATH pytest -s tests/slack_sdk/oauth/installation_store/test_sqlalchemy.py
+          PYTHONPATH=$PWD:$PYTHONPATH pytest -s tests/slack_sdk/oauth/state_store/test_sqlalchemy.py
+      - name: Run async tests (PostgreSQL)
+        env:
+          ASYNC_TEST_DATABASE_URL: postgresql+asyncpg://test_user:password@localhost/test
+        run: |
+          PYTHONPATH=$PWD:$PYTHONPATH pytest -s tests/slack_sdk/oauth/installation_store/test_async_sqlalchemy.py
+          PYTHONPATH=$PWD:$PYTHONPATH pytest -s tests/slack_sdk/oauth/state_store/test_async_sqlalchemy.py
+
   notifications:
     name: Regression notifications
     runs-on: ubuntu-latest
     needs:
       - lint
       - typecheck
       - unittest
+      - databases
     if: ${{ !success() && github.ref == 'refs/heads/main' && github.event_name != 'workflow_dispatch' }}
     steps:
       - name: Send notifications of failing tests

requirements/databases.txt

@@ -0,0 +1,5 @@
+# Database drivers for CI testing
+
+# PostgreSQL drivers
+psycopg2-binary>=2.9,<3
+asyncpg>=0.27,<1

slack_sdk/oauth/installation_store/sqlalchemy/__init__.py

@@ -23,6 +23,7 @@
 from slack_sdk.oauth.installation_store.async_installation_store import (
     AsyncInstallationStore,
 )
+from slack_sdk.oauth.sqlalchemy_utils import normalize_datetime_for_db
 
 
 class SQLAlchemyInstallationStore(InstallationStore):
@@ -140,6 +141,9 @@ def save(self, installation: Installation):
         with self.engine.begin() as conn:
             i = installation.to_dict()
             i["client_id"] = self.client_id
+            i["installed_at"] = normalize_datetime_for_db(i.get("installed_at"))
+            i["bot_token_expires_at"] = normalize_datetime_for_db(i.get("bot_token_expires_at"))
+            i["user_token_expires_at"] = normalize_datetime_for_db(i.get("user_token_expires_at"))
 
             i_column = self.installations.c
             installations_rows = conn.execute(
@@ -171,6 +175,8 @@ def save_bot(self, bot: Bot):
             # bots
             b = bot.to_dict()
             b["client_id"] = self.client_id
+            b["installed_at"] = normalize_datetime_for_db(b.get("installed_at"))
+            b["bot_token_expires_at"] = normalize_datetime_for_db(b.get("bot_token_expires_at"))
 
             b_column = self.bots.c
             bots_rows = conn.execute(
@@ -419,6 +425,9 @@ async def async_save(self, installation: Installation):
         async with self.engine.begin() as conn:
             i = installation.to_dict()
             i["client_id"] = self.client_id
+            i["installed_at"] = normalize_datetime_for_db(i.get("installed_at"))
+            i["bot_token_expires_at"] = normalize_datetime_for_db(i.get("bot_token_expires_at"))
+            i["user_token_expires_at"] = normalize_datetime_for_db(i.get("user_token_expires_at"))
 
             i_column = self.installations.c
             installations_rows = await conn.execute(
@@ -450,6 +459,8 @@ async def async_save_bot(self, bot: Bot):
             # bots
             b = bot.to_dict()
             b["client_id"] = self.client_id
+            b["installed_at"] = normalize_datetime_for_db(b.get("installed_at"))
+            b["bot_token_expires_at"] = normalize_datetime_for_db(b.get("bot_token_expires_at"))
 
             b_column = self.bots.c
             bots_rows = await conn.execute(

slack_sdk/oauth/sqlalchemy_utils/__init__.py

@@ -0,0 +1,33 @@
+from datetime import datetime
+from typing import Optional
+
+
+# TODO: Remove this function in next major release (v4.0.0) after updating all
+# DateTime columns to DateTime(timezone=True). See issue #1832 for context.
+def normalize_datetime_for_db(dt: Optional[datetime]) -> Optional[datetime]:
+    """
+    Normalize timezone-aware datetime to naive UTC datetime for database storage.
+
+    Ensures compatibility with existing databases using TIMESTAMP WITHOUT TIME ZONE.
+    SQLAlchemy DateTime columns without timezone=True create naive timestamp columns
+    in databases like PostgreSQL. This function strips timezone information from
+    timezone-aware datetimes (which are already in UTC) to enable safe comparisons.
+
+    Args:
+        dt: A timezone-aware or naive datetime object, or None
+
+    Returns:
+        A naive datetime in UTC, or None if input is None
+
+    Example:
+        >>> from datetime import datetime, timezone
+        >>> aware_dt = datetime(2024, 1, 1, 12, 0, 0, tzinfo=timezone.utc)
+        >>> naive_dt = normalize_datetime_for_db(aware_dt)
+        >>> naive_dt.tzinfo is None
+        True
+    """
+    if dt is None:
+        return None
+    if dt.tzinfo is not None:
+        return dt.replace(tzinfo=None)
+    return dt

slack_sdk/oauth/state_store/sqlalchemy/__init__.py

@@ -10,6 +10,7 @@
 from sqlalchemy import Table, Column, Integer, String, DateTime, and_, MetaData
 from sqlalchemy.engine import Engine
 from sqlalchemy.ext.asyncio import AsyncEngine
+from slack_sdk.oauth.sqlalchemy_utils import normalize_datetime_for_db
 
 
 class SQLAlchemyOAuthStateStore(OAuthStateStore):
@@ -55,7 +56,7 @@ def logger(self) -> Logger:
 
     def issue(self, *args, **kwargs) -> str:
         state: str = str(uuid4())
-        now = datetime.fromtimestamp(time.time() + self.expiration_seconds, tz=timezone.utc)
+        now = normalize_datetime_for_db(datetime.fromtimestamp(time.time() + self.expiration_seconds, tz=timezone.utc))
         with self.engine.begin() as conn:
             conn.execute(
                 self.oauth_states.insert(),
@@ -65,9 +66,10 @@ def issue(self, *args, **kwargs) -> str:
 
     def consume(self, state: str) -> bool:
         try:
+            now = normalize_datetime_for_db(datetime.now(tz=timezone.utc))
             with self.engine.begin() as conn:
                 c = self.oauth_states.c
-                query = self.oauth_states.select().where(and_(c.state == state, c.expire_at > datetime.now(tz=timezone.utc)))
+                query = self.oauth_states.select().where(and_(c.state == state, c.expire_at > now))
                 result = conn.execute(query)
                 for row in result.mappings():
                     self.logger.debug(f"consume's query result: {row}")
@@ -124,7 +126,7 @@ def logger(self) -> Logger:
 
     async def async_issue(self, *args, **kwargs) -> str:
         state: str = str(uuid4())
-        now = datetime.fromtimestamp(time.time() + self.expiration_seconds, tz=timezone.utc)
+        now = normalize_datetime_for_db(datetime.fromtimestamp(time.time() + self.expiration_seconds, tz=timezone.utc))
         async with self.engine.begin() as conn:
             await conn.execute(
                 self.oauth_states.insert(),
@@ -134,9 +136,10 @@ async def async_issue(self, *args, **kwargs) -> str:
 
     async def async_consume(self, state: str) -> bool:
         try:
+            now = normalize_datetime_for_db(datetime.now(tz=timezone.utc))
             async with self.engine.begin() as conn:
                 c = self.oauth_states.c
-                query = self.oauth_states.select().where(and_(c.state == state, c.expire_at > datetime.now(tz=timezone.utc)))
+                query = self.oauth_states.select().where(and_(c.state == state, c.expire_at > now))
                 result = await conn.execute(query)
                 for row in result.mappings():
                     self.logger.debug(f"consume's query result: {row}")

tests/slack_sdk/oauth/installation_store/test_async_sqlalchemy.py

@@ -1,17 +1,25 @@
+import os
 import unittest
 from tests.helpers import async_test
 from sqlalchemy.ext.asyncio import AsyncEngine, create_async_engine
 
 from slack_sdk.oauth.installation_store import Installation
 from slack_sdk.oauth.installation_store.sqlalchemy import AsyncSQLAlchemyInstallationStore
 
+database_url = os.environ.get("ASYNC_TEST_DATABASE_URL", "sqlite+aiosqlite:///:memory:")
+
+
+def setUpModule():
+    """Emit database configuration for CI visibility across builds."""
+    print(f"\n[InstallationStore/AsyncSQLAlchemy] Database: {database_url}")
+
 
 class TestAsyncSQLAlchemy(unittest.TestCase):
     engine: AsyncEngine
 
     @async_test
     async def setUp(self):
-        self.engine = create_async_engine("sqlite+aiosqlite:///:memory:")
+        self.engine = create_async_engine(database_url)
         self.store = AsyncSQLAlchemyInstallationStore(client_id="111.222", engine=self.engine)
         async with self.engine.begin() as conn:
             await conn.run_sync(self.store.metadata.create_all)
@@ -296,3 +304,27 @@ async def test_issue_1441_mixing_user_and_bot_installations(self):
         self.assertIsNone(installation)
         installation = await store.async_find_installation(enterprise_id=None, team_id="T111")
         self.assertIsNone(installation)
+
+    @async_test
+    async def test_timezone_aware_datetime_compatibility(self):
+        installation = Installation(
+            app_id="A111",
+            enterprise_id="E111",
+            team_id="T111",
+            user_id="U111",
+            bot_id="B111",
+            bot_token="xoxb-111",
+            bot_scopes=["chat:write"],
+            bot_user_id="U222",
+        )
+
+        # First save
+        await self.store.async_save(installation)
+        found = await self.store.async_find_installation(enterprise_id="E111", team_id="T111")
+        self.assertIsNotNone(found)
+        self.assertEqual(found.app_id, "A111")
+
+        # Second save (update) - tests WHERE clause with installed_at comparison
+        await self.store.async_save(installation)
+        found = await self.store.async_find_installation(enterprise_id="E111", team_id="T111")
+        self.assertIsNotNone(found)

tests/slack_sdk/oauth/installation_store/test_sqlalchemy.py

@@ -1,3 +1,4 @@
+import os
 import unittest
 
 import sqlalchemy
@@ -6,12 +7,19 @@
 from slack_sdk.oauth.installation_store import Installation
 from slack_sdk.oauth.installation_store.sqlalchemy import SQLAlchemyInstallationStore
 
+database_url = os.environ.get("TEST_DATABASE_URL", "sqlite:///:memory:")
+
+
+def setUpModule():
+    """Emit database configuration for CI visibility across builds."""
+    print(f"\n[InstallationStore/SQLAlchemy] Database: {database_url}")
+
 
 class TestSQLAlchemy(unittest.TestCase):
     engine: Engine
 
     def setUp(self):
-        self.engine = sqlalchemy.create_engine("sqlite:///:memory:")
+        self.engine = sqlalchemy.create_engine(database_url)
         self.store = SQLAlchemyInstallationStore(client_id="111.222", engine=self.engine)
         self.store.metadata.create_all(self.engine)
 
@@ -289,3 +297,26 @@ def test_issue_1441_mixing_user_and_bot_installations(self):
         self.assertIsNone(installation)
         installation = store.find_installation(enterprise_id=None, team_id="T111")
         self.assertIsNone(installation)
+
+    def test_timezone_aware_datetime_compatibility(self):
+        installation = Installation(
+            app_id="A111",
+            enterprise_id="E111",
+            team_id="T111",
+            user_id="U111",
+            bot_id="B111",
+            bot_token="xoxb-111",
+            bot_scopes=["chat:write"],
+            bot_user_id="U222",
+        )
+
+        # First save
+        self.store.save(installation)
+        found = self.store.find_installation(enterprise_id="E111", team_id="T111")
+        self.assertIsNotNone(found)
+        self.assertEqual(found.app_id, "A111")
+
+        # Second save (update) - tests WHERE clause with installed_at comparison
+        self.store.save(installation)
+        found = self.store.find_installation(enterprise_id="E111", team_id="T111")
+        self.assertIsNotNone(found)

tests/slack_sdk/oauth/state_store/test_async_sqlalchemy.py

@@ -1,17 +1,25 @@
 import asyncio
+import os
 import unittest
 from tests.helpers import async_test
 from sqlalchemy.ext.asyncio import AsyncEngine, create_async_engine
 
 from slack_sdk.oauth.state_store.sqlalchemy import AsyncSQLAlchemyOAuthStateStore
 
+database_url = os.environ.get("ASYNC_TEST_DATABASE_URL", "sqlite+aiosqlite:///:memory:")
+
+
+def setUpModule():
+    """Emit database configuration for CI visibility across builds."""
+    print(f"\n[StateStore/AsyncSQLAlchemy] Database: {database_url}")
+
 
 class TestSQLAlchemy(unittest.TestCase):
     engine: AsyncEngine
 
     @async_test
     async def setUp(self):
-        self.engine = create_async_engine("sqlite+aiosqlite:///:memory:")
+        self.engine = create_async_engine(database_url)
         self.store = AsyncSQLAlchemyOAuthStateStore(engine=self.engine, expiration_seconds=2)
         async with self.engine.begin() as conn:
             await conn.run_sync(self.store.metadata.create_all)
@@ -36,3 +44,17 @@ async def test_expiration(self):
         await asyncio.sleep(3)
         result = await self.store.async_consume(state)
         self.assertFalse(result)
+
+    @async_test
+    async def test_timezone_aware_datetime_compatibility(self):
+        # Issue a state (tests INSERT with timezone-aware datetime)
+        state = await self.store.async_issue()
+        self.assertIsNotNone(state)
+
+        # Consume it immediately (tests WHERE clause comparison with timezone-aware datetime)
+        result = await self.store.async_consume(state)
+        self.assertTrue(result)
+
+        # Second consume should fail (state already consumed)
+        result = await self.store.async_consume(state)
+        self.assertFalse(result)

tests/slack_sdk/oauth/state_store/test_sqlalchemy.py

@@ -1,3 +1,4 @@
+import os
 import time
 import unittest
 
@@ -6,12 +7,19 @@
 
 from slack_sdk.oauth.state_store.sqlalchemy import SQLAlchemyOAuthStateStore
 
+database_url = os.environ.get("TEST_DATABASE_URL", "sqlite:///:memory:")
+
+
+def setUpModule():
+    """Emit database configuration for CI visibility across builds."""
+    print(f"\n[StateStore/SQLAlchemy] Database: {database_url}")
+
 
 class TestSQLAlchemy(unittest.TestCase):
     engine: Engine
 
     def setUp(self):
-        self.engine = sqlalchemy.create_engine("sqlite:///:memory:")
+        self.engine = sqlalchemy.create_engine(database_url)
         self.store = SQLAlchemyOAuthStateStore(engine=self.engine, expiration_seconds=2)
         self.store.metadata.create_all(self.engine)
 
@@ -31,3 +39,16 @@ def test_expiration(self):
         time.sleep(3)
         result = self.store.consume(state)
         self.assertFalse(result)
+
+    def test_timezone_aware_datetime_compatibility(self):
+        # Issue a state (tests INSERT with timezone-aware datetime)
+        state = self.store.issue()
+        self.assertIsNotNone(state)
+
+        # Consume it immediately (tests WHERE clause comparison with timezone-aware datetime)
+        result = self.store.consume(state)
+        self.assertTrue(result)
+
+        # Second consume should fail (state already consumed)
+        result = self.store.consume(state)
+        self.assertFalse(result)