8dde6c81ef
Add PostgreSQL support as the server-mode storage backend alongside existing SQLite for local mode. Verify all ORM models are dialect- agnostic, configure connection pooling for multi-user access, add Docker Compose for local PG development, and wire database URL selection based on deployment mode. Changes: - Add psycopg2-binary dependency to pyproject.toml - Add server_mode, db_pool_size, db_max_overflow, db_pool_recycle settings to Settings with environment variable support - Add resolve_database_url() and is_postgresql() to Settings for mode-aware database URL resolution - Configure UnitOfWork engine creation with pool_size, max_overflow, pool_recycle, and pool_pre_ping for PostgreSQL connections - Update MigrationRunner to handle both SQLite and PostgreSQL backends - Add compare_type=True to Alembic env.py for dialect-aware migrations - Add docker-compose.yml with PostgreSQL 16-alpine for local development - Add Behave BDD feature (14 scenarios) covering settings, pool config, engine creation, ORM dialect compatibility, and migration runner - Add Robot Framework integration tests (12 test cases) for the abstraction layer with requires_postgresql tag for live PG tests Fixes: - Restore require_confirmation parameter to UnitOfWork.__init__ - Add argument validation to UnitOfWork.__init__ (fail-fast) - Add explicit PostgreSQL isolation_level (READ COMMITTED) - Add dispose() method and context manager support to UnitOfWork - Fix MigrationRunner.get_current_revision() to dispose engine - Fix Settings.is_postgresql() to handle ValueError gracefully ISSUES CLOSED: #878
370 lines
13 KiB
Python
370 lines
13 KiB
Python
"""Step definitions for PostgreSQL storage backend BDD scenarios.
|
|
|
|
Steps specific to the ``postgresql_backend.feature`` file. The shared
|
|
``I have imported the database infrastructure modules`` step lives in
|
|
``database_infrastructure_steps.py`` and is reused here via Behave's
|
|
step-sharing mechanism.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import inspect
|
|
import re
|
|
|
|
from behave import given, then, when
|
|
from behave.runner import Context
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Settings helpers
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("I create settings with server_mode disabled")
|
|
def step_settings_local_mode(context: Context) -> None:
|
|
"""Create settings with local (SQLite) mode.
|
|
|
|
Clears ``CLEVERAGENTS_SERVER_MODE`` and restores the default
|
|
``database_url`` so Settings resolves to SQLite.
|
|
"""
|
|
import os
|
|
|
|
from cleveragents.config.settings import Settings
|
|
|
|
saved_db_url = os.environ.pop("CLEVERAGENTS_DATABASE_URL", None)
|
|
os.environ.pop("CLEVERAGENTS_SERVER_MODE", None)
|
|
try:
|
|
context.settings = Settings()
|
|
finally:
|
|
if saved_db_url is not None:
|
|
os.environ["CLEVERAGENTS_DATABASE_URL"] = saved_db_url
|
|
|
|
|
|
@given("I create settings with server_mode enabled")
|
|
def step_settings_server_mode(context: Context) -> None:
|
|
"""Create settings with server mode enabled.
|
|
|
|
The ``before_scenario`` hook injects a per-scenario SQLite URL into
|
|
``CLEVERAGENTS_DATABASE_URL``. We must clear it so that Settings
|
|
falls back to its declared default (``sqlite:///cleveragents.db``),
|
|
allowing ``resolve_database_url`` to detect the default and switch to
|
|
PostgreSQL.
|
|
"""
|
|
import os
|
|
|
|
from cleveragents.config.settings import Settings
|
|
|
|
saved_db_url = os.environ.pop("CLEVERAGENTS_DATABASE_URL", None)
|
|
os.environ["CLEVERAGENTS_SERVER_MODE"] = "true"
|
|
try:
|
|
context.settings = Settings()
|
|
finally:
|
|
os.environ.pop("CLEVERAGENTS_SERVER_MODE", None)
|
|
if saved_db_url is not None:
|
|
os.environ["CLEVERAGENTS_DATABASE_URL"] = saved_db_url
|
|
|
|
|
|
@given("the database_url is the default SQLite value")
|
|
def step_database_url_is_default(context: Context) -> None:
|
|
"""Ensure database_url is the SQLite default.
|
|
|
|
The default is already ``sqlite:///cleveragents.db`` so this is a
|
|
no-op guard ensuring the previous step didn't override it.
|
|
"""
|
|
assert context.settings.database_url.startswith("sqlite")
|
|
|
|
|
|
@given('the database_url is "{url}"')
|
|
def step_database_url_is(context: Context, url: str) -> None:
|
|
"""Recreate settings with an explicit database_url."""
|
|
import os
|
|
|
|
from cleveragents.config.settings import Settings
|
|
|
|
os.environ["CLEVERAGENTS_DATABASE_URL"] = url
|
|
server = "true" if context.settings.server_mode else "false"
|
|
os.environ["CLEVERAGENTS_SERVER_MODE"] = server
|
|
try:
|
|
context.settings = Settings()
|
|
finally:
|
|
os.environ.pop("CLEVERAGENTS_DATABASE_URL", None)
|
|
os.environ.pop("CLEVERAGENTS_SERVER_MODE", None)
|
|
|
|
|
|
@given("I create settings with custom pool configuration")
|
|
def step_settings_custom_pool(context: Context) -> None:
|
|
"""Create settings with custom pool values from the step table.
|
|
|
|
Pydantic-settings reads from environment variables, so we set the
|
|
``CLEVERAGENTS_*`` env vars before constructing the Settings instance
|
|
and clean them up afterward. The ``before_scenario`` per-scenario
|
|
database URL is temporarily cleared so the default applies.
|
|
"""
|
|
import os
|
|
|
|
from cleveragents.config.settings import Settings
|
|
|
|
env_map: dict[str, str] = {
|
|
"db_pool_size": "CLEVERAGENTS_DB_POOL_SIZE",
|
|
"db_max_overflow": "CLEVERAGENTS_DB_MAX_OVERFLOW",
|
|
"db_pool_recycle": "CLEVERAGENTS_DB_POOL_RECYCLE",
|
|
}
|
|
set_vars: list[str] = []
|
|
|
|
saved_db_url = os.environ.pop("CLEVERAGENTS_DATABASE_URL", None)
|
|
os.environ["CLEVERAGENTS_SERVER_MODE"] = "true"
|
|
set_vars.append("CLEVERAGENTS_SERVER_MODE")
|
|
|
|
if context.table is not None:
|
|
for row in context.table:
|
|
key = row["setting"]
|
|
value = row["value"]
|
|
env_name = env_map.get(key, f"CLEVERAGENTS_{key.upper()}")
|
|
os.environ[env_name] = str(value)
|
|
set_vars.append(env_name)
|
|
|
|
try:
|
|
context.settings = Settings()
|
|
finally:
|
|
for var in set_vars:
|
|
os.environ.pop(var, None)
|
|
if saved_db_url is not None:
|
|
os.environ["CLEVERAGENTS_DATABASE_URL"] = saved_db_url
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Settings assertions
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@then("resolving the database URL should raise a ValueError")
|
|
def step_resolve_url_raises_value_error(context: Context) -> None:
|
|
"""Assert that resolve_database_url raises ValueError in server mode."""
|
|
try:
|
|
context.settings.resolve_database_url()
|
|
except ValueError:
|
|
pass # expected
|
|
else:
|
|
raise AssertionError("Expected ValueError from resolve_database_url()")
|
|
|
|
|
|
@then('the resolved database URL should start with "{prefix}"')
|
|
def step_resolved_url_starts_with(context: Context, prefix: str) -> None:
|
|
"""Assert the resolved database URL prefix."""
|
|
url = context.settings.resolve_database_url()
|
|
assert url.startswith(prefix), f"Expected URL to start with '{prefix}', got '{url}'"
|
|
|
|
|
|
@then('the resolved database URL should be "{expected}"')
|
|
def step_resolved_url_equals(context: Context, expected: str) -> None:
|
|
"""Assert the resolved database URL matches exactly."""
|
|
url = context.settings.resolve_database_url()
|
|
assert url == expected, f"Expected '{expected}', got '{url}'"
|
|
|
|
|
|
@then("is_postgresql should return True")
|
|
def step_is_postgresql_true(context: Context) -> None:
|
|
"""Assert is_postgresql returns True."""
|
|
assert context.settings.is_postgresql(), "Expected is_postgresql() to be True"
|
|
|
|
|
|
@then("is_postgresql should return False")
|
|
def step_is_postgresql_false(context: Context) -> None:
|
|
"""Assert is_postgresql returns False."""
|
|
assert not context.settings.is_postgresql(), "Expected is_postgresql() to be False"
|
|
|
|
|
|
@then("the db_pool_size should be {value:d}")
|
|
def step_pool_size_equals(context: Context, value: int) -> None:
|
|
"""Assert db_pool_size setting."""
|
|
assert context.settings.db_pool_size == value, (
|
|
f"Expected {value}, got {context.settings.db_pool_size}"
|
|
)
|
|
|
|
|
|
@then("the db_max_overflow should be {value:d}")
|
|
def step_max_overflow_equals(context: Context, value: int) -> None:
|
|
"""Assert db_max_overflow setting."""
|
|
assert context.settings.db_max_overflow == value, (
|
|
f"Expected {value}, got {context.settings.db_max_overflow}"
|
|
)
|
|
|
|
|
|
@then("the db_pool_recycle should be {value:d}")
|
|
def step_pool_recycle_equals(context: Context, value: int) -> None:
|
|
"""Assert db_pool_recycle setting."""
|
|
assert context.settings.db_pool_recycle == value, (
|
|
f"Expected {value}, got {context.settings.db_pool_recycle}"
|
|
)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# UnitOfWork engine
|
|
# ---------------------------------------------------------------------------
|
|
|
|
_VALID_SQL_IDENTIFIER = re.compile(r"^[a-z_][a-z0-9_]*$")
|
|
|
|
|
|
@given('I create a UnitOfWork with URL "{url}"')
|
|
def step_create_uow(context: Context, url: str) -> None:
|
|
"""Create a UnitOfWork instance with the given URL."""
|
|
from cleveragents.infrastructure.database.unit_of_work import UnitOfWork
|
|
|
|
context.uow = UnitOfWork(
|
|
url,
|
|
prompt_for_migration=lambda _msg: True,
|
|
)
|
|
|
|
|
|
@given('I create a UnitOfWork with URL "{url}" and pool_size {size:d}')
|
|
def step_create_uow_with_pool(context: Context, url: str, size: int) -> None:
|
|
"""Create a UnitOfWork with explicit pool_size."""
|
|
from cleveragents.infrastructure.database.unit_of_work import UnitOfWork
|
|
|
|
context.uow = UnitOfWork(
|
|
url,
|
|
prompt_for_migration=lambda _msg: True,
|
|
pool_size=size,
|
|
)
|
|
|
|
|
|
@when("I access the engine")
|
|
def step_access_engine(context: Context) -> None:
|
|
"""Trigger engine creation."""
|
|
context.engine = context.uow.engine
|
|
|
|
|
|
@then('the engine dialect should be "{dialect}"')
|
|
def step_engine_dialect(context: Context, dialect: str) -> None:
|
|
"""Assert the engine dialect name."""
|
|
actual = context.engine.dialect.name
|
|
assert actual == dialect, f"Expected dialect '{dialect}', got '{actual}'"
|
|
|
|
|
|
@then("the UnitOfWork pool_size should be {value:d}")
|
|
def step_uow_pool_size(context: Context, value: int) -> None:
|
|
"""Assert UnitOfWork.pool_size via read-only property."""
|
|
assert context.uow.pool_size == value
|
|
|
|
|
|
@then("the UnitOfWork max_overflow should be {value:d}")
|
|
def step_uow_max_overflow(context: Context, value: int) -> None:
|
|
"""Assert UnitOfWork.max_overflow via read-only property."""
|
|
assert context.uow.max_overflow == value
|
|
|
|
|
|
@then("the UnitOfWork pool_recycle should be {value:d}")
|
|
def step_uow_pool_recycle(context: Context, value: int) -> None:
|
|
"""Assert UnitOfWork.pool_recycle via read-only property."""
|
|
assert context.uow.pool_recycle == value
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# ORM Model dialect-agnostic checks
|
|
# ---------------------------------------------------------------------------
|
|
|
|
# Standard SQLAlchemy types that are portable across all dialects.
|
|
_PORTABLE_TYPE_NAMES = frozenset(
|
|
{
|
|
"VARCHAR",
|
|
"STRING",
|
|
"TEXT",
|
|
"INTEGER",
|
|
"BIGINTEGER",
|
|
"SMALLINTEGER",
|
|
"FLOAT",
|
|
"NUMERIC",
|
|
"BOOLEAN",
|
|
"DATE",
|
|
"DATETIME",
|
|
"TIME",
|
|
"TIMESTAMP",
|
|
"JSON",
|
|
"JSONB",
|
|
"BLOB",
|
|
"LARGEBINARY",
|
|
"ENUM",
|
|
"UUID",
|
|
"INTERVAL",
|
|
"ARRAY",
|
|
}
|
|
)
|
|
|
|
|
|
@given("I inspect all ORM models from the Base metadata")
|
|
def step_inspect_orm_models(context: Context) -> None:
|
|
"""Collect all tables from the ORM metadata."""
|
|
from cleveragents.infrastructure.database.models import Base
|
|
|
|
context.metadata = Base.metadata
|
|
context.tables = list(Base.metadata.tables.values())
|
|
|
|
|
|
@then("no column should use a SQLite-specific type")
|
|
def step_no_sqlite_specific_types(context: Context) -> None:
|
|
"""Assert no column uses a SQLite-only type."""
|
|
for table in context.tables:
|
|
for col in table.columns:
|
|
type_name = type(col.type).__name__.upper()
|
|
# Known SQLite-only types that are NOT portable
|
|
sqlite_only = {"SQLITEINTEGER", "SQLITETEXT", "SQLITEBLOB"}
|
|
assert type_name not in sqlite_only, (
|
|
f"Column {table.name}.{col.name} uses SQLite-specific type {type_name}"
|
|
)
|
|
|
|
|
|
@then("all columns should use standard SQLAlchemy types")
|
|
def step_all_standard_types(context: Context) -> None:
|
|
"""Assert all columns use portable SQLAlchemy types."""
|
|
for table in context.tables:
|
|
for col in table.columns:
|
|
type_name = type(col.type).__name__.upper()
|
|
# Allow common base types and their variants
|
|
assert type_name in _PORTABLE_TYPE_NAMES or any(
|
|
base in type_name for base in _PORTABLE_TYPE_NAMES
|
|
), f"Column {table.name}.{col.name} uses non-standard type {type_name}"
|
|
|
|
|
|
@then("all table names should be valid SQL identifiers")
|
|
def step_valid_table_names(context: Context) -> None:
|
|
"""Assert all table names are valid (lowercase) SQL identifiers."""
|
|
for table in context.tables:
|
|
assert _VALID_SQL_IDENTIFIER.match(table.name), (
|
|
f"Table name '{table.name}' is not a valid SQL identifier"
|
|
)
|
|
|
|
|
|
@given("I read the models module source code")
|
|
def step_read_models_source(context: Context) -> None:
|
|
"""Read the models.py source for textual analysis."""
|
|
from cleveragents.infrastructure.database import models
|
|
|
|
context.models_source = inspect.getsource(models)
|
|
|
|
|
|
@then('the source should not contain "{forbidden}"')
|
|
def step_source_not_contain(context: Context, forbidden: str) -> None:
|
|
"""Assert the source code does not contain a forbidden string."""
|
|
assert forbidden not in context.models_source, (
|
|
f"models.py should not contain '{forbidden}'"
|
|
)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Migration runner
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given('I create a MigrationRunner with URL "{url}"')
|
|
def step_create_migration_runner(context: Context, url: str) -> None:
|
|
"""Create a MigrationRunner instance."""
|
|
from cleveragents.infrastructure.database.migration_runner import MigrationRunner
|
|
|
|
context.migration_runner = MigrationRunner(url)
|
|
|
|
|
|
@then('the migration runner database_url should start with "{prefix}"')
|
|
def step_runner_url_starts_with(context: Context, prefix: str) -> None:
|
|
"""Assert migration runner database_url prefix."""
|
|
url = context.migration_runner.database_url
|
|
assert url.startswith(prefix), f"Expected '{prefix}', got '{url}'"
|