mirror of
https://github.com/zvx-echo6/central.git
synced 2026-08-26 09:21:36 +00:00
refactor: decouple adapter class-identity (kind) from instance-identity (name) (#120)
Adds a `kind` TEXT column to `config.adapters` (migration 043) so one adapter
class can later back many operator-created instances. Every built-in adapter
row is back-filled with `kind = name`, keeping all 23 adapters working
unchanged.
- Migration 043: ADD COLUMN IF NOT EXISTS kind, back-fill, SET NOT NULL.
- AdapterConfig gains `kind: str | None` with a model_validator that falls
back to `name` when the column is NULL/absent (pre-043 rows).
- config_store.get_adapter / list_adapters: add `kind` to SELECT.
- supervisor._create_adapter AND _start_adapter (api-key precondition):
resolve the class by config.kind (class key) not config.name (instance key);
runtime state keying stays on config.name.
- routes.py GET/POST /adapters/{name} and adapters_list: add `kind` to SELECT,
resolve the adapter class by row["kind"].
- adapter_discovery.py: comment clarifying .name is the kind (class identity).
- Out of scope, left as-is (built-ins only, name==kind): setup wizard paths in
routes.py (~750/~891/~1109) and gui/__init__.py.
- Tests: 17 new pure-unit tests (migration SQL shape, AdapterConfig back-compat,
_create_adapter + _start_adapter kind dispatch). Existing test mock rows
updated to include the `kind` field.
Co-authored-by: Ubuntu <zvx@cortex.echo6.co>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
9450696fe9
commit
13722d07dd
12 changed files with 370 additions and 26 deletions
25
sql/migrations/043_add_adapters_kind_column.sql
Normal file
25
sql/migrations/043_add_adapters_kind_column.sql
Normal file
|
|
@ -0,0 +1,25 @@
|
|||
-- Migration 043: decouple adapter class-identity (kind) from instance-identity (name)
|
||||
--
|
||||
-- Until now config.adapters.name served two roles:
|
||||
-- 1. Instance primary key — the unique name used in runtime state, logs, dedup
|
||||
-- tables, NATS subjects, etc.
|
||||
-- 2. Registry key — the key used to look up the adapter class in the
|
||||
-- supervisor's in-memory registry (discover_adapters returns {class.name: cls}).
|
||||
--
|
||||
-- These roles must be split so that one adapter class can later back many
|
||||
-- operator-created instances (e.g. a generic HTTP adapter). The new `kind`
|
||||
-- column holds the class identity (registry key), while `name` remains the
|
||||
-- unique instance identifier. All built-in adapters have name == kind, so
|
||||
-- every existing row is back-filled with kind = name and behaviour is unchanged.
|
||||
--
|
||||
-- Idempotent: ADD COLUMN IF NOT EXISTS is safe on re-run.
|
||||
-- No DEFAULT is set — future INSERT paths must always supply kind explicitly.
|
||||
|
||||
ALTER TABLE config.adapters ADD COLUMN IF NOT EXISTS kind TEXT;
|
||||
|
||||
-- Back-fill: existing rows get kind = name (class identity == instance identity
|
||||
-- for all 23 built-in adapters shipped before v0.15.0).
|
||||
UPDATE config.adapters SET kind = name WHERE kind IS NULL;
|
||||
|
||||
-- Enforce non-null going forward.
|
||||
ALTER TABLE config.adapters ALTER COLUMN kind SET NOT NULL;
|
||||
|
|
@ -30,5 +30,7 @@ def discover_adapters() -> dict[str, type[SourceAdapter]]:
|
|||
and attr is not SourceAdapter
|
||||
and hasattr(attr, "name")
|
||||
):
|
||||
# attr.name is the *kind* (class identity); AdapterConfig.name
|
||||
# is the instance identity. For all built-ins kind == name.
|
||||
registry[attr.name] = attr
|
||||
return registry
|
||||
|
|
|
|||
|
|
@ -30,7 +30,15 @@ class RegionConfig(BaseModel):
|
|||
class AdapterConfig(BaseModel):
|
||||
"""Configuration for a single adapter."""
|
||||
|
||||
name: str = Field(description="Unique adapter identifier")
|
||||
name: str = Field(description="Unique adapter identifier (instance identity)")
|
||||
kind: str | None = Field(
|
||||
default=None,
|
||||
description=(
|
||||
"Adapter class identifier (registry key / class identity). "
|
||||
"Matches SourceAdapter.name on the class. Defaults to `name` "
|
||||
"for back-compat when the DB column is absent or NULL (pre-043)."
|
||||
),
|
||||
)
|
||||
enabled: bool = Field(default=True, description="Whether adapter is active")
|
||||
cadence_s: int = Field(ge=10, description="Poll interval in seconds")
|
||||
settings: dict[str, Any] = Field(
|
||||
|
|
@ -41,6 +49,13 @@ class AdapterConfig(BaseModel):
|
|||
)
|
||||
updated_at: datetime = Field(description="Last configuration update time")
|
||||
|
||||
@model_validator(mode="after")
|
||||
def _backfill_kind(self) -> "AdapterConfig":
|
||||
"""Default kind to name when the DB column is NULL or absent (pre-043)."""
|
||||
if self.kind is None:
|
||||
self.kind = self.name
|
||||
return self
|
||||
|
||||
@property
|
||||
def is_paused(self) -> bool:
|
||||
"""Check if adapter is currently paused."""
|
||||
|
|
|
|||
|
|
@ -95,7 +95,7 @@ class ConfigStore:
|
|||
async with self._pool.acquire() as conn:
|
||||
row = await conn.fetchrow(
|
||||
"""
|
||||
SELECT name, enabled, cadence_s, settings, paused_at, updated_at
|
||||
SELECT name, kind, enabled, cadence_s, settings, paused_at, updated_at
|
||||
FROM config.adapters
|
||||
WHERE name = $1
|
||||
""",
|
||||
|
|
@ -110,7 +110,7 @@ class ConfigStore:
|
|||
async with self._pool.acquire() as conn:
|
||||
rows = await conn.fetch(
|
||||
"""
|
||||
SELECT name, enabled, cadence_s, settings, paused_at, updated_at
|
||||
SELECT name, kind, enabled, cadence_s, settings, paused_at, updated_at
|
||||
FROM config.adapters
|
||||
ORDER BY name
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -1451,7 +1451,7 @@ async def adapters_list(
|
|||
async with pool.acquire() as conn:
|
||||
rows = await conn.fetch(
|
||||
"""
|
||||
SELECT name, enabled, cadence_s, settings, paused_at, updated_at, last_error
|
||||
SELECT name, kind, enabled, cadence_s, settings, paused_at, updated_at, last_error
|
||||
FROM config.adapters
|
||||
ORDER BY name
|
||||
"""
|
||||
|
|
@ -1460,7 +1460,8 @@ async def adapters_list(
|
|||
adapters = []
|
||||
for row in rows:
|
||||
settings = row["settings"] or {}
|
||||
adapter_cls = adapter_classes.get(row["name"])
|
||||
# Resolve the class by kind (class identity), not instance name.
|
||||
adapter_cls = adapter_classes.get(row["kind"])
|
||||
|
||||
# Check if required API key is missing — resolve via the per-row
|
||||
# settings[api_key_field] (operator-selected alias), falling back
|
||||
|
|
@ -1540,14 +1541,10 @@ async def adapters_edit_form(
|
|||
pool = get_pool()
|
||||
operator = request.state.operator
|
||||
|
||||
# Look up the adapter class
|
||||
adapter_classes = _adapter_classes()
|
||||
adapter_cls = adapter_classes.get(name)
|
||||
|
||||
async with pool.acquire() as conn:
|
||||
row = await conn.fetchrow(
|
||||
"""
|
||||
SELECT name, enabled, cadence_s, settings, paused_at, updated_at, last_error
|
||||
SELECT name, kind, enabled, cadence_s, settings, paused_at, updated_at, last_error
|
||||
FROM config.adapters
|
||||
WHERE name = $1
|
||||
""",
|
||||
|
|
@ -1564,6 +1561,10 @@ async def adapters_edit_form(
|
|||
tile_url = sys_row["map_tile_url"] if sys_row else "https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png"
|
||||
tile_attribution = sys_row["map_attribution"] if sys_row else "© OpenStreetMap contributors"
|
||||
|
||||
# Look up the adapter class by kind (class identity), not instance name.
|
||||
adapter_classes = _adapter_classes()
|
||||
adapter_cls = adapter_classes.get(row["kind"])
|
||||
|
||||
settings = row["settings"] or {}
|
||||
|
||||
# Build adapter dict with class metadata
|
||||
|
|
@ -1609,6 +1610,7 @@ async def adapters_edit_form(
|
|||
settings_obj = adapter_cls.settings_schema(**settings)
|
||||
preview_cfg = AdapterConfig(
|
||||
name=row["name"],
|
||||
kind=row["kind"],
|
||||
enabled=row["enabled"],
|
||||
cadence_s=row["cadence_s"],
|
||||
settings=settings,
|
||||
|
|
@ -1672,10 +1674,6 @@ async def adapters_edit_submit(
|
|||
if not form_csrf or form_csrf != request.state.csrf_token:
|
||||
raise CsrfValidationError("Invalid CSRF token")
|
||||
|
||||
# Look up the adapter class
|
||||
adapter_classes = _adapter_classes()
|
||||
adapter_cls = adapter_classes.get(name)
|
||||
|
||||
# Parse common form fields
|
||||
enabled = "enabled" in form
|
||||
cadence_s_str = form.get("cadence_s", "")
|
||||
|
|
@ -1701,7 +1699,7 @@ async def adapters_edit_submit(
|
|||
# Get current adapter state
|
||||
row = await conn.fetchrow(
|
||||
"""
|
||||
SELECT name, enabled, cadence_s, settings, paused_at, updated_at, last_error
|
||||
SELECT name, kind, enabled, cadence_s, settings, paused_at, updated_at, last_error
|
||||
FROM config.adapters
|
||||
WHERE name = $1
|
||||
""",
|
||||
|
|
@ -1711,6 +1709,10 @@ async def adapters_edit_submit(
|
|||
if row is None:
|
||||
return Response(status_code=404, content="Adapter not found")
|
||||
|
||||
# Look up the adapter class by kind (class identity), not instance name.
|
||||
adapter_classes = _adapter_classes()
|
||||
adapter_cls = adapter_classes.get(row["kind"])
|
||||
|
||||
current_settings = row["settings"] or {}
|
||||
|
||||
# Parse and validate settings via Pydantic if we have the adapter class
|
||||
|
|
|
|||
|
|
@ -355,10 +355,18 @@ class Supervisor:
|
|||
)
|
||||
|
||||
def _create_adapter(self, config: AdapterConfig) -> SourceAdapter:
|
||||
"""Create an adapter instance based on config name."""
|
||||
cls = self._adapters.get(config.name)
|
||||
"""Create an adapter instance based on config kind (class identity).
|
||||
|
||||
config.kind names the adapter *class* in the registry (discover_adapters
|
||||
keys by SourceAdapter.name on the class). config.name is the unique
|
||||
*instance* identifier and is unchanged here — runtime state, logs, and
|
||||
dedup tables continue to key on config.name.
|
||||
"""
|
||||
cls = self._adapters.get(config.kind)
|
||||
if cls is None:
|
||||
raise ValueError(f"Unknown adapter type: {config.name}")
|
||||
raise ValueError(
|
||||
f"Unknown adapter kind: {config.kind!r} (instance: {config.name!r})"
|
||||
)
|
||||
return cls(
|
||||
config=config,
|
||||
config_store=self._config_store,
|
||||
|
|
@ -524,8 +532,9 @@ class Supervisor:
|
|||
# API key precondition — resolve via per-row settings[api_key_field]
|
||||
# (operator-selected alias), falling back to the class-attribute
|
||||
# default when settings hasn't been set. Returns None when no key
|
||||
# is required.
|
||||
adapter_cls = self._adapters.get(config.name)
|
||||
# is required. Resolve the class by config.kind (class identity), not
|
||||
# config.name (instance identity) — mirrors _create_adapter.
|
||||
adapter_cls = self._adapters.get(config.kind)
|
||||
alias = resolve_api_key_alias(adapter_cls, config.settings)
|
||||
if alias is not None:
|
||||
key_value = await self._config_store.get_api_key(alias)
|
||||
|
|
|
|||
230
tests/test_adapter_config_kind.py
Normal file
230
tests/test_adapter_config_kind.py
Normal file
|
|
@ -0,0 +1,230 @@
|
|||
"""Tests for AdapterConfig.kind and supervisor._create_adapter kind-based dispatch.
|
||||
|
||||
All tests are pure-unit (no DB, no NATS) so they run anywhere.
|
||||
"""
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
from datetime import datetime, timezone
|
||||
from pathlib import Path
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
# Provide required env vars before importing central modules.
|
||||
os.environ.setdefault("CENTRAL_DB_DSN", "postgresql://test:test@localhost/test")
|
||||
os.environ.setdefault("CENTRAL_CSRF_SECRET", "testsecret12345678901234567890ab")
|
||||
os.environ.setdefault("CENTRAL_NATS_URL", "nats://localhost:4222")
|
||||
|
||||
from central.config_models import AdapterConfig
|
||||
|
||||
|
||||
_NOW = datetime(2026, 1, 1, tzinfo=timezone.utc)
|
||||
|
||||
|
||||
def _cfg(**kwargs) -> AdapterConfig:
|
||||
"""Build a minimal AdapterConfig with sensible defaults."""
|
||||
defaults = {
|
||||
"name": "usgs_quake",
|
||||
"cadence_s": 120,
|
||||
"updated_at": _NOW,
|
||||
}
|
||||
defaults.update(kwargs)
|
||||
return AdapterConfig(**defaults)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# AdapterConfig.kind back-compat
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestAdapterConfigKind:
|
||||
def test_kind_defaults_to_name_when_absent(self):
|
||||
"""When kind is not supplied, it falls back to name (pre-043 rows)."""
|
||||
cfg = _cfg(name="usgs_quake")
|
||||
assert cfg.kind == "usgs_quake"
|
||||
|
||||
def test_kind_defaults_to_name_when_none(self):
|
||||
"""When kind is explicitly None (NULL from DB), it falls back to name."""
|
||||
cfg = _cfg(name="nws", kind=None)
|
||||
assert cfg.kind == "nws"
|
||||
|
||||
def test_kind_preserved_when_set(self):
|
||||
"""When kind is explicitly supplied, it is kept as-is."""
|
||||
cfg = _cfg(name="my_quake_instance", kind="usgs_quake")
|
||||
assert cfg.kind == "usgs_quake"
|
||||
assert cfg.name == "my_quake_instance"
|
||||
|
||||
def test_name_unchanged_when_kind_differs(self):
|
||||
"""Instance identity (name) is never overwritten by the kind back-fill."""
|
||||
cfg = _cfg(name="my_custom_quake", kind="usgs_quake")
|
||||
assert cfg.name == "my_custom_quake"
|
||||
|
||||
def test_kind_equals_name_for_builtin_pattern(self):
|
||||
"""Built-in adapters: name == kind both before and after the migration."""
|
||||
cfg = _cfg(name="nws", kind="nws")
|
||||
assert cfg.kind == cfg.name == "nws"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# supervisor._create_adapter resolves class by kind
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestCreateAdapterResolvesbyKind:
|
||||
"""Unit-test _create_adapter without a live Supervisor (no NATS, no DB)."""
|
||||
|
||||
def _make_supervisor_with_registry(self, registry: dict):
|
||||
"""Return a minimal Supervisor-like object with _adapters = registry."""
|
||||
# Import lazily to avoid triggering the ENRICHMENT_CACHE_DB_PATH side
|
||||
# effect before conftest patches it (conftest autouse fixture handles it
|
||||
# in the full test run; here we patch manually).
|
||||
from unittest.mock import patch
|
||||
import central.supervisor as sup_mod
|
||||
|
||||
with tempfile.TemporaryDirectory() as td:
|
||||
with patch.object(sup_mod, "ENRICHMENT_CACHE_DB_PATH", Path(td) / "ec.db"):
|
||||
with patch.object(sup_mod, "CURSOR_DB_PATH", Path(td) / "cursor.db"):
|
||||
supervisor = MagicMock()
|
||||
supervisor._adapters = registry
|
||||
supervisor._config_store = MagicMock()
|
||||
# Bind the real _create_adapter method to our mock object.
|
||||
supervisor._create_adapter = (
|
||||
sup_mod.Supervisor._create_adapter.__get__(supervisor)
|
||||
)
|
||||
return supervisor
|
||||
|
||||
def _make_mock_adapter_cls(self, class_name: str):
|
||||
"""Return a mock adapter class whose constructor returns a mock instance."""
|
||||
instance = MagicMock()
|
||||
instance.name = class_name # class-level .name attribute
|
||||
|
||||
cls = MagicMock()
|
||||
cls.return_value = instance
|
||||
return cls
|
||||
|
||||
def test_resolves_class_by_kind_when_kind_differs_from_name(self):
|
||||
"""_create_adapter uses config.kind (class key), not config.name."""
|
||||
usgs_cls = self._make_mock_adapter_cls("usgs_quake")
|
||||
registry = {"usgs_quake": usgs_cls}
|
||||
sup = self._make_supervisor_with_registry(registry)
|
||||
|
||||
cfg = _cfg(name="my_quake_instance", kind="usgs_quake")
|
||||
result = sup._create_adapter(cfg)
|
||||
|
||||
# Class was looked up by kind and instantiated.
|
||||
usgs_cls.assert_called_once()
|
||||
assert result is usgs_cls.return_value
|
||||
|
||||
def test_builtin_pattern_kind_equals_name_still_works(self):
|
||||
"""Regression: existing adapters with name == kind construct correctly."""
|
||||
usgs_cls = self._make_mock_adapter_cls("usgs_quake")
|
||||
registry = {"usgs_quake": usgs_cls}
|
||||
sup = self._make_supervisor_with_registry(registry)
|
||||
|
||||
cfg = _cfg(name="usgs_quake", kind="usgs_quake")
|
||||
result = sup._create_adapter(cfg)
|
||||
|
||||
usgs_cls.assert_called_once()
|
||||
assert result is usgs_cls.return_value
|
||||
|
||||
def test_raises_on_unknown_kind(self):
|
||||
"""_create_adapter raises ValueError mentioning both kind and instance name."""
|
||||
registry = {}
|
||||
sup = self._make_supervisor_with_registry(registry)
|
||||
|
||||
cfg = _cfg(name="my_instance", kind="nonexistent_adapter")
|
||||
with pytest.raises(ValueError) as exc_info:
|
||||
sup._create_adapter(cfg)
|
||||
|
||||
msg = str(exc_info.value)
|
||||
assert "nonexistent_adapter" in msg
|
||||
assert "my_instance" in msg
|
||||
|
||||
def test_instance_identity_passed_through_to_constructor(self):
|
||||
"""The adapter constructor receives the full config (name = instance id)."""
|
||||
some_cls = self._make_mock_adapter_cls("some_adapter")
|
||||
registry = {"some_adapter": some_cls}
|
||||
sup = self._make_supervisor_with_registry(registry)
|
||||
|
||||
cfg = _cfg(name="prod_instance_1", kind="some_adapter")
|
||||
sup._create_adapter(cfg)
|
||||
|
||||
# Check the config passed to constructor has the instance name.
|
||||
call_kwargs = some_cls.call_args
|
||||
passed_config = call_kwargs[1].get("config") or call_kwargs[0][0]
|
||||
assert passed_config.name == "prod_instance_1"
|
||||
|
||||
|
||||
class TestStartAdapterResolvesByKind:
|
||||
"""The api-key precondition in _start_adapter also resolves the class by kind.
|
||||
|
||||
_start_adapter is callable directly with mocks (no live DB), mirroring
|
||||
tests/test_requires_api_key.py::TestSupervisorApiKeyPrecondition. A generic
|
||||
instance where name != kind must still find its class via the registry's
|
||||
kind key — otherwise the lookup returns None, resolve_api_key_alias treats
|
||||
it as "no key required", and the adapter silently skips its api-key check
|
||||
and starts. Resolving by kind keeps the precondition enforced.
|
||||
"""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_precondition_refuses_when_key_missing_for_instance_named_differently(
|
||||
self, tmp_path: Path
|
||||
):
|
||||
from central.supervisor import Supervisor
|
||||
from central.adapters.firms import FIRMSAdapter
|
||||
|
||||
mock_config_store = MagicMock()
|
||||
mock_config_store.get_api_key = AsyncMock(return_value=None) # key missing
|
||||
mock_config_store.set_adapter_last_error = AsyncMock()
|
||||
|
||||
mock_nats = MagicMock()
|
||||
mock_nats.publish = AsyncMock()
|
||||
|
||||
supervisor = Supervisor.__new__(Supervisor)
|
||||
supervisor._config_store = mock_config_store
|
||||
# Registry is keyed by class identity (kind == FIRMSAdapter.name == "firms").
|
||||
supervisor._adapters = {"firms": FIRMSAdapter}
|
||||
supervisor._adapter_states = {}
|
||||
supervisor._nats = mock_nats
|
||||
supervisor._cursor_db_path = tmp_path / "cursors.db"
|
||||
supervisor._log = MagicMock()
|
||||
|
||||
# Instance name differs from kind — the generic-instance case.
|
||||
config = _cfg(
|
||||
name="my_firms_instance",
|
||||
kind="firms",
|
||||
enabled=True,
|
||||
cadence_s=300,
|
||||
settings={"api_key_alias": "firms", "satellites": ["VIIRS_SNPP_NRT"]},
|
||||
)
|
||||
|
||||
await supervisor._start_adapter(config)
|
||||
|
||||
# The class WAS resolved by kind: requires_api_key fired, key was found
|
||||
# missing, and the adapter refused to start. Had the lookup used
|
||||
# config.name ("my_firms_instance", absent from the registry), the
|
||||
# precondition would have been skipped and the adapter would have started.
|
||||
mock_config_store.get_api_key.assert_called_once_with("firms")
|
||||
mock_config_store.set_adapter_last_error.assert_called_once()
|
||||
err_args = mock_config_store.set_adapter_last_error.call_args[0]
|
||||
assert err_args[0] == "my_firms_instance" # error keyed by instance id
|
||||
assert "missing api key" in err_args[1].lower()
|
||||
# Did not start.
|
||||
assert "my_firms_instance" not in supervisor._adapter_states
|
||||
mock_nats.publish.assert_not_called()
|
||||
|
||||
def test_start_path_lookup_expression_uses_kind(self):
|
||||
"""Belt-and-suspenders: the start-path lookup keys on config.kind.
|
||||
|
||||
Direct assertion of the resolution expression (supervisor._adapters.get(
|
||||
config.kind)) independent of the heavier _start_adapter flow above.
|
||||
"""
|
||||
from central.adapters.firms import FIRMSAdapter
|
||||
|
||||
registry = {"firms": FIRMSAdapter}
|
||||
config = _cfg(name="my_firms_instance", kind="firms")
|
||||
|
||||
# Resolving by kind finds the class; resolving by name would miss.
|
||||
assert registry.get(config.kind) is FIRMSAdapter
|
||||
assert registry.get(config.name) is None
|
||||
|
|
@ -42,9 +42,9 @@ class TestAdaptersListAuthenticated:
|
|||
|
||||
mock_conn = AsyncMock()
|
||||
mock_conn.fetch.return_value = [
|
||||
{"name": "firms", "enabled": True, "cadence_s": 300, "settings": {"api_key_alias": "firms"}, "paused_at": None, "updated_at": None, "last_error": None},
|
||||
{"name": "nws", "enabled": True, "cadence_s": 60, "settings": {"contact_email": "test@test.com"}, "paused_at": None, "updated_at": None, "last_error": None},
|
||||
{"name": "usgs_quake", "enabled": True, "cadence_s": 120, "settings": {"feed": "all_hour"}, "paused_at": None, "updated_at": None, "last_error": None},
|
||||
{"name": "firms", "kind": "firms", "enabled": True, "cadence_s": 300, "settings": {"api_key_alias": "firms"}, "paused_at": None, "updated_at": None, "last_error": None},
|
||||
{"name": "nws", "kind": "nws", "enabled": True, "cadence_s": 60, "settings": {"contact_email": "test@test.com"}, "paused_at": None, "updated_at": None, "last_error": None},
|
||||
{"name": "usgs_quake", "kind": "usgs_quake", "enabled": True, "cadence_s": 120, "settings": {"feed": "all_hour"}, "paused_at": None, "updated_at": None, "last_error": None},
|
||||
]
|
||||
|
||||
mock_pool = MagicMock()
|
||||
|
|
@ -97,6 +97,7 @@ class TestAdaptersEditForm:
|
|||
mock_conn.fetchrow.side_effect = [
|
||||
{
|
||||
"name": "nws",
|
||||
"kind": "nws",
|
||||
"enabled": True,
|
||||
"cadence_s": 60,
|
||||
"settings": {"contact_email": "test@example.com", "region": {"north": 49, "south": 24, "east": -66, "west": -125}},
|
||||
|
|
@ -178,6 +179,7 @@ class TestAdaptersEditSubmit:
|
|||
mock_conn = AsyncMock()
|
||||
mock_conn.fetchrow.return_value = {
|
||||
"name": "nws",
|
||||
"kind": "nws",
|
||||
"enabled": True,
|
||||
"cadence_s": 60,
|
||||
"settings": {"contact_email": "old@example.com", "region": {"north": 49, "south": 24, "east": -66, "west": -125}},
|
||||
|
|
@ -227,6 +229,7 @@ class TestAdaptersEditSubmit:
|
|||
mock_conn.fetchrow.side_effect = [
|
||||
{
|
||||
"name": "nws",
|
||||
"kind": "nws",
|
||||
"enabled": True,
|
||||
"cadence_s": 60,
|
||||
"settings": {"contact_email": "test@example.com", "region": {"north": 49, "south": 24, "east": -66, "west": -125}},
|
||||
|
|
@ -286,6 +289,7 @@ class TestAdaptersAudit:
|
|||
mock_conn = AsyncMock()
|
||||
mock_conn.fetchrow.return_value = {
|
||||
"name": "nws",
|
||||
"kind": "nws",
|
||||
"enabled": True,
|
||||
"cadence_s": 60,
|
||||
"settings": {"contact_email": "old@example.com", "region": {"north": 49, "south": 24, "east": -66, "west": -125}},
|
||||
|
|
@ -350,6 +354,7 @@ class TestAdaptersJsonbRegression:
|
|||
mock_conn = AsyncMock()
|
||||
mock_conn.fetchrow.return_value = {
|
||||
"name": "nws",
|
||||
"kind": "nws",
|
||||
"enabled": True,
|
||||
"cadence_s": 60,
|
||||
"settings": {"contact_email": "old@example.com", "region": {"north": 49, "south": 24, "east": -66, "west": -125}}, # dict, as asyncpg returns
|
||||
|
|
@ -402,6 +407,7 @@ class TestAdaptersJsonbRegression:
|
|||
mock_conn = AsyncMock()
|
||||
mock_conn.fetchrow.return_value = {
|
||||
"name": "nws",
|
||||
"kind": "nws",
|
||||
"enabled": True,
|
||||
"cadence_s": 60,
|
||||
"settings": {"contact_email": "old@example.com", "region": {"north": 49, "south": 24, "east": -66, "west": -125}}, # dict
|
||||
|
|
@ -443,7 +449,7 @@ class TestAdaptersJsonbRegression:
|
|||
mock_conn = MagicMock()
|
||||
mock_conn.fetchrow = AsyncMock(side_effect=[
|
||||
# Adapter row
|
||||
{"name": "firms", "enabled": True, "cadence_s": 300, "settings": {},
|
||||
{"name": "firms", "kind": "firms", "enabled": True, "cadence_s": 300, "settings": {},
|
||||
"paused_at": None, "updated_at": None, "last_error": None},
|
||||
# System row
|
||||
{"map_tile_url": "https://tile.example.com", "map_attribution": "Test"},
|
||||
|
|
|
|||
|
|
@ -116,7 +116,7 @@ def _post_req(pairs, cadence_s="1800"):
|
|||
def _pool(settings=_SETTINGS):
|
||||
conn = AsyncMock()
|
||||
conn.fetchrow.side_effect = [
|
||||
{"name": "tomtom_incidents", "enabled": True, "cadence_s": 1800, "settings": settings,
|
||||
{"name": "tomtom_incidents", "kind": "tomtom_incidents", "enabled": True, "cadence_s": 1800, "settings": settings,
|
||||
"paused_at": None, "updated_at": None, "last_error": None},
|
||||
{"map_tile_url": None, "map_attribution": None},
|
||||
]
|
||||
|
|
|
|||
48
tests/test_migration_043.py
Normal file
48
tests/test_migration_043.py
Normal file
|
|
@ -0,0 +1,48 @@
|
|||
"""v0.15.0 migration 043: add kind column to config.adapters.
|
||||
|
||||
No live Postgres required — asserts the migration SQL is structurally correct:
|
||||
adds the column idempotently, back-fills kind = name for pre-existing rows, and
|
||||
enforces NOT NULL going forward.
|
||||
"""
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
_SQL = Path("sql/migrations/043_add_adapters_kind_column.sql").read_text()
|
||||
_NORM = " ".join(_SQL.split()) # whitespace-insensitive matching
|
||||
|
||||
|
||||
def test_uses_add_column_if_not_exists():
|
||||
"""Must be idempotent — safe to re-run after partial application."""
|
||||
assert "ADD COLUMN IF NOT EXISTS kind TEXT" in _NORM
|
||||
|
||||
|
||||
def test_backfills_kind_equals_name_for_null_rows():
|
||||
"""Existing rows must get kind = name so built-in adapters keep working."""
|
||||
assert "UPDATE config.adapters SET kind = name WHERE kind IS NULL" in _NORM
|
||||
|
||||
|
||||
def test_enforces_not_null_after_backfill():
|
||||
"""kind must be NOT NULL once every row is back-filled."""
|
||||
assert "ALTER COLUMN kind SET NOT NULL" in _NORM
|
||||
|
||||
|
||||
def test_does_not_add_default_clause():
|
||||
"""No DEFAULT — future INSERT paths must always supply kind explicitly."""
|
||||
# The ADD COLUMN line should not contain DEFAULT
|
||||
add_line = next(
|
||||
(line for line in _SQL.splitlines() if "ADD COLUMN" in line), ""
|
||||
)
|
||||
assert "DEFAULT" not in add_line.upper()
|
||||
|
||||
|
||||
def test_does_not_drop_any_columns():
|
||||
"""Pure additive migration — must not drop anything."""
|
||||
upper = _NORM.upper()
|
||||
assert "DROP COLUMN" not in upper
|
||||
assert "DROP TABLE" not in upper
|
||||
|
||||
|
||||
def test_explains_name_vs_kind_split_in_comment():
|
||||
"""Header comment must explain the instance-vs-class identity split."""
|
||||
assert "instance" in _SQL.lower()
|
||||
assert "class" in _SQL.lower()
|
||||
|
|
@ -27,6 +27,7 @@ class TestRegionPickerInTemplate:
|
|||
mock_conn.fetchrow.side_effect = [
|
||||
{ # Adapter row
|
||||
"name": "firms",
|
||||
"kind": "firms",
|
||||
"enabled": True,
|
||||
"cadence_s": 300,
|
||||
"settings": {
|
||||
|
|
@ -94,6 +95,7 @@ class TestRegionValidation:
|
|||
mock_conn = AsyncMock()
|
||||
mock_conn.fetchrow.return_value = {
|
||||
"name": "firms",
|
||||
"kind": "firms",
|
||||
"enabled": True,
|
||||
"cadence_s": 300,
|
||||
"settings": {"api_key_alias": "firms", "satellites": ["VIIRS_SNPP_NRT"], "region": {"north": 49.5, "south": 31.0, "east": -102.0, "west": -124.5}},
|
||||
|
|
@ -153,6 +155,7 @@ class TestRegionValidation:
|
|||
mock_conn.fetchrow.side_effect = [
|
||||
{
|
||||
"name": "firms",
|
||||
"kind": "firms",
|
||||
"enabled": True,
|
||||
"cadence_s": 300,
|
||||
"settings": {"api_key_alias": "firms", "satellites": ["VIIRS_SNPP_NRT"], "region": {"north": 49.5, "south": 31.0, "east": -102.0, "west": -124.5}},
|
||||
|
|
@ -209,6 +212,7 @@ class TestRegionValidation:
|
|||
mock_conn.fetchrow.side_effect = [
|
||||
{
|
||||
"name": "firms",
|
||||
"kind": "firms",
|
||||
"enabled": True,
|
||||
"cadence_s": 300,
|
||||
"settings": {"api_key_alias": "firms", "satellites": ["VIIRS_SNPP_NRT"], "region": {"north": 49.5, "south": 31.0, "east": -102.0, "west": -124.5}},
|
||||
|
|
@ -265,6 +269,7 @@ class TestRegionValidation:
|
|||
mock_conn.fetchrow.side_effect = [
|
||||
{
|
||||
"name": "firms",
|
||||
"kind": "firms",
|
||||
"enabled": True,
|
||||
"cadence_s": 300,
|
||||
"settings": {"api_key_alias": "firms", "satellites": ["VIIRS_SNPP_NRT"], "region": {"north": 49.5, "south": 31.0, "east": -102.0, "west": -124.5}},
|
||||
|
|
@ -323,6 +328,7 @@ class TestRegionAuditLog:
|
|||
mock_conn = AsyncMock()
|
||||
mock_conn.fetchrow.return_value = {
|
||||
"name": "firms",
|
||||
"kind": "firms",
|
||||
"enabled": True,
|
||||
"cadence_s": 300,
|
||||
"settings": {
|
||||
|
|
|
|||
|
|
@ -74,7 +74,7 @@ class TestRoutesApiKeyMissing:
|
|||
mock_pool = MagicMock()
|
||||
mock_conn = MagicMock()
|
||||
mock_conn.fetch = AsyncMock(return_value=[
|
||||
{"name": "firms", "enabled": False, "cadence_s": 300, "settings": {}, "paused_at": None, "updated_at": None, "last_error": None},
|
||||
{"name": "firms", "kind": "firms", "enabled": False, "cadence_s": 300, "settings": {}, "paused_at": None, "updated_at": None, "last_error": None},
|
||||
])
|
||||
mock_conn.fetchval = AsyncMock(return_value=None) # No API key exists
|
||||
mock_conn.__aenter__ = AsyncMock(return_value=mock_conn)
|
||||
|
|
@ -307,6 +307,7 @@ class TestAdaptersEditSubmitErrorRerender:
|
|||
# First call: adapter row
|
||||
{
|
||||
"name": "firms",
|
||||
"kind": "firms",
|
||||
"enabled": False,
|
||||
"cadence_s": 300,
|
||||
"settings": {"api_key_alias": "firms", "satellites": ["VIIRS_SNPP_NRT"]},
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue