From c64851a7881c32781dc0cdf7725e36ded5379cb4 Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Sat, 15 Nov 2025 16:52:49 -0800 Subject: [PATCH] Extract parsing and nginx helpers from lib.rs (minimal handler) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Handler reduced: 170 lines → 52 lines New Modules: - parsing.rs (172 lines): Config validation and parameter binding construction - nginx_helpers.rs (62 lines): NGINX-specific utilities Extracted from lib.rs: - parse_config(): Validate raw config into ValidatedConfig - parse_parameter_bindings(): Convert strings to ParameterBinding types - get_doc_root_and_uri(): Extract nginx path information - send_response(): Create and send nginx buffer - log_error(): Consistent error logging Handler Now (52 lines): 1. Get config 2. Parse config → validated types (parsing.rs) 3. Get paths (nginx_helpers.rs) 4. Resolve template (domain.rs pure function) 5. Resolve parameters (domain.rs with injected resolver) 6. Create processor with DI (adapters.rs) 7. Process request (domain.rs pure core) 8. Send response (nginx_helpers.rs) Benefits: ✓ Handler is now orchestration only ✓ No business logic in lib.rs ✓ Each step is a single function call ✓ Clear data flow ✓ Easy to follow Test Coverage: 54 tests (+7 parsing tests) Module Size: lib.rs 302 lines (down from 423) Production: Verified working The handler is now truly minimal - just glue code! --- src/lib.rs | 166 ++++++----------------------------------- src/nginx_helpers.rs | 62 ++++++++++++++++ src/parsing.rs | 172 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 257 insertions(+), 143 deletions(-) create mode 100644 src/nginx_helpers.rs create mode 100644 src/parsing.rs diff --git a/src/lib.rs b/src/lib.rs index d0ca975..d56209b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,6 +3,8 @@ mod adapters; mod config; mod domain; +mod nginx_helpers; +mod parsing; mod query; mod template; mod types; @@ -11,13 +13,12 @@ mod variable; use adapters::{HandlebarsAdapter, NginxVariableResolver, SqliteQueryExecutor}; use config::{MainConfig, ModuleConfig}; use domain::RequestProcessor; +use nginx_helpers::{get_doc_root_and_uri, log_error, send_response}; use ngx::ffi::{ NGX_CONF_TAKE1, NGX_CONF_TAKE2, NGX_HTTP_LOC_CONF, NGX_HTTP_MAIN_CONF, NGX_HTTP_MODULE, NGX_HTTP_LOC_CONF_OFFSET, NGX_RS_MODULE_SIGNATURE, nginx_version, ngx_command_t, ngx_conf_t, ngx_http_module_t, ngx_int_t, ngx_module_t, ngx_str_t, ngx_uint_t, }; -use ngx::core::Buffer; -use ngx::ffi::ngx_chain_t; use ngx::http::{HttpModule, HttpModuleLocationConf, HttpModuleMainConf, NgxHttpCoreModule}; use ngx::{core::Status, http, http_request_handler, ngx_log_debug_http, ngx_modules, ngx_string}; use std::os::raw::{c_char, c_void}; @@ -245,141 +246,44 @@ extern "C" fn ngx_http_howto_commands_add_param( std::ptr::null_mut() } -// HTTP request handler using functional core with dependency injection +// HTTP request handler - minimal glue code orchestrating domain layer http_request_handler!(howto_access_handler, |request: &mut http::Request| { let co = Module::location_conf(request).expect("module config is none"); - // Check if all required config values are set + // Skip if not configured if co.db_path.is_empty() || co.query.is_empty() || co.template_path.is_empty() { return Status::NGX_OK; } - ngx_log_debug_http!(request, "sqlite module handler called"); - - // Parse and validate configuration into domain types - let db_path = match types::DatabasePath::parse(&co.db_path) { - Ok(p) => p, - Err(e) => { - ngx_log_debug_http!(request, "invalid db path: {}", e); - return http::HTTPStatus::INTERNAL_SERVER_ERROR.into(); - } + // Parse configuration into validated domain types + let validated_config = match parsing::parse_config(co) { + Ok(config) => config, + Err(e) => return log_error(request, "config parse error", &e, http::HTTPStatus::INTERNAL_SERVER_ERROR), }; - let query = match types::SqlQuery::parse(&co.query) { - Ok(q) => q, - Err(e) => { - ngx_log_debug_http!(request, "invalid query: {}", e); - return http::HTTPStatus::INTERNAL_SERVER_ERROR.into(); - } + // Get document root and URI from nginx + let (doc_root, uri) = match get_doc_root_and_uri(request) { + Ok(paths) => paths, + Err(e) => return log_error(request, "path resolution error", &e, http::HTTPStatus::INTERNAL_SERVER_ERROR), }; - let template_path = match types::TemplatePath::parse(&co.template_path) { - Ok(t) => t, - Err(e) => { - ngx_log_debug_http!(request, "invalid template path: {}", e); - return http::HTTPStatus::INTERNAL_SERVER_ERROR.into(); - } - }; + // Resolve template path (pure function) + let resolved_template = domain::resolve_template_path(&doc_root, &uri, &validated_config.template_path); - // Parse parameter bindings - let mut bindings = Vec::new(); - for (param_name, var_name) in &co.query_params { - let binding = if var_name.starts_with('$') { - let variable = match types::NginxVariable::parse(var_name) { - Ok(v) => v, - Err(e) => { - ngx_log_debug_http!(request, "invalid variable: {}", e); - return http::HTTPStatus::INTERNAL_SERVER_ERROR.into(); - } - }; - - if param_name.is_empty() { - types::ParameterBinding::Positional { variable } - } else { - let name = match types::ParamName::parse(param_name) { - Ok(n) => n, - Err(e) => { - ngx_log_debug_http!(request, "invalid param name: {}", e); - return http::HTTPStatus::INTERNAL_SERVER_ERROR.into(); - } - }; - types::ParameterBinding::Named { name, variable } - } - } else { - // Literal value - if param_name.is_empty() { - types::ParameterBinding::PositionalLiteral { - value: var_name.clone(), - } - } else { - let name = match types::ParamName::parse(param_name) { - Ok(n) => n, - Err(e) => { - ngx_log_debug_http!(request, "invalid param name: {}", e); - return http::HTTPStatus::INTERNAL_SERVER_ERROR.into(); - } - }; - types::ParameterBinding::NamedLiteral { - name, - value: var_name.clone(), - } - } - }; - bindings.push(binding); - } - - let validated_config = domain::ValidatedConfig { - db_path, - query, - template_path, - parameters: bindings, - }; - - // Resolve template path relative to document root and URI - let core_loc_conf = - NgxHttpCoreModule::location_conf(request).expect("failed to get core location conf"); - let doc_root = match (*core_loc_conf).root.to_str() { - Ok(s) => s, - Err(e) => { - ngx_log_debug_http!(request, "failed to decode root path: {}", e); - return http::HTTPStatus::INTERNAL_SERVER_ERROR.into(); - } - }; - let uri = match request.path().to_str() { - Ok(s) => s, - Err(e) => { - ngx_log_debug_http!(request, "failed to decode URI path: {}", e); - return http::HTTPStatus::INTERNAL_SERVER_ERROR.into(); - } - }; - - let resolved_template = domain::resolve_template_path(doc_root, uri, &validated_config.template_path); - - ngx_log_debug_http!(request, "resolved template path: {}", resolved_template.full_path()); - - // Resolve parameters using nginx variable resolver + // Resolve parameters from nginx variables let var_resolver = NginxVariableResolver::new(request); let resolved_params = match domain::resolve_parameters(&validated_config.parameters, &var_resolver) { Ok(params) => params, - Err(e) => { - ngx_log_debug_http!(request, "failed to resolve parameters: {}", e); - return http::HTTPStatus::BAD_REQUEST.into(); - } + Err(e) => return log_error(request, "parameter resolution error", &e, http::HTTPStatus::BAD_REQUEST), }; - ngx_log_debug_http!(request, "resolved {} parameters", resolved_params.len()); - - // Create domain processor with adapters (dependency injection) + // Create processor with injected dependencies let mut reg = handlebars::Handlebars::new(); let reg_ptr: *mut handlebars::Handlebars<'static> = unsafe { std::mem::transmute(&mut reg) }; let hbs_adapter = unsafe { HandlebarsAdapter::new(reg_ptr) }; - let processor = RequestProcessor::new( - SqliteQueryExecutor, - hbs_adapter, - hbs_adapter, - ); + let processor = RequestProcessor::new(SqliteQueryExecutor, hbs_adapter, hbs_adapter); - // Get global template directory from main config + // Get global template directory let main_conf = Module::main_conf(request).expect("main config is none"); let global_dir = if !main_conf.global_templates_dir.is_empty() { Some(main_conf.global_templates_dir.as_str()) @@ -390,33 +294,9 @@ http_request_handler!(howto_access_handler, |request: &mut http::Request| { // Process request through functional core let body = match processor.process(&validated_config, &resolved_template, &resolved_params, global_dir) { Ok(html) => html, - Err(e) => { - ngx_log_debug_http!(request, "request processing failed: {}", e); - return http::HTTPStatus::INTERNAL_SERVER_ERROR.into(); - } + Err(e) => return log_error(request, "request processing error", &e, http::HTTPStatus::INTERNAL_SERVER_ERROR), }; - // Create output buffer (imperative shell) - let mut buf = match request.pool().create_buffer_from_str(&body) { - Some(buf) => buf, - None => return http::HTTPStatus::INTERNAL_SERVER_ERROR.into(), - }; - - buf.set_last_buf(request.is_main()); - buf.set_last_in_chain(true); - - let mut out = ngx_chain_t { - buf: buf.as_ngx_buf_mut(), - next: std::ptr::null_mut(), - }; - - request.discard_request_body(); - request.set_status(http::HTTPStatus::OK); - let rc = request.send_header(); - if rc == Status::NGX_ERROR || rc > Status::NGX_OK || request.header_only() { - return rc; - } - - request.output_filter(&mut out); - Status::NGX_DONE + // Send response + send_response(request, &body) }); diff --git a/src/nginx_helpers.rs b/src/nginx_helpers.rs new file mode 100644 index 0000000..05113bc --- /dev/null +++ b/src/nginx_helpers.rs @@ -0,0 +1,62 @@ +//! NGINX-specific helper functions + +use ngx::core::Buffer; +use ngx::ffi::ngx_chain_t; +use ngx::http::{HttpModuleLocationConf, NgxHttpCoreModule, Request}; +use ngx::{core::Status, http, ngx_log_debug_http}; + +/// Get document root and URI from request +pub fn get_doc_root_and_uri(request: &mut Request) -> Result<(String, String), String> { + let core_loc_conf = NgxHttpCoreModule::location_conf(request) + .ok_or_else(|| "failed to get core location conf".to_string())?; + + let doc_root = (*core_loc_conf) + .root + .to_str() + .map_err(|e| format!("failed to decode root path: {}", e))? + .to_string(); + + let uri = request + .path() + .to_str() + .map_err(|e| format!("failed to decode URI path: {}", e))? + .to_string(); + + Ok((doc_root, uri)) +} + +/// Create and send nginx response buffer +pub fn send_response(request: &mut Request, body: &str) -> Status { + + // Create output buffer + let mut buf = match request.pool().create_buffer_from_str(body) { + Some(buf) => buf, + None => return http::HTTPStatus::INTERNAL_SERVER_ERROR.into(), + }; + + buf.set_last_buf(request.is_main()); + buf.set_last_in_chain(true); + + let mut out = ngx_chain_t { + buf: buf.as_ngx_buf_mut(), + next: std::ptr::null_mut(), + }; + + request.discard_request_body(); + request.set_status(http::HTTPStatus::OK); + + let rc = request.send_header(); + if rc == Status::NGX_ERROR || rc > Status::NGX_OK || request.header_only() { + return rc; + } + + request.output_filter(&mut out); + Status::NGX_DONE +} + +/// Log and return error status +pub fn log_error(request: &mut Request, context: &str, error: &str, status: http::HTTPStatus) -> Status { + ngx_log_debug_http!(request, "{}: {}", context, error); + status.into() +} + diff --git a/src/parsing.rs b/src/parsing.rs new file mode 100644 index 0000000..11fc9f3 --- /dev/null +++ b/src/parsing.rs @@ -0,0 +1,172 @@ +//! Parse raw configuration strings into validated domain types + +use crate::config::ModuleConfig; +use crate::domain::ValidatedConfig; +use crate::types::{DatabasePath, NginxVariable, ParamName, ParameterBinding, SqlQuery, TemplatePath}; + +/// Parse raw configuration into validated domain configuration +pub fn parse_config(config: &ModuleConfig) -> Result { + let db_path = DatabasePath::parse(&config.db_path) + .map_err(|e| format!("invalid db_path: {}", e))?; + + let query = SqlQuery::parse(&config.query) + .map_err(|e| format!("invalid query: {}", e))?; + + let template_path = TemplatePath::parse(&config.template_path) + .map_err(|e| format!("invalid template_path: {}", e))?; + + let parameters = parse_parameter_bindings(&config.query_params)?; + + Ok(ValidatedConfig { + db_path, + query, + template_path, + parameters, + }) +} + +/// Parse parameter configuration into typed bindings +fn parse_parameter_bindings( + params: &[(String, String)], +) -> Result, String> { + let mut bindings = Vec::new(); + + for (param_name, var_name) in params { + let binding = if var_name.starts_with('$') { + // Variable reference + let variable = NginxVariable::parse(var_name) + .map_err(|e| format!("invalid variable '{}': {}", var_name, e))?; + + if param_name.is_empty() { + ParameterBinding::Positional { variable } + } else { + let name = ParamName::parse(param_name) + .map_err(|e| format!("invalid param name '{}': {}", param_name, e))?; + ParameterBinding::Named { name, variable } + } + } else { + // Literal value + if param_name.is_empty() { + ParameterBinding::PositionalLiteral { + value: var_name.clone(), + } + } else { + let name = ParamName::parse(param_name) + .map_err(|e| format!("invalid param name '{}': {}", param_name, e))?; + ParameterBinding::NamedLiteral { + name, + value: var_name.clone(), + } + } + }; + + bindings.push(binding); + } + + Ok(bindings) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_parse_config_valid() { + let config = ModuleConfig { + db_path: "test.db".to_string(), + query: "SELECT * FROM books".to_string(), + template_path: "list.hbs".to_string(), + query_params: vec![], + }; + + let validated = parse_config(&config).unwrap(); + assert_eq!(validated.db_path.as_str(), "test.db"); + assert!(validated.query.as_str().contains("SELECT")); + } + + #[test] + fn test_parse_config_invalid_query() { + let config = ModuleConfig { + db_path: "test.db".to_string(), + query: "DELETE FROM books".to_string(), + template_path: "list.hbs".to_string(), + query_params: vec![], + }; + + let result = parse_config(&config); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("SELECT")); + } + + #[test] + fn test_parse_config_invalid_template() { + let config = ModuleConfig { + db_path: "test.db".to_string(), + query: "SELECT * FROM books".to_string(), + template_path: "list.html".to_string(), + query_params: vec![], + }; + + let result = parse_config(&config); + assert!(result.is_err()); + assert!(result.unwrap_err().contains(".hbs")); + } + + #[test] + fn test_parse_parameter_bindings_positional() { + let params = vec![(String::new(), "$arg_id".to_string())]; + let bindings = parse_parameter_bindings(¶ms).unwrap(); + + assert_eq!(bindings.len(), 1); + match &bindings[0] { + ParameterBinding::Positional { variable } => { + assert_eq!(variable.name(), "arg_id"); + } + _ => panic!("expected positional binding"), + } + } + + #[test] + fn test_parse_parameter_bindings_named() { + let params = vec![(":book_id".to_string(), "$arg_id".to_string())]; + let bindings = parse_parameter_bindings(¶ms).unwrap(); + + assert_eq!(bindings.len(), 1); + match &bindings[0] { + ParameterBinding::Named { name, variable } => { + assert_eq!(name.as_str(), ":book_id"); + assert_eq!(variable.name(), "arg_id"); + } + _ => panic!("expected named binding"), + } + } + + #[test] + fn test_parse_parameter_bindings_literal() { + let params = vec![(String::new(), "constant".to_string())]; + let bindings = parse_parameter_bindings(¶ms).unwrap(); + + assert_eq!(bindings.len(), 1); + match &bindings[0] { + ParameterBinding::PositionalLiteral { value } => { + assert_eq!(value, "constant"); + } + _ => panic!("expected positional literal binding"), + } + } + + #[test] + fn test_parse_parameter_bindings_invalid_variable() { + let params = vec![(String::new(), "arg_id".to_string())]; + let bindings = parse_parameter_bindings(¶ms).unwrap(); + + // Without $, it's treated as a literal + match &bindings[0] { + ParameterBinding::PositionalLiteral { value } => { + assert_eq!(value, "arg_id"); + } + _ => panic!("expected literal binding"), + } + } +} +