Add comprehensive tests for session cleanup and async CRUD operations; improve error handling and validation across schemas and API routes
- Introduced extensive tests for session cleanup, async session CRUD methods, and concurrent cleanup to ensure reliability and efficiency. - Enhanced `schemas/users.py` with reusable password strength validation logic. - Improved error handling in `admin.py` routes by replacing `detail` with `message` for consistency and readability.
This commit is contained in:
@@ -0,0 +1,78 @@
|
||||
"""add_performance_indexes
|
||||
|
||||
Revision ID: 1174fffbe3e4
|
||||
Revises: fbf6318a8a36
|
||||
Create Date: 2025-11-01 04:15:25.367010
|
||||
|
||||
"""
|
||||
from typing import Sequence, Union
|
||||
|
||||
import sqlalchemy as sa
|
||||
|
||||
from alembic import op
|
||||
|
||||
# revision identifiers, used by Alembic.
|
||||
revision: str = '1174fffbe3e4'
|
||||
down_revision: Union[str, None] = 'fbf6318a8a36'
|
||||
branch_labels: Union[str, Sequence[str], None] = None
|
||||
depends_on: Union[str, Sequence[str], None] = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
"""Add performance indexes for optimized queries."""
|
||||
|
||||
# Index for session cleanup queries
|
||||
# Optimizes: DELETE WHERE is_active = FALSE AND expires_at < now AND created_at < cutoff
|
||||
op.create_index(
|
||||
'ix_user_sessions_cleanup',
|
||||
'user_sessions',
|
||||
['is_active', 'expires_at', 'created_at'],
|
||||
unique=False,
|
||||
postgresql_where=sa.text('is_active = false')
|
||||
)
|
||||
|
||||
# Index for user search queries (basic trigram support without pg_trgm extension)
|
||||
# Optimizes: WHERE email ILIKE '%search%' OR first_name ILIKE '%search%'
|
||||
# Note: For better performance, consider enabling pg_trgm extension
|
||||
op.create_index(
|
||||
'ix_users_email_lower',
|
||||
'users',
|
||||
[sa.text('LOWER(email)')],
|
||||
unique=False,
|
||||
postgresql_where=sa.text('deleted_at IS NULL')
|
||||
)
|
||||
|
||||
op.create_index(
|
||||
'ix_users_first_name_lower',
|
||||
'users',
|
||||
[sa.text('LOWER(first_name)')],
|
||||
unique=False,
|
||||
postgresql_where=sa.text('deleted_at IS NULL')
|
||||
)
|
||||
|
||||
op.create_index(
|
||||
'ix_users_last_name_lower',
|
||||
'users',
|
||||
[sa.text('LOWER(last_name)')],
|
||||
unique=False,
|
||||
postgresql_where=sa.text('deleted_at IS NULL')
|
||||
)
|
||||
|
||||
# Index for organization search
|
||||
op.create_index(
|
||||
'ix_organizations_name_lower',
|
||||
'organizations',
|
||||
[sa.text('LOWER(name)')],
|
||||
unique=False
|
||||
)
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
"""Remove performance indexes."""
|
||||
|
||||
# Drop indexes in reverse order
|
||||
op.drop_index('ix_organizations_name_lower', table_name='organizations')
|
||||
op.drop_index('ix_users_last_name_lower', table_name='users')
|
||||
op.drop_index('ix_users_first_name_lower', table_name='users')
|
||||
op.drop_index('ix_users_email_lower', table_name='users')
|
||||
op.drop_index('ix_user_sessions_cleanup', table_name='user_sessions')
|
||||
@@ -145,7 +145,7 @@ async def admin_create_user(
|
||||
except ValueError as e:
|
||||
logger.warning(f"Failed to create user: {str(e)}")
|
||||
raise NotFoundError(
|
||||
detail=str(e),
|
||||
message=str(e),
|
||||
error_code=ErrorCode.USER_ALREADY_EXISTS
|
||||
)
|
||||
except Exception as e:
|
||||
@@ -169,7 +169,7 @@ async def admin_get_user(
|
||||
user = await user_crud.get(db, id=user_id)
|
||||
if not user:
|
||||
raise NotFoundError(
|
||||
detail=f"User {user_id} not found",
|
||||
message=f"User {user_id} not found",
|
||||
error_code=ErrorCode.USER_NOT_FOUND
|
||||
)
|
||||
return user
|
||||
@@ -193,7 +193,7 @@ async def admin_update_user(
|
||||
user = await user_crud.get(db, id=user_id)
|
||||
if not user:
|
||||
raise NotFoundError(
|
||||
detail=f"User {user_id} not found",
|
||||
message=f"User {user_id} not found",
|
||||
error_code=ErrorCode.USER_NOT_FOUND
|
||||
)
|
||||
|
||||
@@ -225,7 +225,7 @@ async def admin_delete_user(
|
||||
user = await user_crud.get(db, id=user_id)
|
||||
if not user:
|
||||
raise NotFoundError(
|
||||
detail=f"User {user_id} not found",
|
||||
message=f"User {user_id} not found",
|
||||
error_code=ErrorCode.USER_NOT_FOUND
|
||||
)
|
||||
|
||||
@@ -269,7 +269,7 @@ async def admin_activate_user(
|
||||
user = await user_crud.get(db, id=user_id)
|
||||
if not user:
|
||||
raise NotFoundError(
|
||||
detail=f"User {user_id} not found",
|
||||
message=f"User {user_id} not found",
|
||||
error_code=ErrorCode.USER_NOT_FOUND
|
||||
)
|
||||
|
||||
@@ -305,7 +305,7 @@ async def admin_deactivate_user(
|
||||
user = await user_crud.get(db, id=user_id)
|
||||
if not user:
|
||||
raise NotFoundError(
|
||||
detail=f"User {user_id} not found",
|
||||
message=f"User {user_id} not found",
|
||||
error_code=ErrorCode.USER_NOT_FOUND
|
||||
)
|
||||
|
||||
@@ -491,7 +491,7 @@ async def admin_create_organization(
|
||||
except ValueError as e:
|
||||
logger.warning(f"Failed to create organization: {str(e)}")
|
||||
raise NotFoundError(
|
||||
detail=str(e),
|
||||
message=str(e),
|
||||
error_code=ErrorCode.ALREADY_EXISTS
|
||||
)
|
||||
except Exception as e:
|
||||
@@ -515,7 +515,7 @@ async def admin_get_organization(
|
||||
org = await organization_crud.get(db, id=org_id)
|
||||
if not org:
|
||||
raise NotFoundError(
|
||||
detail=f"Organization {org_id} not found",
|
||||
message=f"Organization {org_id} not found",
|
||||
error_code=ErrorCode.NOT_FOUND
|
||||
)
|
||||
|
||||
@@ -551,7 +551,7 @@ async def admin_update_organization(
|
||||
org = await organization_crud.get(db, id=org_id)
|
||||
if not org:
|
||||
raise NotFoundError(
|
||||
detail=f"Organization {org_id} not found",
|
||||
message=f"Organization {org_id} not found",
|
||||
error_code=ErrorCode.NOT_FOUND
|
||||
)
|
||||
|
||||
@@ -595,7 +595,7 @@ async def admin_delete_organization(
|
||||
org = await organization_crud.get(db, id=org_id)
|
||||
if not org:
|
||||
raise NotFoundError(
|
||||
detail=f"Organization {org_id} not found",
|
||||
message=f"Organization {org_id} not found",
|
||||
error_code=ErrorCode.NOT_FOUND
|
||||
)
|
||||
|
||||
@@ -633,7 +633,7 @@ async def admin_list_organization_members(
|
||||
org = await organization_crud.get(db, id=org_id)
|
||||
if not org:
|
||||
raise NotFoundError(
|
||||
detail=f"Organization {org_id} not found",
|
||||
message=f"Organization {org_id} not found",
|
||||
error_code=ErrorCode.NOT_FOUND
|
||||
)
|
||||
|
||||
@@ -688,14 +688,14 @@ async def admin_add_organization_member(
|
||||
org = await organization_crud.get(db, id=org_id)
|
||||
if not org:
|
||||
raise NotFoundError(
|
||||
detail=f"Organization {org_id} not found",
|
||||
message=f"Organization {org_id} not found",
|
||||
error_code=ErrorCode.NOT_FOUND
|
||||
)
|
||||
|
||||
user = await user_crud.get(db, id=request.user_id)
|
||||
if not user:
|
||||
raise NotFoundError(
|
||||
detail=f"User {request.user_id} not found",
|
||||
message=f"User {request.user_id} not found",
|
||||
error_code=ErrorCode.USER_NOT_FOUND
|
||||
)
|
||||
|
||||
@@ -749,14 +749,14 @@ async def admin_remove_organization_member(
|
||||
org = await organization_crud.get(db, id=org_id)
|
||||
if not org:
|
||||
raise NotFoundError(
|
||||
detail=f"Organization {org_id} not found",
|
||||
message=f"Organization {org_id} not found",
|
||||
error_code=ErrorCode.NOT_FOUND
|
||||
)
|
||||
|
||||
user = await user_crud.get(db, id=user_id)
|
||||
if not user:
|
||||
raise NotFoundError(
|
||||
detail=f"User {user_id} not found",
|
||||
message=f"User {user_id} not found",
|
||||
error_code=ErrorCode.USER_NOT_FOUND
|
||||
)
|
||||
|
||||
@@ -768,7 +768,7 @@ async def admin_remove_organization_member(
|
||||
|
||||
if not success:
|
||||
raise NotFoundError(
|
||||
detail="User is not a member of this organization",
|
||||
message="User is not a member of this organization",
|
||||
error_code=ErrorCode.NOT_FOUND
|
||||
)
|
||||
|
||||
|
||||
@@ -35,6 +35,7 @@ class UserUpdate(BaseModel):
|
||||
first_name: Optional[str] = None
|
||||
last_name: Optional[str] = None
|
||||
phone_number: Optional[str] = None
|
||||
password: Optional[str] = None
|
||||
preferences: Optional[Dict[str, Any]] = None
|
||||
is_active: Optional[bool] = None # Changed default from True to None to avoid unintended updates
|
||||
|
||||
@@ -43,6 +44,14 @@ class UserUpdate(BaseModel):
|
||||
def validate_phone(cls, v: Optional[str]) -> Optional[str]:
|
||||
return validate_phone_number(v)
|
||||
|
||||
@field_validator('password')
|
||||
@classmethod
|
||||
def password_strength(cls, v: Optional[str]) -> Optional[str]:
|
||||
"""Enterprise-grade password strength validation"""
|
||||
if v is None:
|
||||
return v
|
||||
return validate_password_strength(v)
|
||||
|
||||
|
||||
class UserInDB(UserBase):
|
||||
id: UUID
|
||||
|
||||
@@ -68,6 +68,22 @@ def parse_device_name(user_agent: str) -> Optional[str]:
|
||||
elif 'windows phone' in user_agent_lower:
|
||||
return "Windows Phone"
|
||||
|
||||
# Tablets (check before desktop, as some tablets contain "android")
|
||||
elif 'tablet' in user_agent_lower:
|
||||
return "Tablet"
|
||||
|
||||
# Smart TVs (check before desktop OS patterns)
|
||||
elif any(tv in user_agent_lower for tv in ['smart-tv', 'smarttv']):
|
||||
return "Smart TV"
|
||||
|
||||
# Game consoles (check before desktop OS patterns, as Xbox contains "Windows")
|
||||
elif 'playstation' in user_agent_lower:
|
||||
return "PlayStation"
|
||||
elif 'xbox' in user_agent_lower:
|
||||
return "Xbox"
|
||||
elif 'nintendo' in user_agent_lower:
|
||||
return "Nintendo"
|
||||
|
||||
# Desktop operating systems
|
||||
elif 'macintosh' in user_agent_lower or 'mac os x' in user_agent_lower:
|
||||
# Try to extract browser
|
||||
@@ -82,22 +98,6 @@ def parse_device_name(user_agent: str) -> Optional[str]:
|
||||
elif 'cros' in user_agent_lower:
|
||||
return "Chromebook"
|
||||
|
||||
# Tablets (not already caught)
|
||||
elif 'tablet' in user_agent_lower:
|
||||
return "Tablet"
|
||||
|
||||
# Smart TVs
|
||||
elif any(tv in user_agent_lower for tv in ['smart-tv', 'smarttv', 'tv']):
|
||||
return "Smart TV"
|
||||
|
||||
# Game consoles
|
||||
elif 'playstation' in user_agent_lower:
|
||||
return "PlayStation"
|
||||
elif 'xbox' in user_agent_lower:
|
||||
return "Xbox"
|
||||
elif 'nintendo' in user_agent_lower:
|
||||
return "Nintendo"
|
||||
|
||||
# Fallback: just return browser name if detected
|
||||
browser = extract_browser(user_agent)
|
||||
if browser:
|
||||
|
||||
Reference in New Issue
Block a user