From 67df6053afe6d7c5a136d65cc9b3c9491c4f5958 Mon Sep 17 00:00:00 2001 From: CIzz22 Date: Wed, 18 Feb 2026 08:42:48 +0000 Subject: [PATCH 1/4] Update 'Jenkinsfile' --- Jenkinsfile | 137 ++++++++++++++++++++++------------------------------ 1 file changed, 58 insertions(+), 79 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 709e03c..e379513 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -1,117 +1,96 @@ pipeline { agent any - + environment { - // Replace with your Docker Hub username/organization DOCKER_HUB_USERNAME = 'aimodocker' - // Use credentials for Docker Hub - DOCKER_CREDENTIALS = credentials('aimodocker') - // Replace with your image name + // This creates DOCKER_AUTH_USR and DOCKER_AUTH_PSW + DOCKER_AUTH = credentials('aimodocker') IMAGE_NAME = 'oh-service' - // Replace with your docker compose service name - SERVICE_NAME = 'oh-app' - // Variable for Git commit hash - GIT_COMMIT_HASH = '' + SERVICE_NAME = 'ahm-app' + + SECURITY_PREFIX = 'security' - // Replace with the SSH credentials for development server - // SSH_CREDENTIALS = credentials('backend-server-digitaltwin') - // SSH_CREDENTIALS_USR = 'aimo' - // SSH_SERVER_IP = '192.168.1.82' + // Initialize variables to be updated in script blocks + GIT_COMMIT_HASH = "" + IMAGE_TAG = "" + SECONDARY_TAG = "" } - + stages { - stage('Checkout') { + stage('Checkout & Setup') { steps { script { - // Checkout and get git commit hash checkout scm - def commitHash = sh(script: 'git rev-parse --short HEAD', returnStdout: true).trim() - GIT_COMMIT_HASH = commitHash - echo "Git commit hash: ${GIT_COMMIT_HASH}" + GIT_COMMIT_HASH = sh(script: 'git rev-parse --short HEAD', returnStdout: true).trim() + + // Use env.BRANCH_NAME or logic to handle detached HEAD if necessary + def branch = env.BRANCH_NAME ?: 'unknown' + echo "Current Branch: ${branch}" + + if (branch == 'main') { + IMAGE_TAG = GIT_COMMIT_HASH + SECONDARY_TAG = 'latest' + } else if (branch == 'oh_security') { + IMAGE_TAG = "${SECURITY_PREFIX}-${GIT_COMMIT_HASH}" + SECONDARY_TAG = "${SECURITY_PREFIX}-latest" + } else { + IMAGE_TAG = "temp-${GIT_COMMIT_HASH}" + SECONDARY_TAG = "" // Ensure it's empty for other branches + } + + echo "Primary Tag: ${IMAGE_TAG}" } } } - + stage('Docker Login') { steps { - sh ''' - echo ${DOCKER_CREDENTIALS_PSW} | docker login -u ${DOCKER_CREDENTIALS_USR} --password-stdin - ''' + // Fixed variable names based on the 'DOCKER_AUTH' environment key + sh "echo ${DOCKER_AUTH_PSW} | docker login -u ${DOCKER_AUTH_USR} --password-stdin" } } - - stage('Build Docker Image') { + + stage('Build & Tag') { steps { script { - // Build with commit hash tag - sh """ - docker build -t ${DOCKER_HUB_USERNAME}/${IMAGE_NAME}:latest . - docker tag ${DOCKER_HUB_USERNAME}/${IMAGE_NAME}:latest ${DOCKER_HUB_USERNAME}/${IMAGE_NAME}:${GIT_COMMIT_HASH} - """ + def fullImageName = "${DOCKER_HUB_USERNAME}/${IMAGE_NAME}" + sh "docker build -t ${fullImageName}:${IMAGE_TAG} ." + + if (SECONDARY_TAG) { + sh "docker tag ${fullImageName}:${IMAGE_TAG} ${fullImageName}:${SECONDARY_TAG}" + } } } } - + stage('Push to Docker Hub') { steps { - sh """ - # Push both tags - docker push ${DOCKER_HUB_USERNAME}/${IMAGE_NAME}:${GIT_COMMIT_HASH} - docker push ${DOCKER_HUB_USERNAME}/${IMAGE_NAME}:latest - """ + script { + def fullImageName = "${DOCKER_HUB_USERNAME}/${IMAGE_NAME}" + sh "docker push ${fullImageName}:${IMAGE_TAG}" + + if (SECONDARY_TAG) { + sh "docker push ${fullImageName}:${SECONDARY_TAG}" + } + } } } - - // stage('Watchtower Deployment') { - // steps { - // sh """ - // # Push both tags - // docker push ${DOCKER_HUB_USERNAME}/${IMAGE_NAME}:${GIT_COMMIT_HASH} - // docker push ${DOCKER_HUB_USERNAME}/${IMAGE_NAME}:latest - // """ - // } - // } - - // stage('Deploy') { - // steps { - // script { - // sshagent(credentials: ['backend-server-digitaltwin']) { - // sh """ - // ssh -o StrictHostKeyChecking=no -p 12558 aimo@0.tcp.ap.ngrok.io ' - // cd ~/digital-twin/Docker - // sudo docker compose pull ${SERVICE_NAME} - // sudo docker compose up -d ${SERVICE_NAME} - // ' - // """ - // } - // } - // } - // } } - + post { always { - // Clean up - sh 'docker logout' - - // Clean up local images script { - try { - sh """ - # Push both tags - docker rmi ${DOCKER_HUB_USERNAME}/${IMAGE_NAME}:${GIT_COMMIT_HASH} - docker rmi ${DOCKER_HUB_USERNAME}/${IMAGE_NAME}:latest - """ - } catch (err) { - echo "Failed to clean up images: ${err}" + sh 'docker logout' + def fullImageName = "${DOCKER_HUB_USERNAME}/${IMAGE_NAME}" + // Clean up images to save agent disk space + sh "docker rmi ${fullImageName}:${IMAGE_TAG} || true" + if (SECONDARY_TAG) { + sh "docker rmi ${fullImageName}:${SECONDARY_TAG} || true" } } } success { - echo "Successfully built, pushed, and deployed Docker image with tags: latest and ${GIT_COMMIT_HASH}" - } - failure { - echo 'Failed to build/push/deploy Docker image!' + echo "Successfully processed ${env.BRANCH_NAME}." } } } \ No newline at end of file From 5084362f18150982e80d0be90a408394a97ac28b Mon Sep 17 00:00:00 2001 From: Cizz22 Date: Thu, 12 Feb 2026 16:54:04 +0700 Subject: [PATCH 2/4] refactor: Improve exception handling with structured logging, add specific validation error handling, and silence uvicorn access logs. --- src/exceptions.py | 73 ++++++++++++++++++++++++++++++++++++----------- src/logging.py | 3 ++ src/main.py | 34 +++++++++++++++++----- 3 files changed, 87 insertions(+), 23 deletions(-) diff --git a/src/exceptions.py b/src/exceptions.py index 6db15c9..bd74c4a 100644 --- a/src/exceptions.py +++ b/src/exceptions.py @@ -15,6 +15,9 @@ from sqlalchemy.exc import (DataError, DBAPIError, IntegrityError, from src.enums import ResponseStatus +from starlette.exceptions import HTTPException as StarletteHTTPException + +log = logging.getLogger(__name__) class ErrorDetail(BaseModel): field: Optional[str] = None @@ -57,7 +60,7 @@ def get_request_context(request: Request): return { "endpoint": request.url.path, - "url": request.url, + "url": str(request.url), "method": request.method, "remote_addr": get_client_ip(), } @@ -71,7 +74,6 @@ def handle_sqlalchemy_error(error: SQLAlchemyError): original_error = error.orig except AttributeError: original_error = None - print(original_error) if isinstance(error, IntegrityError): if "unique constraint" in str(error).lower(): @@ -95,7 +97,7 @@ def handle_sqlalchemy_error(error: SQLAlchemyError): return "Database error.", 500 else: # Log the full error for debugging purposes - logging.error(f"Unexpected database error: {str(error)}") + log.error(f"Unexpected database error: {str(error)}") return "An unexpected database error occurred.", 500 @@ -106,28 +108,62 @@ def handle_exception(request: Request, exc: Exception): request_info = get_request_context(request) if isinstance(exc, RateLimitExceeded): - _rate_limit_exceeded_handler(request, exc) - if isinstance(exc, HTTPException): - logging.error( - f"HTTP exception | Code: {exc.status_code} | Error: {exc.detail} | Request: {request_info}", - extra={"error_category": "http"}, + return _rate_limit_exceeded_handler(request, exc) + + if isinstance(exc, RequestValidationError): + log.error( + "Validation error occurred", + extra={ + "error_category": "validation", + "errors": exc.errors(), + "request": request_info, + }, + ) + return JSONResponse( + status_code=422, + content={ + "data": None, + "message": "Validation Error", + "status": ResponseStatus.ERROR, + "errors": exc.errors(), + }, + ) + + if isinstance(exc, (HTTPException, StarletteHTTPException)): + log.error( + "HTTP exception occurred", + extra={ + "error_category": "http", + "status_code": exc.status_code, + "detail": exc.detail if hasattr(exc, "detail") else str(exc), + "request": request_info, + }, ) return JSONResponse( status_code=exc.status_code, content={ "data": None, - "message": str(exc.detail), + "message": str(exc.detail) if hasattr(exc, "detail") else str(exc), "status": ResponseStatus.ERROR, - "errors": [ErrorDetail(message=str(exc.detail)).model_dump()], + "errors": [ + ErrorDetail( + message=str(exc.detail) if hasattr(exc, "detail") else str(exc) + ).model_dump() + ], }, ) if isinstance(exc, SQLAlchemyError): error_message, status_code = handle_sqlalchemy_error(exc) - logging.error( - f"Database Error | Error: {str(error_message)} | Request: {request_info}", - extra={"error_category": "database"}, + log.error( + "Database error occurred", + extra={ + "error_category": "database", + "error_message": error_message, + "request": request_info, + "exception": str(exc), + }, ) return JSONResponse( @@ -141,9 +177,14 @@ def handle_exception(request: Request, exc: Exception): ) # Log unexpected errors - logging.error( - f"Unexpected Error | Error: {str(exc)} | Request: {request_info}", - extra={"error_category": "unexpected"}, + log.error( + "Unexpected error occurred", + extra={ + "error_category": "unexpected", + "error_message": str(exc), + "request": request_info, + }, + exc_info=True, ) return JSONResponse( diff --git a/src/logging.py b/src/logging.py index bdf3731..32d9316 100644 --- a/src/logging.py +++ b/src/logging.py @@ -123,5 +123,8 @@ def configure_logging(): logger.handlers = [] logger.propagate = True + # Silence the chatty uvicorn access logs as we have custom middleware logging + logging.getLogger("uvicorn.access").setLevel(logging.WARNING) + # sometimes the slack client can be too verbose logging.getLogger("slack_sdk.web.base_client").setLevel(logging.CRITICAL) \ No newline at end of file diff --git a/src/main.py b/src/main.py index 9eaabcb..e1225a7 100644 --- a/src/main.py +++ b/src/main.py @@ -30,24 +30,30 @@ from src.exceptions import handle_exception from src.logging import configure_logging from src.middleware import RequestValidationMiddleware from src.rate_limiter import limiter +from fastapi.exceptions import RequestValidationError +from starlette.exceptions import HTTPException as StarletteHTTPException + log = logging.getLogger(__name__) # we configure the logging level and format configure_logging() -# we define the exception handlers -exception_handlers = {Exception: handle_exception} - # we create the ASGI for the app app = FastAPI( - exception_handlers=exception_handlers, openapi_url="", title="LCCA API", description="Welcome to LCCA's API documentation!", version="0.1.0", ) -app.state.limiter = limiter + +# we define the exception handlers +app.add_exception_handler(Exception, handle_exception) +app.add_exception_handler(HTTPException, handle_exception) +app.add_exception_handler(StarletteHTTPException, handle_exception) +app.add_exception_handler(RequestValidationError, handle_exception) app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler) + +app.state.limiter = limiter app.add_middleware(GZipMiddleware, minimum_size=1000) # credentials: "include", @@ -142,10 +148,24 @@ async def db_session_middleware(request: Request, call_next): process_time = (time.time() - start_time) * 1000 log.info( - f"Request: {request.method} {request.url.path} Status: {response.status_code} Duration: {process_time:.2f}ms" + "Request finished", + extra={ + "method": request.method, + "path": request.url.path, + "status_code": response.status_code, + "duration_ms": round(process_time, 2), + }, ) except Exception as e: - log.error(f"Request failed: {request.method} {request.url.path} Error: {str(e)}") + log.error( + "Request failed", + extra={ + "method": request.method, + "path": request.url.path, + "error": str(e), + }, + exc_info=True, + ) raise e from None finally: await request.state.db.close() From 4f5c95029db55e812ea1a37d92c5f01a74de1164 Mon Sep 17 00:00:00 2001 From: Cizz22 Date: Mon, 23 Feb 2026 12:32:35 +0700 Subject: [PATCH 3/4] feat: Introduce user context and unique error IDs for enhanced logging and error traceability. --- .env | 2 +- src/auth/service.py | 11 ++++++++ src/config.py | 2 +- src/context.py | 28 ++++++++++++++++++-- src/contribution_util.py | 12 ++++----- src/exceptions.py | 57 +++++++++++++++++++++++++++------------- src/logging.py | 43 ++++++++++++++++++++---------- src/main.py | 51 ++++++++++++++++++++++++++++++++--- 8 files changed, 161 insertions(+), 45 deletions(-) diff --git a/.env b/.env index 3ba6479..6a77a29 100644 --- a/.env +++ b/.env @@ -1,5 +1,5 @@ ENV=development -LOG_LEVEL=ERROR +LOG_LEVEL=INFO PORT=3021 HOST=0.0.0.0 diff --git a/src/auth/service.py b/src/auth/service.py index 82516f6..9bb8347 100644 --- a/src/auth/service.py +++ b/src/auth/service.py @@ -44,6 +44,17 @@ class JWTBearer(HTTPBearer): ) request.state.user = message + + from src.context import set_user_id, set_username, set_role + if hasattr(message, "user_id"): + set_user_id(str(message.user_id)) + if hasattr(message, "username"): + set_username(message.username) + elif hasattr(message, "name"): + set_username(message.name) + if hasattr(message, "role"): + set_role(message.role) + return message else: raise HTTPException(status_code=403, detail="Invalid authorization code.") diff --git a/src/config.py b/src/config.py index d7c92b7..4015d58 100644 --- a/src/config.py +++ b/src/config.py @@ -45,7 +45,7 @@ def get_config(): config = get_config() -LOG_LEVEL = config("LOG_LEVEL", default=logging.WARNING) +LOG_LEVEL = config("LOG_LEVEL", default="INFO") ENV = config("ENV", default="local") PORT = config("PORT", cast=int, default=8000) HOST = config("HOST", default="localhost") diff --git a/src/context.py b/src/context.py index 4c968a2..779a3e4 100644 --- a/src/context.py +++ b/src/context.py @@ -2,8 +2,14 @@ from contextvars import ContextVar from typing import Optional, Final REQUEST_ID_CTX_KEY: Final[str] = "request_id" -_request_id_ctx_var: ContextVar[Optional[str]] = ContextVar( - REQUEST_ID_CTX_KEY, default=None) +USER_ID_CTX_KEY: Final[str] = "user_id" +USERNAME_CTX_KEY: Final[str] = "username" +ROLE_CTX_KEY: Final[str] = "role" + +_request_id_ctx_var: ContextVar[Optional[str]] = ContextVar(REQUEST_ID_CTX_KEY, default=None) +_user_id_ctx_var: ContextVar[Optional[str]] = ContextVar(USER_ID_CTX_KEY, default=None) +_username_ctx_var: ContextVar[Optional[str]] = ContextVar(USERNAME_CTX_KEY, default=None) +_role_ctx_var: ContextVar[Optional[str]] = ContextVar(ROLE_CTX_KEY, default=None) def get_request_id() -> Optional[str]: @@ -16,3 +22,21 @@ def set_request_id(request_id: str): def reset_request_id(token): _request_id_ctx_var.reset(token) + +def get_user_id() -> Optional[str]: + return _user_id_ctx_var.get() + +def set_user_id(user_id: str): + return _user_id_ctx_var.set(user_id) + +def get_username() -> Optional[str]: + return _username_ctx_var.get() + +def set_username(username: str): + return _username_ctx_var.set(username) + +def get_role() -> Optional[str]: + return _role_ctx_var.get() + +def set_role(role: str): + return _role_ctx_var.set(role) diff --git a/src/contribution_util.py b/src/contribution_util.py index 84644d7..b77a6f2 100644 --- a/src/contribution_util.py +++ b/src/contribution_util.py @@ -256,12 +256,12 @@ def calculate_contribution_accurate(availabilities: Dict[str, float], structure_ key=lambda x: x[1]['birnbaum_importance'], reverse=True) - print("\n=== COMPONENT IMPORTANCE ANALYSIS ===") - print(f"System Availability: {system_info['system_availability']:.6f} ({system_info['system_availability']*100:.4f}%)") - print(f"System Unavailability: {system_info['system_unavailability']:.6f}") - print("\nComponent Rankings (by Birnbaum Importance):") - print(f"{'Component':<20} {'Availability':<12} {'Birnbaum':<12} {'Criticality':<12} {'F-V':<12} {'Contribution%':<12}") - print("-" * 92) + # print("\n=== COMPONENT IMPORTANCE ANALYSIS ===") + # print(f"System Availability: {system_info['system_availability']:.6f} ({system_info['system_availability']*100:.4f}%)") + # print(f"System Unavailability: {system_info['system_unavailability']:.6f}") + # print("\nComponent Rankings (by Birnbaum Importance):") + # print(f"{'Component':<20} {'Availability':<12} {'Birnbaum':<12} {'Criticality':<12} {'F-V':<12} {'Contribution%':<12}") + # print("-" * 92) for component, measures in sorted_components: print(f"{component:<20} {measures['component_availability']:<12.6f} " diff --git a/src/exceptions.py b/src/exceptions.py index bd74c4a..593b089 100644 --- a/src/exceptions.py +++ b/src/exceptions.py @@ -105,15 +105,38 @@ def handle_exception(request: Request, exc: Exception): """ Global exception handler for Fastapi application. """ + import uuid + error_id = str(uuid.uuid1()) request_info = get_request_context(request) + + # Store error_id in request.state for middleware/logging + request.state.error_id = error_id if isinstance(exc, RateLimitExceeded): - return _rate_limit_exceeded_handler(request, exc) + log.warning( + f"Rate limit exceeded | Error ID: {error_id}", + extra={ + "error_id": error_id, + "error_category": "rate_limit", + "request": request_info, + "detail": str(exc.description) if hasattr(exc, "description") else str(exc), + }, + ) + return JSONResponse( + status_code=429, + content={ + "data": None, + "message": "Rate limit exceeded", + "status": ResponseStatus.ERROR, + "error_id": error_id + } + ) if isinstance(exc, RequestValidationError): - log.error( - "Validation error occurred", + log.warning( + f"Validation error occurred | Error ID: {error_id}", extra={ + "error_id": error_id, "error_category": "validation", "errors": exc.errors(), "request": request_info, @@ -122,17 +145,18 @@ def handle_exception(request: Request, exc: Exception): return JSONResponse( status_code=422, content={ - "data": None, + "data": exc.errors(), "message": "Validation Error", "status": ResponseStatus.ERROR, - "errors": exc.errors(), + "error_id": error_id }, ) if isinstance(exc, (HTTPException, StarletteHTTPException)): log.error( - "HTTP exception occurred", + f"HTTP exception occurred | Error ID: {error_id}", extra={ + "error_id": error_id, "error_category": "http", "status_code": exc.status_code, "detail": exc.detail if hasattr(exc, "detail") else str(exc), @@ -146,19 +170,16 @@ def handle_exception(request: Request, exc: Exception): "data": None, "message": str(exc.detail) if hasattr(exc, "detail") else str(exc), "status": ResponseStatus.ERROR, - "errors": [ - ErrorDetail( - message=str(exc.detail) if hasattr(exc, "detail") else str(exc) - ).model_dump() - ], + "error_id": error_id }, ) if isinstance(exc, SQLAlchemyError): error_message, status_code = handle_sqlalchemy_error(exc) log.error( - "Database error occurred", + f"Database error occurred | Error ID: {error_id}", extra={ + "error_id": error_id, "error_category": "database", "error_message": error_message, "request": request_info, @@ -172,14 +193,15 @@ def handle_exception(request: Request, exc: Exception): "data": None, "message": error_message, "status": ResponseStatus.ERROR, - "errors": [ErrorDetail(message=error_message).model_dump()], + "error_id": error_id }, ) # Log unexpected errors log.error( - "Unexpected error occurred", + f"Unexpected error occurred | Error ID: {error_id}", extra={ + "error_id": error_id, "error_category": "unexpected", "error_message": str(exc), "request": request_info, @@ -191,10 +213,9 @@ def handle_exception(request: Request, exc: Exception): status_code=500, content={ "data": None, - "message": str(exc), + "message": "An unexpected error occurred", "status": ResponseStatus.ERROR, - "errors": [ - ErrorDetail(message="An unexpected error occurred.").model_dump() - ], + "error_id": error_id }, ) + diff --git a/src/logging.py b/src/logging.py index 32d9316..7266016 100644 --- a/src/logging.py +++ b/src/logging.py @@ -35,29 +35,45 @@ class JSONFormatter(logging.Formatter): Custom formatter to output logs in JSON format. """ def format(self, record): - from src.context import get_request_id - + from src.context import get_request_id, get_user_id, get_username, get_role request_id = None + user_id = None + username = None + role = None + try: request_id = get_request_id() + user_id = get_user_id() + username = get_username() + role = get_role() except Exception: pass + # Standard fields from requirements log_record = { - "timestamp": datetime.datetime.fromtimestamp(record.created).astimezone().isoformat(), + "timestamp": datetime.datetime.fromtimestamp(record.created).strftime("%Y-%m-%d %H:%M:%S"), "level": record.levelname, + "name": record.name, "message": record.getMessage(), - "logger_name": record.name, - "location": f"{record.module}:{record.funcName}:{record.lineno}", - "module": record.module, - "funcName": record.funcName, - "lineno": record.lineno, - "pid": os.getpid(), - "request_id": request_id, } - + # Add Context information if available + if user_id: + log_record["user_id"] = user_id + if username: + log_record["username"] = username + if role: + log_record["role"] = role + if request_id: + log_record["request_id"] = request_id + + # Add Error context if available + if hasattr(record, "error_id"): + log_record["error_id"] = record.error_id + elif "error_id" in record.__dict__: + log_record["error_id"] = record.error_id + # Capture exception info if available if record.exc_info: log_record["exception"] = self.formatException(record.exc_info) @@ -67,15 +83,14 @@ class JSONFormatter(logging.Formatter): log_record["stack_trace"] = self.formatStack(record.stack_info) # Add any extra attributes passed to the log call - # We skip standard attributes to avoid duplication standard_attrs = { "args", "asctime", "created", "exc_info", "exc_text", "filename", "funcName", "levelname", "levelno", "lineno", "module", "msecs", "message", "msg", "name", "pathname", "process", "processName", - "relativeCreated", "stack_info", "thread", "threadName" + "relativeCreated", "stack_info", "thread", "threadName", "error_id" } for key, value in record.__dict__.items(): - if key not in standard_attrs: + if key not in standard_attrs and not key.startswith("_"): log_record[key] = value log_json = json.dumps(log_record) diff --git a/src/main.py b/src/main.py index e1225a7..d6110cf 100644 --- a/src/main.py +++ b/src/main.py @@ -51,7 +51,7 @@ app.add_exception_handler(Exception, handle_exception) app.add_exception_handler(HTTPException, handle_exception) app.add_exception_handler(StarletteHTTPException, handle_exception) app.add_exception_handler(RequestValidationError, handle_exception) -app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler) +app.add_exception_handler(RateLimitExceeded, handle_exception) app.state.limiter = limiter app.add_middleware(GZipMiddleware, minimum_size=1000) @@ -147,22 +147,67 @@ async def db_session_middleware(request: Request, call_next): response = await call_next(request) process_time = (time.time() - start_time) * 1000 + from src.context import get_username, get_role, get_user_id, set_user_id, set_username, set_role + + # Pull from context or fallback to request.state.user + username = get_username() + role = get_role() + user_id = get_user_id() + + user_obj = getattr(request.state, "user", None) + if user_obj: + # message is UserBase dict/obj in this project + if isinstance(user_obj, dict): + u_id = user_obj.get("user_id") + u_name = user_obj.get("name") or user_obj.get("username") + u_role = user_obj.get("role") + else: + u_id = getattr(user_obj, "user_id", None) + u_name = getattr(user_obj, "name", None) or getattr(user_obj, "username", None) + u_role = getattr(user_obj, "role", None) + + if not user_id and u_id: + user_id = str(u_id) + set_user_id(user_id) + if not username and u_name: + username = u_name + set_username(username) + if not role and u_role: + role = u_role + set_role(role) + + user_info_str = "" + if username: + user_info_str = f" | User: {username}" + if role: + user_info_str += f" ({role})" + log.info( - "Request finished", + f"HTTP {request.method} {request.url.path} completed in {round(process_time, 2)}ms{user_info_str}", extra={ "method": request.method, "path": request.url.path, "status_code": response.status_code, "duration_ms": round(process_time, 2), + "user_id": user_id, + "role": role, }, ) except Exception as e: + # Generate an error_id here if it hasn't been generated yet + error_id = getattr(request.state, "error_id", None) + if not error_id: + import uuid + error_id = str(uuid.uuid1()) + request.state.error_id = error_id + log.error( - "Request failed", + f"Request failed | Error ID: {error_id}", extra={ "method": request.method, "path": request.url.path, "error": str(e), + "error_id": error_id, }, exc_info=True, ) From b7ad1c9a6bf54281efb03962b398ca5a47a3d96e Mon Sep 17 00:00:00 2001 From: Cizz22 Date: Tue, 24 Feb 2026 11:21:19 +0700 Subject: [PATCH 4/4] refactor: Overhaul testing framework with extensive unit tests, pytest configuration, and improved schema validation. --- poetry.lock | 40 +++++++- pyproject.toml | 2 + pytest.ini | 6 ++ src/database/schema.py | 2 +- src/middleware.py | 57 ++++++++++- src/utils.py | 5 +- tests/conftest.py | 106 ++++++++++---------- tests/database.py | 3 - tests/factories.py | 28 ------ tests/unit/test_budget_constrains.py | 44 ++++++++ tests/unit/test_context.py | 14 +++ tests/unit/test_contribution_util.py | 53 ++++++++++ tests/unit/test_exceptions.py | 31 ++++++ tests/unit/test_middleware_dispatch.py | 56 +++++++++++ tests/unit/test_reliability_calc.py | 64 ++++++++++++ tests/unit/test_schemas.py | 49 +++++++++ tests/unit/test_security_middleware.py | 58 +++++++++++ tests/unit/test_target_reliability_utils.py | 33 ++++++ tests/unit/test_utils.py | 36 +++++++ 19 files changed, 596 insertions(+), 91 deletions(-) create mode 100644 pytest.ini delete mode 100644 tests/database.py delete mode 100644 tests/factories.py create mode 100644 tests/unit/test_budget_constrains.py create mode 100644 tests/unit/test_context.py create mode 100644 tests/unit/test_contribution_util.py create mode 100644 tests/unit/test_exceptions.py create mode 100644 tests/unit/test_middleware_dispatch.py create mode 100644 tests/unit/test_reliability_calc.py create mode 100644 tests/unit/test_schemas.py create mode 100644 tests/unit/test_security_middleware.py create mode 100644 tests/unit/test_target_reliability_utils.py create mode 100644 tests/unit/test_utils.py diff --git a/poetry.lock b/poetry.lock index 73f375d..6dd16be 100644 --- a/poetry.lock +++ b/poetry.lock @@ -148,6 +148,25 @@ files = [ frozenlist = ">=1.1.0" typing-extensions = {version = ">=4.2", markers = "python_version < \"3.13\""} +[[package]] +name = "aiosqlite" +version = "0.20.0" +description = "asyncio bridge to the standard sqlite3 module" +optional = false +python-versions = ">=3.8" +groups = ["main"] +files = [ + {file = "aiosqlite-0.20.0-py3-none-any.whl", hash = "sha256:36a1deaca0cac40ebe32aac9977a6e2bbc7f5189f23f4a54d5908986729e5bd6"}, + {file = "aiosqlite-0.20.0.tar.gz", hash = "sha256:6d35c8c256637f4672f843c31021464090805bf925385ac39473fb16eaaca3d7"}, +] + +[package.dependencies] +typing_extensions = ">=4.0" + +[package.extras] +dev = ["attribution (==1.7.0)", "black (==24.2.0)", "coverage[toml] (==7.4.1)", "flake8 (==7.0.0)", "flake8-bugbear (==24.2.6)", "flit (==3.9.0)", "mypy (==1.8.0)", "ufmt (==2.3.0)", "usort (==1.0.8.post1)"] +docs = ["sphinx (==7.2.6)", "sphinx-mdinclude (==0.5.3)"] + [[package]] name = "annotated-types" version = "0.7.0" @@ -2050,6 +2069,25 @@ pluggy = ">=1.5,<2" [package.extras] dev = ["argcomplete", "attrs (>=19.2)", "hypothesis (>=3.56)", "mock", "pygments (>=2.7.2)", "requests", "setuptools", "xmlschema"] +[[package]] +name = "pytest-asyncio" +version = "0.24.0" +description = "Pytest support for asyncio" +optional = false +python-versions = ">=3.8" +groups = ["main"] +files = [ + {file = "pytest_asyncio-0.24.0-py3-none-any.whl", hash = "sha256:a811296ed596b69bf0b6f3dc40f83bcaf341b155a269052d82efa2b25ac7037b"}, + {file = "pytest_asyncio-0.24.0.tar.gz", hash = "sha256:d081d828e576d85f875399194281e92bf8a68d60d72d1a2faf2feddb6c46b276"}, +] + +[package.dependencies] +pytest = ">=8.2,<9" + +[package.extras] +docs = ["sphinx (>=5.3)", "sphinx-rtd-theme (>=1.0)"] +testing = ["coverage (>=6.2)", "hypothesis (>=5.7.1)"] + [[package]] name = "python-dateutil" version = "2.9.0.post0" @@ -3008,4 +3046,4 @@ propcache = ">=0.2.1" [metadata] lock-version = "2.1" python-versions = "^3.11" -content-hash = "6c2a5a5a8e6a2732bd9e94de4bac3a7c0d3e63d959d5793b23eb327c7a95f3f8" +content-hash = "256c8104c6eeb5b288dd0cdf02fe7cbad4f75aa93fc71f8d44da8b605d72f886" diff --git a/pyproject.toml b/pyproject.toml index 4272aa1..762975c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,6 +12,8 @@ fastapi = { extras = ["standard"], version = "^0.115.4" } sqlalchemy = "^2.0.36" httpx = "^0.27.2" pytest = "^8.3.3" +pytest-asyncio = "^0.24.0" +aiosqlite = "^0.20.0" faker = "^30.8.2" factory-boy = "^3.3.1" sqlalchemy-utils = "^0.41.2" diff --git a/pytest.ini b/pytest.ini new file mode 100644 index 0000000..3259ad7 --- /dev/null +++ b/pytest.ini @@ -0,0 +1,6 @@ +[pytest] +asyncio_mode = auto +testpaths = tests +python_files = test_*.py +filterwarnings = + ignore::pydantic.PydanticDeprecatedSince20 diff --git a/src/database/schema.py b/src/database/schema.py index 9a79777..42ff626 100644 --- a/src/database/schema.py +++ b/src/database/schema.py @@ -8,7 +8,7 @@ class CommonParams(DefultBase): # This ensures no extra query params are allowed current_user: Optional[str] = Field(None, alias="currentUser") page: int = Field(1, gt=0, lt=2147483647) - items_per_page: int = Field(5, gt=-2, lt=2147483647, alias="itemsPerPage") + items_per_page: int = Field(5, gt=0, le=50, multiple_of=5, alias="itemsPerPage") query_str: Optional[str] = Field(None, alias="q") filter_spec: Optional[str] = Field(None, alias="filter") sort_by: List[str] = Field(default_factory=list, alias="sortBy[]") diff --git a/src/middleware.py b/src/middleware.py index b654422..0ca720b 100644 --- a/src/middleware.py +++ b/src/middleware.py @@ -18,17 +18,35 @@ MAX_QUERY_PARAMS = 50 MAX_QUERY_LENGTH = 2000 MAX_JSON_BODY_SIZE = 1024 * 100 # 100 KB -# Very targeted patterns. Avoid catastrophic regex nonsense. XSS_PATTERN = re.compile( - r"( 50: + raise HTTPException( + status_code=400, + detail=f"Pagination size '{key}' cannot exceed 50", + ) + if size_val % 5 != 0: + raise HTTPException( + status_code=400, + detail=f"Pagination size '{key}' must be a multiple of 5", + ) + except ValueError: + raise HTTPException( + status_code=400, + detail=f"Pagination size '{key}' must be an integer", + ) + # ------------------------- # 4. Content-Type sanity # ------------------------- diff --git a/src/utils.py b/src/utils.py index ed4a615..f8d7659 100644 --- a/src/utils.py +++ b/src/utils.py @@ -22,7 +22,8 @@ def parse_relative_expression(date_str: str) -> Optional[datetime]: unit, offset = match.groups() offset = int(offset) if offset else 0 # Use UTC timezone for consistency - today = datetime.now(timezone.tzname("Asia/Jakarta")) + jakarta_tz = pytz.timezone("Asia/Jakarta") + today = datetime.now(jakarta_tz) if unit == "H": # For hours, keep minutes and seconds result_time = today + timedelta(hours=offset) @@ -64,7 +65,7 @@ def parse_date_string(date_str: str) -> Optional[datetime]: minute=0, second=0, microsecond=0, - tzinfo=timezone.tzname("Asia/Jakarta"), + tzinfo=pytz.timezone("Asia/Jakarta"), ) return dt except ValueError: diff --git a/tests/conftest.py b/tests/conftest.py index a678ff3..6708226 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,68 +1,68 @@ -import asyncio -from typing import AsyncGenerator, Generator +# import asyncio +# from typing import AsyncGenerator, Generator -import pytest -from httpx import AsyncClient -from sqlalchemy.ext.asyncio import AsyncSession, create_async_engine -from sqlalchemy.orm import sessionmaker -from sqlalchemy.pool import StaticPool -from sqlalchemy_utils import database_exists, drop_database -from starlette.config import environ -from starlette.testclient import TestClient +# import pytest +# from httpx import AsyncClient +# from sqlalchemy.ext.asyncio import AsyncSession, create_async_engine +# from sqlalchemy.orm import sessionmaker +# from sqlalchemy.pool import StaticPool +# from sqlalchemy_utils import database_exists, drop_database +# from starlette.config import environ +# from starlette.testclient import TestClient -# from src.database import Base, get_db -# from src.main import app +# # from src.database import Base, get_db +# # from src.main import app -# Test database URL -TEST_DATABASE_URL = "sqlite+aiosqlite:///:memory:" +# # Test database URL +# TEST_DATABASE_URL = "sqlite+aiosqlite:///:memory:" -engine = create_async_engine( - TEST_DATABASE_URL, - connect_args={"check_same_thread": False}, - poolclass=StaticPool, -) +# engine = create_async_engine( +# TEST_DATABASE_URL, +# connect_args={"check_same_thread": False}, +# poolclass=StaticPool, +# ) -async_session = sessionmaker( - engine, - class_=AsyncSession, - expire_on_commit=False, - autocommit=False, - autoflush=False, -) +# async_session = sessionmaker( +# engine, +# class_=AsyncSession, +# expire_on_commit=False, +# autocommit=False, +# autoflush=False, +# ) -async def override_get_db() -> AsyncGenerator[AsyncSession, None]: - async with async_session() as session: - try: - yield session - await session.commit() - except Exception: - await session.rollback() - raise - finally: - await session.close() +# async def override_get_db() -> AsyncGenerator[AsyncSession, None]: +# async with async_session() as session: +# try: +# yield session +# await session.commit() +# except Exception: +# await session.rollback() +# raise +# finally: +# await session.close() -app.dependency_overrides[get_db] = override_get_db +# app.dependency_overrides[get_db] = override_get_db -@pytest.fixture(scope="session") -def event_loop() -> Generator: - loop = asyncio.get_event_loop_policy().new_event_loop() - yield loop - loop.close() +# @pytest.fixture(scope="session") +# def event_loop() -> Generator: +# loop = asyncio.get_event_loop_policy().new_event_loop() +# yield loop +# loop.close() -@pytest.fixture(autouse=True) -async def setup_db() -> AsyncGenerator[None, None]: - async with engine.begin() as conn: - await conn.run_sync(Base.metadata.create_all) - yield - async with engine.begin() as conn: - await conn.run_sync(Base.metadata.drop_all) +# @pytest.fixture(autouse=True) +# async def setup_db() -> AsyncGenerator[None, None]: +# async with engine.begin() as conn: +# await conn.run_sync(Base.metadata.create_all) +# yield +# async with engine.begin() as conn: +# await conn.run_sync(Base.metadata.drop_all) -@pytest.fixture -async def client() -> AsyncGenerator[AsyncClient, None]: - async with AsyncClient(app=app, base_url="http://test") as client: - yield client +# @pytest.fixture +# async def client() -> AsyncGenerator[AsyncClient, None]: +# async with AsyncClient(app=app, base_url="http://test") as client: +# yield client diff --git a/tests/database.py b/tests/database.py deleted file mode 100644 index 89b84ae..0000000 --- a/tests/database.py +++ /dev/null @@ -1,3 +0,0 @@ -from sqlalchemy.orm import scoped_session, sessionmaker - -Session = scoped_session(sessionmaker()) diff --git a/tests/factories.py b/tests/factories.py deleted file mode 100644 index c15b514..0000000 --- a/tests/factories.py +++ /dev/null @@ -1,28 +0,0 @@ -import uuid -from datetime import datetime - -from factory import (LazyAttribute, LazyFunction, SelfAttribute, Sequence, - SubFactory, post_generation) -from factory.alchemy import SQLAlchemyModelFactory -from factory.fuzzy import FuzzyChoice, FuzzyDateTime, FuzzyInteger, FuzzyText -from faker import Faker -from faker.providers import misc - -from .database import Session - -# from pytz import UTC - - -fake = Faker() -fake.add_provider(misc) - - -class BaseFactory(SQLAlchemyModelFactory): - """Base Factory.""" - - class Meta: - """Factory configuration.""" - - abstract = True - sqlalchemy_session = Session - sqlalchemy_session_persistence = "commit" diff --git a/tests/unit/test_budget_constrains.py b/tests/unit/test_budget_constrains.py new file mode 100644 index 0000000..c8c9c94 --- /dev/null +++ b/tests/unit/test_budget_constrains.py @@ -0,0 +1,44 @@ +import pytest +from src.calculation_budget_constrains.service import greedy_selection, knapsack_selection + +def test_greedy_selection(): + equipments = [ + {"id": 1, "total_cost": 100, "priority_score": 10, "cost": 100}, + {"id": 2, "total_cost": 50, "priority_score": 20, "cost": 50}, + {"id": 3, "total_cost": 60, "priority_score": 15, "cost": 60}, + ] + budget = 120 + # Items sorted by priority_score: id 2 (20), id 3 (15), id 1 (10) + # 2 (50) + 3 (60) = 110. Item 1 (100) won't fit. + selected, excluded = greedy_selection(equipments, budget) + + selected_ids = [e["id"] for e in selected] + assert 2 in selected_ids + assert 3 in selected_ids + assert len(selected) == 2 + assert excluded[0]["id"] == 1 + +def test_knapsack_selection_basic(): + # Similar items but where greedy might fail if cost/value ratio is tricky + # item 1: value 10, cost 60 + # item 2: value 7, cost 35 + # item 3: value 7, cost 35 + # budget: 70 + # Greedy would take item 1 (value 10, remaining budget 10, can't take more) + # Optimal would take item 2 and 3 (value 14, remaining budget 0) + + scale = 1 # No scaling for simplicity in this test + equipments = [ + {"id": 1, "total_cost": 60, "priority_score": 10}, + {"id": 2, "total_cost": 35, "priority_score": 7}, + {"id": 3, "total_cost": 35, "priority_score": 7}, + ] + budget = 70 + + selected, excluded = knapsack_selection(equipments, budget, scale=1) + + selected_ids = [e["id"] for e in selected] + assert 2 in selected_ids + assert 3 in selected_ids + assert len(selected) == 2 + assert 1 not in selected_ids diff --git a/tests/unit/test_context.py b/tests/unit/test_context.py new file mode 100644 index 0000000..7ab857d --- /dev/null +++ b/tests/unit/test_context.py @@ -0,0 +1,14 @@ +from src.context import set_request_id, get_request_id, set_user_id, get_user_id + +def test_request_id_context(): + test_id = "test-request-id-123" + set_request_id(test_id) + assert get_request_id() == test_id + +def test_user_id_context(): + test_uid = "user-456" + set_user_id(test_uid) + assert get_user_id() == test_uid + +def test_context_default_none(): + assert get_request_id() is None or get_request_id() != "" diff --git a/tests/unit/test_contribution_util.py b/tests/unit/test_contribution_util.py new file mode 100644 index 0000000..e4c157d --- /dev/null +++ b/tests/unit/test_contribution_util.py @@ -0,0 +1,53 @@ +import pytest +from decimal import Decimal +from src.contribution_util import prod, system_availability, get_all_components, birnbaum_importance + +def test_prod(): + assert prod([1, 2, 3]) == 6.0 + assert prod([0.5, 0.5]) == 0.25 + assert prod([]) == 1.0 + +def test_system_availability_series(): + structure = {"series": ["A", "B"]} + availabilities = {"A": 0.9, "B": 0.8} + # 0.9 * 0.8 = 0.72 + assert system_availability(structure, availabilities) == pytest.approx(0.72) + +def test_system_availability_parallel(): + structure = {"parallel": ["A", "B"]} + availabilities = {"A": 0.9, "B": 0.8} + # 1 - (1-0.9)*(1-0.8) = 1 - 0.1*0.2 = 1 - 0.02 = 0.98 + assert system_availability(structure, availabilities) == pytest.approx(0.98) + +def test_system_availability_nested(): + # (A in series with (B in parallel with C)) + structure = { + "series": [ + "A", + {"parallel": ["B", "C"]} + ] + } + availabilities = {"A": 0.9, "B": 0.8, "C": 0.7} + # B||C = 1 - (1-0.8)*(1-0.7) = 1 - 0.2*0.3 = 0.94 + # A && (B||C) = 0.9 * 0.94 = 0.846 + assert system_availability(structure, availabilities) == pytest.approx(0.846) + +def test_get_all_components(): + structure = { + "series": [ + "A", + {"parallel": ["B", "C"]} + ] + } + assert get_all_components(structure) == {"A", "B", "C"} + +def test_birnbaum_importance(): + structure = {"series": ["A", "B"]} + availabilities = {"A": 0.9, "B": 0.8} + # I_B(A) = A_sys(A=1) - A_sys(A=0) + # A_sys(A=1, B=0.8) = 1 * 0.8 = 0.8 + # A_sys(A=0, B=0.8) = 0 * 0.8 = 0 + # I_B(A) = 0.8 + assert birnbaum_importance(structure, availabilities, "A") == pytest.approx(0.8) + # I_B(B) = A_sys(B=1, A=0.9) - A_sys(B=0, A=0.9) = 0.9 - 0 = 0.9 + assert birnbaum_importance(structure, availabilities, "B") == pytest.approx(0.9) diff --git a/tests/unit/test_exceptions.py b/tests/unit/test_exceptions.py new file mode 100644 index 0000000..187314b --- /dev/null +++ b/tests/unit/test_exceptions.py @@ -0,0 +1,31 @@ +import pytest +from sqlalchemy.exc import IntegrityError, DataError, DBAPIError +from src.exceptions import handle_sqlalchemy_error + +def test_handle_sqlalchemy_error_unique_constraint(): + err = IntegrityError("Unique constraint", params=None, orig=Exception("unique constraint violation")) + msg, status = handle_sqlalchemy_error(err) + assert status == 409 + assert "already exists" in msg + +def test_handle_sqlalchemy_error_foreign_key(): + err = IntegrityError("Foreign key constraint", params=None, orig=Exception("foreign key constraint violation")) + msg, status = handle_sqlalchemy_error(err) + assert status == 400 + assert "Related record not found" in msg + +def test_handle_sqlalchemy_error_data_error(): + err = DataError("Invalid data", params=None, orig=None) + msg, status = handle_sqlalchemy_error(err) + assert status == 400 + assert "Invalid data" in msg + +def test_handle_sqlalchemy_error_generic_dbapi(): + class MockError: + def __str__(self): + return "Some generic database error" + + err = DBAPIError("Generic error", params=None, orig=MockError()) + msg, status = handle_sqlalchemy_error(err) + assert status == 500 + assert "Database error" in msg diff --git a/tests/unit/test_middleware_dispatch.py b/tests/unit/test_middleware_dispatch.py new file mode 100644 index 0000000..6517990 --- /dev/null +++ b/tests/unit/test_middleware_dispatch.py @@ -0,0 +1,56 @@ +import pytest +from unittest.mock import AsyncMock, MagicMock +from fastapi import HTTPException +from src.middleware import RequestValidationMiddleware + +@pytest.mark.asyncio +async def test_request_validation_middleware_query_length(): + middleware = RequestValidationMiddleware(app=MagicMock()) + request = MagicMock() + request.url.query = "a" * 2001 + + with pytest.raises(HTTPException) as excinfo: + await middleware.dispatch(request, AsyncMock()) + assert excinfo.value.status_code == 414 + +@pytest.mark.asyncio +async def test_request_validation_middleware_too_many_params(): + middleware = RequestValidationMiddleware(app=MagicMock()) + request = MagicMock() + request.url.query = "a=1" + request.query_params.multi_items.return_value = [("param", "val")] * 51 + + with pytest.raises(HTTPException) as excinfo: + await middleware.dispatch(request, AsyncMock()) + assert excinfo.value.status_code == 400 + assert "Too many query parameters" in excinfo.value.detail + +@pytest.mark.asyncio +async def test_request_validation_middleware_xss_detection(): + middleware = RequestValidationMiddleware(app=MagicMock()) + request = MagicMock() + request.url.query = "q=