12 KiB
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 (
responseobject). - Request delay (
delay_ms,X-Delay-ms).
While functional, this monolithic dependency leads to:
- Lack of Granularity: Every route that uses
common_route_paramsinherits 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_idand the randomaccount_id_randomstring. It also handles the authentication/authorization logic forx_account_id. - Dependency Function:
get_account_context()
Proposed app/lib_general.py additions:
# --- 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:
# --- 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:
# --- 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:
# --- 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_timebased on header/query delay parameters. - Dependency Function:
get_delay_params()
Proposed app/lib_general.py additions:
# --- 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:
# 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:
- Approval of the Overall Structure: Do you agree with breaking down
common_route_paramsinto these more granular dependencies? AccountContextLogic:- How should
x_no_account_idbe handled precisely? Is it just a flag to bypassx_account_idvalidation, or does it imply a specific "anonymous" account context? - How should
x_no_account_id_tokenbe 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?
- How should
StatusFilterParamsValidation: Should theenabledandhiddenquery parameters (e.g.,'enabled','disabled','all','hidden','not_hidden','all') be strictly validated withinget_status_filter_params(e.g., raisingHTTPException 400if an invalid string is provided)?
Your feedback on these points is crucial before proceeding with implementation.