Fix: Thread-safe DB connection manager with pooling (#212)

[muhammadtihame]

Summary

This PR implements thread-safe database connection management with connection pooling and proper resource cleanup.

Changes

• Added backend/app/database/core.py (async SQLAlchemy engine + session manager)
• Added tests/test_db_pool.py to verify concurrency and rollback behavior
• Added docs/DATABASE_CONNECTION.md documentation
• Updated backend/app/core/config/settings.py for database configuration
• Updated pyproject.toml and backend/requirements.txt to include new dependencies

Verification

Run the following tests:

pytest tests/test_db_pool.py


<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit

* **New Features**
  * Added optional environment-configurable database URL and asynchronous database connectivity with pooled connections for improved performance and reliability.

* **Documentation**
  * Added a guide covering database connection setup, pooling behavior, and best practices.

* **Tests**
  * Added tests for connection pool sizing, concurrent session handling, and rollback/cleanup on errors.

* **Chores**
  * Added async DB runtime and test dependencies.

<sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub>
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds async database support via SQLAlchemy/asyncpg: new optional database_url setting, an async engine/session module with pooling and a get_db() generator, updated dependencies, documentation, and tests for pool behavior and session lifecycle.

Changes

Cohort / File(s)	Summary
Configuration backend/app/core/config/settings.py	Added optional database_url: Optional[str] = None to Settings.
Async DB core backend/app/database/core.py	New async engine (create_async_engine) and async_session_maker created from DATABASE_URL; engine may be None with a warning; added async def get_db() -> AsyncGenerator[AsyncSession, None] that raises if engine uninitialized, yields sessions, and handles rollback/closure on errors.
Project deps pyproject.toml, backend/requirements.txt	Added sqlalchemy and asyncpg to project dependencies; added asyncpg==0.29.0 and pytest-asyncio==0.23.5 to backend requirements.
Documentation docs/DATABASE_CONNECTION.md	New doc describing AsyncIO SQLAlchemy setup, pool parameters, DATABASE_URL usage, get_db DI pattern, and testing considerations.
Tests tests/test_db_pool.py	New async tests mocking engine/session: asserts pool size/timeout, simulates 50 concurrent session acquisitions, and verifies rollback+close on error.

Sequence Diagram(s)

sequenceDiagram
    participant Client as FastAPI Route
    participant GetDb as get_db()
    participant Maker as async_session_maker
    participant Pool as Connection Pool
    participant Session as AsyncSession

    Client->>GetDb: request session
    alt Engine initialized
        GetDb->>Maker: create session
        Maker->>Pool: acquire connection
        Pool-->>Maker: provide connection
        Maker-->>Session: return AsyncSession
        GetDb-->>Client: yield session
        Client->>Session: execute queries
        alt Success
            Client->>GetDb: return normally
            GetDb->>Session: close session
            Session->>Pool: release connection
        else Exception
            Client->>GetDb: exception raised
            GetDb->>Session: rollback
            GetDb->>Session: close session
            Session->>Pool: release connection
        end
    else Engine not initialized
        GetDb-->>Client: raise RuntimeError
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐇 I nibble bytes and hop through queues,

Connections pooled, a cozy fuse.
Sessions yield and roll back light,
Async fields gleam in rabbit sight.
Hooray — the DB sleeps snug at night!

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix: Thread-safe DB connection manager with pooling (#212)' directly and accurately describes the main change: implementing a thread-safe database connection manager with connection pooling, which is the primary objective of the PR across all modified files.
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@backend/app/database/core.py`:
- Around line 46-57: Remove the redundant explicit close in the async session
context manager: when using the async with async_session_maker() as session
block you should not call await session.close() in the finally; delete that call
so the context manager handles cleanup. Also replace logger.error(...) with
logger.exception(...) in the except block to include the traceback, while
preserving await session.rollback() and re-raising the exception; reference
async_session_maker, session, logger.error -> logger.exception, session.rollback
to locate the changes.

In `@tests/test_db_pool.py`:
- Around line 7-9: The test currently patches app.core.config.settings at module
scope but imports engine and get_db after the patch context, which is fragile
because engine is created at import time and module caching prevents
reinitialization; update tests/test_db_pool.py to apply the patch while
importing/reloading the database module so engine is created with the mocked
configuration—use a pytest fixture that applies patch.dict or patch to set
DATABASE_URL (or patch app.core.config.settings), then
importlib.reload(app.database.core) inside that fixture and yield db_core.engine
and db_core.get_db (or move the import of engine/get_db inside the with
patch(...) block) so engine initialization sees the mocked value.
- Around line 44-45: The async context manager mock is incomplete: instead of
assigning __aexit__.return_value = None make both __aenter__ and __aexit__
awaitable (e.g., replace the current assignments on mock_session with AsyncMock
usage: set mock_session.__aenter__ = AsyncMock(return_value=mock_session) and
mock_session.__aexit__ = AsyncMock(return_value=None)), and apply the same
change to the other async context-manager mock later in the file so both
enter/exit are proper awaitables.

🧹 Nitpick comments (1)

tests/test_db_pool.py (1)

49-52: Unused loop variable session — rename to _session.

Per the static analysis hint, the loop variable is unused within the body. Renaming to _session signals intentional non-use.

-            async for session in get_db():
+            async for _session in get_db():

The same applies to line 80.

[muhammadtihame]

Fixes #212

[muhammadtihame]

Addressed CodeRabbit feedback:

• Removed redundant session.close()
• Switched to logger.exception for better tracing
• Refactored tests to use importlib.reload with fixture
• Fixed async mock aexit using AsyncMock