Improve the app.installation_store field consistency

Author: seratch

slack_bolt/app/app.py

@@ -55,6 +55,7 @@
 from slack_bolt.middleware.message_listener_matches import MessageListenerMatches
 from slack_bolt.middleware.url_verification import UrlVerification
 from slack_bolt.oauth import OAuthFlow
+from slack_bolt.oauth.internals import select_consistent_installation_store
 from slack_bolt.oauth.oauth_settings import OAuthSettings
 from slack_bolt.request import BoltRequest
 from slack_bolt.response import BoltResponse
@@ -157,17 +158,28 @@ def __init__(
 
         if oauth_flow:
             self._oauth_flow = oauth_flow
-            if self._installation_store is None:
-                self._installation_store = self._oauth_flow.settings.installation_store
+            installation_store = select_consistent_installation_store(
+                client_id=self._oauth_flow.client_id,
+                app_store=self._installation_store,
+                oauth_flow_store=self._oauth_flow.settings.installation_store,
+                logger=self._framework_logger,
+            )
+            self._installation_store = installation_store
+            self._oauth_flow.settings.installation_store = installation_store
+
             if self._oauth_flow._client is None:
                 self._oauth_flow._client = self._client
             if self._authorize is None:
                 self._authorize = self._oauth_flow.settings.authorize
         elif oauth_settings is not None:
-            if self._installation_store:
-                # Consistently use a single installation_store
-                oauth_settings.installation_store = self._installation_store
-
+            installation_store = select_consistent_installation_store(
+                client_id=oauth_settings.client_id,
+                app_store=self._installation_store,
+                oauth_flow_store=oauth_settings.installation_store,
+                logger=self._framework_logger,
+            )
+            self._installation_store = installation_store
+            oauth_settings.installation_store = installation_store
             self._oauth_flow = OAuthFlow(
                 client=self.client, logger=self.logger, settings=oauth_settings
             )

slack_bolt/app/async_app.py

@@ -7,6 +7,7 @@
 from slack_bolt.middleware.message_listener_matches.async_message_listener_matches import (
     AsyncMessageListenerMatches,
 )
+from slack_bolt.oauth.async_internals import select_consistent_installation_store
 from slack_bolt.workflows.step.async_step import AsyncWorkflowStep
 from slack_bolt.workflows.step.async_step_middleware import AsyncWorkflowStepMiddleware
 from slack_sdk.oauth.installation_store.async_installation_store import (
@@ -167,18 +168,28 @@ def __init__(
 
         if oauth_flow:
             self._async_oauth_flow = oauth_flow
-            if self._async_installation_store is None:
-                self._async_installation_store = (
-                    self._async_oauth_flow.settings.installation_store
-                )
+            installation_store = select_consistent_installation_store(
+                client_id=self._async_oauth_flow.client_id,
+                app_store=self._async_installation_store,
+                oauth_flow_store=self._async_oauth_flow.settings.installation_store,
+                logger=self._framework_logger,
+            )
+            self._async_installation_store = installation_store
+            self._async_oauth_flow.settings.installation_store = installation_store
+
             if self._async_oauth_flow._async_client is None:
                 self._async_oauth_flow._async_client = self._async_client
             if self._async_authorize is None:
                 self._async_authorize = self._async_oauth_flow.settings.authorize
         elif oauth_settings is not None:
-            if self._async_installation_store:
-                # Consistently use a single installation_store
-                oauth_settings.installation_store = self._async_installation_store
+            installation_store = select_consistent_installation_store(
+                client_id=oauth_settings.client_id,
+                app_store=self._async_installation_store,
+                oauth_flow_store=oauth_settings.installation_store,
+                logger=self._framework_logger,
+            )
+            self._async_installation_store = installation_store
+            oauth_settings.installation_store = installation_store
 
             self._async_oauth_flow = AsyncOAuthFlow(
                 client=self._async_client, logger=self.logger, settings=oauth_settings

slack_bolt/logger/messages.py

@@ -64,6 +64,10 @@ def warning_token_skipped() -> str:
     )
 
 
+def warning_installation_store_conflicts() -> str:
+    return "As you gave both `installation_store` and `oauth_settings`/`auth_flow`, the top level one is unused."
+
+
 def warning_unhandled_request(req: Union[BoltRequest, "AsyncBoltRequest"]) -> str:  # type: ignore
     return f"Unhandled request ({req.body})"
 

slack_bolt/oauth/async_internals.py

@@ -0,0 +1,45 @@
+from logging import Logger
+from typing import Optional
+
+from slack_sdk.oauth.installation_store import FileInstallationStore
+from slack_sdk.oauth.installation_store.async_installation_store import (
+    AsyncInstallationStore,
+)
+
+from ..logger.messages import warning_installation_store_conflicts
+
+# key: client_id, value: AsyncInstallationStore
+default_installation_stores = {}
+
+
+def get_or_create_default_installation_store(client_id: str) -> AsyncInstallationStore:
+    store = default_installation_stores.get(client_id)
+    if store is None:
+        store = FileInstallationStore(client_id=client_id)
+        default_installation_stores[client_id] = store
+    return store
+
+
+def select_consistent_installation_store(
+    client_id: str,
+    app_store: Optional[AsyncInstallationStore],
+    oauth_flow_store: Optional[AsyncInstallationStore],
+    logger: Logger,
+) -> Optional[AsyncInstallationStore]:
+    default = get_or_create_default_installation_store(client_id)
+    if app_store is not None:
+        if oauth_flow_store is not None:
+            if oauth_flow_store is default:
+                # only app_store is intentionally set in this case
+                return app_store
+
+            # if both are intentionally set, prioritize app_store
+            if oauth_flow_store is not app_store:
+                logger.warning(warning_installation_store_conflicts())
+            return oauth_flow_store
+        else:
+            # only app_store is available
+            return app_store
+    else:
+        # only oauth_flow_store is available
+        return oauth_flow_store

slack_bolt/oauth/async_oauth_settings.py

@@ -8,7 +8,6 @@
     AuthorizeUrlGenerator,
     RedirectUriPageRenderer,
 )
-from slack_sdk.oauth.installation_store import FileInstallationStore
 from slack_sdk.oauth.installation_store.async_installation_store import (
     AsyncInstallationStore,
 )
@@ -20,6 +19,7 @@
     AsyncAuthorize,
 )
 from slack_bolt.error import BoltError
+from slack_bolt.oauth.async_internals import get_or_create_default_installation_store
 from slack_bolt.oauth.callback_options import CallbackOptions
 
 
@@ -122,8 +122,8 @@ def __init__(
             authorization_url or "https://slack.com/oauth/v2/authorize"
         )
         # Installation Management
-        self.installation_store = installation_store or FileInstallationStore(
-            client_id=client_id
+        self.installation_store = (
+            installation_store or get_or_create_default_installation_store(client_id)
         )
         self.authorize = AsyncInstallationStoreAuthorize(
             logger=logger, installation_store=self.installation_store,

slack_bolt/oauth/internals.py

@@ -1,11 +1,15 @@
 from logging import Logger
-from typing import Optional, Union
+from typing import Optional
+from typing import Union
 
+from slack_sdk.oauth import InstallationStore
 from slack_sdk.oauth import OAuthStateUtils, RedirectUriPageRenderer
+from slack_sdk.oauth.installation_store import FileInstallationStore
 from slack_sdk.oauth.installation_store import Installation
 
 from slack_bolt.request import BoltRequest
 from slack_bolt.response import BoltResponse
+from ..logger.messages import warning_installation_store_conflicts
 
 
 class CallbackResponseBuilder:
@@ -83,3 +87,40 @@ def _build_default_install_page_html(url: str) -> str:
 </body>
 </html>
 """
+
+
+# key: client_id, value: InstallationStore
+default_installation_stores = {}
+
+
+def get_or_create_default_installation_store(client_id: str) -> InstallationStore:
+    store = default_installation_stores.get(client_id)
+    if store is None:
+        store = FileInstallationStore(client_id=client_id)
+        default_installation_stores[client_id] = store
+    return store
+
+
+def select_consistent_installation_store(
+    client_id: str,
+    app_store: Optional[InstallationStore],
+    oauth_flow_store: Optional[InstallationStore],
+    logger: Logger,
+) -> Optional[InstallationStore]:
+    default = get_or_create_default_installation_store(client_id)
+    if app_store is not None:
+        if oauth_flow_store is not None:
+            if oauth_flow_store is default:
+                # only app_store is intentionally set in this case
+                return app_store
+
+            # if both are intentionally set, prioritize app_store
+            if oauth_flow_store is not app_store:
+                logger.warning(warning_installation_store_conflicts())
+            return oauth_flow_store
+        else:
+            # only app_store is available
+            return app_store
+    else:
+        # only oauth_flow_store is available
+        return oauth_flow_store

slack_bolt/oauth/oauth_settings.py

@@ -10,11 +10,11 @@
     AuthorizeUrlGenerator,
     RedirectUriPageRenderer,
 )
-from slack_sdk.oauth.installation_store import FileInstallationStore
 from slack_sdk.oauth.state_store import FileOAuthStateStore
 
 from slack_bolt.authorization.authorize import Authorize, InstallationStoreAuthorize
 from slack_bolt.error import BoltError
+from slack_bolt.oauth.internals import get_or_create_default_installation_store
 from slack_bolt.oauth.callback_options import CallbackOptions
 
 
@@ -116,8 +116,8 @@ def __init__(
             authorization_url or "https://slack.com/oauth/v2/authorize"
         )
         # Installation Management
-        self.installation_store = installation_store or FileInstallationStore(
-            client_id=client_id
+        self.installation_store = (
+            installation_store or get_or_create_default_installation_store(client_id)
         )
         self.authorize = InstallationStoreAuthorize(
             logger=logger, installation_store=self.installation_store,

tests/scenario_tests/test_app.py

@@ -121,3 +121,37 @@ def authorize() -> AuthorizeResult:
 
         with pytest.raises(BoltError):
             App(signing_secret="valid", authorize=authorize, oauth_flow=oauth_flow)
+
+    def test_installation_store_conflicts(self):
+        store1 = FileInstallationStore()
+        store2 = FileInstallationStore()
+        app = App(
+            signing_secret="valid",
+            oauth_settings=OAuthSettings(
+                client_id="111.222", client_secret="valid", installation_store=store1,
+            ),
+            installation_store=store2,
+        )
+        assert app.installation_store is store1
+
+        app = App(
+            signing_secret="valid",
+            oauth_flow=OAuthFlow(
+                settings=OAuthSettings(
+                    client_id="111.222",
+                    client_secret="valid",
+                    installation_store=store1,
+                )
+            ),
+            installation_store=store2,
+        )
+        assert app.installation_store is store1
+
+        app = App(
+            signing_secret="valid",
+            oauth_flow=OAuthFlow(
+                settings=OAuthSettings(client_id="111.222", client_secret="valid",)
+            ),
+            installation_store=store1,
+        )
+        assert app.installation_store is store1

tests/scenario_tests_async/test_app.py

@@ -8,7 +8,6 @@
 from slack_bolt.error import BoltError
 from slack_bolt.oauth.async_oauth_flow import AsyncOAuthFlow
 from slack_bolt.oauth.async_oauth_settings import AsyncOAuthSettings
-from slack_bolt.oauth.oauth_settings import OAuthSettings
 from tests.utils import remove_os_env_temporarily, restore_os_env
 
 
@@ -90,18 +89,22 @@ def test_valid_multi_auth_client_id_absence(self):
         with pytest.raises(BoltError):
             AsyncApp(
                 signing_secret="valid",
-                oauth_settings=OAuthSettings(client_id=None, client_secret="valid"),
+                oauth_settings=AsyncOAuthSettings(
+                    client_id=None, client_secret="valid"
+                ),
             )
 
     def test_valid_multi_auth_secret_absence(self):
         with pytest.raises(BoltError):
             AsyncApp(
                 signing_secret="valid",
-                oauth_settings=OAuthSettings(client_id="111.222", client_secret=None),
+                oauth_settings=AsyncOAuthSettings(
+                    client_id="111.222", client_secret=None
+                ),
             )
 
     def test_authorize_conflicts(self):
-        oauth_settings = OAuthSettings(
+        oauth_settings = AsyncOAuthSettings(
             client_id="111.222",
             client_secret="valid",
             installation_store=FileInstallationStore(),
@@ -127,3 +130,37 @@ def authorize() -> AuthorizeResult:
 
         with pytest.raises(BoltError):
             AsyncApp(signing_secret="valid", authorize=authorize, oauth_flow=oauth_flow)
+
+    def test_installation_store_conflicts(self):
+        store1 = FileInstallationStore()
+        store2 = FileInstallationStore()
+        app = AsyncApp(
+            signing_secret="valid",
+            oauth_settings=AsyncOAuthSettings(
+                client_id="111.222", client_secret="valid", installation_store=store1,
+            ),
+            installation_store=store2,
+        )
+        assert app.installation_store is store1
+
+        app = AsyncApp(
+            signing_secret="valid",
+            oauth_flow=AsyncOAuthFlow(
+                settings=AsyncOAuthSettings(
+                    client_id="111.222",
+                    client_secret="valid",
+                    installation_store=store1,
+                )
+            ),
+            installation_store=store2,
+        )
+        assert app.installation_store is store1
+
+        app = AsyncApp(
+            signing_secret="valid",
+            oauth_flow=AsyncOAuthFlow(
+                settings=AsyncOAuthSettings(client_id="111.222", client_secret="valid",)
+            ),
+            installation_store=store1,
+        )
+        assert app.installation_store is store1