From 80bb4b296f6fd8266a21c31a2545d988c98ddab2 Mon Sep 17 00:00:00 2001 From: Scott Idem Date: Wed, 3 Dec 2025 20:47:47 -0500 Subject: [PATCH] Restored lost file --- .../COMMON_PARAMETERS_REFACTORING_PROPOSAL.md | 255 ++++++++++++++++++ 1 file changed, 255 insertions(+) create mode 100644 documentation/COMMON_PARAMETERS_REFACTORING_PROPOSAL.md diff --git a/documentation/COMMON_PARAMETERS_REFACTORING_PROPOSAL.md b/documentation/COMMON_PARAMETERS_REFACTORING_PROPOSAL.md new file mode 100644 index 0000000..a192bc1 --- /dev/null +++ b/documentation/COMMON_PARAMETERS_REFACTORING_PROPOSAL.md @@ -0,0 +1,255 @@ +# 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.