From 25cb697560b81c9f4992598647ec3da4e0601c86 Mon Sep 17 00:00:00 2001 From: Nadu_Dev Date: Sun, 26 Apr 2026 11:59:07 +0530 Subject: [PATCH 01/18] feat: update environment configuration and add engineering standards documentation --- .env.example | 13 +++++++--- GEMINI.md | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 GEMINI.md diff --git a/.env.example b/.env.example index fbce886..d0bda3d 100644 --- a/.env.example +++ b/.env.example @@ -7,13 +7,17 @@ APP_PORT = "REPLACE_WITH_APP_PORT" APP_ENV = "REPLACE_WITH_APP_ENV" ALLOWED_ORIGINS = "REPLACE_WITH_ALLOWED_ORIGINS" RUST_LOG = "REPLACE_WITH_RUST_LOG" +DOWNLOAD_DIR = "REPLACE_WITH_DOWNLOAD_DIR" YTDLP_PATH = "REPLACE_WITH_YTDLP_PATH" -YTDLP_OUTPUT_DIR = "REPLACE_WITH_YTDLP_OUTPUT_DIR" -YTDLP_TIMEOUT_SECS = "REPLACE_WITH_YTDLP_TIMEOUT_SECS" YTDLP_EXTERNAL_DOWNLOADER = "REPLACE_WITH_YTDLP_EXTERNAL_DOWNLOADER" YTDLP_EXTERNAL_DOWNLOADER_ARGS = "REPLACE_WITH_YTDLP_EXTERNAL_DOWNLOADER_ARGS" CAPTCHA_SECRET_KEY = "REPLACE_WITH_CAPTCHA_SECRET_KEY" MAX_CONCURRENT_DOWNLOADS = "REPLACE_WITH_MAX_CONCURRENT_DOWNLOADS" +MASTER_API_KEY = "REPLACE_WITH_MASTER_API_KEY" +GITHUB_PAT = "REPLACE_WITH_GITHUB_PAT" +GITHUB_USERNAME = "REPLACE_WITH_GITHUB_USERNAME" +GITHUB_GRAPHQL_URL = "https://api.github.com/graphql" +WARP_LICENSE_KEY = "REPLACE_WITH_WARP_LICENSE_KEY" ### ------------------------- ### Terraform / Infra - If you need to deploy @@ -21,6 +25,7 @@ MAX_CONCURRENT_DOWNLOADS = "REPLACE_WITH_MAX_CONCURRENT_DOWNLOADS" AWS_ACCESS_KEY_ID = "REPLACE_WITH_AWS_ACCESS_KEY_ID" AWS_SECRET_ACCESS_KEY = "REPLACE_WITH_AWS_SECRET_ACCESS_KEY" AWS_ENDPOINT_URL_S3 = "https://REPLACE_WITH_R2_ACCOUNT_ID.r2.cloudflarestorage.com" +AWS_S3_BUCKET_NAME = "REPLACE_WITH_AWS_S3_BUCKET_NAME" # Terraform provider secrets TF_VAR_DO_TOKEN = "REPLACE_WITH_DIGITALOCEAN_TOKEN" TF_VAR_CLOUDFLARE_API_TOKEN = "REPLACE_WITH_CLOUDFLARE_API_TOKEN" @@ -38,4 +43,6 @@ TF_VAR_RUST_LOG = "REPLACE_WITH_TF_VAR_RUST_LOG" TF_VAR_MAX_CONCURRENT_DOWNLOADS = "REPLACE_WITH_TF_VAR_MAX_CONCURRENT_DOWNLOADS" TF_VAR_APP_HOST = "REPLACE_WITH_TF_VAR_APP_HOST" TF_VAR_YTDLP_PATH = "REPLACE_WITH_TF_VAR_YTDLP_PATH" -TF_VAR_YTDLP_OUTPUT_DIR = "REPLACE_WITH_TF_VAR_YTDLP_OUTPUT_DIR" +TF_VAR_DOWNLOAD_DIR = "REPLACE_WITH_TF_VAR_DOWNLOAD_DIR" +TF_VAR_WARP_LICENSE_KEY = "REPLACE_WITH_TF_VAR_WARP_LICENSE_KEY" +TF_VAR_MASTER_API_KEY = "REPLACE_WITH_TF_VAR_MASTER_API_KEY" diff --git a/GEMINI.md b/GEMINI.md new file mode 100644 index 0000000..240d222 --- /dev/null +++ b/GEMINI.md @@ -0,0 +1,70 @@ +# Nadzu Backend - Engineering Standards & Policies + +This document serves as the foundational mandate for all engineering work on this codebase. It applies to both human developers and AI agents. Strict adherence is required to maintain the "Anti-Corruption Layer" and high-performance nature of the system. + +## 1. Architectural Integrity + +### DTO vs. Domain Model Separation +* **External DTOs (`*_dto.rs`)**: Strictly for mapping external API responses (e.g., GitHub, YouTube). They must mirror the external schema (e.g., `camelCase`). +* **Domain Models (`src/models/`)**: Clean, optimized structures used by our business logic and returned to our frontend. +* **Anti-Corruption Layer**: Every service must implement a transformation pass (e.g., `transform_calendar`) to convert "dirty" DTOs into "pure" Domain Models. **Never leak external API structures into the rest of the application.** + +### Service Layer Responsibility +* Services must handle business logic, caching, and external communication. +* Controllers must only handle request extraction, calling services, and mapping results to HTTP responses. + +## 2. Performance & Memory Safety + +### Zero-Allocation Strategy +* Use `std::borrow::Cow<'static, str>` for static metadata (colors, labels, constant status messages). +* Avoid `String::clone()` or `.to_string()` inside loops. +* Pre-allocate vector capacities when the size is known or estimable (e.g., `Vec::with_capacity(365)`). + +### Iteration Optimization +* Perform data transformations in a **single pass**. +* Calculate metadata (min/max values, counts) during the primary loop to leverage CPU cache and minimize cycles. + +## 3. Security Standards + +### Constant-Time Validation +* Sensitive comparisons (API keys, tokens) must use `constant_time_eq` to prevent timing attacks. +* Validation logic should be centralized in `AppConfig`. + +### Information Hiding +* Internal state (file paths, format flags, system IDs) must **never** be exposed in API responses. +* Use specific "Response" versions of models (e.g., `YtdlpJobResponse`) to filter sensitive fields. + +## 4. Configuration Management + +### Result-Based Loading +* `AppConfig::from_env()` must return a `Result`. +* Avoid `std::process::exit` or `panic!` deep in the logic; handle startup failures gracefully in `app.rs`. + +### Immutable State +* Config fields should be private where appropriate, using constructors and public methods (`check_api_key`) to enforce security policies. + +## 5. Error Handling & API Contract + +### Typed Errors +* Use the `AppError` enum for all internal failures. +* Map domain errors to correct HTTP status codes: + * Validation Error $\rightarrow$ `422 Unprocessable Entity` + * Upstream/External Failure $\rightarrow$ `502 Bad Gateway` + * Auth Failure $\rightarrow$ `401 Unauthorized` + +### Consistency +* Responses should be flat and idiomatic where possible (avoiding unnecessary "job" or "data" wrappers unless required by the specific API design). + +## 6. Development Workflow + +### Tooling +* **Clippy**: Must be zero-warning. +* **Rustfmt**: Must be applied to every file. +* **Makefile**: Use `make c` for a full validation suite before concluding any task. + +### Documentation +* All public-facing methods and services must have `///` (Rustdoc) comments explaining intent and behavior. +* Complex logic (like the Midnight Snap caching strategy) must be documented inline. + +--- +*Follow these rules to ensure the codebase remains scalable, secure, and blazingly fast.* From 8bca0b0f58a64e1f28bc63ded3e0d4d2622b204c Mon Sep 17 00:00:00 2001 From: Nadu_Dev Date: Sun, 26 Apr 2026 11:59:40 +0530 Subject: [PATCH 02/18] feat: add data models for contributions, health, validation, and ytdlp --- ...ontributions_model.rs => contributions.rs} | 11 ++-- src/models/github_dto.rs | 47 ++++++++++++++ src/models/{health_model.rs => health.rs} | 0 .../{validation_model.rs => validation.rs} | 2 +- src/models/{ytdlp_model.rs => ytdlp.rs} | 26 +------- src/models/ytdlp_dto.rs | 63 +++++++++++++++++++ 6 files changed, 120 insertions(+), 29 deletions(-) rename src/models/{contributions_model.rs => contributions.rs} (90%) create mode 100644 src/models/github_dto.rs rename src/models/{health_model.rs => health.rs} (100%) rename src/models/{validation_model.rs => validation.rs} (93%) rename src/models/{ytdlp_model.rs => ytdlp.rs} (56%) create mode 100644 src/models/ytdlp_dto.rs diff --git a/src/models/contributions_model.rs b/src/models/contributions.rs similarity index 90% rename from src/models/contributions_model.rs rename to src/models/contributions.rs index 84b3923..6e0a630 100644 --- a/src/models/contributions_model.rs +++ b/src/models/contributions.rs @@ -1,4 +1,5 @@ use serde::{Deserialize, Serialize}; +use std::borrow::Cow; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ContributionsResponse { @@ -31,15 +32,15 @@ pub struct ContributionSummary { #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ContributionLegend { pub level: u32, - pub label: String, + pub label: Cow<'static, str>, pub min: u32, pub max: u32, - pub color: String, + pub color: Cow<'static, str>, } #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ContributionMonth { - pub label: String, + pub label: Cow<'static, str>, #[serde(rename = "weekIndex")] pub week_index: usize, } @@ -51,10 +52,10 @@ pub struct ContributionCell { pub week_index: usize, pub weekday: u8, #[serde(rename = "weekdayLabel")] - pub weekday_label: String, + pub weekday_label: Cow<'static, str>, pub count: u32, pub level: u32, - pub color: String, + pub color: Cow<'static, str>, #[serde(rename = "isFuture")] pub is_future: bool, #[serde(rename = "isInCurrentMonth")] diff --git a/src/models/github_dto.rs b/src/models/github_dto.rs new file mode 100644 index 0000000..ba8b457 --- /dev/null +++ b/src/models/github_dto.rs @@ -0,0 +1,47 @@ +use serde::Deserialize; + +#[derive(Debug, Deserialize)] +pub struct GithubGqlResponse { + pub data: Option, + pub errors: Option>, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct GithubGraphQLUser { + pub user: Option, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct GithubUserNode { + pub contributions_collection: GithubContributionsCollection, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct GithubContributionsCollection { + pub contribution_calendar: GithubContributionCalendar, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct GithubContributionCalendar { + pub total_contributions: u32, + pub weeks: Vec, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct GithubWeek { + pub contribution_days: Vec, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct GithubContributionDay { + pub date: String, + pub weekday: u8, // 0 = Sunday + pub contribution_count: u32, + pub contribution_level: String, +} diff --git a/src/models/health_model.rs b/src/models/health.rs similarity index 100% rename from src/models/health_model.rs rename to src/models/health.rs diff --git a/src/models/validation_model.rs b/src/models/validation.rs similarity index 93% rename from src/models/validation_model.rs rename to src/models/validation.rs index c027313..399d6c3 100644 --- a/src/models/validation_model.rs +++ b/src/models/validation.rs @@ -20,7 +20,7 @@ pub struct ValidateUserRequest { #[derive(Debug, Serialize)] pub struct ValidatedUserResponse { pub success: bool, - pub message: String, + pub message: std::borrow::Cow<'static, str>, pub data: UserData, } diff --git a/src/models/ytdlp_model.rs b/src/models/ytdlp.rs similarity index 56% rename from src/models/ytdlp_model.rs rename to src/models/ytdlp.rs index b12560f..d7b0962 100644 --- a/src/models/ytdlp_model.rs +++ b/src/models/ytdlp.rs @@ -1,27 +1,7 @@ use serde::{Deserialize, Serialize}; -use validator::Validate; - -#[derive(Debug, Deserialize, Validate, Clone)] -pub struct YtdlpDownloadRequest { - #[validate(url(message = "url must be a valid URL"))] - pub url: String, - pub quality: Option, - pub format: Option, - pub folder: Option, -} - -#[derive(Debug, Serialize, Clone)] -pub struct YtdlpEnqueueResponse { - pub status: &'static str, - pub message: &'static str, - pub job: YtdlpJob, -} - -#[derive(Debug, Serialize, Clone)] -pub struct YtdlpListResponse { - pub jobs: Vec, -} +/// Internal domain model representing a download job. +/// This includes internal state like `output_dir` and `format_flag` which are not exposed to clients. #[derive(Debug, Serialize, Clone)] pub struct YtdlpJob { pub id: String, @@ -42,7 +22,7 @@ pub struct YtdlpJob { pub error: Option, } -#[derive(Debug, Serialize, Clone)] +#[derive(Debug, Serialize, Deserialize, Clone, Copy, PartialEq, Eq)] #[serde(rename_all = "snake_case")] pub enum YtdlpJobStatus { Queued, diff --git a/src/models/ytdlp_dto.rs b/src/models/ytdlp_dto.rs new file mode 100644 index 0000000..1adc1f6 --- /dev/null +++ b/src/models/ytdlp_dto.rs @@ -0,0 +1,63 @@ +use serde::{Deserialize, Serialize}; +use std::borrow::Cow; +use validator::Validate; + +use super::ytdlp::{YtdlpJob, YtdlpJobStatus}; + +#[derive(Debug, Deserialize, Validate, Clone)] +pub struct YtdlpDownloadRequest { + #[validate(url(message = "url must be a valid URL"))] + pub url: String, + pub quality: Option, + pub format: Option, + pub folder: Option, +} + +#[derive(Debug, Serialize, Clone)] +pub struct YtdlpEnqueueResponse { + pub status: Cow<'static, str>, + pub message: Cow<'static, str>, + pub job: YtdlpJobResponse, +} + +#[derive(Debug, Serialize, Clone)] +pub struct YtdlpListResponse { + pub jobs: Vec, +} + +#[derive(Debug, Serialize, Clone)] +pub struct YtdlpJobResponse { + pub id: String, + pub url: String, + pub status: YtdlpJobStatus, + pub started_at_unix: Option, + pub finished_at_unix: Option, + pub progress_percent: Option, + pub progress_total: Option, + pub progress_speed: Option, + pub progress_eta: Option, + pub progress_message: Option, + pub updated_at_unix: Option, + pub files: Option>, + pub error: Option, +} + +impl From for YtdlpJobResponse { + fn from(job: YtdlpJob) -> Self { + Self { + id: job.id, + url: job.url, + status: job.status, + started_at_unix: job.started_at_unix, + finished_at_unix: job.finished_at_unix, + progress_percent: job.progress_percent, + progress_total: job.progress_total, + progress_speed: job.progress_speed, + progress_eta: job.progress_eta, + progress_message: job.progress_message, + updated_at_unix: job.updated_at_unix, + files: job.files, + error: job.error, + } + } +} From 4b3f4279a83b57b2091a3b84cd833e9cace80ce3 Mon Sep 17 00:00:00 2001 From: Nadu_Dev Date: Sun, 26 Apr 2026 11:59:52 +0530 Subject: [PATCH 03/18] feat: enhance configuration loading with error handling and add upstream error type to AppError --- src/app.rs | 9 ++++- src/config.rs | 106 ++++++++++++++++++++++++++++++++++++++++---------- src/error.rs | 8 ++++ 3 files changed, 100 insertions(+), 23 deletions(-) diff --git a/src/app.rs b/src/app.rs index 72ee8b5..6d62752 100644 --- a/src/app.rs +++ b/src/app.rs @@ -47,7 +47,13 @@ pub async fn run() { .on_response(DefaultOnResponse::new().level(Level::INFO)); // 4. Load application config and build shared app state - let config = Arc::new(AppConfig::from_env()); + let config = match AppConfig::from_env() { + Ok(cfg) => Arc::new(cfg), + Err(err) => { + error!("Failed to load configuration: {err}"); + std::process::exit(1); + } + }; let ytdlp_manager = Arc::new(YtdlpManager::new(config.clone())); let rate_limiters = Arc::new(RateLimiters::new()); log_rate_limit_mode(&config); @@ -117,6 +123,5 @@ async fn shutdown_signal() { if let Err(err) = tokio::signal::ctrl_c().await { error!("failed to listen for CTRL+C: {err}"); } - println!(); info!("Initiating graceful shutdown"); } diff --git a/src/config.rs b/src/config.rs index 727f861..268f058 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,4 +1,15 @@ use std::env; +use thiserror::Error; + +use crate::middleware::constant_time_eq; + +#[derive(Debug, Error)] +pub enum ConfigError { + #[error("Missing required environment variable: {0}")] + MissingVar(String), + #[error("Invalid value for {key}: {details}")] + InvalidValue { key: String, details: String }, +} /// Helper: Fetches an env var, applies a default, and parses to the required type. fn env_or(key: &str, default: &str) -> T { @@ -24,6 +35,7 @@ fn env_opt(key: &str) -> Option { .filter(|v| !v.is_empty()) } +/// Application configuration loaded from environment variables. #[derive(Debug, Clone)] pub struct AppConfig { pub name: String, @@ -37,39 +49,91 @@ pub struct AppConfig { pub ytdlp_external_downloader_args: Option, pub max_concurrent_downloads: usize, pub captcha_secret_key: Option, - pub master_api_key: String, + master_api_key: String, // Private: use check_api_key pub github_pat: Option, pub github_username: Option, pub github_graphql_url: String, } impl AppConfig { - /// Loads the application configuration from environment variables. - pub fn from_env() -> Self { + /// Internal constructor for creating config instances. + #[allow(clippy::too_many_arguments)] + pub const fn new( + name: String, + env: String, + host: String, + port: u16, + allowed_origins: Option, + download_dir: String, + ytdlp_path: String, + ytdlp_external_downloader: Option, + ytdlp_external_downloader_args: Option, + max_concurrent_downloads: usize, + captcha_secret_key: Option, + master_api_key: String, + github_pat: Option, + github_username: Option, + github_graphql_url: String, + ) -> Self { Self { - name: env_or("APP_NAME", "nadzu-backend"), - env: env_or("APP_ENV", "production"), - host: env_or("APP_HOST", "127.0.0.1"), - port: env_or("APP_PORT", "8080"), - allowed_origins: env_opt("ALLOWED_ORIGINS"), - download_dir: env_or("DOWNLOAD_DIR", "downloads"), - ytdlp_path: env_or("YTDLP_PATH", "yt-dlp"), - ytdlp_external_downloader: env_opt("YTDLP_EXTERNAL_DOWNLOADER"), - ytdlp_external_downloader_args: env_opt("YTDLP_EXTERNAL_DOWNLOADER_ARGS"), - max_concurrent_downloads: env_or("MAX_CONCURRENT_DOWNLOADS", "3"), - captcha_secret_key: env_opt("CAPTCHA_SECRET_KEY"), - master_api_key: env_opt("MASTER_API_KEY").unwrap_or_else(|| { - tracing::error!("MASTER_API_KEY must be set to a non-empty value"); - std::process::exit(1) - }), - github_pat: env_opt("GITHUB_PAT"), - github_username: env_opt("GITHUB_USERNAME"), - github_graphql_url: env_or("GITHUB_GRAPHQL_URL", "https://api.github.com/graphql"), + name, + env, + host, + port, + allowed_origins, + download_dir, + ytdlp_path, + ytdlp_external_downloader, + ytdlp_external_downloader_args, + max_concurrent_downloads, + captcha_secret_key, + master_api_key, + github_pat, + github_username, + github_graphql_url, } } + /// Loads the application configuration from environment variables. + pub fn from_env() -> Result { + let master_api_key = env_opt("MASTER_API_KEY") + .ok_or_else(|| ConfigError::MissingVar("MASTER_API_KEY".to_string()))?; + + Ok(Self::new( + env_or("APP_NAME", "nadzu-backend"), + env_or("APP_ENV", "production"), + env_or("APP_HOST", "127.0.0.1"), + env_or("APP_PORT", "8080"), + env_opt("ALLOWED_ORIGINS"), + env_or("DOWNLOAD_DIR", "downloads"), + env_or("YTDLP_PATH", "yt-dlp"), + env_opt("YTDLP_EXTERNAL_DOWNLOADER"), + env_opt("YTDLP_EXTERNAL_DOWNLOADER_ARGS"), + env_or("MAX_CONCURRENT_DOWNLOADS", "3"), + env_opt("CAPTCHA_SECRET_KEY"), + master_api_key, + env_opt("GITHUB_PAT"), + env_opt("GITHUB_USERNAME"), + env_or("GITHUB_GRAPHQL_URL", "https://api.github.com/graphql"), + )) + } + + /// Securely checks if the provided key matches the master API key using constant-time comparison. + #[must_use] + pub fn check_api_key(&self, provided_key: &str) -> bool { + constant_time_eq(provided_key, &self.master_api_key) + } + /// Returns the full address string for the server. pub fn addr(&self) -> String { format!("{}:{}", self.host, self.port) } + + /// Helper for testing to inject a master API key. + #[cfg(test)] + #[must_use] + pub fn with_master_key(mut self, key: &str) -> Self { + self.master_api_key = key.to_string(); + self + } } diff --git a/src/error.rs b/src/error.rs index 8d9e54e..6797087 100644 --- a/src/error.rs +++ b/src/error.rs @@ -35,6 +35,9 @@ pub enum AppError { #[error("Conflict: {0}")] Conflict(String), + #[error("Upstream Service Error: {0}")] + UpstreamError(String), + #[error("Service Unavailable: {0}")] ServiceUnavailable(String), } @@ -75,6 +78,11 @@ impl IntoResponse for AppError { msg.clone(), Some("CONFLICT".to_string()), ), + Self::UpstreamError(msg) => ( + StatusCode::BAD_GATEWAY, + msg.clone(), + Some("UPSTREAM_ERROR".to_string()), + ), Self::ServiceUnavailable(msg) => ( StatusCode::SERVICE_UNAVAILABLE, msg.clone(), From 0e54f1ea2055c7145e338a481da57c5eab75ce24 Mon Sep 17 00:00:00 2001 From: Nadu_Dev Date: Sun, 26 Apr 2026 12:00:38 +0530 Subject: [PATCH 04/18] fix: update model imports in controllers for consistency --- .../api/v1/contributions_controller.rs | 5 +- src/controllers/api/v1/ytdlp_controller.rs | 55 +++++++++---------- src/controllers/health_controller.rs | 2 +- src/controllers/validation_controller.rs | 5 +- 4 files changed, 34 insertions(+), 33 deletions(-) diff --git a/src/controllers/api/v1/contributions_controller.rs b/src/controllers/api/v1/contributions_controller.rs index 22d80dd..9913fe6 100644 --- a/src/controllers/api/v1/contributions_controller.rs +++ b/src/controllers/api/v1/contributions_controller.rs @@ -4,13 +4,16 @@ use axum::{ }; use serde::Deserialize; -use crate::{error::AppError, models::contributions_model::ContributionsResponse, state::AppState}; +use crate::{error::AppError, models::contributions::ContributionsResponse, state::AppState}; +/// Request query parameters for the contributions endpoint. #[derive(Debug, Deserialize)] pub struct ContributionsQuery { pub username: Option, } +/// Retrieves GitHub contributions for a specific user. +/// Defaults to the configured system user if none is provided. pub async fn get_contributions( State(state): State, Query(query): Query, diff --git a/src/controllers/api/v1/ytdlp_controller.rs b/src/controllers/api/v1/ytdlp_controller.rs index 8213a22..a8eaf51 100644 --- a/src/controllers/api/v1/ytdlp_controller.rs +++ b/src/controllers/api/v1/ytdlp_controller.rs @@ -8,8 +8,7 @@ use axum::{ sse::{Event, KeepAlive, Sse}, }, }; -use serde_json::json; -use std::{convert::Infallible, path::PathBuf, time::Duration}; +use std::{borrow::Cow, convert::Infallible, path::PathBuf, time::Duration}; use tokio::{sync::mpsc, time::sleep}; use tokio_stream::wrappers::ReceiverStream; use tower::ServiceExt; @@ -19,8 +18,11 @@ use tracing::info; use crate::{ error::AppError, extractors::validated_json::ValidatedJson, - models::ytdlp_model::{ - YtdlpDownloadRequest, YtdlpEnqueueResponse, YtdlpJobStatus, YtdlpListResponse, + models::{ + ytdlp::YtdlpJobStatus, + ytdlp_dto::{ + YtdlpDownloadRequest, YtdlpEnqueueResponse, YtdlpJobResponse, YtdlpListResponse, + }, }, state::AppState, }; @@ -35,9 +37,9 @@ pub async fn create_download_job( Ok(( StatusCode::ACCEPTED, Json(YtdlpEnqueueResponse { - status: "accepted", - message: "Download enqueued", - job, + status: Cow::Borrowed("accepted"), + message: Cow::Borrowed("Download enqueued"), + job: YtdlpJobResponse::from(job), }), )) } @@ -52,7 +54,7 @@ pub async fn get_download_job( .get_job(&id) .ok_or_else(|| AppError::NotFound(format!("Job {id} not found")))?; - Ok((StatusCode::OK, Json(json!({ "job": job })))) + Ok((StatusCode::OK, Json(YtdlpJobResponse::from(job)))) } /// Lists all current download jobs. @@ -60,7 +62,14 @@ pub async fn list_download_jobs( State(state): State, ) -> Result { let jobs = state.ytdlp_manager.list_jobs(); - Ok((StatusCode::OK, Json(YtdlpListResponse { jobs }))) + let response_jobs: Vec = + jobs.into_iter().map(YtdlpJobResponse::from).collect(); + Ok(( + StatusCode::OK, + Json(YtdlpListResponse { + jobs: response_jobs, + }), + )) } /// Streams progress of a download job via SSE. @@ -105,32 +114,22 @@ pub async fn stream_download_progress( loop { if let Some(job) = manager.get_job(&stream_job_id) { - let payload = json!({ - "status": job.status.clone(), - "progress_percent": job.progress_percent, - "progress_total": job.progress_total.clone(), - "progress_speed": job.progress_speed.clone(), - "progress_eta": job.progress_eta.clone(), - "progress_message": job.progress_message.clone(), - "updated_at_unix": job.updated_at_unix - }); - let snapshot = payload.to_string(); + let job_resp = YtdlpJobResponse::from(job); + let snapshot = serde_json::to_string(&job_resp).unwrap_or_default(); if snapshot != last_snapshot { + last_snapshot.clone_from(&snapshot); if tx - .send(Ok(Event::default() - .event("progress") - .data(snapshot.clone()))) + .send(Ok(Event::default().event("progress").data(snapshot))) .await .is_err() { break; } - last_snapshot = snapshot; } if matches!( - job.status, + job_resp.status, YtdlpJobStatus::Finished | YtdlpJobStatus::Failed ) { let _ = tx @@ -138,7 +137,7 @@ pub async fn stream_download_progress( .await; info!( "sse stream complete path={} job_id={} client_ip={} status={:?}", - stream_path, stream_job_id, stream_client_ip, job.status + stream_path, stream_job_id, stream_client_ip, job_resp.status ); break; } @@ -173,7 +172,6 @@ pub async fn stream_download_progress( /// Returns a list of supported sites by yt-dlp. pub async fn get_supported_sites() -> Result { - // Read the compiled site lists generated by the docker entrypoint let file_path = PathBuf::from("/home/app/sites.txt"); tokio::fs::read_to_string(&file_path).await.map_or_else( |_| { @@ -185,7 +183,7 @@ pub async fn get_supported_sites() -> Result { let sites: Vec<&str> = content.lines().filter(|line| !line.is_empty()).collect(); Ok(( StatusCode::OK, - Json(json!({ "status": "ok", "sites": sites })), + Json(serde_json::json!({ "status": "ok", "sites": sites })), )) }, ) @@ -202,7 +200,6 @@ pub async fn download_file( .get_job(&id) .ok_or_else(|| AppError::NotFound(format!("Job {id} not found")))?; - // Prefer video/audio files (avoid serving thumbnails or sidecar files) let filename = job .files .as_ref() @@ -226,7 +223,7 @@ pub async fn download_file( let res = ServeFile::new(file_path) .oneshot(req) .await - .map_err(|err| AppError::Internal(anyhow::anyhow!("Failed to serve file: {err}")))?; + .map_err(|err| AppError::Internal(anyhow::Error::new(err)))?; let mut response = res.into_response(); if let Ok(hv) = HeaderValue::from_str(&format!("attachment; filename=\"{filename}\"")) { diff --git a/src/controllers/health_controller.rs b/src/controllers/health_controller.rs index d3b60f6..4292af7 100644 --- a/src/controllers/health_controller.rs +++ b/src/controllers/health_controller.rs @@ -1,4 +1,4 @@ -use crate::{error::AppError, models::health_model::Health, state::AppState}; +use crate::{error::AppError, models::health::Health, state::AppState}; use axum::{Json, extract::State}; /// Health check endpoint. diff --git a/src/controllers/validation_controller.rs b/src/controllers/validation_controller.rs index 2e1e517..574f5db 100644 --- a/src/controllers/validation_controller.rs +++ b/src/controllers/validation_controller.rs @@ -1,9 +1,10 @@ use crate::{ error::AppError, extractors::validated_json::ValidatedJson, - models::validation_model::{UserData, ValidateUserRequest, ValidatedUserResponse}, + models::validation::{UserData, ValidateUserRequest, ValidatedUserResponse}, }; use axum::Json; +use std::borrow::Cow; /// Validates user data using the custom validator extractor. pub async fn validate_user( @@ -11,7 +12,7 @@ pub async fn validate_user( ) -> Result, AppError> { let response = ValidatedUserResponse { success: true, - message: "Validation successful".to_string(), + message: Cow::Borrowed("Validation successful"), data: UserData { name: payload.name, email: payload.email, From 1a40c848073475a38d4755ee9cefe19055606e93 Mon Sep 17 00:00:00 2001 From: Nadu_Dev Date: Sun, 26 Apr 2026 12:01:01 +0530 Subject: [PATCH 05/18] refactor: standardize API key and CAPTCHA token handling across middleware --- src/extractors/validated_json.rs | 3 ++- src/middleware/api_key.rs | 8 +++----- src/middleware/auth.rs | 17 +++-------------- src/middleware/captcha.rs | 14 +++++++++----- src/middleware/cors.rs | 11 +++++++---- 5 files changed, 24 insertions(+), 29 deletions(-) diff --git a/src/extractors/validated_json.rs b/src/extractors/validated_json.rs index c2e965e..838760d 100644 --- a/src/extractors/validated_json.rs +++ b/src/extractors/validated_json.rs @@ -42,7 +42,8 @@ where }) .collect(); - let msg = format!("Validation failed: {error_map:?}"); + let msg = serde_json::to_string(&error_map) + .unwrap_or_else(|_| "Validation failed".to_string()); AppError::Validation(msg).into_response() })?; diff --git a/src/middleware/api_key.rs b/src/middleware/api_key.rs index 9c95e5c..8e959de 100644 --- a/src/middleware/api_key.rs +++ b/src/middleware/api_key.rs @@ -1,14 +1,12 @@ use axum::http::HeaderMap; -use crate::config::AppConfig; - -pub const API_KEY_HEADER: &str = "x-api-key"; +use crate::{config::AppConfig, middleware::X_API_KEY}; /// Checks if the request headers contain a valid master API key. #[must_use] pub fn has_valid_master_api_key(headers: &HeaderMap, config: &AppConfig) -> bool { headers - .get(API_KEY_HEADER) + .get(X_API_KEY) .and_then(|v| v.to_str().ok()) - .is_some_and(|v| v == config.master_api_key) + .is_some_and(|v| config.check_api_key(v)) } diff --git a/src/middleware/auth.rs b/src/middleware/auth.rs index bc44740..0c4eae2 100644 --- a/src/middleware/auth.rs +++ b/src/middleware/auth.rs @@ -4,18 +4,7 @@ use axum::{ response::Response, }; -use crate::{error::AppError, middleware::api_key::API_KEY_HEADER, state::AppState}; - -/// Performs a constant-time comparison of two strings to prevent timing attacks. -fn constant_time_eq(a: &str, b: &str) -> bool { - if a.len() != b.len() { - return false; - } - a.bytes() - .zip(b.bytes()) - .fold(0u8, |acc, (x, y)| acc | (x ^ y)) - == 0 -} +use crate::{error::AppError, middleware::X_API_KEY, state::AppState}; /// Middleware that requires a valid master API key to be present in the headers. pub async fn require_api_key( @@ -25,11 +14,11 @@ pub async fn require_api_key( ) -> Result { let api_key = req .headers() - .get(API_KEY_HEADER) + .get(X_API_KEY) .and_then(|value| value.to_str().ok()); match api_key { - Some(key) if constant_time_eq(key, &state.config.master_api_key) => Ok(next.run(req).await), + Some(key) if state.config.check_api_key(key) => Ok(next.run(req).await), _ => Err(AppError::Unauthorized( "Invalid or missing API key".to_string(), )), diff --git a/src/middleware/captcha.rs b/src/middleware/captcha.rs index 7147702..c236dcf 100644 --- a/src/middleware/captcha.rs +++ b/src/middleware/captcha.rs @@ -6,7 +6,11 @@ use axum::{ }; use serde::Deserialize; -use crate::{error::AppError, middleware::api_key::has_valid_master_api_key, state::AppState}; +use crate::{ + error::AppError, + middleware::{X_CAPTCHA_TOKEN, api_key::has_valid_master_api_key}, + state::AppState, +}; #[derive(Debug, Deserialize)] struct CaptchaProviderResponse { @@ -30,7 +34,7 @@ pub async fn verify_captcha_token( let captcha_token = req .headers() - .get("x-captcha-token") + .get(X_CAPTCHA_TOKEN) .and_then(|value| value.to_str().ok()) .map(str::trim) .filter(|value| !value.is_empty()) @@ -42,7 +46,7 @@ pub async fn verify_captcha_token( .as_deref() .filter(|s| !s.trim().is_empty()) .ok_or_else(|| { - AppError::Internal(anyhow::anyhow!("CAPTCHA_SECRET_KEY is not configured")) + AppError::ServiceUnavailable("CAPTCHA_SECRET_KEY is not configured".to_string()) })?; let response = state @@ -52,13 +56,13 @@ pub async fn verify_captcha_token( .form(&[("secret", secret_key), ("response", captcha_token)]) .send() .await - .map_err(|err| AppError::Internal(anyhow::anyhow!("Failed to verify captcha: {err}")))?; + .map_err(|err| AppError::UpstreamError(format!("Failed to verify captcha: {err}")))?; let body = response .json::() .await .map_err(|err| { - AppError::Internal(anyhow::anyhow!("Failed to parse captcha response: {err}")) + AppError::UpstreamError(format!("Failed to parse captcha response: {err}")) })?; if !body.success { diff --git a/src/middleware/cors.rs b/src/middleware/cors.rs index 68b376f..f2f302e 100644 --- a/src/middleware/cors.rs +++ b/src/middleware/cors.rs @@ -1,7 +1,10 @@ -use crate::{config::AppConfig, middleware::api_key::API_KEY_HEADER}; +use crate::{ + config::AppConfig, + middleware::{X_API_KEY, X_CAPTCHA_TOKEN}, +}; use axum::http::{ Method, - header::{ACCEPT, AUTHORIZATION, CONTENT_TYPE, HeaderName}, + header::{ACCEPT, AUTHORIZATION, CONTENT_TYPE}, }; use tower_http::cors::{Any, CorsLayer}; use tracing::{info, warn}; @@ -108,8 +111,8 @@ pub fn build_cors(config: &AppConfig) -> CorsLayer { AUTHORIZATION, CONTENT_TYPE, ACCEPT, - HeaderName::from_static(API_KEY_HEADER), - HeaderName::from_static("x-captcha-token"), + X_API_KEY, + X_CAPTCHA_TOKEN, ]) .allow_credentials(true) } From 2924513be9104c34cefb4e2cacdcdc59a965b3d1 Mon Sep 17 00:00:00 2001 From: Nadu_Dev Date: Sun, 26 Apr 2026 12:01:15 +0530 Subject: [PATCH 06/18] feat: enhance rate limiting logic with improved error handling and documentation --- src/middleware/mod.rs | 17 +++++++++++++ src/middleware/rate_limit.rs | 49 ++++++++++++++++++------------------ 2 files changed, 41 insertions(+), 25 deletions(-) diff --git a/src/middleware/mod.rs b/src/middleware/mod.rs index dbbdced..70af45a 100644 --- a/src/middleware/mod.rs +++ b/src/middleware/mod.rs @@ -1,5 +1,22 @@ +use axum::http::header::HeaderName; + pub mod api_key; pub mod auth; pub mod captcha; pub mod cors; pub mod rate_limit; + +pub const X_API_KEY: HeaderName = HeaderName::from_static("x-api-key"); +pub const X_CAPTCHA_TOKEN: HeaderName = HeaderName::from_static("x-captcha-token"); + +/// Performs a constant-time comparison of two strings to prevent timing attacks. +#[must_use] +pub fn constant_time_eq(a: &str, b: &str) -> bool { + if a.len() != b.len() { + return false; + } + a.bytes() + .zip(b.bytes()) + .fold(0u8, |acc, (x, y)| acc | (x ^ y)) + == 0 +} diff --git a/src/middleware/rate_limit.rs b/src/middleware/rate_limit.rs index ab22d60..542379c 100644 --- a/src/middleware/rate_limit.rs +++ b/src/middleware/rate_limit.rs @@ -26,6 +26,8 @@ const ENHANCED_RATE_LIMITER_BURST_SIZE: u32 = 100; type KeyedLimiter = RateLimiter, DefaultClock, NoOpMiddleware>; +/// Container for the application's rate limiting logic. +/// Supports a "Normal" tier for public users and an "Enhanced" tier for authorized API key holders. #[derive(Clone, Debug)] pub struct RateLimiters { normal: Arc, @@ -34,6 +36,9 @@ pub struct RateLimiters { impl RateLimiters { /// Creates a new instance of `RateLimiters` with normal and enhanced buckets. + /// + /// # Panics + /// Panics if the hardcoded rate limit constants are invalid (e.g., zero). #[must_use] pub fn new() -> Self { Self { @@ -63,16 +68,11 @@ impl Default for RateLimiters { } } +/// Internal helper to build a DashMap-backed rate limiter. +#[allow(clippy::expect_used)] fn build_limiter(per_second: u32, burst_size: u32) -> KeyedLimiter { - let Some(per_second) = NonZeroU32::new(per_second) else { - tracing::error!("rate limiter per_second must be greater than 0"); - std::process::exit(1) - }; - - let Some(burst_size) = NonZeroU32::new(burst_size) else { - tracing::error!("rate limiter burst_size must be greater than 0"); - std::process::exit(1) - }; + let per_second = NonZeroU32::new(per_second).expect("RATE_LIMITER_PER_SECOND must be > 0"); + let burst_size = NonZeroU32::new(burst_size).expect("RATE_LIMITER_BURST_SIZE must be > 0"); let quota = Quota::per_second(per_second).allow_burst(burst_size); RateLimiter::, DefaultClock, NoOpMiddleware>::dashmap( @@ -104,6 +104,8 @@ fn request_client_key(req: &Request, config: &AppConfig) -> String { } /// Middleware that enforces tiered rate limits based on API key presence and client IP. +/// +/// Rejects requests exceeding the quota with a `403 Forbidden` error. pub async fn enforce_tiered_rate_limit( State(state): State, req: Request, @@ -134,22 +136,19 @@ pub async fn enforce_tiered_rate_limit( Ok(next.run(req).await) } +/// Logs the current rate limiting configuration to the tracing output. pub fn log_rate_limit_mode(config: &AppConfig) { - if is_production(config) { - info!( - "Rate Limiter: production mode (keyed by client IP), normal={}/s burst={}, enhanced={}/s burst={}", - RATE_LIMITER_PER_SECOND, - RATE_LIMITER_BURST_SIZE, - ENHANCED_RATE_LIMITER_PER_SECOND, - ENHANCED_RATE_LIMITER_BURST_SIZE - ); + let mode = if is_production(config) { + "production" } else { - info!( - "Rate Limiter: development mode (global key), normal={}/s burst={}, enhanced={}/s burst={}", - RATE_LIMITER_PER_SECOND, - RATE_LIMITER_BURST_SIZE, - ENHANCED_RATE_LIMITER_PER_SECOND, - ENHANCED_RATE_LIMITER_BURST_SIZE - ); - } + "development" + }; + info!( + "Rate Limiter: {} mode, normal={}/s burst={}, enhanced={}/s burst={}", + mode, + RATE_LIMITER_PER_SECOND, + RATE_LIMITER_BURST_SIZE, + ENHANCED_RATE_LIMITER_PER_SECOND, + ENHANCED_RATE_LIMITER_BURST_SIZE + ); } From 74b7a11dab47fc27a9f85ad621411727e316d7b9 Mon Sep 17 00:00:00 2001 From: Nadu_Dev Date: Sun, 26 Apr 2026 12:01:40 +0530 Subject: [PATCH 07/18] refactor: update module structure and rename contributions router function for consistency --- src/models/mod.rs | 10 ++++++---- src/routes/api/v1/contributions_routes.rs | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/models/mod.rs b/src/models/mod.rs index 6fdbd45..2c11e65 100644 --- a/src/models/mod.rs +++ b/src/models/mod.rs @@ -1,4 +1,6 @@ -pub mod contributions_model; -pub mod health_model; -pub mod validation_model; -pub mod ytdlp_model; +pub mod contributions; +pub mod github_dto; +pub mod health; +pub mod validation; +pub mod ytdlp; +pub mod ytdlp_dto; diff --git a/src/routes/api/v1/contributions_routes.rs b/src/routes/api/v1/contributions_routes.rs index cd85e17..2962e88 100644 --- a/src/routes/api/v1/contributions_routes.rs +++ b/src/routes/api/v1/contributions_routes.rs @@ -2,6 +2,6 @@ use axum::{Router, routing::get}; use crate::{controllers::api::v1::contributions_controller::get_contributions, state::AppState}; -pub fn create_contributions_router(_state: AppState) -> Router { +pub fn router(_state: AppState) -> Router { Router::new().route("/", get(get_contributions)) } From 799617dd2d9905164a47e1b5b16a39af330e9a7b Mon Sep 17 00:00:00 2001 From: Nadu_Dev Date: Sun, 26 Apr 2026 12:01:56 +0530 Subject: [PATCH 08/18] refactor: update contributions router function for consistency and improve error handling in YtdlpManager --- src/routes/api/v1/mod.rs | 5 +-- src/services/ytdlp/mod.rs | 69 +++++++++++++++++++++++---------------- 2 files changed, 41 insertions(+), 33 deletions(-) diff --git a/src/routes/api/v1/mod.rs b/src/routes/api/v1/mod.rs index ea66758..113b7da 100644 --- a/src/routes/api/v1/mod.rs +++ b/src/routes/api/v1/mod.rs @@ -9,8 +9,5 @@ mod ytdlp_routes; pub fn router(state: AppState) -> Router { Router::new() .merge(ytdlp_routes::router(state.clone())) - .nest( - "/api/v1/contributions", - contributions_routes::create_contributions_router(state), - ) + .nest("/api/v1/contributions", contributions_routes::router(state)) } diff --git a/src/services/ytdlp/mod.rs b/src/services/ytdlp/mod.rs index a78c6a6..5b01687 100644 --- a/src/services/ytdlp/mod.rs +++ b/src/services/ytdlp/mod.rs @@ -1,6 +1,10 @@ use crate::{ config::AppConfig, - models::ytdlp_model::{YtdlpDownloadRequest, YtdlpJob, YtdlpJobStatus}, + error::AppError, + models::{ + ytdlp::{YtdlpJob, YtdlpJobStatus}, + ytdlp_dto::YtdlpDownloadRequest, + }, }; use dashmap::DashMap; use std::{ @@ -31,6 +35,8 @@ struct ParsedProgress { eta: Option, } +/// Manager for handling yt-dlp download jobs. +/// Manages a pool of concurrent downloads using semaphores and tracks job state in a concurrent map. #[derive(Clone, Debug)] pub struct YtdlpManager { cfg: Arc, @@ -40,7 +46,7 @@ pub struct YtdlpManager { } impl YtdlpManager { - /// Creates a new instance of `YtdlpManager`. + /// Creates a new instance of `YtdlpManager` and starts the background cleanup task. #[must_use] pub fn new(cfg: Arc) -> Self { let manager = Self { @@ -50,7 +56,6 @@ impl YtdlpManager { job_counter: Arc::new(std::sync::atomic::AtomicU64::new(1)), }; - // Use a weak reference to avoid keeping the manager alive indefinitely let jobs_weak = Arc::downgrade(&manager.jobs); tokio::spawn(async move { @@ -88,7 +93,7 @@ impl YtdlpManager { } /// Enqueues a download job and returns its initial state. - #[allow(clippy::expect_used)] + /// Spawns an asynchronous task to perform the actual download. pub fn enqueue_download(&self, payload: YtdlpDownloadRequest) -> YtdlpJob { let id = self.next_id(); let quality = payload.quality.as_deref().unwrap_or("best").to_string(); @@ -97,8 +102,8 @@ impl YtdlpManager { let output_dir_res = self.resolve_output_dir(payload.folder.as_deref()); let output_dir = output_dir_res - .clone() - .unwrap_or_else(|_| self.cfg.download_dir.clone()); + .as_ref() + .map_or_else(|_| self.cfg.download_dir.clone(), Clone::clone); let job = YtdlpJob { id: id.clone(), @@ -124,12 +129,16 @@ impl YtdlpManager { if let Err(error) = output_dir_res { self.update_job(&id, |job| { job.status = YtdlpJobStatus::Failed; - job.error = Some(error); + job.error = Some(error.to_string()); job.finished_at_unix = Some(now_unix()); }); + + #[allow(clippy::expect_used)] return self - .get_job(&id) - .expect("job should exist immediately after insert"); + .jobs + .get(&id) + .map(|e| e.value().clone()) + .expect("job should exist"); } let manager = self.clone(); @@ -142,12 +151,12 @@ impl YtdlpManager { job } - /// Retrieves a job by ID. + /// Retrieves a job by ID from the concurrent state map. pub fn get_job(&self, id: &str) -> Option { self.jobs.get(id).map(|entry| entry.value().clone()) } - /// Lists all jobs. + /// Returns a list of all currently tracked download jobs. pub fn list_jobs(&self) -> Vec { self.jobs .iter() @@ -163,13 +172,15 @@ impl YtdlpManager { format!("ytdlp-{ts}-{counter}") } - fn resolve_output_dir(&self, folder: Option<&str>) -> Result { + fn resolve_output_dir(&self, folder: Option<&str>) -> Result { let mut dir = PathBuf::from(&self.cfg.download_dir); if let Some(folder_str) = folder.filter(|f| !f.is_empty()) { let folder_path = Path::new(folder_str); if folder_path.is_absolute() { - return Err("folder must be a relative safe path".to_string()); + return Err(AppError::Validation( + "folder must be a relative safe path".to_string(), + )); } for component in folder_path.components() { @@ -177,7 +188,9 @@ impl YtdlpManager { component, Component::ParentDir | Component::RootDir | Component::Prefix(_) ) { - return Err("folder must be a relative safe path".to_string()); + return Err(AppError::Validation( + "folder must be a relative safe path".to_string(), + )); } } dir.push(folder_path); @@ -397,7 +410,6 @@ impl YtdlpManager { let sanitized_line = redact_client_progress_line(trimmed); - // Keep raw downloader output at debug level to avoid noisy info logs. debug!( "ytdlp progress id={} line={}", id, @@ -420,7 +432,7 @@ impl YtdlpManager { if parsed.eta.is_some() { job.progress_eta = parsed.eta; } - job.progress_message = Some(aria2_message.clone()); + job.progress_message = Some(aria2_message); job.updated_at_unix = Some(now_unix()); }); return; @@ -435,35 +447,34 @@ impl YtdlpManager { } } -// Reusable utility for client-safe progress text. fn redact_client_progress_line(line: &str) -> String { if line.contains("Destination") { return "[download] Destination: [REDACTED_PATH]".to_string(); } - line.split_whitespace() - .map(|token| { - if is_sensitive_token(token) { - "[REDACTED_PATH]".to_string() - } else { - token.to_string() - } - }) - .collect::>() - .join(" ") + let mut result = String::with_capacity(line.len() + 15); + for (i, token) in line.split_whitespace().enumerate() { + if i > 0 { + result.push(' '); + } + if is_sensitive_token(token) { + result.push_str("[REDACTED_PATH]"); + } else { + result.push_str(token); + } + } + result } fn is_sensitive_token(token: &str) -> bool { let cleaned = token.trim_matches(|c| c == '"' || c == '\'' || c == ',' || c == ';'); - // Redact common env interpolation patterns. if (cleaned.starts_with("${") && cleaned.ends_with('}')) || cleaned.starts_with('$') { return true; } let normalized = cleaned.replace('\\', "/").to_ascii_lowercase(); - // Redact known sensitive server-local path roots and absolute local paths. normalized.contains("/run/secrets") || normalized.contains("/home/app") || normalized.starts_with("/home/") From 47718011aa1e09423ae0e057394adfd18f254aa9 Mon Sep 17 00:00:00 2001 From: Nadu_Dev Date: Sun, 26 Apr 2026 12:02:04 +0530 Subject: [PATCH 09/18] refactor: reorganize contributions module structure and improve caching logic --- src/services/contributions.rs | 350 +++++++++++++++------------------- 1 file changed, 151 insertions(+), 199 deletions(-) diff --git a/src/services/contributions.rs b/src/services/contributions.rs index bbc386c..04a892f 100644 --- a/src/services/contributions.rs +++ b/src/services/contributions.rs @@ -1,21 +1,26 @@ use dashmap::DashMap; use reqwest::Client; -use serde::Deserialize; -use serde_json::json; +use serde::Serialize; +use std::borrow::Cow; use std::sync::Arc; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use tracing::info; use crate::{ error::AppError, - models::contributions_model::{ - ContributionCell, ContributionLegend, ContributionMeta, ContributionMonth, - ContributionRange, ContributionSummary, ContributionsResponse, + models::{ + contributions::{ + ContributionCell, ContributionLegend, ContributionMeta, ContributionMonth, + ContributionRange, ContributionSummary, ContributionsResponse, + }, + github_dto::{GithubContributionCalendar, GithubGqlResponse}, }, }; -const CACHE_TTL_SECONDS: u32 = 86400; -const CACHE_MAX_CAPACITY: u32 = 1000; +/// Cache Time-to-Live in seconds (3 hours). +/// This ensures mid-day updates are visible while staying within API rate limits. +const CACHE_TTL_SECONDS: u64 = 10800; +const CACHE_MAX_CAPACITY: usize = 1000; const SCHEMA_VERSION: u32 = 1; const PROVIDER_GITHUB: &str = "github"; const USER_AGENT: &str = "nadzu-backend"; @@ -25,65 +30,54 @@ const MONTH_LABELS: [&str; 12] = [ "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec", ]; -const CONTRIBUTION_COLORS: [&str; 5] = [ - "#2b2c3494", // Level 0 - "#9be9a8", // Level 1 - "#40c463", // Level 2 - "#30a14e", // Level 3 - "#216e39", // Level 4 +const CONTRIBUTION_COLORS: [Cow<'static, str>; 5] = [ + Cow::Borrowed("#2b2c3494"), // Level 0 (None) + Cow::Borrowed("#9be9a8"), // Level 1 (Low) + Cow::Borrowed("#40c463"), // Level 2 (Medium) + Cow::Borrowed("#30a14e"), // Level 3 (High) + Cow::Borrowed("#216e39"), // Level 4 (Very High) ]; -#[derive(Debug, Deserialize)] -struct GithubGqlResponse { - data: Option, - errors: Option>, -} - -#[derive(Debug, Deserialize)] -#[serde(rename_all = "camelCase")] -struct GithubGraphQLUser { - user: Option, -} - -#[derive(Debug, Deserialize)] -#[serde(rename_all = "camelCase")] -struct GithubUserNode { - contributions_collection: GithubContributionsCollection, -} - -#[derive(Debug, Deserialize)] -#[serde(rename_all = "camelCase")] -struct GithubContributionsCollection { - contribution_calendar: GithubContributionCalendar, -} - -#[derive(Debug, Deserialize)] -#[serde(rename_all = "camelCase")] -struct GithubContributionCalendar { - total_contributions: u32, - weeks: Vec, -} +const LEGEND_LABELS: [&str; 5] = ["No contributions", "Low", "Medium", "High", "Very high"]; + +const GITHUB_CONTRIBUTIONS_QUERY: &str = r" + query($username: String!) { + user(login: $username) { + contributionsCollection { + contributionCalendar { + totalContributions + weeks { + contributionDays { + date + weekday + contributionCount + contributionLevel + } + } + } + } + } + } +"; -#[derive(Debug, Deserialize)] -#[serde(rename_all = "camelCase")] -struct GithubWeek { - contribution_days: Vec, +#[derive(Debug, Serialize)] +struct GithubGqlRequest { + query: &'static str, + variables: GithubGqlVariables, } -#[derive(Debug, Deserialize)] -#[serde(rename_all = "camelCase")] -struct GithubContributionDay { - date: String, - weekday: u8, // 0 = Sunday - contribution_count: u32, - contribution_level: String, +#[derive(Debug, Serialize)] +struct GithubGqlVariables { + username: String, } +/// Service for fetching and caching GitHub contribution data. pub struct ContributionsService { http_client: Client, pat: String, default_username: String, graphql_url: String, + /// Cache stores (Username -> (Response, `ExpiryTimestamp`)) cache: Arc>, } @@ -98,6 +92,7 @@ impl std::fmt::Debug for ContributionsService { } impl ContributionsService { + /// Creates a new `ContributionsService` and starts the background cache cleanup task. pub fn new( http_client: Client, pat: String, @@ -107,8 +102,9 @@ impl ContributionsService { let cache = Arc::new(DashMap::new()); let cache_weak = Arc::downgrade(&cache); + // Background task to prune expired entries every 10 minutes tokio::spawn(async move { - let mut interval = tokio::time::interval(Duration::from_mins(10)); // Clean every 10 mins + let mut interval = tokio::time::interval(Duration::from_secs(600)); loop { interval.tick().await; if let Some(cache) = cache_weak.upgrade() { @@ -130,16 +126,20 @@ impl ContributionsService { } } + /// Seeds the cache with a predefined response (primarily for testing). pub fn seed_cache(&self, username: &str, response: ContributionsResponse, ttl_secs: u64) { let expires_at = now_unix() + ttl_secs; self.cache .insert(username.to_string(), (response, expires_at)); } + /// Returns the configured default GitHub username. pub fn get_default_username(&self) -> &str { &self.default_username } + /// Retrieves contributions for the given username, utilizing cache if available and valid. + /// Implements a "Midnight Snap" strategy to ensure the calendar refreshes at UTC midnight. pub async fn get_contributions( &self, username: &str, @@ -151,13 +151,14 @@ impl ContributionsService { return Err(AppError::Validation("Username cannot be empty".into())); } - //not allow any other username other than default for now. + // Restrict to default username for security/scope if username != self.default_username { return Err(AppError::Validation( "Only the default username is allowed".into(), )); } + // 1. Check Cache if let Some(entry) = self.cache.get(&cache_key) { let (cached_resp, expires_at) = entry.value(); if *expires_at > now { @@ -167,19 +168,25 @@ impl ContributionsService { } } + // 2. Fetch fresh data let resp_result = self.fetch_and_process(username, SystemTime::now()).await; match resp_result { Ok(new_resp) => { - let expires_at = now + u64::from(CACHE_TTL_SECONDS); - // Bounded: only insert if under capacity to prevent memory leaks - if self.cache.len() < CACHE_MAX_CAPACITY as usize { + // Determine TTL: 3 hours OR seconds until UTC midnight, whichever is sooner. + let seconds_since_midnight = now % 86400; + let seconds_until_midnight = 86400 - seconds_since_midnight; + let ttl = CACHE_TTL_SECONDS.min(seconds_until_midnight); + + let expires_at = now + ttl; + + if self.cache.len() < CACHE_MAX_CAPACITY { self.cache.insert(cache_key, (new_resp.clone(), expires_at)); } Ok(new_resp) } Err(e) => { - // Stale-cache fallback + // Fallback to stale cache on upstream failure if let Some(entry) = self.cache.get(&cache_key) { let mut resp = entry.value().0.clone(); resp.meta.cached = true; @@ -190,35 +197,18 @@ impl ContributionsService { } } + /// Fetches data from GitHub GraphQL API and processes it into the internal model. async fn fetch_and_process( &self, username: &str, fetched_at: SystemTime, ) -> Result { - let query = r" - query($username: String!) { - user(login: $username) { - contributionsCollection { - contributionCalendar { - totalContributions - weeks { - contributionDays { - date - weekday - contributionCount - contributionLevel - } - } - } - } - } - } - "; - - let payload = json!({ - "query": query, - "variables": { "username": username } - }); + let payload = GithubGqlRequest { + query: GITHUB_CONTRIBUTIONS_QUERY, + variables: GithubGqlVariables { + username: username.to_string(), + }, + }; let resp = self .http_client @@ -229,10 +219,10 @@ impl ContributionsService { .json(&payload) .send() .await - .map_err(|e| AppError::Internal(anyhow::anyhow!("Network or timeout error: {e}")))?; + .map_err(|e| AppError::UpstreamError(format!("Network or timeout error: {e}")))?; if !resp.status().is_success() { - return Err(AppError::Internal(anyhow::anyhow!( + return Err(AppError::UpstreamError(format!( "Upstream API returned status: {}", resp.status() ))); @@ -241,14 +231,14 @@ impl ContributionsService { let gh_resp: GithubGqlResponse = resp .json() .await - .map_err(|e| AppError::Internal(anyhow::anyhow!("JSON parsing error: {e}")))?; + .map_err(|e| AppError::UpstreamError(format!("JSON parsing error: {e}")))?; if let Some(errs) = gh_resp.errors && !errs.is_empty() { - return Err(AppError::Internal(anyhow::anyhow!( - "GitHub GraphQL returned errors" - ))); + return Err(AppError::UpstreamError( + "GitHub GraphQL returned errors".to_string(), + )); } let calendar = gh_resp @@ -256,51 +246,49 @@ impl ContributionsService { .and_then(|d| d.user) .map(|u| u.contributions_collection.contribution_calendar) .ok_or_else(|| { - AppError::Internal(anyhow::anyhow!("User not found or missing fields")) + AppError::UpstreamError("User not found or missing fields".to_string()) })?; - Ok(Self::transform_calendar(username, &calendar, fetched_at)) + Ok(Self::transform_calendar(username, calendar, fetched_at)) } - #[allow(clippy::too_many_lines)] + /// Transforms GitHub's raw calendar data into the simplified `ContributionsResponse`. fn transform_calendar( username: &str, - calendar: &GithubContributionCalendar, + calendar: GithubContributionCalendar, fetched_at: SystemTime, ) -> ContributionsResponse { - let mut cells = Vec::new(); + let mut cells = Vec::with_capacity(calendar.weeks.len() * 7); let mut months = Vec::new(); let mut max_daily_count = 0; - let mut last_month: Option = None; + let mut level_mins = [u32::MAX; 5]; + let mut level_maxs = [0u32; 5]; + level_mins[0] = 0; + + // Compare against "Today" in UTC to identify future cells or the current month let (current_year, current_month_num, current_day) = get_utc_date(SystemTime::now()); let current_date_str = format!("{current_year:04}-{current_month_num:02}-{current_day:02}"); - let fetched_at_str = format_iso_time(fetched_at); - for (week_idx, week) in calendar.weeks.iter().enumerate() { + let total_contributions = calendar.total_contributions; + let total_weeks = u32::try_from(calendar.weeks.len()).unwrap_or_default(); + + for (week_idx, week) in calendar.weeks.into_iter().enumerate() { let mut month_added_this_week = false; - for day in &week.contribution_days { + for day in week.contribution_days { if day.contribution_count > max_daily_count { max_daily_count = day.contribution_count; } - // Parse YYYY-MM-DD let (y_day, m_day, _) = parse_ymd(&day.date).unwrap_or((1970, 1, 1)); - if let Some(lm) = last_month { - if lm != m_day && !month_added_this_week { - months.push(ContributionMonth { - label: get_month_label(m_day), - week_index: week_idx, - }); - month_added_this_week = true; - } - } else if !month_added_this_week { + // Detect month transitions to add labels to the grid + if !month_added_this_week && (last_month != Some(m_day)) { months.push(ContributionMonth { - label: get_month_label(m_day), + label: Cow::Borrowed(get_month_label(m_day)), week_index: week_idx, }); month_added_this_week = true; @@ -318,87 +306,44 @@ impl ContributionsService { _ => 0, }; - let weekday_label = WEEKDAY_LABELS - .get(day.weekday as usize) - .unwrap_or(&"") - .to_string(); + // Track min/max for legend in the same pass + if (1..5).contains(&level) { + level_mins[level] = level_mins[level].min(day.contribution_count); + level_maxs[level] = level_maxs[level].max(day.contribution_count); + } + #[allow(clippy::cast_possible_truncation)] cells.push(ContributionCell { - date: day.date.clone(), + date: day.date, week_index: week_idx, weekday: day.weekday, - weekday_label, + weekday_label: Cow::Borrowed( + WEEKDAY_LABELS.get(day.weekday as usize).unwrap_or(&""), + ), count: day.contribution_count, - level, - color: CONTRIBUTION_COLORS[level as usize].to_string(), + level: level as u32, + color: CONTRIBUTION_COLORS[level].clone(), is_future, is_in_current_month, }); } } - let total_weeks = u32::try_from(calendar.weeks.len()).unwrap_or_default(); - - let mut level_mins = [u32::MAX; 5]; - let mut level_maxs = [0u32; 5]; - level_mins[0] = 0; - level_maxs[0] = 0; - - for cell in &cells { - let l = cell.level as usize; - if l < 5 && l > 0 { - if cell.count < level_mins[l] { - level_mins[l] = cell.count; - } - if cell.count > level_maxs[l] { - level_maxs[l] = cell.count; - } - } - } - - for min_val in level_mins.iter_mut().skip(1) { - if *min_val == u32::MAX { - *min_val = 0; - } - } - - let legend = vec![ - ContributionLegend { - level: 0, - label: "No contributions".into(), - min: level_mins[0], - max: level_maxs[0], - color: CONTRIBUTION_COLORS[0].to_string(), - }, - ContributionLegend { - level: 1, - label: "Low".into(), - min: level_mins[1], - max: level_maxs[1], - color: CONTRIBUTION_COLORS[1].to_string(), - }, - ContributionLegend { - level: 2, - label: "Medium".into(), - min: level_mins[2], - max: level_maxs[2], - color: CONTRIBUTION_COLORS[2].to_string(), - }, - ContributionLegend { - level: 3, - label: "High".into(), - min: level_mins[3], - max: level_maxs[3], - color: CONTRIBUTION_COLORS[3].to_string(), - }, - ContributionLegend { - level: 4, - label: "Very high".into(), - min: level_mins[4], - max: level_maxs[4], - color: CONTRIBUTION_COLORS[4].to_string(), - }, - ]; + // Finalize Legend + #[allow(clippy::cast_possible_truncation)] + let legend = (0..5) + .map(|i| ContributionLegend { + level: i as u32, + label: Cow::Borrowed(LEGEND_LABELS[i]), + min: if level_mins[i] == u32::MAX { + 0 + } else { + level_mins[i] + }, + max: level_maxs[i], + color: CONTRIBUTION_COLORS[i].clone(), + }) + .collect(); let from_date = cells.first().map(|c| c.date.clone()).unwrap_or_default(); let to_date = cells.last().map(|c| c.date.clone()).unwrap_or_default(); @@ -411,17 +356,18 @@ impl ContributionsService { timezone: "UTC".into(), }, summary: ContributionSummary { - total_contributions: calendar.total_contributions, + total_contributions, total_weeks, max_daily_count, }, + legend, months, cells, meta: ContributionMeta { provider: PROVIDER_GITHUB.into(), cached: false, - cache_ttl_seconds: CACHE_TTL_SECONDS, + cache_ttl_seconds: u32::try_from(CACHE_TTL_SECONDS).unwrap_or(86400), fetched_at: fetched_at_str, schema_version: SCHEMA_VERSION, }, @@ -429,24 +375,26 @@ impl ContributionsService { } } +/// Parses "YYYY-MM-DD" into (Year, Month, Day) fn parse_ymd(date: &str) -> Option<(u32, u32, u32)> { if date.len() < 10 { return None; } - let y = date.get(0..4)?.parse().ok()?; - let m = date.get(5..7)?.parse().ok()?; - let d = date.get(8..10)?.parse().ok()?; - Some((y, m, d)) + Some(( + date.get(0..4)?.parse().ok()?, + date.get(5..7)?.parse().ok()?, + date.get(8..10)?.parse().ok()?, + )) } -fn get_month_label(month: u32) -> String { - if (1..=12).contains(&month) { - MONTH_LABELS[(month - 1) as usize].to_string() - } else { - String::new() - } +/// Returns the short month name for a given month index (1-12) +fn get_month_label(month: u32) -> &'static str { + MONTH_LABELS + .get((month.saturating_sub(1)) as usize) + .unwrap_or(&"") } +/// Calculates UTC Year, Month, Day from `SystemTime` #[allow( clippy::cast_possible_truncation, clippy::cast_lossless, @@ -462,15 +410,16 @@ fn get_utc_date(sys_time: SystemTime) -> (u32, u32, u32) { let era = z / 146_097; let doe = (z - era * 146_097) as u32; let yoe = (doe - doe / 1460 + doe / 36524 - doe / 146_096) / 365; - let y = (yoe as u64) + (era * 400); + let y = (u64::from(yoe)) + (era * 400); let doy = doe - (365 * yoe + yoe / 4 - yoe / 100); let mp = (5 * doy + 2) / 153; let d = doy - (153 * mp + 2) / 5 + 1; let m = if mp < 10 { mp + 3 } else { mp - 9 }; - let year = y + (if m <= 2 { 1 } else { 0 }); - (year as u32, m as u32, d as u32) + let year = y + u64::from(m <= 2); + (year as u32, m, d) } +/// Formats `SystemTime` into ISO 8601 string (UTC) fn format_iso_time(sys_time: SystemTime) -> String { let secs = sys_time .duration_since(UNIX_EPOCH) @@ -478,12 +427,15 @@ fn format_iso_time(sys_time: SystemTime) -> String { .as_secs(); let (y, m, d) = get_utc_date(sys_time); let rem = secs % 86400; - let hh = rem / 3600; - let mm = (rem % 3600) / 60; - let ss = rem % 60; - format!("{y:04}-{m:02}-{d:02}T{hh:02}:{mm:02}:{ss:02}Z") + format!( + "{y:04}-{m:02}-{d:02}T{:02}:{:02}:{:02}Z", + rem / 3600, + (rem % 3600) / 60, + rem % 60 + ) } +/// Returns current Unix timestamp in seconds fn now_unix() -> u64 { SystemTime::now() .duration_since(UNIX_EPOCH) From 1b3234f553590f994d738226d367247fac45289e Mon Sep 17 00:00:00 2001 From: Nadu_Dev Date: Sun, 26 Apr 2026 12:02:11 +0530 Subject: [PATCH 10/18] refactor: update model imports and improve AppConfig initialization in tests --- tests/api/common.rs | 39 +++++++++--------- tests/api/contributions_tests.rs | 43 ++++++++++--------- tests/api/validation_tests.rs | 7 +--- tests/api/ytdlp_tests.rs | 16 ++++--- tests/layer_unit_tests.rs | 71 +++++++++++++------------------- 5 files changed, 79 insertions(+), 97 deletions(-) diff --git a/tests/api/common.rs b/tests/api/common.rs index 404dd16..c8fac3a 100644 --- a/tests/api/common.rs +++ b/tests/api/common.rs @@ -11,7 +11,7 @@ use nadzu::{ cors::build_cors, rate_limit::{RateLimiters, enforce_tiered_rate_limit}, }, - models::ytdlp_model::YtdlpDownloadRequest, + models::ytdlp_dto::YtdlpDownloadRequest, routes::create_router, services::{contributions::ContributionsService, ytdlp::YtdlpManager}, state::AppState, @@ -20,8 +20,7 @@ use serde_json::{Value, json}; use std::sync::Arc; use tower::ServiceExt; -pub use nadzu::middleware::api_key::API_KEY_HEADER; - +pub const API_KEY_HEADER: &str = "x-api-key"; pub const CAPTCHA_TOKEN_HEADER: &str = "x-captcha-token"; pub const CONTENT_TYPE_JSON: &str = "application/json"; pub const TEST_MASTER_API_KEY: &str = "test_master_key"; @@ -38,23 +37,23 @@ pub fn create_test_state_with_options( secret_key: Option<&str>, allowed_origins: Option<&str>, ) -> AppState { - let config = Arc::new(AppConfig { - name: "test".into(), - env: "test".into(), - host: "127.0.0.1".into(), - port: 8080, - allowed_origins: allowed_origins.map(str::to_string), - download_dir: "downloads".into(), - ytdlp_path: "yt-dlp".into(), - ytdlp_external_downloader: None, - ytdlp_external_downloader_args: None, - max_concurrent_downloads: 3, - captcha_secret_key: secret_key.map(str::to_string), - master_api_key: TEST_MASTER_API_KEY.into(), - github_pat: None, - github_username: None, - github_graphql_url: "https://api.github.com/graphql".into(), - }); + let config = Arc::new(AppConfig::new( + "test".into(), + "test".into(), + "127.0.0.1".into(), + 8080, + allowed_origins.map(str::to_string), + "downloads".into(), + "yt-dlp".into(), + None, + None, + 3, + secret_key.map(str::to_string), + TEST_MASTER_API_KEY.into(), + None, + None, + "https://api.github.com/graphql".into(), + )); let ytdlp_manager = Arc::new(YtdlpManager::new(config.clone())); let rate_limiters = Arc::new(RateLimiters::new()); diff --git a/tests/api/contributions_tests.rs b/tests/api/contributions_tests.rs index 9e9ff50..d5064a6 100644 --- a/tests/api/contributions_tests.rs +++ b/tests/api/contributions_tests.rs @@ -1,6 +1,6 @@ use crate::common::{TEST_MASTER_API_KEY, create_test_state_with_options, get, send_json}; use axum::http::StatusCode; -use nadzu::models::contributions_model::{ +use nadzu::models::contributions::{ ContributionMeta, ContributionRange, ContributionSummary, ContributionsResponse, }; use nadzu::routes::create_router; @@ -58,29 +58,32 @@ async fn get_contributions_hits_mock_server_when_cache_empty() { let mock_server = MockServer::start().await; // Create state pointing to mock server - let config = nadzu::config::AppConfig { - name: "test".into(), - env: "test".into(), - host: "127.0.0.1".into(), - port: 8080, - allowed_origins: None, - download_dir: "downloads".into(), - ytdlp_path: "yt-dlp".into(), - ytdlp_external_downloader: None, - ytdlp_external_downloader_args: None, - max_concurrent_downloads: 3, - captcha_secret_key: None, - master_api_key: TEST_MASTER_API_KEY.into(), - github_pat: Some("fake_pat".into()), - github_username: Some("nxdun".into()), - github_graphql_url: mock_server.uri(), - }; + let config = nadzu::config::AppConfig::new( + "test".into(), + "test".into(), + "127.0.0.1".into(), + 8080, + None, + "downloads".into(), + "yt-dlp".into(), + None, + None, + 3, + None, + TEST_MASTER_API_KEY.into(), + Some("fake_pat".into()), + Some("nxdun".into()), + mock_server.uri(), + ); let http_client = reqwest::Client::new(); let contributions_service = Arc::new(ContributionsService::new( http_client.clone(), - config.github_pat.clone().unwrap(), - config.github_username.clone().unwrap(), + config.github_pat.clone().unwrap_or_default(), + config + .github_username + .clone() + .unwrap_or_else(|| "nxdun".to_string()), config.github_graphql_url.clone(), )); diff --git a/tests/api/validation_tests.rs b/tests/api/validation_tests.rs index 82bf76d..5046d88 100644 --- a/tests/api/validation_tests.rs +++ b/tests/api/validation_tests.rs @@ -60,10 +60,5 @@ async fn validate_user_rejects_semantically_invalid_payload() { assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY); assert_eq!(body["status"], 422); assert_eq!(body["error_code"], "VALIDATION_ERROR"); - assert!( - body["message"] - .as_str() - .unwrap() - .contains("Validation failed") - ); + assert!(body["message"].as_str().unwrap().contains("messages")); } diff --git a/tests/api/ytdlp_tests.rs b/tests/api/ytdlp_tests.rs index 2e737d1..980928a 100644 --- a/tests/api/ytdlp_tests.rs +++ b/tests/api/ytdlp_tests.rs @@ -37,9 +37,8 @@ async fn ytdlp_enqueue_fails_when_secret_key_missing() { ) .await; - assert_eq!(status, StatusCode::INTERNAL_SERVER_ERROR); - assert_eq!(body["message"], "Internal Server Error"); - assert_eq!(body["error_code"], "INTERNAL_SERVER_ERROR"); + assert_eq!(status, StatusCode::SERVICE_UNAVAILABLE); + assert_eq!(body["error_code"], "SERVICE_UNAVAILABLE"); } #[tokio::test] @@ -56,9 +55,8 @@ async fn ytdlp_enqueue_fails_when_secret_key_empty() { ) .await; - assert_eq!(status, StatusCode::INTERNAL_SERVER_ERROR); - assert_eq!(body["message"], "Internal Server Error"); - assert_eq!(body["error_code"], "INTERNAL_SERVER_ERROR"); + assert_eq!(status, StatusCode::SERVICE_UNAVAILABLE); + assert_eq!(body["error_code"], "SERVICE_UNAVAILABLE"); } #[tokio::test] @@ -90,9 +88,9 @@ async fn ytdlp_get_job_returns_seeded_job() { let (status, body) = send_json(&app, get(&uri)).await; assert_eq!(status, StatusCode::OK); - assert_eq!(body["job"]["id"], job_id); - assert_eq!(body["job"]["url"], SAMPLE_YTDLP_URL); - assert_eq!(body["job"]["status"], "queued"); + assert_eq!(body["id"], job_id); + assert_eq!(body["url"], SAMPLE_YTDLP_URL); + assert_eq!(body["status"], "queued"); } #[tokio::test] diff --git a/tests/layer_unit_tests.rs b/tests/layer_unit_tests.rs index 5bd6476..505d931 100644 --- a/tests/layer_unit_tests.rs +++ b/tests/layer_unit_tests.rs @@ -2,40 +2,36 @@ use axum::http::HeaderMap; use nadzu::{ config::AppConfig, - middleware::{ - api_key::{API_KEY_HEADER, has_valid_master_api_key}, - rate_limit::is_production, - }, - models::health_model::Health, + middleware::{api_key::has_valid_master_api_key, rate_limit::is_production}, + models::health::Health, }; fn test_config(env: &str) -> AppConfig { - AppConfig { - name: "nadzu-test".to_string(), - env: env.to_string(), - host: "127.0.0.1".to_string(), - port: 8080, - allowed_origins: None, - download_dir: "downloads".to_string(), - ytdlp_path: "yt-dlp".to_string(), - ytdlp_external_downloader: None, - ytdlp_external_downloader_args: None, - max_concurrent_downloads: 3, - captcha_secret_key: None, - master_api_key: "master_key".to_string(), - github_pat: None, - github_username: None, - github_graphql_url: "https://api.github.com/graphql".to_string(), - } + AppConfig::new( + "nadzu-test".to_string(), + env.to_string(), + "127.0.0.1".to_string(), + 8080, + None, + "downloads".to_string(), + "yt-dlp".to_string(), + None, + None, + 3, + None, + "master_key".to_string(), + None, + None, + "https://api.github.com/graphql".to_string(), + ) } #[test] #[allow(clippy::unwrap_used)] fn has_valid_master_api_key_returns_true_for_matching_header() { - // Utility layer contract: header parser and key matcher should accept valid key. let config = test_config("test"); let mut headers = HeaderMap::new(); - headers.insert(API_KEY_HEADER, "master_key".parse().unwrap()); + headers.insert("x-api-key", "master_key".parse().unwrap()); assert!(has_valid_master_api_key(&headers, &config)); } @@ -43,14 +39,13 @@ fn has_valid_master_api_key_returns_true_for_matching_header() { #[test] #[allow(clippy::unwrap_used)] fn has_valid_master_api_key_returns_false_for_missing_or_wrong_header() { - // Utility layer contract: missing/wrong keys must be rejected consistently. let config = test_config("test"); let empty_headers = HeaderMap::new(); assert!(!has_valid_master_api_key(&empty_headers, &config)); let mut wrong_headers = HeaderMap::new(); - wrong_headers.insert(API_KEY_HEADER, "wrong_key".parse().unwrap()); + wrong_headers.insert("x-api-key", "wrong_key".parse().unwrap()); assert!(!has_valid_master_api_key(&wrong_headers, &config)); let mut unrelated_headers = HeaderMap::new(); @@ -60,7 +55,6 @@ fn has_valid_master_api_key_returns_false_for_missing_or_wrong_header() { #[test] fn is_production_uses_environment_flag() { - // Rate-limit policy helper should only treat literal production env as production. assert!(is_production(&test_config("production"))); assert!(!is_production(&test_config("test"))); assert!(!is_production(&test_config("staging"))); @@ -68,7 +62,6 @@ fn is_production_uses_environment_flag() { #[test] fn health_ok_model_contains_expected_values() { - // Model layer contract for health payload should remain stable. let health = Health::ok(); assert_eq!(health.status, "ok"); @@ -77,18 +70,12 @@ fn health_ok_model_contains_expected_values() { } #[test] -fn app_config_from_env_exits_when_master_api_key_missing() -> std::io::Result<()> { - let helper_binary = env!("CARGO_BIN_EXE_config_exit"); - - let output = std::process::Command::new(helper_binary) - .env_remove("MASTER_API_KEY") - .output()?; - - assert!( - !output.status.success(), - "expected exit status to be non-zero" - ); - assert_eq!(output.status.code(), Some(1)); - - Ok(()) +fn app_config_from_env_fails_when_master_api_key_missing() { + // Clear env to ensure a clean state for the test + #[allow(deprecated)] + unsafe { + std::env::remove_var("MASTER_API_KEY"); + } + let result = AppConfig::from_env(); + assert!(result.is_err()); } From 7a75f1d2bff18179cad40065200b55083f8b1234 Mon Sep 17 00:00:00 2001 From: Nadu_Dev Date: Sun, 26 Apr 2026 12:05:17 +0530 Subject: [PATCH 11/18] refactor: update cache pruning interval from 600 seconds to 10 minutes --- src/services/contributions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/contributions.rs b/src/services/contributions.rs index 04a892f..fede238 100644 --- a/src/services/contributions.rs +++ b/src/services/contributions.rs @@ -104,7 +104,7 @@ impl ContributionsService { // Background task to prune expired entries every 10 minutes tokio::spawn(async move { - let mut interval = tokio::time::interval(Duration::from_secs(600)); + let mut interval = tokio::time::interval(Duration::from_mins(10)); loop { interval.tick().await; if let Some(cache) = cache_weak.upgrade() { From 67ba737d77cd707adc24352e6463b37f453e8607 Mon Sep 17 00:00:00 2001 From: Nadu_Dev Date: Sun, 26 Apr 2026 12:22:13 +0530 Subject: [PATCH 12/18] refactor: update environment variable formatting and enhance error handling in AppError --- .env.example | 6 +++--- src/config.rs | 1 + src/error.rs | 13 ++++++++----- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/.env.example b/.env.example index d0bda3d..caf00bf 100644 --- a/.env.example +++ b/.env.example @@ -43,6 +43,6 @@ TF_VAR_RUST_LOG = "REPLACE_WITH_TF_VAR_RUST_LOG" TF_VAR_MAX_CONCURRENT_DOWNLOADS = "REPLACE_WITH_TF_VAR_MAX_CONCURRENT_DOWNLOADS" TF_VAR_APP_HOST = "REPLACE_WITH_TF_VAR_APP_HOST" TF_VAR_YTDLP_PATH = "REPLACE_WITH_TF_VAR_YTDLP_PATH" -TF_VAR_DOWNLOAD_DIR = "REPLACE_WITH_TF_VAR_DOWNLOAD_DIR" -TF_VAR_WARP_LICENSE_KEY = "REPLACE_WITH_TF_VAR_WARP_LICENSE_KEY" -TF_VAR_MASTER_API_KEY = "REPLACE_WITH_TF_VAR_MASTER_API_KEY" +TF_VAR_DOWNLOAD_DIR="REPLACE_WITH_TF_VAR_DOWNLOAD_DIR" +TF_VAR_MASTER_API_KEY="REPLACE_WITH_TF_VAR_MASTER_API_KEY" +TF_VAR_WARP_LICENSE_KEY="REPLACE_WITH_TF_VAR_WARP_LICENSE_KEY" diff --git a/src/config.rs b/src/config.rs index 268f058..9d55865 100644 --- a/src/config.rs +++ b/src/config.rs @@ -7,6 +7,7 @@ use crate::middleware::constant_time_eq; pub enum ConfigError { #[error("Missing required environment variable: {0}")] MissingVar(String), + // TODO: Use this variant when validating env values in `from_env()` #[error("Invalid value for {key}: {details}")] InvalidValue { key: String, details: String }, } diff --git a/src/error.rs b/src/error.rs index 6797087..e0ccf3c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -78,11 +78,14 @@ impl IntoResponse for AppError { msg.clone(), Some("CONFLICT".to_string()), ), - Self::UpstreamError(msg) => ( - StatusCode::BAD_GATEWAY, - msg.clone(), - Some("UPSTREAM_ERROR".to_string()), - ), + Self::UpstreamError(msg) => { + tracing::error!("Upstream service error: {}", msg); + ( + StatusCode::BAD_GATEWAY, + "Upstream service error".to_string(), + Some("UPSTREAM_ERROR".to_string()), + ) + } Self::ServiceUnavailable(msg) => ( StatusCode::SERVICE_UNAVAILABLE, msg.clone(), From 5c23a9fb9733366559495a0db39fcf085974bce5 Mon Sep 17 00:00:00 2001 From: Nadu_Dev Date: Sun, 26 Apr 2026 12:23:17 +0530 Subject: [PATCH 13/18] refactor: optimize job response construction and enhance CORS matcher initialization --- src/controllers/api/v1/ytdlp_controller.rs | 6 ++++-- src/middleware/cors.rs | 2 +- src/models/github_dto.rs | 7 +++++++ src/routes/api/v1/mod.rs | 5 +++++ 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/controllers/api/v1/ytdlp_controller.rs b/src/controllers/api/v1/ytdlp_controller.rs index a8eaf51..2f4ef42 100644 --- a/src/controllers/api/v1/ytdlp_controller.rs +++ b/src/controllers/api/v1/ytdlp_controller.rs @@ -62,8 +62,10 @@ pub async fn list_download_jobs( State(state): State, ) -> Result { let jobs = state.ytdlp_manager.list_jobs(); - let response_jobs: Vec = - jobs.into_iter().map(YtdlpJobResponse::from).collect(); + let mut response_jobs = Vec::with_capacity(jobs.len()); + for job in jobs { + response_jobs.push(YtdlpJobResponse::from(job)); + } Ok(( StatusCode::OK, Json(YtdlpListResponse { diff --git a/src/middleware/cors.rs b/src/middleware/cors.rs index f2f302e..b828a61 100644 --- a/src/middleware/cors.rs +++ b/src/middleware/cors.rs @@ -50,7 +50,7 @@ pub fn build_cors(config: &AppConfig) -> CorsLayer { info!("CORS allowed origins: {:?}", raw_origins); // PRE-COMPUTE MATCHERS (Runs only once on startup) - let mut matchers = Vec::new(); + let mut matchers = Vec::with_capacity(raw_origins.len()); for allowed in raw_origins { if allowed.contains('*') { let parts: Vec<&str> = allowed.split('*').collect(); diff --git a/src/models/github_dto.rs b/src/models/github_dto.rs index ba8b457..a127fe9 100644 --- a/src/models/github_dto.rs +++ b/src/models/github_dto.rs @@ -1,29 +1,34 @@ use serde::Deserialize; +/// Top-level GraphQL response containing data and errors from GitHub. #[derive(Debug, Deserialize)] pub struct GithubGqlResponse { pub data: Option, pub errors: Option>, } +/// Wrapper for the user node in the GraphQL response (camelCase mapped). #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct GithubGraphQLUser { pub user: Option, } +/// Contains the user's contributions collection. #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct GithubUserNode { pub contributions_collection: GithubContributionsCollection, } +/// The contributions container holding the calendar. #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct GithubContributionsCollection { pub contribution_calendar: GithubContributionCalendar, } +/// The contribution calendar containing total contributions and weekly buckets. #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct GithubContributionCalendar { @@ -31,12 +36,14 @@ pub struct GithubContributionCalendar { pub weeks: Vec, } +/// A weekly bucket of contribution days. #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct GithubWeek { pub contribution_days: Vec, } +/// A single-day record of contributions (date, weekday, count, level). #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct GithubContributionDay { diff --git a/src/routes/api/v1/mod.rs b/src/routes/api/v1/mod.rs index 113b7da..07ceef5 100644 --- a/src/routes/api/v1/mod.rs +++ b/src/routes/api/v1/mod.rs @@ -5,6 +5,11 @@ use crate::state::AppState; mod contributions_routes; mod ytdlp_routes; +/// Constructs and returns the primary API v1 router. +/// +/// This router merges the `ytdlp_routes::router` and nests the +/// `contributions_routes::router` under the `/api/v1/contributions` prefix. +/// It takes `AppState` by value to share ownership across routes. #[allow(unreachable_pub)] pub fn router(state: AppState) -> Router { Router::new() From 2014414225bd3c29b308dc0c8b1eca957d677b85 Mon Sep 17 00:00:00 2001 From: Nadu_Dev Date: Sun, 26 Apr 2026 12:23:24 +0530 Subject: [PATCH 14/18] refactor: improve contributions service initialization and enhance validation error handling in tests --- tests/api/contributions_tests.rs | 7 ++----- tests/api/validation_tests.rs | 10 +++++++++- tests/layer_unit_tests.rs | 9 +++++++++ 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/tests/api/contributions_tests.rs b/tests/api/contributions_tests.rs index d5064a6..5ae8c09 100644 --- a/tests/api/contributions_tests.rs +++ b/tests/api/contributions_tests.rs @@ -79,11 +79,8 @@ async fn get_contributions_hits_mock_server_when_cache_empty() { let http_client = reqwest::Client::new(); let contributions_service = Arc::new(ContributionsService::new( http_client.clone(), - config.github_pat.clone().unwrap_or_default(), - config - .github_username - .clone() - .unwrap_or_else(|| "nxdun".to_string()), + config.github_pat.clone().unwrap(), + config.github_username.clone().unwrap(), config.github_graphql_url.clone(), )); diff --git a/tests/api/validation_tests.rs b/tests/api/validation_tests.rs index 5046d88..835b92d 100644 --- a/tests/api/validation_tests.rs +++ b/tests/api/validation_tests.rs @@ -60,5 +60,13 @@ async fn validate_user_rejects_semantically_invalid_payload() { assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY); assert_eq!(body["status"], 422); assert_eq!(body["error_code"], "VALIDATION_ERROR"); - assert!(body["message"].as_str().unwrap().contains("messages")); + let message_str = body["message"].as_str().unwrap(); + let message_val: serde_json::Value = serde_json::from_str(message_str).unwrap(); + assert!(message_val.is_array()); + let validation_errors = message_val.as_array().unwrap(); + assert!(!validation_errors.is_empty()); + let first_error = &validation_errors[0]; + assert!(first_error.is_object()); + assert!(first_error["field"].is_string()); + assert!(first_error["messages"].is_array()); } diff --git a/tests/layer_unit_tests.rs b/tests/layer_unit_tests.rs index 505d931..6b52ff2 100644 --- a/tests/layer_unit_tests.rs +++ b/tests/layer_unit_tests.rs @@ -71,6 +71,7 @@ fn health_ok_model_contains_expected_values() { #[test] fn app_config_from_env_fails_when_master_api_key_missing() { + let original_master_key = std::env::var("MASTER_API_KEY").ok(); // Clear env to ensure a clean state for the test #[allow(deprecated)] unsafe { @@ -78,4 +79,12 @@ fn app_config_from_env_fails_when_master_api_key_missing() { } let result = AppConfig::from_env(); assert!(result.is_err()); + + // Restore the original environment variable if it existed + if let Some(key) = original_master_key { + #[allow(deprecated)] + unsafe { + std::env::set_var("MASTER_API_KEY", key); + } + } } From 952b0c11b087f9225d459362f3336d08b70c524b Mon Sep 17 00:00:00 2001 From: Nadu_Dev Date: Mon, 27 Apr 2026 10:12:38 +0530 Subject: [PATCH 15/18] refactor: standardize environment variable formatting and enhance error handling in configuration --- .env.example | 76 ++++++++++++++++---------------- src/app.rs | 2 +- src/config.rs | 59 +++++++++++++++++++++---- src/error.rs | 44 ++++++++++-------- src/middleware/captcha.rs | 3 +- tests/api/contributions_tests.rs | 2 +- tests/layer_unit_tests.rs | 50 +++++++++++++++------ 7 files changed, 153 insertions(+), 83 deletions(-) diff --git a/.env.example b/.env.example index caf00bf..65c0e89 100644 --- a/.env.example +++ b/.env.example @@ -1,48 +1,48 @@ ### ------------------------- ### Application Runtime / Local ### ------------------------- -APP_NAME = "REPLACE_WITH_APP_NAME" -APP_HOST = "REPLACE_WITH_APP_HOST" -APP_PORT = "REPLACE_WITH_APP_PORT" -APP_ENV = "REPLACE_WITH_APP_ENV" -ALLOWED_ORIGINS = "REPLACE_WITH_ALLOWED_ORIGINS" -RUST_LOG = "REPLACE_WITH_RUST_LOG" -DOWNLOAD_DIR = "REPLACE_WITH_DOWNLOAD_DIR" -YTDLP_PATH = "REPLACE_WITH_YTDLP_PATH" -YTDLP_EXTERNAL_DOWNLOADER = "REPLACE_WITH_YTDLP_EXTERNAL_DOWNLOADER" -YTDLP_EXTERNAL_DOWNLOADER_ARGS = "REPLACE_WITH_YTDLP_EXTERNAL_DOWNLOADER_ARGS" -CAPTCHA_SECRET_KEY = "REPLACE_WITH_CAPTCHA_SECRET_KEY" -MAX_CONCURRENT_DOWNLOADS = "REPLACE_WITH_MAX_CONCURRENT_DOWNLOADS" -MASTER_API_KEY = "REPLACE_WITH_MASTER_API_KEY" -GITHUB_PAT = "REPLACE_WITH_GITHUB_PAT" -GITHUB_USERNAME = "REPLACE_WITH_GITHUB_USERNAME" -GITHUB_GRAPHQL_URL = "https://api.github.com/graphql" -WARP_LICENSE_KEY = "REPLACE_WITH_WARP_LICENSE_KEY" +APP_NAME=REPLACE_WITH_APP_NAME +APP_HOST=REPLACE_WITH_APP_HOST +APP_PORT=REPLACE_WITH_APP_PORT +APP_ENV=REPLACE_WITH_APP_ENV +ALLOWED_ORIGINS=REPLACE_WITH_ALLOWED_ORIGINS +RUST_LOG=REPLACE_WITH_RUST_LOG +DOWNLOAD_DIR=REPLACE_WITH_DOWNLOAD_DIR +YTDLP_PATH=REPLACE_WITH_YTDLP_PATH +YTDLP_EXTERNAL_DOWNLOADER=REPLACE_WITH_YTDLP_EXTERNAL_DOWNLOADER +YTDLP_EXTERNAL_DOWNLOADER_ARGS=REPLACE_WITH_YTDLP_EXTERNAL_DOWNLOADER_ARGS +CAPTCHA_SECRET_KEY=REPLACE_WITH_CAPTCHA_SECRET_KEY +MAX_CONCURRENT_DOWNLOADS=REPLACE_WITH_MAX_CONCURRENT_DOWNLOADS +MASTER_API_KEY=REPLACE_WITH_MASTER_API_KEY +GITHUB_PAT=REPLACE_WITH_GITHUB_PAT +GITHUB_USERNAME=REPLACE_WITH_GITHUB_USERNAME +GITHUB_GRAPHQL_URL=https://api.github.com/graphql +WARP_LICENSE_KEY=REPLACE_WITH_WARP_LICENSE_KEY ### ------------------------- ### Terraform / Infra - If you need to deploy ### ------------------------- -AWS_ACCESS_KEY_ID = "REPLACE_WITH_AWS_ACCESS_KEY_ID" -AWS_SECRET_ACCESS_KEY = "REPLACE_WITH_AWS_SECRET_ACCESS_KEY" -AWS_ENDPOINT_URL_S3 = "https://REPLACE_WITH_R2_ACCOUNT_ID.r2.cloudflarestorage.com" -AWS_S3_BUCKET_NAME = "REPLACE_WITH_AWS_S3_BUCKET_NAME" +AWS_ACCESS_KEY_ID=REPLACE_WITH_AWS_ACCESS_KEY_ID +AWS_SECRET_ACCESS_KEY=REPLACE_WITH_AWS_SECRET_ACCESS_KEY +AWS_ENDPOINT_URL_S3=https://REPLACE_WITH_R2_ACCOUNT_ID.r2.cloudflarestorage.com +AWS_S3_BUCKET_NAME=REPLACE_WITH_AWS_S3_BUCKET_NAME # Terraform provider secrets -TF_VAR_DO_TOKEN = "REPLACE_WITH_DIGITALOCEAN_TOKEN" -TF_VAR_CLOUDFLARE_API_TOKEN = "REPLACE_WITH_CLOUDFLARE_API_TOKEN" +TF_VAR_DO_TOKEN=REPLACE_WITH_DIGITALOCEAN_TOKEN +TF_VAR_CLOUDFLARE_API_TOKEN=REPLACE_WITH_CLOUDFLARE_API_TOKEN # Production DOCKER registry credentials (GHCR) -TF_VAR_GHCR_PAT = "REPLACE_WITH_GHCR_PAT" -TF_VAR_GITHUB_USERNAME = "REPLACE_WITH_GITHUB_USERNAME" +TF_VAR_GHCR_PAT=REPLACE_WITH_GHCR_PAT +TF_VAR_GITHUB_USERNAME=REPLACE_WITH_GITHUB_USERNAME # ENV values to pass to Production -TF_VAR_APP_ENV = "REPLACE_WITH_TF_VAR_APP_ENV" -TF_VAR_CAPTCHA_SECRET_KEY = "REPLACE_WITH_TF_VAR_CAPTCHA_SECRET_KEY" -TF_VAR_APP_PORT = "REPLACE_WITH_TF_VAR_APP_PORT" -TF_VAR_HOST_PORT = "REPLACE_WITH_TF_VAR_HOST_PORT" -TF_VAR_APP_NAME = "REPLACE_WITH_TF_VAR_APP_NAME" -TF_VAR_ALLOWED_ORIGINS = "REPLACE_WITH_TF_VAR_ALLOWED_ORIGINS" -TF_VAR_RUST_LOG = "REPLACE_WITH_TF_VAR_RUST_LOG" -TF_VAR_MAX_CONCURRENT_DOWNLOADS = "REPLACE_WITH_TF_VAR_MAX_CONCURRENT_DOWNLOADS" -TF_VAR_APP_HOST = "REPLACE_WITH_TF_VAR_APP_HOST" -TF_VAR_YTDLP_PATH = "REPLACE_WITH_TF_VAR_YTDLP_PATH" -TF_VAR_DOWNLOAD_DIR="REPLACE_WITH_TF_VAR_DOWNLOAD_DIR" -TF_VAR_MASTER_API_KEY="REPLACE_WITH_TF_VAR_MASTER_API_KEY" -TF_VAR_WARP_LICENSE_KEY="REPLACE_WITH_TF_VAR_WARP_LICENSE_KEY" +TF_VAR_APP_ENV=REPLACE_WITH_TF_VAR_APP_ENV +TF_VAR_CAPTCHA_SECRET_KEY=REPLACE_WITH_TF_VAR_CAPTCHA_SECRET_KEY +TF_VAR_APP_PORT=REPLACE_WITH_TF_VAR_APP_PORT +TF_VAR_HOST_PORT=REPLACE_WITH_TF_VAR_HOST_PORT +TF_VAR_APP_NAME=REPLACE_WITH_TF_VAR_APP_NAME +TF_VAR_ALLOWED_ORIGINS=REPLACE_WITH_TF_VAR_ALLOWED_ORIGINS +TF_VAR_RUST_LOG=REPLACE_WITH_TF_VAR_RUST_LOG +TF_VAR_MAX_CONCURRENT_DOWNLOADS=REPLACE_WITH_TF_VAR_MAX_CONCURRENT_DOWNLOADS +TF_VAR_APP_HOST=REPLACE_WITH_TF_VAR_APP_HOST +TF_VAR_YTDLP_PATH=REPLACE_WITH_TF_VAR_YTDLP_PATH +TF_VAR_DOWNLOAD_DIR=REPLACE_WITH_TF_VAR_DOWNLOAD_DIR +TF_VAR_MASTER_API_KEY=REPLACE_WITH_TF_VAR_MASTER_API_KEY +TF_VAR_WARP_LICENSE_KEY=REPLACE_WITH_TF_VAR_WARP_LICENSE_KEY diff --git a/src/app.rs b/src/app.rs index 6d62752..576642a 100644 --- a/src/app.rs +++ b/src/app.rs @@ -62,7 +62,7 @@ pub async fn run() { let contributions_service = Arc::new(crate::services::contributions::ContributionsService::new( http_client.clone(), - config.github_pat.clone().unwrap_or_default(), + config.github_pat().unwrap_or_default().to_string(), config .github_username .clone() diff --git a/src/config.rs b/src/config.rs index 9d55865..a183dca 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,4 +1,4 @@ -use std::env; +use std::{env, fmt}; use thiserror::Error; use crate::middleware::constant_time_eq; @@ -7,7 +7,6 @@ use crate::middleware::constant_time_eq; pub enum ConfigError { #[error("Missing required environment variable: {0}")] MissingVar(String), - // TODO: Use this variant when validating env values in `from_env()` #[error("Invalid value for {key}: {details}")] InvalidValue { key: String, details: String }, } @@ -28,6 +27,15 @@ fn env_or(key: &str, default: &str) -> T { }) } +/// Helper: Fetches and parses an environment variable, returning ConfigError::InvalidValue on parse failure. +fn env_parse(key: &str, default: &str) -> Result { + let val = env::var(key).unwrap_or_else(|_| default.to_string()); + val.parse::().map_err(|_| ConfigError::InvalidValue { + key: key.to_string(), + details: format!("'{}' is not a valid {}", val, std::any::type_name::()), + }) +} + /// Helper: Fetches an optional env var and trims it, returning None if empty. fn env_opt(key: &str) -> Option { env::var(key) @@ -37,7 +45,7 @@ fn env_opt(key: &str) -> Option { } /// Application configuration loaded from environment variables. -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct AppConfig { pub name: String, pub env: String, @@ -49,13 +57,38 @@ pub struct AppConfig { pub ytdlp_external_downloader: Option, pub ytdlp_external_downloader_args: Option, pub max_concurrent_downloads: usize, - pub captcha_secret_key: Option, - master_api_key: String, // Private: use check_api_key - pub github_pat: Option, + captcha_secret_key: Option, + master_api_key: String, + github_pat: Option, pub github_username: Option, pub github_graphql_url: String, } +impl fmt::Debug for AppConfig { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("AppConfig") + .field("name", &self.name) + .field("env", &self.env) + .field("host", &self.host) + .field("port", &self.port) + .field("allowed_origins", &self.allowed_origins) + .field("download_dir", &self.download_dir) + .field("ytdlp_path", &self.ytdlp_path) + .field("ytdlp_external_downloader", &self.ytdlp_external_downloader) + .field( + "ytdlp_external_downloader_args", + &self.ytdlp_external_downloader_args, + ) + .field("max_concurrent_downloads", &self.max_concurrent_downloads) + .field("captcha_secret_key", &"***") + .field("master_api_key", &"***") + .field("github_pat", &"***") + .field("github_username", &self.github_username) + .field("github_graphql_url", &self.github_graphql_url) + .finish() + } +} + impl AppConfig { /// Internal constructor for creating config instances. #[allow(clippy::too_many_arguments)] @@ -104,13 +137,13 @@ impl AppConfig { env_or("APP_NAME", "nadzu-backend"), env_or("APP_ENV", "production"), env_or("APP_HOST", "127.0.0.1"), - env_or("APP_PORT", "8080"), + env_parse("APP_PORT", "8080")?, env_opt("ALLOWED_ORIGINS"), env_or("DOWNLOAD_DIR", "downloads"), env_or("YTDLP_PATH", "yt-dlp"), env_opt("YTDLP_EXTERNAL_DOWNLOADER"), env_opt("YTDLP_EXTERNAL_DOWNLOADER_ARGS"), - env_or("MAX_CONCURRENT_DOWNLOADS", "3"), + env_parse("MAX_CONCURRENT_DOWNLOADS", "3")?, env_opt("CAPTCHA_SECRET_KEY"), master_api_key, env_opt("GITHUB_PAT"), @@ -125,6 +158,16 @@ impl AppConfig { constant_time_eq(provided_key, &self.master_api_key) } + /// Returns the GitHub Personal Access Token if configured. + pub fn github_pat(&self) -> Option<&str> { + self.github_pat.as_deref() + } + + /// Returns the CAPTCHA secret key if configured. + pub fn captcha_secret_key(&self) -> Option<&str> { + self.captcha_secret_key.as_deref() + } + /// Returns the full address string for the server. pub fn addr(&self) -> String { format!("{}:{}", self.host, self.port) diff --git a/src/error.rs b/src/error.rs index e0ccf3c..8b54416 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,3 +1,5 @@ +use std::borrow::Cow; + use axum::{ Json, http::StatusCode, @@ -10,9 +12,9 @@ use tracing::error; #[derive(Debug, Serialize)] struct ErrorResponse { status: u16, - message: String, + message: Cow<'static, str>, #[serde(skip_serializing_if = "Option::is_none")] - error_code: Option, + error_code: Option>, } #[derive(Debug, Error)] @@ -44,52 +46,56 @@ pub enum AppError { impl IntoResponse for AppError { fn into_response(self) -> Response { - let (status, message, error_code) = match &self { + let (status, message, error_code): ( + StatusCode, + Cow<'static, str>, + Option>, + ) = match &self { Self::Internal(err) => { error!("Internal error occurred: {:?}", err); ( StatusCode::INTERNAL_SERVER_ERROR, - "Internal Server Error".to_string(), - Some("INTERNAL_SERVER_ERROR".to_string()), + Cow::Borrowed("Internal Server Error"), + Some(Cow::Borrowed("INTERNAL_SERVER_ERROR")), ) } Self::NotFound(msg) => ( StatusCode::NOT_FOUND, - msg.clone(), - Some("NOT_FOUND".to_string()), + Cow::Owned(msg.clone()), + Some(Cow::Borrowed("NOT_FOUND")), ), Self::Validation(msg) => ( StatusCode::UNPROCESSABLE_ENTITY, - msg.clone(), - Some("VALIDATION_ERROR".to_string()), + Cow::Owned(msg.clone()), + Some(Cow::Borrowed("VALIDATION_ERROR")), ), Self::Unauthorized(msg) => ( StatusCode::UNAUTHORIZED, - msg.clone(), - Some("UNAUTHORIZED".to_string()), + Cow::Owned(msg.clone()), + Some(Cow::Borrowed("UNAUTHORIZED")), ), Self::Forbidden(msg) => ( StatusCode::FORBIDDEN, - msg.clone(), - Some("FORBIDDEN".to_string()), + Cow::Owned(msg.clone()), + Some(Cow::Borrowed("FORBIDDEN")), ), Self::Conflict(msg) => ( StatusCode::CONFLICT, - msg.clone(), - Some("CONFLICT".to_string()), + Cow::Owned(msg.clone()), + Some(Cow::Borrowed("CONFLICT")), ), Self::UpstreamError(msg) => { tracing::error!("Upstream service error: {}", msg); ( StatusCode::BAD_GATEWAY, - "Upstream service error".to_string(), - Some("UPSTREAM_ERROR".to_string()), + Cow::Borrowed("Upstream service error"), + Some(Cow::Borrowed("UPSTREAM_ERROR")), ) } Self::ServiceUnavailable(msg) => ( StatusCode::SERVICE_UNAVAILABLE, - msg.clone(), - Some("SERVICE_UNAVAILABLE".to_string()), + Cow::Owned(msg.clone()), + Some(Cow::Borrowed("SERVICE_UNAVAILABLE")), ), }; diff --git a/src/middleware/captcha.rs b/src/middleware/captcha.rs index c236dcf..410e200 100644 --- a/src/middleware/captcha.rs +++ b/src/middleware/captcha.rs @@ -42,8 +42,7 @@ pub async fn verify_captcha_token( let secret_key = state .config - .captcha_secret_key - .as_deref() + .captcha_secret_key() .filter(|s| !s.trim().is_empty()) .ok_or_else(|| { AppError::ServiceUnavailable("CAPTCHA_SECRET_KEY is not configured".to_string()) diff --git a/tests/api/contributions_tests.rs b/tests/api/contributions_tests.rs index 5ae8c09..f1456ec 100644 --- a/tests/api/contributions_tests.rs +++ b/tests/api/contributions_tests.rs @@ -79,7 +79,7 @@ async fn get_contributions_hits_mock_server_when_cache_empty() { let http_client = reqwest::Client::new(); let contributions_service = Arc::new(ContributionsService::new( http_client.clone(), - config.github_pat.clone().unwrap(), + config.github_pat().unwrap().to_string(), config.github_username.clone().unwrap(), config.github_graphql_url.clone(), )); diff --git a/tests/layer_unit_tests.rs b/tests/layer_unit_tests.rs index 6b52ff2..52d00de 100644 --- a/tests/layer_unit_tests.rs +++ b/tests/layer_unit_tests.rs @@ -69,22 +69,44 @@ fn health_ok_model_contains_expected_values() { assert_eq!(health.version, env!("CARGO_PKG_VERSION")); } -#[test] -fn app_config_from_env_fails_when_master_api_key_missing() { - let original_master_key = std::env::var("MASTER_API_KEY").ok(); - // Clear env to ensure a clean state for the test - #[allow(deprecated)] - unsafe { - std::env::remove_var("MASTER_API_KEY"); +use std::sync::Mutex; + +static ENV_LOCK: Mutex<()> = Mutex::new(()); + +struct EnvGuard { + key: &'static str, + original_value: Option, +} + +impl EnvGuard { + fn new(key: &'static str, new_value: Option<&str>) -> Self { + let original_value = std::env::var(key).ok(); + match new_value { + Some(v) => unsafe { std::env::set_var(key, v) }, + None => unsafe { std::env::remove_var(key) }, + } + Self { + key, + original_value, + } } - let result = AppConfig::from_env(); - assert!(result.is_err()); +} - // Restore the original environment variable if it existed - if let Some(key) = original_master_key { - #[allow(deprecated)] - unsafe { - std::env::set_var("MASTER_API_KEY", key); +impl Drop for EnvGuard { + fn drop(&mut self) { + if let Some(ref value) = self.original_value { + unsafe { std::env::set_var(self.key, value) }; + } else { + unsafe { std::env::remove_var(self.key) }; } } } + +#[test] +fn app_config_from_env_fails_when_master_api_key_missing() { + let _lock = ENV_LOCK.lock().unwrap(); + let _guard = EnvGuard::new("MASTER_API_KEY", None); + + let result = AppConfig::from_env(); + assert!(result.is_err()); +} From 45e38e5123289bd75e7fa7e90eb40a016eb55e4e Mon Sep 17 00:00:00 2001 From: Nadu_Dev Date: Mon, 27 Apr 2026 10:16:13 +0530 Subject: [PATCH 16/18] refactor: enhance documentation and suppress clippy warnings in tests --- src/config.rs | 2 +- tests/api/contributions_tests.rs | 1 + tests/layer_unit_tests.rs | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/config.rs b/src/config.rs index a183dca..7bb7e2d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -27,7 +27,7 @@ fn env_or(key: &str, default: &str) -> T { }) } -/// Helper: Fetches and parses an environment variable, returning ConfigError::InvalidValue on parse failure. +/// Helper: Fetches and parses an environment variable, returning `ConfigError::InvalidValue` on parse failure. fn env_parse(key: &str, default: &str) -> Result { let val = env::var(key).unwrap_or_else(|_| default.to_string()); val.parse::().map_err(|_| ConfigError::InvalidValue { diff --git a/tests/api/contributions_tests.rs b/tests/api/contributions_tests.rs index f1456ec..aff8c9e 100644 --- a/tests/api/contributions_tests.rs +++ b/tests/api/contributions_tests.rs @@ -54,6 +54,7 @@ async fn get_contributions_returns_seeded_cache() { } #[tokio::test] +#[allow(clippy::unwrap_used)] async fn get_contributions_hits_mock_server_when_cache_empty() { let mock_server = MockServer::start().await; diff --git a/tests/layer_unit_tests.rs b/tests/layer_unit_tests.rs index 52d00de..475270f 100644 --- a/tests/layer_unit_tests.rs +++ b/tests/layer_unit_tests.rs @@ -104,6 +104,7 @@ impl Drop for EnvGuard { #[test] fn app_config_from_env_fails_when_master_api_key_missing() { + #[allow(clippy::unwrap_used)] let _lock = ENV_LOCK.lock().unwrap(); let _guard = EnvGuard::new("MASTER_API_KEY", None); From bd01f7e138e0a00dc47adca6860da1d277bb1e41 Mon Sep 17 00:00:00 2001 From: Nadu_Dev Date: Mon, 27 Apr 2026 10:26:03 +0530 Subject: [PATCH 17/18] refactor: replace hardcoded API key string with constant in unit tests --- tests/layer_unit_tests.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/layer_unit_tests.rs b/tests/layer_unit_tests.rs index 475270f..a0c2291 100644 --- a/tests/layer_unit_tests.rs +++ b/tests/layer_unit_tests.rs @@ -2,7 +2,7 @@ use axum::http::HeaderMap; use nadzu::{ config::AppConfig, - middleware::{api_key::has_valid_master_api_key, rate_limit::is_production}, + middleware::{api_key::has_valid_master_api_key, rate_limit::is_production, X_API_KEY}, models::health::Health, }; @@ -31,7 +31,7 @@ fn test_config(env: &str) -> AppConfig { fn has_valid_master_api_key_returns_true_for_matching_header() { let config = test_config("test"); let mut headers = HeaderMap::new(); - headers.insert("x-api-key", "master_key".parse().unwrap()); + headers.insert(X_API_KEY, "master_key".parse().unwrap()); assert!(has_valid_master_api_key(&headers, &config)); } @@ -45,8 +45,14 @@ fn has_valid_master_api_key_returns_false_for_missing_or_wrong_header() { assert!(!has_valid_master_api_key(&empty_headers, &config)); let mut wrong_headers = HeaderMap::new(); - wrong_headers.insert("x-api-key", "wrong_key".parse().unwrap()); + wrong_headers.insert(X_API_KEY, "wrong_key".parse().unwrap()); assert!(!has_valid_master_api_key(&wrong_headers, &config)); +} + +#[test] +#[allow(clippy::unwrap_used)] +fn has_valid_master_api_key_ignores_unrelated_headers() { + let config = test_config("test"); let mut unrelated_headers = HeaderMap::new(); unrelated_headers.insert("x-not-api-key", "master_key".parse().unwrap()); From a820173b8006eef9be6748c30e8686148954fa08 Mon Sep 17 00:00:00 2001 From: Nadu_Dev Date: Mon, 27 Apr 2026 10:26:22 +0530 Subject: [PATCH 18/18] refactor: reorder middleware imports for improved readability --- tests/layer_unit_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/layer_unit_tests.rs b/tests/layer_unit_tests.rs index a0c2291..9412a5e 100644 --- a/tests/layer_unit_tests.rs +++ b/tests/layer_unit_tests.rs @@ -2,7 +2,7 @@ use axum::http::HeaderMap; use nadzu::{ config::AppConfig, - middleware::{api_key::has_valid_master_api_key, rate_limit::is_production, X_API_KEY}, + middleware::{X_API_KEY, api_key::has_valid_master_api_key, rate_limit::is_production}, models::health::Health, };