# Common Parameters Refactoring Proposal for Aether API V3 ## 1. Problem Statement Currently, the `common_route_params` dependency in `app/lib_general.py` bundles many disparate concerns: - Account authentication/authorization context (`x_account_id`). - Pagination (`limit`, `offset`). - Status filtering (`enabled`, `hidden`). - Pydantic serialization options (`by_alias`, `exclude_unset`). - FastAPI internal (`response` object). - Request delay (`delay_ms`, `X-Delay-ms`). While functional, this monolithic dependency leads to: - **Lack of Granularity:** Every route that uses `common_route_params` inherits all its parameters, even if it only needs a subset. - **Reduced Clarity:** The purpose of each parameter is obscured within a single, large object. - **Limited Flexibility:** Difficult to customize or override specific common parameters without affecting others. - **Suboptimal OpenAPI Docs:** FastAPI generates a single, large set of parameters for `commons`, which can be less precise in the Swagger UI. ## 2. Proposed Solution: Granular Dependencies The proposal is to break down `common_route_params` into smaller, more focused FastAPI dependency injection functions and Pydantic models. This leverages FastAPI's powerful dependency injection system to achieve clarity, flexibility, and better OpenAPI documentation. All new models and dependency functions will reside in `app/lib_general.py`. ### A. `AccountContext` (Pydantic Model & Dependency Function) * **Purpose:** Encapsulates the resolved account context, including the integer `account_id` and the random `account_id_random` string. It also handles the authentication/authorization logic for `x_account_id`. * **Dependency Function:** `get_account_context()` **Proposed `app/lib_general.py` additions:** ```python # --- Pydantic Model for Account Context --- class AccountContext(BaseModel): account_id: int account_id_random: str # Add any other account-specific context needed here # --- Dependency Function for Account Context --- def get_account_context( x_account_id: Optional[str] = Header(None, min_length=11, max_length=22), x_no_account_id: Optional[str] = Header(None, min_length=11, max_length=22), x_no_account_id_token: Optional[str] = Query(None, min_length=11, max_length=22), # Changed from Header to Query for easier testing ) -> AccountContext: """ Resolves the account context from headers/query parameters. Raises HTTPException 403 if no valid account is found. """ log.setLevel(logging.DEBUG) # Adjust as needed log.debug(locals()) resolved_account_id = None resolved_account_id_random = None if x_account_id: resolved_account_id_random = x_account_id if resolved_account_id := redis_lookup_id_random(table_name='account', record_id_random=x_account_id): log.info(f'Found account from x-account-id: {resolved_account_id}') else: log.warning(f'Invalid x-account-id provided: {x_account_id}') raise HTTPException(status_code=403, detail='Invalid X-Account-ID.') elif x_no_account_id and len(x_no_account_id) >= 11: # User specified no account ID but might have a special bypass token/logic # USER INPUT REQUIRED: Clarify logic for x_no_account_id vs x_no_account_id_token # For now, treat as no specific account and potentially allow access based on other factors log.info(f'X-No-Account-ID header found: {x_no_account_id}. Proceeding without specific account context.') resolved_account_id = 0 # Or some other indicator for "no specific account" resolved_account_id_random = '--- NO ACCOUNT ---' elif x_no_account_id_token and len(x_no_account_id_token) >= 11: # USER INPUT REQUIRED: How should x_no_account_id_token be validated? # Current logic in common_route_params looks up account based on this token. resolved_account_id_random = x_no_account_id_token if resolved_account_id := redis_lookup_id_random(table_name='account', record_id_random=x_no_account_id_token): log.info(f'Found account from x_no_account_id_token: {resolved_account_id}') else: log.warning(f'Invalid x_no_account_id_token provided: {x_no_account_id_token}') raise HTTPException(status_code=403, detail='Invalid X-No-Account-ID-Token.') else: log.warning('No valid account context provided via X-Account-ID or X-No-Account-ID-Token.') raise HTTPException(status_code=403, detail='Account context required. Please provide X-Account-ID or X-No-Account-ID-Token.') return AccountContext(account_id=resolved_account_id, account_id_random=resolved_account_id_random) ``` --- ### B. `PaginationParams` (Pydantic Model & Dependency Function) * **Purpose:** Encapsulates pagination parameters (`limit`, `offset`). * **Dependency Function:** `get_pagination_params()` **Proposed `app/lib_general.py` additions:** ```python # --- Pydantic Model for Pagination --- class PaginationParams(BaseModel): limit: int = 100 # Default limit offset: int = 0 # --- Dependency Function for Pagination --- def get_pagination_params( limit: int = Query(100, ge=0, description="Maximum number of items to return"), offset: int = Query(0, ge=0, description="Number of items to skip (for pagination)"), ) -> PaginationParams: return PaginationParams(limit=limit, offset=offset) ``` --- ### C. `StatusFilterParams` (Pydantic Model & Dependency Function) * **Purpose:** Encapsulates status filtering parameters (`enabled`, `hidden`). * **Dependency Function:** `get_status_filter_params()` **Proposed `app/lib_general.py` additions:** ```python # --- Pydantic Model for Status Filtering --- class StatusFilterParams(BaseModel): enabled: str = 'enabled' # 'enabled', 'disabled', 'all' hidden: str = 'not_hidden' # 'hidden', 'not_hidden', 'all' # --- Dependency Function for Status Filtering --- def get_status_filter_params( enabled: str = Query('enabled', description="Filter by object enabled status ('enabled', 'disabled', 'all')"), hidden: str = Query('not_hidden', description="Filter by object hidden status ('hidden', 'not_hidden', 'all')"), ) -> StatusFilterParams: # USER INPUT REQUIRED: Should we validate 'enabled' and 'hidden' values here # (e.g., raise HTTP 400 if not 'enabled', 'disabled', 'all')? return StatusFilterParams(enabled=enabled, hidden=hidden) ``` --- ### D. `SerializationParams` (Pydantic Model & Dependency Function) * **Purpose:** Encapsulates Pydantic serialization options (`by_alias`, `exclude_unset`). * **Dependency Function:** `get_serialization_params()` **Proposed `app/lib_general.py` additions:** ```python # --- Pydantic Model for Serialization Options --- class SerializationParams(BaseModel): by_alias: bool = True exclude_unset: bool = False # --- Dependency Function for Serialization Options --- def get_serialization_params( by_alias: bool = Query(True, description="Whether to use field aliases for serialization"), exclude_unset: bool = Query(False, description="Whether to exclude unset fields from the response"), ) -> SerializationParams: return SerializationParams(by_alias=by_alias, exclude_unset=exclude_unset) ``` --- ### E. `DelayParams` (Pydantic Model & Dependency Function) * **Purpose:** Encapsulates the calculated `sleep_time` based on header/query delay parameters. * **Dependency Function:** `get_delay_params()` **Proposed `app/lib_general.py` additions:** ```python # --- Pydantic Model for Delay --- class DelayParams(BaseModel): sleep_time_ms: int = 0 # Raw delay value in ms sleep_time_s: float = 0.0 # Converted to seconds for time.sleep() # --- Dependency Function for Delay --- def get_delay_params( x_delay_ms: Optional[int] = Header(0, alias='X-Delay-ms', description="Delay response for X milliseconds (header)"), delay_ms: Optional[int] = Query(0, description="Delay response for X milliseconds (query parameter)"), ) -> DelayParams: calculated_delay_ms = max(x_delay_ms or 0, delay_ms or 0) return DelayParams(sleep_time_ms=calculated_delay_ms, sleep_time_s=calculated_delay_ms / 1000.0) ``` --- ### F. Refactoring `common_route_params` and `common_route_params_no_account_id` The existing `common_route_params` and `common_route_params_no_account_id` functions, along with their associated Pydantic models, would be deprecated or removed, as their functionality is now covered by these new granular dependencies. ## 3. How this affects route functions in `api_crud_v3.py` Route functions will now explicitly declare only the dependencies they need. This significantly cleans up the function signatures and makes their requirements explicit. **Example `get_obj_li` function signature update:** ```python # Old: # async def get_obj_li( # obj_type_l1: str, # for_obj_type: Optional[str] = None, # for_obj_id: Optional[str] = None, # hidden: str = 'not_hidden', # order_by_li: Optional[str] = None, # jp: Optional[Union[str, None]] = None, # x_delay_ms: Optional[int] = Header(0, alias='X-Delay-ms'), # delay_ms: Optional[int] = Query(0), # commons: Common_Route_Params = Depends(common_route_params), # ): # New (using new dependencies): from fastapi import Depends, Header, Query, HTTPException from pydantic import BaseModel from typing import Optional, Union # ... other typing imports (existing) import time # (existing) # ... (assuming new AccountContext, PaginationParams, etc. and their get_X_params functions are defined in lib_general) @router.get('/{obj_type_l1}/', response_model=Resp_Body_Base) async def get_obj_li( obj_type_l1: str, account_context: AccountContext = Depends(get_account_context), pagination: PaginationParams = Depends(get_pagination_params), status_filter: StatusFilterParams = Depends(get_status_filter_params), serialization: SerializationParams = Depends(get_serialization_params), delay_params: DelayParams = Depends(get_delay_params), for_obj_type: Optional[str] = Query(None), # Still needed for parent filtering for_obj_id: Optional[str] = Query(None), # Still needed for parent filtering order_by_li: Optional[str] = None, jp: Optional[Union[str, None]] = None, # The 'response: Response' object will be handled by FastAPI directly, or can be passed through a dependency if really needed. ): # Apply delay if delay_params.sleep_time_s > 0: time.sleep(delay_params.sleep_time_s) # Use parameters: # account_id = account_context.account_id # account_id_random = account_context.account_id_random # limit = pagination.limit # offset = pagination.offset # enabled_status = status_filter.enabled # hidden_status = status_filter.hidden # by_alias_option = serialization.by_alias # exclude_unset_option = serialization.exclude_unset # ... rest of your logic, now using these granular parameters ... ``` ## 4. User Input Required Please review this proposal. Specifically, I need your input on: 1. **Approval of the Overall Structure:** Do you agree with breaking down `common_route_params` into these more granular dependencies? 2. **`AccountContext` Logic:** * How should `x_no_account_id` be handled precisely? Is it just a flag to bypass `x_account_id` validation, or does it imply a specific "anonymous" account context? * How should `x_no_account_id_token` be validated? The current code looks up an account based on it, but is there a specific token validation mechanism beyond just lookup? Should it have a default value for unauthenticated use? 3. **`StatusFilterParams` Validation:** Should the `enabled` and `hidden` query parameters (e.g., `'enabled'`, `'disabled'`, `'all'`, `'hidden'`, `'not_hidden'`, `'all'`) be strictly validated within `get_status_filter_params` (e.g., raising `HTTPException 400` if an invalid string is provided)? Your feedback on these points is crucial before proceeding with implementation.