-
Notifications
You must be signed in to change notification settings - Fork 32
🎨 feat(api-server): Add TTL caching to authentication repositories 🚨 #8474
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
pcrespov
merged 13 commits into
ITISFoundation:master
from
pcrespov:is8461/cache-public-api-auth
Oct 8, 2025
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
462a3bf
adds tests
pcrespov 45af695
🐛 Fix: Update fake API key generation to use _asdict() for row data
pcrespov 1c7dea6
new test on depedency
pcrespov 6b62230
🐛 Fix: Refactor app startup and engine initialization in test setup
pcrespov c561abf
aiocache
pcrespov f1db9f5
adds cache tests
pcrespov 2af43ba
refactor and split tests
pcrespov a6e3227
refactor: improve cache handling in authentication tests
pcrespov 3f7bb7c
constant
pcrespov 36e0ce5
mypy
pcrespov 32e2886
@GitHK review: doc and rename
pcrespov 829e3bd
@GitHK review: rm test
pcrespov dfaadbf
missing improt
pcrespov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
54 changes: 54 additions & 0 deletions
54
services/api-server/src/simcore_service_api_server/repository/_base.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
58 changes: 58 additions & 0 deletions
58
services/api-server/tests/unit/_with_db/authentication/conftest.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| # pylint: disable=redefined-outer-name | ||
| # pylint: disable=unused-argument | ||
| # pylint: disable=unused-variable | ||
| # pylint: disable=too-many-arguments | ||
|
|
||
| from collections.abc import AsyncGenerator, AsyncIterator, Callable | ||
|
|
||
| import pytest | ||
| from asgi_lifespan import LifespanManager | ||
| from fastapi import FastAPI | ||
| from models_library.api_schemas_api_server.api_keys import ApiKeyInDB | ||
| from pydantic import PositiveInt | ||
| from simcore_service_api_server.clients.postgres import get_engine | ||
| from simcore_service_api_server.repository.api_keys import ApiKeysRepository | ||
| from simcore_service_api_server.repository.users import UsersRepository | ||
| from sqlalchemy.ext.asyncio import AsyncEngine | ||
|
|
||
| MAX_TIME_FOR_APP_TO_STARTUP = 10 | ||
| MAX_TIME_FOR_APP_TO_SHUTDOWN = 10 | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| async def app_started(app: FastAPI, is_pdb_enabled: bool) -> AsyncIterator[FastAPI]: | ||
| # LifespanManager will trigger app's startup&shutown event handlers | ||
| async with LifespanManager( | ||
| app, | ||
| startup_timeout=None if is_pdb_enabled else MAX_TIME_FOR_APP_TO_STARTUP, | ||
| shutdown_timeout=None if is_pdb_enabled else MAX_TIME_FOR_APP_TO_SHUTDOWN, | ||
| ): | ||
| yield app | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| async def async_engine(app_started: FastAPI) -> AsyncEngine: | ||
| # Overrides | ||
| return get_engine(app_started) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def api_key_repo( | ||
| async_engine: AsyncEngine, | ||
| ) -> ApiKeysRepository: | ||
| return ApiKeysRepository(db_engine=async_engine) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def users_repo( | ||
| async_engine: AsyncEngine, | ||
| ) -> UsersRepository: | ||
| return UsersRepository(db_engine=async_engine) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| async def api_key_in_db( | ||
| create_fake_api_keys: Callable[[PositiveInt], AsyncGenerator[ApiKeyInDB, None]], | ||
| ) -> ApiKeyInDB: | ||
| """Creates a single API key in DB for testing purposes""" | ||
| return await anext(create_fake_api_keys(1)) |
150 changes: 150 additions & 0 deletions
150
services/api-server/tests/unit/_with_db/authentication/test_api_dependency_authentication.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| # pylint: disable=redefined-outer-name | ||
| # pylint: disable=unused-argument | ||
| # pylint: disable=unused-variable | ||
| # pylint: disable=too-many-arguments | ||
|
|
||
| import time | ||
|
|
||
| import pytest | ||
| from fastapi.security import HTTPBasicCredentials | ||
| from models_library.api_schemas_api_server.api_keys import ApiKeyInDB | ||
| from pytest_mock import MockerFixture | ||
| from simcore_service_api_server.api.dependencies.authentication import ( | ||
| get_current_identity, | ||
| ) | ||
| from simcore_service_api_server.repository.api_keys import ApiKeysRepository | ||
| from simcore_service_api_server.repository.users import UsersRepository | ||
|
|
||
|
|
||
| async def test_rest_dependency_authentication( | ||
| api_key_in_db: ApiKeyInDB, | ||
| api_key_repo: ApiKeysRepository, | ||
| users_repo: UsersRepository, | ||
| ): | ||
|
|
||
| # Generate a fake API key | ||
| # Act | ||
| result = await get_current_identity( | ||
| apikeys_repo=api_key_repo, | ||
| users_repo=users_repo, | ||
| credentials=HTTPBasicCredentials( | ||
| username=api_key_in_db.api_key, password=api_key_in_db.api_secret | ||
| ), | ||
| ) | ||
|
|
||
| # Assert | ||
| assert result is not None | ||
| assert result.user_id == api_key_in_db.user_id | ||
| assert result.product_name == api_key_in_db.product_name | ||
|
|
||
|
|
||
| @pytest.mark.skip(reason="This test is intended to be used for local profiling only") | ||
| async def test_cache_effectiveness_in_rest_authentication_dependencies( | ||
pcrespov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| api_key_in_db: ApiKeyInDB, | ||
| api_key_repo: ApiKeysRepository, | ||
| users_repo: UsersRepository, | ||
| mocker: MockerFixture, | ||
| ): | ||
| """Test that caching reduces database calls and improves performance.""" | ||
|
|
||
| # Generate a fake API key | ||
| credentials = HTTPBasicCredentials( | ||
| username=api_key_in_db.api_key, password=api_key_in_db.api_secret | ||
| ) | ||
|
|
||
| # Get cache instances from repository methods | ||
| # pylint: disable=no-member | ||
| api_keys_cache = api_key_repo.get_user.cache | ||
| users_cache = users_repo.get_active_user_email.cache | ||
|
|
||
| # Clear any existing cache | ||
| await api_keys_cache.clear() | ||
| await users_cache.clear() | ||
|
|
||
| # Spy on the connection's execute method by patching AsyncConnection.execute | ||
| from sqlalchemy.ext.asyncio import AsyncConnection # noqa: PLC0415 | ||
|
|
||
| execute_spy = mocker.spy(AsyncConnection, "execute") | ||
|
|
||
| # Test with cache enabled (default behavior) | ||
| # First call - should hit database | ||
| start_time = time.time() | ||
| result1 = await get_current_identity( | ||
| apikeys_repo=api_key_repo, | ||
| users_repo=users_repo, | ||
| credentials=credentials, | ||
| ) | ||
| first_call_time = time.time() - start_time | ||
|
|
||
| # Second call - should use cache | ||
| start_time = time.time() | ||
| result2 = await get_current_identity( | ||
| apikeys_repo=api_key_repo, | ||
| users_repo=users_repo, | ||
| credentials=credentials, | ||
| ) | ||
| second_call_time = time.time() - start_time | ||
|
|
||
| cached_db_calls = execute_spy.call_count | ||
| execute_spy.reset_mock() | ||
|
|
||
| # Verify cache was used | ||
| api_key_cache_key = f"api_auth:{api_key_in_db.api_key}" | ||
| user_cache_key = f"user_email:{api_key_in_db.user_id}" | ||
|
|
||
| assert await api_keys_cache.exists(api_key_cache_key), "API key should be cached" | ||
| assert await users_cache.exists(user_cache_key), "User email should be cached" | ||
|
|
||
| # Test with cache disabled by clearing cache before each call | ||
| await api_keys_cache.clear() | ||
| await users_cache.clear() | ||
|
|
||
| # First call without cache | ||
| start_time = time.time() | ||
| result3 = await get_current_identity( | ||
| apikeys_repo=api_key_repo, | ||
| users_repo=users_repo, | ||
| credentials=credentials, | ||
| ) | ||
| no_cache_first_time = time.time() - start_time | ||
|
|
||
| # Clear cache again to simulate no caching | ||
| await api_keys_cache.clear() | ||
| await users_cache.clear() | ||
|
|
||
| # Second call without cache | ||
| start_time = time.time() | ||
| result4 = await get_current_identity( | ||
| apikeys_repo=api_key_repo, | ||
| users_repo=users_repo, | ||
| credentials=credentials, | ||
| ) | ||
| no_cache_second_time = time.time() - start_time | ||
|
|
||
| no_cache_db_calls = execute_spy.call_count | ||
|
|
||
| # ASSERTIONS | ||
| # All results should be identical | ||
| assert result1.user_id == result2.user_id == result3.user_id == result4.user_id | ||
| assert ( | ||
| result1.product_name | ||
| == result2.product_name | ||
| == result3.product_name | ||
| == result4.product_name | ||
| ) | ||
| assert result1.email == result2.email == result3.email == result4.email | ||
|
|
||
| # With cache: second call should be significantly faster | ||
| assert ( | ||
| second_call_time < first_call_time * 0.5 | ||
| ), "Cache should make subsequent calls faster" | ||
|
|
||
| # Without cache: both calls should take similar time | ||
| time_ratio = abs(no_cache_second_time - no_cache_first_time) / max( | ||
| no_cache_first_time, no_cache_second_time | ||
| ) | ||
| assert time_ratio < 0.5, "Without cache, call times should be similar" | ||
|
|
||
| # With cache: fewer total database calls (due to caching) | ||
| # Without cache: more database calls (no caching) | ||
| assert no_cache_db_calls > cached_db_calls, "Cache should reduce database calls" | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.