From 9c27fa02b040dd030652b9aff414c424d868e41c Mon Sep 17 00:00:00 2001 From: Samir Boulahtit Date: Fri, 13 Feb 2026 20:58:22 +0100 Subject: [PATCH] refactor: move capacity_forecast_service from billing to monitoring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves the billing (core) → monitoring (optional) architecture violation by moving CapacityForecastService to the monitoring module where it belongs. - Create BillingMetricsProvider to expose subscription counts via stats_aggregator - Move CapacitySnapshot model from billing to monitoring - Replace direct MerchantSubscription queries with stats_aggregator calls - Fix middleware test mocks to cover StoreDomain/MerchantDomain fallback chains Co-Authored-By: Claude Opus 4.6 --- app/modules/billing/definition.py | 9 ++ app/modules/billing/models/__init__.py | 2 - app/modules/billing/models/subscription.py | 58 --------- app/modules/billing/services/__init__.py | 6 - .../billing/services/billing_metrics.py | 115 ++++++++++++++++++ app/modules/monitoring/models/__init__.py | 5 +- .../monitoring/models/capacity_snapshot.py | 71 +++++++++++ .../routes/api/admin_platform_health.py | 6 +- app/modules/monitoring/services/__init__.py | 6 + .../services/capacity_forecast_service.py | 30 ++--- app/modules/monitoring/tasks/capacity.py | 2 +- app/modules/monitoring/tests/__init__.py | 0 app/modules/monitoring/tests/unit/__init__.py | 0 .../unit/test_capacity_forecast_service.py | 6 +- .../unit/middleware/test_platform_context.py | 12 +- tests/unit/middleware/test_store_context.py | 8 +- 16 files changed, 234 insertions(+), 102 deletions(-) create mode 100644 app/modules/billing/services/billing_metrics.py create mode 100644 app/modules/monitoring/models/capacity_snapshot.py rename app/modules/{billing => monitoring}/services/capacity_forecast_service.py (93%) create mode 100644 app/modules/monitoring/tests/__init__.py create mode 100644 app/modules/monitoring/tests/unit/__init__.py rename app/modules/{billing => monitoring}/tests/unit/test_capacity_forecast_service.py (98%) diff --git a/app/modules/billing/definition.py b/app/modules/billing/definition.py index f8e665ca..36e05886 100644 --- a/app/modules/billing/definition.py +++ b/app/modules/billing/definition.py @@ -89,6 +89,13 @@ def _get_store_router(): return store_router +def _get_metrics_provider(): + """Lazy import of metrics provider to avoid circular imports.""" + from app.modules.billing.services.billing_metrics import billing_metrics_provider + + return billing_metrics_provider + + def _get_feature_provider(): """Lazy import of feature provider to avoid circular imports.""" from app.modules.billing.services.billing_features import billing_feature_provider @@ -271,6 +278,8 @@ billing_module = ModuleDefinition( ], # Feature provider for feature flags feature_provider=_get_feature_provider, + # Metrics provider for subscription metrics + metrics_provider=_get_metrics_provider, ) diff --git a/app/modules/billing/models/__init__.py b/app/modules/billing/models/__init__.py index f727ccdb..0ef111f8 100644 --- a/app/modules/billing/models/__init__.py +++ b/app/modules/billing/models/__init__.py @@ -22,7 +22,6 @@ from app.modules.billing.models.subscription import ( AddOnProduct, BillingHistory, BillingPeriod, - CapacitySnapshot, StoreAddOn, StripeWebhookEvent, SubscriptionStatus, @@ -46,7 +45,6 @@ __all__ = [ "StoreAddOn", "StripeWebhookEvent", "BillingHistory", - "CapacitySnapshot", # Merchant Subscription "MerchantSubscription", # Feature Limits diff --git a/app/modules/billing/models/subscription.py b/app/modules/billing/models/subscription.py index 2e60a992..05f8ac8d 100644 --- a/app/modules/billing/models/subscription.py +++ b/app/modules/billing/models/subscription.py @@ -345,61 +345,3 @@ class BillingHistory(Base, TimestampMixin): def __repr__(self): return f"" - - -# ============================================================================ -# Capacity Planning -# ============================================================================ - - -class CapacitySnapshot(Base, TimestampMixin): - """ - Daily snapshot of platform capacity metrics. - - Used for growth trending and capacity forecasting. - Captured daily by background job. - """ - - __tablename__ = "capacity_snapshots" - - id = Column(Integer, primary_key=True, index=True) - snapshot_date = Column(DateTime(timezone=True), nullable=False, unique=True, index=True) - - # Store metrics - total_stores = Column(Integer, default=0, nullable=False) - active_stores = Column(Integer, default=0, nullable=False) - trial_stores = Column(Integer, default=0, nullable=False) - - # Subscription metrics - total_subscriptions = Column(Integer, default=0, nullable=False) - active_subscriptions = Column(Integer, default=0, nullable=False) - - # Resource metrics - total_products = Column(Integer, default=0, nullable=False) - total_orders_month = Column(Integer, default=0, nullable=False) - total_team_members = Column(Integer, default=0, nullable=False) - - # Storage metrics - storage_used_gb = Column(Numeric(10, 2), default=0, nullable=False) - db_size_mb = Column(Numeric(10, 2), default=0, nullable=False) - - # Capacity metrics (theoretical limits from subscriptions) - theoretical_products_limit = Column(Integer, nullable=True) - theoretical_orders_limit = Column(Integer, nullable=True) - theoretical_team_limit = Column(Integer, nullable=True) - - # Tier distribution (JSON: {"essential": 10, "professional": 5, ...}) - tier_distribution = Column(JSON, nullable=True) - - # Performance metrics - avg_response_ms = Column(Integer, nullable=True) - peak_cpu_percent = Column(Numeric(5, 2), nullable=True) - peak_memory_percent = Column(Numeric(5, 2), nullable=True) - - # Indexes - __table_args__ = ( - Index("ix_capacity_snapshots_date", "snapshot_date"), - ) - - def __repr__(self) -> str: - return f"" diff --git a/app/modules/billing/services/__init__.py b/app/modules/billing/services/__init__.py index dc19b3df..281d4fe5 100644 --- a/app/modules/billing/services/__init__.py +++ b/app/modules/billing/services/__init__.py @@ -21,10 +21,6 @@ from app.modules.billing.services.billing_service import ( BillingService, billing_service, ) -from app.modules.billing.services.capacity_forecast_service import ( - CapacityForecastService, - capacity_forecast_service, -) from app.modules.billing.services.feature_service import ( FeatureService, feature_service, @@ -68,8 +64,6 @@ __all__ = [ "SubscriptionNotCancelledError", "FeatureService", "feature_service", - "CapacityForecastService", - "capacity_forecast_service", "PlatformPricingService", "platform_pricing_service", "UsageService", diff --git a/app/modules/billing/services/billing_metrics.py b/app/modules/billing/services/billing_metrics.py new file mode 100644 index 00000000..7fbc4e8c --- /dev/null +++ b/app/modules/billing/services/billing_metrics.py @@ -0,0 +1,115 @@ +# app/modules/billing/services/billing_metrics.py +""" +Metrics provider for the billing module. + +Provides metrics for: +- Subscription counts (total, active, trial) +""" + +import logging + +from sqlalchemy import func +from sqlalchemy.orm import Session + +from app.modules.contracts.metrics import ( + MetricsContext, + MetricValue, +) + +logger = logging.getLogger(__name__) + + +class BillingMetricsProvider: + """ + Metrics provider for billing module. + + Provides subscription metrics at the platform level. + """ + + @property + def metrics_category(self) -> str: + return "billing" + + def get_store_metrics( + self, + db: Session, + store_id: int, + context: MetricsContext | None = None, + ) -> list[MetricValue]: + """ + Get metrics for a specific store. + + Subscriptions are merchant-level, not store-level, so no store metrics. + """ + return [] + + def get_platform_metrics( + self, + db: Session, + platform_id: int, + context: MetricsContext | None = None, + ) -> list[MetricValue]: + """ + Get subscription metrics aggregated for a platform. + + Provides: + - Total subscriptions + - Active subscriptions (active + trial) + - Trial subscriptions + """ + from app.modules.billing.models import MerchantSubscription, SubscriptionStatus + + try: + total_subs = ( + db.query(func.count(MerchantSubscription.id)).scalar() or 0 + ) + + active_subs = ( + db.query(func.count(MerchantSubscription.id)) + .filter(MerchantSubscription.status.in_(["active", "trial"])) + .scalar() + or 0 + ) + + trial_subs = ( + db.query(func.count(MerchantSubscription.id)) + .filter(MerchantSubscription.status == SubscriptionStatus.TRIAL.value) + .scalar() + or 0 + ) + + return [ + MetricValue( + key="billing.total_subscriptions", + value=total_subs, + label="Total Subscriptions", + category="billing", + icon="credit-card", + description="Total number of merchant subscriptions", + ), + MetricValue( + key="billing.active_subscriptions", + value=active_subs, + label="Active Subscriptions", + category="billing", + icon="check-circle", + description="Subscriptions with active or trial status", + ), + MetricValue( + key="billing.trial_subscriptions", + value=trial_subs, + label="Trial Subscriptions", + category="billing", + icon="clock", + description="Subscriptions currently in trial period", + ), + ] + except Exception as e: + logger.warning(f"Failed to get billing platform metrics: {e}") + return [] + + +# Singleton instance +billing_metrics_provider = BillingMetricsProvider() + +__all__ = ["BillingMetricsProvider", "billing_metrics_provider"] diff --git a/app/modules/monitoring/models/__init__.py b/app/modules/monitoring/models/__init__.py index ac092cf7..dfe2353c 100644 --- a/app/modules/monitoring/models/__init__.py +++ b/app/modules/monitoring/models/__init__.py @@ -2,11 +2,10 @@ """ Monitoring module database models. -Re-exports monitoring-related models from their source locations. +Provides monitoring-related models including capacity snapshots. """ -# CapacitySnapshot is in billing module (tracks system capacity over time) -from app.modules.billing.models import CapacitySnapshot +from app.modules.monitoring.models.capacity_snapshot import CapacitySnapshot # Admin notification and logging models from app.modules.messaging.models import AdminNotification diff --git a/app/modules/monitoring/models/capacity_snapshot.py b/app/modules/monitoring/models/capacity_snapshot.py new file mode 100644 index 00000000..3a941a3d --- /dev/null +++ b/app/modules/monitoring/models/capacity_snapshot.py @@ -0,0 +1,71 @@ +# app/modules/monitoring/models/capacity_snapshot.py +""" +Capacity snapshot model for platform capacity monitoring. + +Stores daily snapshots of platform metrics for growth trending and capacity forecasting. +""" + +from sqlalchemy import ( + Column, + DateTime, + Index, + Integer, + Numeric, +) +from sqlalchemy.dialects.sqlite import JSON + +from app.core.database import Base +from models.database.base import TimestampMixin + + +class CapacitySnapshot(Base, TimestampMixin): + """ + Daily snapshot of platform capacity metrics. + + Used for growth trending and capacity forecasting. + Captured daily by background job. + """ + + __tablename__ = "capacity_snapshots" + + id = Column(Integer, primary_key=True, index=True) + snapshot_date = Column(DateTime(timezone=True), nullable=False, unique=True, index=True) + + # Store metrics + total_stores = Column(Integer, default=0, nullable=False) + active_stores = Column(Integer, default=0, nullable=False) + trial_stores = Column(Integer, default=0, nullable=False) + + # Subscription metrics + total_subscriptions = Column(Integer, default=0, nullable=False) + active_subscriptions = Column(Integer, default=0, nullable=False) + + # Resource metrics + total_products = Column(Integer, default=0, nullable=False) + total_orders_month = Column(Integer, default=0, nullable=False) + total_team_members = Column(Integer, default=0, nullable=False) + + # Storage metrics + storage_used_gb = Column(Numeric(10, 2), default=0, nullable=False) + db_size_mb = Column(Numeric(10, 2), default=0, nullable=False) + + # Capacity metrics (theoretical limits from subscriptions) + theoretical_products_limit = Column(Integer, nullable=True) + theoretical_orders_limit = Column(Integer, nullable=True) + theoretical_team_limit = Column(Integer, nullable=True) + + # Tier distribution (JSON: {"essential": 10, "professional": 5, ...}) + tier_distribution = Column(JSON, nullable=True) + + # Performance metrics + avg_response_ms = Column(Integer, nullable=True) + peak_cpu_percent = Column(Numeric(5, 2), nullable=True) + peak_memory_percent = Column(Numeric(5, 2), nullable=True) + + # Indexes + __table_args__ = ( + Index("ix_capacity_snapshots_date", "snapshot_date"), + ) + + def __repr__(self) -> str: + return f"" diff --git a/app/modules/monitoring/routes/api/admin_platform_health.py b/app/modules/monitoring/routes/api/admin_platform_health.py index 0d9c2391..92937f7d 100644 --- a/app/modules/monitoring/routes/api/admin_platform_health.py +++ b/app/modules/monitoring/routes/api/admin_platform_health.py @@ -172,7 +172,7 @@ async def get_growth_trends( Returns growth rates and projections for key metrics. """ - from app.modules.billing.services.capacity_forecast_service import ( + from app.modules.monitoring.services.capacity_forecast_service import ( capacity_forecast_service, ) @@ -189,7 +189,7 @@ async def get_scaling_recommendations( Returns prioritized list of recommendations. """ - from app.modules.billing.services.capacity_forecast_service import ( + from app.modules.monitoring.services.capacity_forecast_service import ( capacity_forecast_service, ) @@ -206,7 +206,7 @@ async def capture_snapshot( Normally run automatically by daily background job. """ - from app.modules.billing.services.capacity_forecast_service import ( + from app.modules.monitoring.services.capacity_forecast_service import ( capacity_forecast_service, ) diff --git a/app/modules/monitoring/services/__init__.py b/app/modules/monitoring/services/__init__.py index 62d84ff3..96f61dad 100644 --- a/app/modules/monitoring/services/__init__.py +++ b/app/modules/monitoring/services/__init__.py @@ -9,6 +9,10 @@ from app.modules.monitoring.services.admin_audit_service import ( AdminAuditService, admin_audit_service, ) +from app.modules.monitoring.services.capacity_forecast_service import ( + CapacityForecastService, + capacity_forecast_service, +) from app.modules.monitoring.services.background_tasks_service import ( BackgroundTasksService, background_tasks_service, @@ -25,6 +29,8 @@ from app.modules.monitoring.services.platform_health_service import ( __all__ = [ "admin_audit_service", "AdminAuditService", + "capacity_forecast_service", + "CapacityForecastService", "background_tasks_service", "BackgroundTasksService", "log_service", diff --git a/app/modules/billing/services/capacity_forecast_service.py b/app/modules/monitoring/services/capacity_forecast_service.py similarity index 93% rename from app/modules/billing/services/capacity_forecast_service.py rename to app/modules/monitoring/services/capacity_forecast_service.py index 54d8e180..b8a43348 100644 --- a/app/modules/billing/services/capacity_forecast_service.py +++ b/app/modules/monitoring/services/capacity_forecast_service.py @@ -1,4 +1,4 @@ -# app/modules/billing/services/capacity_forecast_service.py +# app/modules/monitoring/services/capacity_forecast_service.py """ Capacity forecasting service for growth trends and scaling recommendations. @@ -16,13 +16,9 @@ from decimal import Decimal from sqlalchemy import func from sqlalchemy.orm import Session -from app.modules.billing.models import ( - CapacitySnapshot, - MerchantSubscription, - SubscriptionStatus, -) from app.modules.contracts.metrics import MetricsContext from app.modules.core.services.stats_aggregator import stats_aggregator +from app.modules.monitoring.models.capacity_snapshot import CapacitySnapshot from app.modules.tenancy.models import Platform, Store, StoreUser logger = logging.getLogger(__name__) @@ -75,22 +71,7 @@ class CapacityForecastService: or 0 ) - # Subscription metrics - total_subs = db.query(func.count(MerchantSubscription.id)).scalar() or 0 - active_subs = ( - db.query(func.count(MerchantSubscription.id)) - .filter(MerchantSubscription.status.in_(["active", "trial"])) - .scalar() - or 0 - ) - trial_stores = ( - db.query(func.count(MerchantSubscription.id)) - .filter(MerchantSubscription.status == SubscriptionStatus.TRIAL.value) - .scalar() - or 0 - ) - - # Resource metrics via provider pattern (avoids direct catalog/orders imports) + # Resource metrics via provider pattern (avoids cross-module imports) start_of_month = now.replace(day=1, hour=0, minute=0, second=0, microsecond=0) platform = db.query(Platform).first() platform_id = platform.id if platform else 1 @@ -100,6 +81,11 @@ class CapacityForecastService: context=MetricsContext(date_from=start_of_month), ) + # Subscription metrics via stats aggregator (avoids billing → monitoring violation) + total_subs = stats.get("billing.total_subscriptions", 0) + active_subs = stats.get("billing.active_subscriptions", 0) + trial_stores = stats.get("billing.trial_subscriptions", 0) + total_products = stats.get("catalog.total_products", 0) total_team = ( db.query(func.count(StoreUser.id)) diff --git a/app/modules/monitoring/tasks/capacity.py b/app/modules/monitoring/tasks/capacity.py index 071c3562..adeccd72 100644 --- a/app/modules/monitoring/tasks/capacity.py +++ b/app/modules/monitoring/tasks/capacity.py @@ -27,7 +27,7 @@ def capture_capacity_snapshot(self): Returns: dict: Snapshot summary with store and product counts. """ - from app.modules.billing.services.capacity_forecast_service import ( + from app.modules.monitoring.services.capacity_forecast_service import ( capacity_forecast_service, ) diff --git a/app/modules/monitoring/tests/__init__.py b/app/modules/monitoring/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/app/modules/monitoring/tests/unit/__init__.py b/app/modules/monitoring/tests/unit/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/app/modules/billing/tests/unit/test_capacity_forecast_service.py b/app/modules/monitoring/tests/unit/test_capacity_forecast_service.py similarity index 98% rename from app/modules/billing/tests/unit/test_capacity_forecast_service.py rename to app/modules/monitoring/tests/unit/test_capacity_forecast_service.py index 258614f4..a969b7ac 100644 --- a/app/modules/billing/tests/unit/test_capacity_forecast_service.py +++ b/app/modules/monitoring/tests/unit/test_capacity_forecast_service.py @@ -1,4 +1,4 @@ -# tests/unit/services/test_capacity_forecast_service.py +# app/modules/monitoring/tests/unit/test_capacity_forecast_service.py """ Unit tests for CapacityForecastService. @@ -14,8 +14,8 @@ from decimal import Decimal import pytest -from app.modules.billing.models import CapacitySnapshot -from app.modules.billing.services.capacity_forecast_service import ( +from app.modules.monitoring.models import CapacitySnapshot +from app.modules.monitoring.services.capacity_forecast_service import ( INFRASTRUCTURE_SCALING, CapacityForecastService, capacity_forecast_service, diff --git a/tests/unit/middleware/test_platform_context.py b/tests/unit/middleware/test_platform_context.py index 2aa566ff..9acd64b8 100644 --- a/tests/unit/middleware/test_platform_context.py +++ b/tests/unit/middleware/test_platform_context.py @@ -312,7 +312,10 @@ class TestPlatformContextManager: def test_get_platform_from_domain_not_found(self): """Test domain lookup when platform not found.""" mock_db = Mock(spec=Session) - mock_db.query.return_value.filter.return_value.filter.return_value.first.return_value = None + # Ensure all query chain variants return None for .first() + query_mock = mock_db.query.return_value + query_mock.filter.return_value.first.return_value = None + query_mock.filter.return_value.filter.return_value.first.return_value = None context = {"detection_method": "domain", "domain": "unknown.lu"} @@ -367,8 +370,11 @@ class TestPlatformContextManager: def test_get_platform_inactive_not_returned(self): """Test that inactive platforms are not returned.""" mock_db = Mock(spec=Session) - # First call returns None (is_active filter excludes it) - mock_db.query.return_value.filter.return_value.filter.return_value.first.return_value = None + # Ensure all query chain variants return None for .first() + # (primary Platform lookup and StoreDomain/MerchantDomain fallbacks) + query_mock = mock_db.query.return_value + query_mock.filter.return_value.first.return_value = None + query_mock.filter.return_value.filter.return_value.first.return_value = None context = {"detection_method": "domain", "domain": "inactive.lu"} diff --git a/tests/unit/middleware/test_store_context.py b/tests/unit/middleware/test_store_context.py index 325a64b6..8c426da6 100644 --- a/tests/unit/middleware/test_store_context.py +++ b/tests/unit/middleware/test_store_context.py @@ -245,7 +245,13 @@ class TestStoreContextManager: def test_get_store_from_custom_domain_not_found(self): """Test custom domain not found in database.""" mock_db = Mock(spec=Session) - mock_db.query.return_value.filter.return_value.filter.return_value.filter.return_value.first.return_value = None + # Ensure all query chain variants return None for .first() + # (primary StoreDomain lookup and MerchantDomain fallback) + query_mock = mock_db.query.return_value + query_mock.filter.return_value.first.return_value = None + query_mock.filter.return_value.filter.return_value.first.return_value = None + query_mock.filter.return_value.filter.return_value.filter.return_value.first.return_value = None + query_mock.filter.return_value.order_by.return_value.first.return_value = None context = {"detection_method": "custom_domain", "domain": "nonexistent.com"}