refactor: fix architecture violations with provider patterns and dependency inversion
Major changes: - Add AuditProvider protocol for cross-module audit logging - Move customer order operations to orders module (dependency inversion) - Add customer order metrics via MetricsProvider pattern - Fix missing db parameter in get_admin_context() calls - Move ProductMedia relationship to catalog module (proper ownership) - Add marketplace breakdown stats to marketplace_widgets New files: - contracts/audit.py - AuditProviderProtocol - core/services/audit_aggregator.py - Aggregates audit providers - monitoring/services/audit_provider.py - Monitoring audit implementation - orders/services/customer_order_service.py - Customer order operations - orders/routes/api/vendor_customer_orders.py - Customer order endpoints - catalog/services/product_media_service.py - Product media service - Architecture documentation for patterns Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -20,7 +20,7 @@ from sqlalchemy.orm import Session
|
||||
|
||||
from app.api.deps import get_current_admin_api
|
||||
from app.core.database import get_db
|
||||
from app.modules.contracts.widgets import ListWidget
|
||||
from app.modules.contracts.widgets import BreakdownWidget, ListWidget
|
||||
from app.modules.core.schemas.dashboard import (
|
||||
AdminDashboardResponse,
|
||||
ImportStatsResponse,
|
||||
@@ -219,29 +219,37 @@ def get_comprehensive_stats(
|
||||
"/stats/marketplace", response_model=list[MarketplaceStatsResponse]
|
||||
)
|
||||
def get_marketplace_stats(
|
||||
request: Request,
|
||||
db: Session = Depends(get_db),
|
||||
current_admin: UserContext = Depends(get_current_admin_api),
|
||||
):
|
||||
"""Get statistics broken down by marketplace (Admin only)."""
|
||||
# For detailed marketplace breakdown, we still use the analytics service
|
||||
# as the MetricsProvider pattern is for aggregated stats
|
||||
try:
|
||||
from app.modules.analytics.services.stats_service import stats_service
|
||||
"""Get statistics broken down by marketplace (Admin only).
|
||||
|
||||
marketplace_stats = stats_service.get_marketplace_breakdown_stats(db=db)
|
||||
return [
|
||||
MarketplaceStatsResponse(
|
||||
marketplace=stat["marketplace"],
|
||||
total_products=stat["total_products"],
|
||||
unique_vendors=stat["unique_vendors"],
|
||||
unique_brands=stat["unique_brands"],
|
||||
)
|
||||
for stat in marketplace_stats
|
||||
]
|
||||
except ImportError:
|
||||
# Analytics module not available
|
||||
logger.warning("Analytics module not available for marketplace breakdown stats")
|
||||
return []
|
||||
Uses the widget_aggregator to get marketplace breakdown data from the
|
||||
marketplace module's WidgetProvider, avoiding cross-module violations.
|
||||
"""
|
||||
platform_id = _get_platform_id(request, current_admin)
|
||||
|
||||
# Get widgets from marketplace module via aggregator
|
||||
widgets = widget_aggregator.get_admin_dashboard_widgets(db=db, platform_id=platform_id)
|
||||
|
||||
# Extract marketplace breakdown widget
|
||||
marketplace_widgets = widgets.get("marketplace", [])
|
||||
for widget in marketplace_widgets:
|
||||
if widget.key == "marketplace.breakdown" and isinstance(widget.data, BreakdownWidget):
|
||||
# Transform breakdown items to MarketplaceStatsResponse
|
||||
return [
|
||||
MarketplaceStatsResponse(
|
||||
marketplace=item.label,
|
||||
total_products=int(item.value),
|
||||
unique_vendors=int(item.secondary_value or 0),
|
||||
unique_brands=0, # Not included in breakdown widget
|
||||
)
|
||||
for item in widget.data.items
|
||||
]
|
||||
|
||||
# No breakdown widget found
|
||||
return []
|
||||
|
||||
|
||||
@admin_dashboard_router.get("/stats/platform", response_model=PlatformStatsResponse)
|
||||
|
||||
@@ -20,8 +20,8 @@ from app.core.config import settings as app_settings
|
||||
from app.core.database import get_db
|
||||
from app.exceptions import ResourceNotFoundException
|
||||
from app.modules.tenancy.exceptions import ConfirmationRequiredException
|
||||
from app.modules.monitoring.services.admin_audit_service import admin_audit_service
|
||||
from app.modules.core.services.admin_settings_service import admin_settings_service
|
||||
from app.modules.core.services.audit_aggregator import audit_aggregator
|
||||
from models.schema.auth import UserContext
|
||||
from app.modules.tenancy.schemas.admin import (
|
||||
AdminSettingCreate,
|
||||
@@ -116,7 +116,7 @@ def create_setting(
|
||||
)
|
||||
|
||||
# Log action
|
||||
admin_audit_service.log_action(
|
||||
audit_aggregator.log(
|
||||
db=db,
|
||||
admin_user_id=current_admin.id,
|
||||
action="create_setting",
|
||||
@@ -147,7 +147,7 @@ def update_setting(
|
||||
)
|
||||
|
||||
# Log action
|
||||
admin_audit_service.log_action(
|
||||
audit_aggregator.log(
|
||||
db=db,
|
||||
admin_user_id=current_admin.id,
|
||||
action="update_setting",
|
||||
@@ -176,7 +176,7 @@ def upsert_setting(
|
||||
)
|
||||
|
||||
# Log action
|
||||
admin_audit_service.log_action(
|
||||
audit_aggregator.log(
|
||||
db=db,
|
||||
admin_user_id=current_admin.id,
|
||||
action="upsert_setting",
|
||||
@@ -233,7 +233,7 @@ def set_rows_per_page(
|
||||
db=db, setting_data=setting_data, admin_user_id=current_admin.id
|
||||
)
|
||||
|
||||
admin_audit_service.log_action(
|
||||
audit_aggregator.log(
|
||||
db=db,
|
||||
admin_user_id=current_admin.id,
|
||||
action="update_setting",
|
||||
@@ -288,7 +288,7 @@ def delete_setting(
|
||||
)
|
||||
|
||||
# Log action
|
||||
admin_audit_service.log_action(
|
||||
audit_aggregator.log(
|
||||
db=db,
|
||||
admin_user_id=current_admin.id,
|
||||
action="delete_setting",
|
||||
@@ -586,7 +586,7 @@ def update_email_settings(
|
||||
updated_keys.append(field)
|
||||
|
||||
# Log action
|
||||
admin_audit_service.log_action(
|
||||
audit_aggregator.log(
|
||||
db=db,
|
||||
admin_user_id=current_admin.id,
|
||||
action="update_email_settings",
|
||||
@@ -625,7 +625,7 @@ def reset_email_settings(
|
||||
deleted_count += 1
|
||||
|
||||
# Log action
|
||||
admin_audit_service.log_action(
|
||||
audit_aggregator.log(
|
||||
db=db,
|
||||
admin_user_id=current_admin.id,
|
||||
action="reset_email_settings",
|
||||
@@ -688,7 +688,7 @@ def send_test_email(
|
||||
# Check if email was actually sent (send_raw returns EmailLog, not boolean)
|
||||
if email_log.status == "sent":
|
||||
# Log action
|
||||
admin_audit_service.log_action(
|
||||
audit_aggregator.log(
|
||||
db=db,
|
||||
admin_user_id=current_admin.id,
|
||||
action="send_test_email",
|
||||
|
||||
216
app/modules/core/services/audit_aggregator.py
Normal file
216
app/modules/core/services/audit_aggregator.py
Normal file
@@ -0,0 +1,216 @@
|
||||
# app/modules/core/services/audit_aggregator.py
|
||||
"""
|
||||
Audit aggregator service for collecting audit providers from all modules.
|
||||
|
||||
This service lives in core because audit logging is infrastructure functionality
|
||||
that should be available across all modules. It discovers AuditProviders from
|
||||
enabled modules, providing a unified interface for audit logging.
|
||||
|
||||
Benefits:
|
||||
- Audit logging always works (aggregator is in core)
|
||||
- Each module can provide its own audit backend
|
||||
- Optional modules are truly optional (monitoring can be removed without breaking app)
|
||||
- Easy to add new audit backends (just implement AuditProviderProtocol in your module)
|
||||
|
||||
Usage:
|
||||
from app.modules.core.services.audit_aggregator import audit_aggregator
|
||||
from app.modules.contracts.audit import AuditEvent
|
||||
|
||||
# Log an admin action
|
||||
audit_aggregator.log_action(
|
||||
db=db,
|
||||
event=AuditEvent(
|
||||
admin_user_id=123,
|
||||
action="create_setting",
|
||||
target_type="setting",
|
||||
target_id="max_vendors",
|
||||
details={"category": "system"},
|
||||
)
|
||||
)
|
||||
|
||||
# Or use the convenience method with individual parameters
|
||||
audit_aggregator.log(
|
||||
db=db,
|
||||
admin_user_id=123,
|
||||
action="create_setting",
|
||||
target_type="setting",
|
||||
target_id="max_vendors",
|
||||
details={"category": "system"},
|
||||
)
|
||||
"""
|
||||
|
||||
import logging
|
||||
from typing import TYPE_CHECKING, Any
|
||||
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.modules.contracts.audit import (
|
||||
AuditEvent,
|
||||
AuditProviderProtocol,
|
||||
)
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from app.modules.base import ModuleDefinition
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class AuditAggregatorService:
|
||||
"""
|
||||
Aggregates audit providers from all modules.
|
||||
|
||||
This service discovers AuditProviders from enabled modules and provides
|
||||
a unified interface for audit logging. It handles graceful degradation
|
||||
when modules are disabled or providers fail.
|
||||
"""
|
||||
|
||||
def _get_enabled_providers(
|
||||
self, db: Session
|
||||
) -> list[tuple["ModuleDefinition", AuditProviderProtocol]]:
|
||||
"""
|
||||
Get audit providers from enabled modules.
|
||||
|
||||
Note: Unlike metrics/widget providers, audit logging doesn't filter
|
||||
by platform_id - it's a system-wide concern. We still need db to
|
||||
check if optional modules are enabled.
|
||||
|
||||
Args:
|
||||
db: Database session
|
||||
|
||||
Returns:
|
||||
List of (module, provider) tuples for enabled modules with providers
|
||||
"""
|
||||
from app.modules.registry import MODULES
|
||||
|
||||
providers: list[tuple[ModuleDefinition, AuditProviderProtocol]] = []
|
||||
|
||||
for module in MODULES.values():
|
||||
# Skip modules without audit providers
|
||||
if not module.has_audit_provider():
|
||||
continue
|
||||
|
||||
# Core and internal modules are always enabled
|
||||
# For optional modules, we don't strictly check enablement for audit
|
||||
# because audit logging is system infrastructure
|
||||
|
||||
# Get the provider instance
|
||||
try:
|
||||
provider = module.get_audit_provider_instance()
|
||||
if provider is not None:
|
||||
providers.append((module, provider))
|
||||
except Exception as e:
|
||||
logger.warning(
|
||||
f"Failed to get audit provider for module {module.code}: {e}"
|
||||
)
|
||||
|
||||
return providers
|
||||
|
||||
def log_action(self, db: Session, event: AuditEvent) -> bool:
|
||||
"""
|
||||
Log an audit event using all available providers.
|
||||
|
||||
Called by services/routes to record admin actions.
|
||||
Uses all available audit providers - if multiple are configured,
|
||||
all will receive the event (e.g., database + external service).
|
||||
|
||||
Args:
|
||||
db: Database session
|
||||
event: The audit event to log
|
||||
|
||||
Returns:
|
||||
True if at least one provider logged successfully, False otherwise
|
||||
"""
|
||||
providers = self._get_enabled_providers(db)
|
||||
|
||||
if not providers:
|
||||
# No audit providers available - this is acceptable
|
||||
logger.debug(
|
||||
f"No audit providers available for action {event.action} "
|
||||
f"on {event.target_type}:{event.target_id}"
|
||||
)
|
||||
return False
|
||||
|
||||
any_success = False
|
||||
for module, provider in providers:
|
||||
try:
|
||||
success = provider.log_action(db, event)
|
||||
if success:
|
||||
any_success = True
|
||||
logger.debug(
|
||||
f"Audit logged via {module.code}: {event.action} "
|
||||
f"on {event.target_type}:{event.target_id}"
|
||||
)
|
||||
else:
|
||||
logger.warning(
|
||||
f"Audit provider {module.code} failed to log {event.action}"
|
||||
)
|
||||
except Exception as e:
|
||||
logger.warning(
|
||||
f"Failed to log audit via {module.code}: {e}"
|
||||
)
|
||||
# Continue with other providers - graceful degradation
|
||||
|
||||
return any_success
|
||||
|
||||
def log(
|
||||
self,
|
||||
db: Session,
|
||||
admin_user_id: int,
|
||||
action: str,
|
||||
target_type: str,
|
||||
target_id: str,
|
||||
details: dict[str, Any] | None = None,
|
||||
ip_address: str | None = None,
|
||||
user_agent: str | None = None,
|
||||
request_id: str | None = None,
|
||||
) -> bool:
|
||||
"""
|
||||
Convenience method to log an audit event with individual parameters.
|
||||
|
||||
This is a shorthand for creating an AuditEvent and calling log_action.
|
||||
Useful for simple logging calls without explicitly constructing the event.
|
||||
|
||||
Args:
|
||||
db: Database session
|
||||
admin_user_id: ID of the admin performing the action
|
||||
action: Action performed (e.g., "create_vendor", "update_setting")
|
||||
target_type: Type of target (e.g., "vendor", "user", "setting")
|
||||
target_id: ID of the target entity (as string)
|
||||
details: Additional context about the action
|
||||
ip_address: IP address of the admin (optional)
|
||||
user_agent: User agent string (optional)
|
||||
request_id: Request ID for correlation (optional)
|
||||
|
||||
Returns:
|
||||
True if at least one provider logged successfully, False otherwise
|
||||
"""
|
||||
event = AuditEvent(
|
||||
admin_user_id=admin_user_id,
|
||||
action=action,
|
||||
target_type=target_type,
|
||||
target_id=str(target_id),
|
||||
details=details,
|
||||
ip_address=ip_address,
|
||||
user_agent=user_agent,
|
||||
request_id=request_id,
|
||||
)
|
||||
return self.log_action(db, event)
|
||||
|
||||
def get_available_backends(self, db: Session) -> list[str]:
|
||||
"""
|
||||
Get list of available audit backends.
|
||||
|
||||
Args:
|
||||
db: Database session
|
||||
|
||||
Returns:
|
||||
List of backend names from enabled providers
|
||||
"""
|
||||
providers = self._get_enabled_providers(db)
|
||||
return [provider.audit_backend for _, provider in providers]
|
||||
|
||||
|
||||
# Singleton instance
|
||||
audit_aggregator = AuditAggregatorService()
|
||||
|
||||
__all__ = ["AuditAggregatorService", "audit_aggregator"]
|
||||
Reference in New Issue
Block a user