Use ngx_log_error! macro and fix formatting

- Switched from ngx_log_error_core() to ngx_log_error! macro
- Changed error_log level from debug to info (cleaner output)
- Formatting cleanup across all modules (cargo fmt)
- Removed trailing newlines and fixed indentation

Logging now properly uses nginx's macro system for better
integration with nginx's log handling.
This commit is contained in:
Edward Langley
2025-11-15 17:16:55 -08:00
parent 86b79c0a85
commit 56c6045e3b
14 changed files with 146 additions and 159 deletions

View File

@ -7,4 +7,3 @@ fn main() {
println!("cargo:rustc-link-arg=dynamic_lookup");
}
}

View File

@ -8,7 +8,7 @@ events {
worker_connections 1024;
}
error_log logs/error.log debug;
error_log logs/error.log info;
http {
# Global templates for shared components

View File

@ -41,8 +41,7 @@ impl QueryExecutor for SqliteQueryExecutor {
query: &SqlQuery,
params: &[(String, String)],
) -> Result<Vec<HashMap<String, Value>>, String> {
query::execute_query(db_path.as_str(), query.as_str(), params)
.map_err(|e| e.to_string())
query::execute_query(db_path.as_str(), query.as_str(), params).map_err(|e| e.to_string())
}
}
@ -54,7 +53,7 @@ pub struct HandlebarsAdapter {
impl HandlebarsAdapter {
/// Create adapter from mutable handlebars registry
///
///
/// # Safety
/// Caller must ensure the registry outlives this adapter
pub unsafe fn new(registry: *mut Handlebars<'static>) -> Self {
@ -81,7 +80,11 @@ impl TemplateLoader for HandlebarsAdapter {
impl TemplateRenderer for HandlebarsAdapter {
fn render(&self, template_name: &str, data: &Value) -> Result<String, String> {
unsafe { (*self.registry).render(template_name, data).map_err(|e| e.to_string()) }
unsafe {
(*self.registry)
.render(template_name, data)
.map_err(|e| e.to_string())
}
}
}
@ -136,9 +139,7 @@ mod tests {
let reg_ptr: *mut Handlebars<'static> = unsafe { std::mem::transmute(&mut reg) };
let adapter = unsafe { HandlebarsAdapter::new(reg_ptr) };
adapter
.register_template("test", &template_path)
.unwrap();
adapter.register_template("test", &template_path).unwrap();
let data = serde_json::json!({"name": "World"});
let rendered = adapter.render("test", &data).unwrap();
@ -148,4 +149,3 @@ mod tests {
let _ = fs::remove_dir_all(temp_dir);
}
}

View File

@ -130,4 +130,3 @@ mod tests {
assert_eq!(config.global_templates_dir, "templates/global");
}
}

View File

@ -25,7 +25,7 @@ impl ResolvedTemplate {
pub fn full_path(&self) -> &str {
&self.full_path
}
pub fn directory(&self) -> &str {
&self.directory
}
@ -231,11 +231,9 @@ mod tests {
#[test]
fn test_resolve_parameters_positional() {
let bindings = vec![
ParameterBinding::Positional {
variable: NginxVariable::parse("$arg_id").unwrap(),
},
];
let bindings = vec![ParameterBinding::Positional {
variable: NginxVariable::parse("$arg_id").unwrap(),
}];
let resolver = MockVariableResolver;
let resolved = resolve_parameters(&bindings, &resolver).unwrap();
@ -247,12 +245,10 @@ mod tests {
#[test]
fn test_resolve_parameters_named() {
let bindings = vec![
ParameterBinding::Named {
name: ParamName::parse(":book_id").unwrap(),
variable: NginxVariable::parse("$arg_id").unwrap(),
},
];
let bindings = vec![ParameterBinding::Named {
name: ParamName::parse(":book_id").unwrap(),
variable: NginxVariable::parse("$arg_id").unwrap(),
}];
let resolver = MockVariableResolver;
let resolved = resolve_parameters(&bindings, &resolver).unwrap();
@ -264,11 +260,9 @@ mod tests {
#[test]
fn test_resolve_parameters_literal() {
let bindings = vec![
ParameterBinding::PositionalLiteral {
value: "constant".to_string(),
},
];
let bindings = vec![ParameterBinding::PositionalLiteral {
value: "constant".to_string(),
}];
let resolver = MockVariableResolver;
let resolved = resolve_parameters(&bindings, &resolver).unwrap();
@ -291,11 +285,8 @@ mod tests {
directory: "templates".to_string(),
};
let processor = RequestProcessor::new(
MockQueryExecutor,
MockTemplateLoader,
MockTemplateRenderer,
);
let processor =
RequestProcessor::new(MockQueryExecutor, MockTemplateLoader, MockTemplateRenderer);
let result = processor.process(&config, &resolved_template, &[], None);
@ -303,4 +294,3 @@ mod tests {
assert!(result.unwrap().contains("Rendered"));
}
}

View File

@ -6,7 +6,7 @@ use crate::domain::{RequestProcessor, ValidatedConfig};
use crate::logging;
use crate::nginx_helpers::{get_doc_root_and_uri, send_response};
use crate::parsing;
use crate::{domain, Module};
use crate::{Module, domain};
use ngx::core::Status;
use ngx::http::{HttpModuleLocationConf, HttpModuleMainConf};
@ -31,15 +31,15 @@ impl<'a> ValidConfigToken<'a> {
/// Process a request with guaranteed valid configuration
/// Returns Status directly - no Result needed, types prove correctness
pub fn process_request(
request: &mut ngx::http::Request,
config: ValidConfigToken,
) -> Status {
pub fn process_request(request: &mut ngx::http::Request, config: ValidConfigToken) -> Status {
logging::log(
request,
logging::LogLevel::Debug,
"handler",
&format!("Processing request for {}", request.unparsed_uri().to_str().unwrap_or("unknown")),
&format!(
"Processing request for {}",
request.unparsed_uri().to_str().unwrap_or("unknown")
),
);
// Parse config into validated types
@ -55,13 +55,19 @@ pub fn process_request(
let (doc_root, uri) = match get_doc_root_and_uri(request) {
Ok(paths) => paths,
Err(e) => {
logging::log(request, logging::LogLevel::Error, "nginx", &format!("Path resolution failed: {}", e));
logging::log(
request,
logging::LogLevel::Error,
"nginx",
&format!("Path resolution failed: {}", e),
);
return ngx::http::HTTPStatus::INTERNAL_SERVER_ERROR.into();
}
};
// Resolve template path (pure function - cannot fail)
let resolved_template = domain::resolve_template_path(&doc_root, &uri, &validated_config.template_path);
let resolved_template =
domain::resolve_template_path(&doc_root, &uri, &validated_config.template_path);
logging::log(
request,
@ -72,27 +78,33 @@ pub fn process_request(
// Resolve parameters
let var_resolver = NginxVariableResolver::new(request);
let resolved_params = match domain::resolve_parameters(&validated_config.parameters, &var_resolver) {
Ok(params) => {
if !params.is_empty() {
logging::log(
request,
logging::LogLevel::Debug,
"params",
&format!("Resolved {} parameters", params.len()),
);
let resolved_params =
match domain::resolve_parameters(&validated_config.parameters, &var_resolver) {
Ok(params) => {
if !params.is_empty() {
logging::log(
request,
logging::LogLevel::Debug,
"params",
&format!("Resolved {} parameters", params.len()),
);
}
params
}
params
}
Err(e) => {
logging::log_param_error(request, "variable", &e);
return ngx::http::HTTPStatus::BAD_REQUEST.into();
}
};
Err(e) => {
logging::log_param_error(request, "variable", &e);
return ngx::http::HTTPStatus::BAD_REQUEST.into();
}
};
// Execute and render
let html = execute_with_processor(&validated_config, &resolved_template, &resolved_params, request);
let html = execute_with_processor(
&validated_config,
&resolved_template,
&resolved_params,
request,
);
// Send response
send_response(request, &html)
}
@ -107,21 +119,12 @@ fn execute_with_processor(
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);
let main_conf = Module::main_conf(request).expect("main config is none");
let global_dir = if !main_conf.global_templates_dir.is_empty() {
logging::log_template_loading(
request,
"global",
0,
&main_conf.global_templates_dir,
);
logging::log_template_loading(request, "global", 0, &main_conf.global_templates_dir);
Some(main_conf.global_templates_dir.as_str())
} else {
None
@ -137,7 +140,11 @@ fn execute_with_processor(
"success",
&format!(
"Rendered {} with {} params",
resolved_template.full_path().split('/').last().unwrap_or("template"),
resolved_template
.full_path()
.split('/')
.last()
.unwrap_or("template"),
resolved_params.len()
),
);
@ -235,4 +242,3 @@ mod tests {
assert!(token.is_none());
}
}

View File

@ -13,11 +13,11 @@ mod types;
mod variable;
use config::{MainConfig, ModuleConfig};
use handler_types::{process_request, ValidConfigToken};
use handler_types::{ValidConfigToken, process_request};
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,
NGX_CONF_TAKE1, NGX_CONF_TAKE2, NGX_HTTP_LOC_CONF, NGX_HTTP_LOC_CONF_OFFSET,
NGX_HTTP_MAIN_CONF, NGX_HTTP_MODULE, 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::http::{HttpModule, HttpModuleLocationConf, HttpModuleMainConf, NgxHttpCoreModule};
use ngx::{core::Status, http, http_request_handler, ngx_modules, ngx_string};
@ -209,8 +209,8 @@ extern "C" fn ngx_http_howto_commands_set_template_path(
conf.template_path = (*args.add(1)).to_string();
// Set the content handler for this location
let clcf = NgxHttpCoreModule::location_conf_mut(&*cf)
.expect("failed to get core location conf");
let clcf =
NgxHttpCoreModule::location_conf_mut(&*cf).expect("failed to get core location conf");
clcf.handler = Some(howto_access_handler);
};

View File

@ -15,29 +15,21 @@ pub enum LogLevel {
/// Log a message with context using nginx's native logging
pub fn log(request: &mut Request, level: LogLevel, module: &str, message: &str) {
let r: *mut ngx::ffi::ngx_http_request_t = request.into();
let log_level = match level {
LogLevel::Error => 3, // NGX_LOG_ERR
LogLevel::Warn => 4, // NGX_LOG_WARN
LogLevel::Info => 6, // NGX_LOG_INFO
LogLevel::Debug => 7, // NGX_LOG_DEBUG
LogLevel::Error => 3, // NGX_LOG_ERR
LogLevel::Warn => 4, // NGX_LOG_WARN
LogLevel::Info => 6, // NGX_LOG_INFO
LogLevel::Debug => 7, // NGX_LOG_DEBUG
};
let r: *mut ngx::ffi::ngx_http_request_t = request.into();
unsafe {
let connection = (*r).connection;
if !connection.is_null() {
let log_ptr = (*connection).log;
if !log_ptr.is_null() {
// ngx_log_error! requires a string literal, so we use the C API directly
let c_msg = std::ffi::CString::new(format!("[sqlite-serve:{}] {}", module, message))
.unwrap_or_else(|_| std::ffi::CString::new("log message error").unwrap());
ngx::ffi::ngx_log_error_core(
log_level as ngx::ffi::ngx_uint_t,
log_ptr,
0,
c_msg.as_ptr(),
);
let log = (*connection).log;
if !log.is_null() {
ngx_log_error!(log_level, log, "[sqlite-serve:{}] {}", module, message);
}
}
}
@ -128,4 +120,3 @@ mod tests {
assert_eq!(levels.len(), 4);
}
}

View File

@ -28,7 +28,6 @@ pub fn get_doc_root_and_uri(request: &mut Request) -> Result<(String, String), S
/// 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,
@ -45,7 +44,7 @@ pub fn send_response(request: &mut Request, body: &str) -> Status {
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;
@ -57,8 +56,12 @@ pub fn send_response(request: &mut Request, body: &str) -> Status {
/// Log and return error status (deprecated - use logging module directly)
#[allow(dead_code)]
pub fn log_error(request: &mut Request, context: &str, error: &str, status: http::HTTPStatus) -> Status {
pub fn log_error(
request: &mut Request,
context: &str,
error: &str,
status: http::HTTPStatus,
) -> Status {
logging::log(request, logging::LogLevel::Error, context, error);
status.into()
}

View File

@ -2,15 +2,16 @@
use crate::config::ModuleConfig;
use crate::domain::ValidatedConfig;
use crate::types::{DatabasePath, NginxVariable, ParamName, ParameterBinding, SqlQuery, TemplatePath};
use crate::types::{
DatabasePath, NginxVariable, ParamName, ParameterBinding, SqlQuery, TemplatePath,
};
/// Parse raw configuration into validated domain configuration
pub fn parse_config(config: &ModuleConfig) -> Result<ValidatedConfig, String> {
let db_path = DatabasePath::parse(&config.db_path)
.map_err(|e| format!("invalid db_path: {}", e))?;
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 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))?;
@ -26,9 +27,7 @@ pub fn parse_config(config: &ModuleConfig) -> Result<ValidatedConfig, String> {
}
/// Parse parameter configuration into typed bindings
fn parse_parameter_bindings(
params: &[(String, String)],
) -> Result<Vec<ParameterBinding>, String> {
fn parse_parameter_bindings(params: &[(String, String)]) -> Result<Vec<ParameterBinding>, String> {
let mut bindings = Vec::new();
for (param_name, var_name) in params {
@ -169,4 +168,3 @@ mod tests {
}
}
}

View File

@ -31,11 +31,9 @@ pub fn execute_query(
let value: Value = match row.get_ref(i)? {
rusqlite::types::ValueRef::Null => Value::Null,
rusqlite::types::ValueRef::Integer(v) => Value::Number(v.into()),
rusqlite::types::ValueRef::Real(v) => {
serde_json::Number::from_f64(v)
.map(Value::Number)
.unwrap_or(Value::Null)
}
rusqlite::types::ValueRef::Real(v) => serde_json::Number::from_f64(v)
.map(Value::Number)
.unwrap_or(Value::Null),
rusqlite::types::ValueRef::Text(v) => {
Value::String(String::from_utf8_lossy(v).to_string())
}
@ -90,11 +88,8 @@ mod tests {
{
let conn = Connection::open(temp_path).unwrap();
conn.execute(
"CREATE TABLE test (id INTEGER, name TEXT, value REAL)",
[],
)
.unwrap();
conn.execute("CREATE TABLE test (id INTEGER, name TEXT, value REAL)", [])
.unwrap();
conn.execute(
"INSERT INTO test VALUES (1, 'first', 1.5), (2, 'second', 2.5)",
[],
@ -104,10 +99,7 @@ mod tests {
let results = execute_query(temp_path, "SELECT * FROM test ORDER BY id", &[]).unwrap();
assert_eq!(results.len(), 2);
assert_eq!(
results[0].get("id").unwrap(),
&Value::Number(1.into())
);
assert_eq!(results[0].get("id").unwrap(), &Value::Number(1.into()));
assert_eq!(
results[0].get("name").unwrap(),
&Value::String("first".to_string())
@ -157,8 +149,11 @@ mod tests {
{
let conn = Connection::open(temp_path).unwrap();
conn.execute("CREATE TABLE books (id INTEGER, title TEXT, year INTEGER)", [])
.unwrap();
conn.execute(
"CREATE TABLE books (id INTEGER, title TEXT, year INTEGER)",
[],
)
.unwrap();
conn.execute(
"INSERT INTO books VALUES (1, 'Old Book', 2000), (2, 'New Book', 2020), (3, 'Newer Book', 2023)",
[],
@ -218,10 +213,7 @@ mod tests {
let row = &results[0];
assert_eq!(row.get("id").unwrap(), &Value::Number(42.into()));
assert_eq!(
row.get("name").unwrap(),
&Value::String("test".to_string())
);
assert_eq!(row.get("name").unwrap(), &Value::String("test".to_string()));
assert_eq!(row.get("price").unwrap().as_f64().unwrap(), 3.14);
assert_eq!(
row.get("data").unwrap(),
@ -242,8 +234,11 @@ mod tests {
{
let conn = Connection::open(temp_path).unwrap();
conn.execute("CREATE TABLE books (id INTEGER, genre TEXT, rating REAL)", [])
.unwrap();
conn.execute(
"CREATE TABLE books (id INTEGER, genre TEXT, rating REAL)",
[],
)
.unwrap();
conn.execute(
"INSERT INTO books VALUES
(1, 'Fiction', 4.5),
@ -324,4 +319,3 @@ mod tests {
let _ = fs::remove_file(temp_path);
}
}

View File

@ -157,4 +157,3 @@ mod tests {
let _ = fs::remove_dir_all(temp_dir);
}
}

View File

@ -36,16 +36,16 @@ impl SqlQuery {
pub fn parse(query: impl Into<String>) -> Result<Self, String> {
let query = query.into();
let trimmed = query.trim().to_uppercase();
if trimmed.is_empty() {
return Err("query cannot be empty".to_string());
}
// Ensure it's a SELECT query (read-only)
if !trimmed.starts_with("SELECT") {
return Err("only SELECT queries are allowed".to_string());
}
Ok(SqlQuery(query))
}
@ -62,16 +62,16 @@ impl TemplatePath {
/// Parse and validate a template path
pub fn parse(path: impl AsRef<Path>) -> Result<Self, String> {
let path = path.as_ref();
if path.as_os_str().is_empty() {
return Err("template path cannot be empty".to_string());
}
// Ensure it's a .hbs file
if path.extension().and_then(|e| e.to_str()) != Some("hbs") {
return Err("template must be a .hbs file".to_string());
}
Ok(TemplatePath(path.to_path_buf()))
}
@ -92,28 +92,28 @@ impl NginxVariable {
/// Parse a nginx variable name
pub fn parse(name: impl Into<String>) -> Result<Self, String> {
let name = name.into();
if name.is_empty() {
return Err("variable name cannot be empty".to_string());
}
if !name.starts_with('$') {
return Err(format!("variable name must start with $: {}", name));
}
// Get the part after $
let var_name = &name[1..];
if var_name.is_empty() {
return Err("variable name after $ cannot be empty".to_string());
}
Ok(NginxVariable(name))
}
pub fn as_str(&self) -> &str {
&self.0
}
/// Get the variable name without the $ prefix
pub fn name(&self) -> &str {
&self.0[1..]
@ -128,23 +128,23 @@ impl ParamName {
/// Parse a SQL parameter name
pub fn parse(name: impl Into<String>) -> Result<Self, String> {
let name = name.into();
if name.is_empty() {
return Err("parameter name cannot be empty".to_string());
}
if !name.starts_with(':') {
return Err(format!("parameter name must start with :: {}", name));
}
Ok(ParamName(name))
}
/// Create an empty (positional) parameter name
pub fn positional() -> Self {
ParamName(String::new())
}
pub fn is_positional(&self) -> bool {
self.0.is_empty()
}
@ -157,10 +157,20 @@ impl ParamName {
/// A parameter binding (param name + variable or literal)
#[derive(Debug, Clone)]
pub enum ParameterBinding {
Positional { variable: NginxVariable },
PositionalLiteral { value: String },
Named { name: ParamName, variable: NginxVariable },
NamedLiteral { name: ParamName, value: String },
Positional {
variable: NginxVariable,
},
PositionalLiteral {
value: String,
},
Named {
name: ParamName,
variable: NginxVariable,
},
NamedLiteral {
name: ParamName,
value: String,
},
}
impl ParameterBinding {
@ -300,4 +310,3 @@ mod tests {
assert!(result.unwrap_err().contains(":"));
}
}

View File

@ -86,8 +86,8 @@ mod tests {
fn test_named_params_parsing() {
// Test parameter name parsing logic
let test_cases = vec![
(2, false, ""), // sqlite_param $arg_id
(3, true, ":book_id"), // sqlite_param :book_id $arg_id
(2, false, ""), // sqlite_param $arg_id
(3, true, ":book_id"), // sqlite_param :book_id $arg_id
];
for (nelts, expected_is_named, expected_param_name) in test_cases {
@ -103,4 +103,3 @@ mod tests {
}
}
}