From 3a56ba3ae7d4b427607ce493bede4f079c7502a5 Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Wed, 24 Jul 2024 11:13:38 -0600 Subject: [PATCH 1/6] feat: "libify" the Validator - Returns `Result<()>` instead of bools from validation method - Removes `validate_str` - Adds crate-specific error based on `thiserror` - Adds some doctests --- Cargo.lock | 5 +- Cargo.toml | 6 ++ src/lib.rs | 174 ++++++++++++++++++++++++++++++++------------- src/main.rs | 40 ++++++++--- tests/ogc_tests.rs | 20 ++---- 5 files changed, 171 insertions(+), 74 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 28f8d52..49f11d7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -115,6 +115,7 @@ dependencies = [ "serde", "serde_derive", "serde_json", + "thiserror", ] [[package]] @@ -626,9 +627,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.71" +version = "2.0.72" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b146dcf730474b4bcd16c311627b31ede9ab149045db4d6088b3becaea046462" +checksum = "dc4b9b9bf2add8093d3f2c0204471e951b2285580335de42f9d2534f3ae7a8af" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index fa60cff..f08a949 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,3 +14,9 @@ rstest = "0.21.0" serde = "1.0.204" serde_derive = "1.0.204" serde_json = "1.0.120" +thiserror = "1.0.63" + +[[bin]] +name = "cql2" +test = false +doc = false diff --git a/src/lib.rs b/src/lib.rs index 236c9e7..9844874 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,4 @@ -use boon::{Compiler, SchemaIndex, Schemas}; +use boon::{Compiler, SchemaIndex, Schemas, ValidationError}; use geozero::geojson::GeoJsonWriter; use geozero::wkt::Wkt; use geozero::{CoordDimensions, GeozeroGeometry, ToJson}; @@ -6,61 +6,73 @@ use pest::iterators::{Pair, Pairs}; use pest::pratt_parser::PrattParser; use pest::Parser; use serde_derive::{Deserialize, Serialize}; +use serde_json::Value; use std::fs; +use thiserror::Error; + +/// Crate-specific error enum. +#[derive(Debug, Error)] +pub enum Error { + /// [boon::CompileError] + #[error(transparent)] + BoonCompile(#[from] boon::CompileError), + + /// [serde_json::Error] + #[error(transparent)] + SerdeJson(#[from] serde_json::Error), +} + +/// A re-usable json-schema validator for CQL2. pub struct Validator { schemas: Schemas, index: SchemaIndex, } impl Validator { - pub fn new() -> Validator { + /// Creates a new validator. + /// + /// # Examples + /// + /// ``` + /// use cql2::Validator; + /// + /// let validator = Validator::new().unwrap(); + /// ``` + pub fn new() -> Result { let mut schemas = Schemas::new(); let mut compiler = Compiler::new(); - let schema_json = serde_json::from_str(include_str!("cql2.json")) - .expect("Could not parse schema to json"); - compiler - .add_resource("/tmp/cql2.json", schema_json) - .expect("Could not add schema to compiler"); - let index = compiler - .compile("/tmp/cql2.json", &mut schemas) - .expect("Could not compile schema"); - Validator { schemas, index } - } - - pub fn validate(self, obj: serde_json::Value) -> bool { - let valid = self.schemas.validate(&obj, self.index); - match valid { - Ok(()) => true, - Err(e) => { - let debug_level: &str = - &std::env::var("CQL2_DEBUG_LEVEL").unwrap_or("1".to_string()); - match debug_level { - "3" => { - println!("-----------\n{e:#?}\n---------------") - } - "2" => { - println!("-----------\n{e:?}\n---------------") - } - "1" => { - println!("-----------\n{e}\n---------------") - } - _ => { - println!("-----------\nCQL2 Is Invalid!\n---------------") - } - } - - false - } - } + let schema_json = serde_json::from_str(include_str!("cql2.json"))?; + compiler.add_resource("/tmp/cql2.json", schema_json)?; + let index = compiler.compile("/tmp/cql2.json", &mut schemas)?; + Ok(Validator { schemas, index }) } - pub fn validate_str(self, obj: &str) -> bool { - self.validate(serde_json::from_str(obj).expect("Could not convert string to json.")) - } -} -impl Default for Validator { - fn default() -> Self { - Self::new() + /// Validates a [serde_json::Value]. + /// + /// # Examples + /// + /// ``` + /// use cql2::Validator; + /// use serde_json::json; + /// + /// let validator = Validator::new().unwrap(); + /// + /// let valid = json!({ + /// "op": "=", + /// "args": [ + /// { "property": "landsat:scene_id" }, + /// "LC82030282019133LGN00" + /// ] + /// }); + /// validator.validate(&valid).unwrap(); + /// + /// let invalid = json!({ + /// "op": "not an operator!", + /// }); + /// validator.validate(&invalid).unwrap_err(); + /// ``` + pub fn validate<'a, 'b>(&'a self, value: &'b Value) -> Result<(), ValidationError<'a, 'b>> { + self.schemas.validate(value, self.index) } } @@ -96,14 +108,74 @@ impl Expr { fn as_sql() -> String { return "sql".to_string(); } */ - pub fn as_json(&self) -> String { - serde_json::to_string(&self).unwrap() + + /// Converts this expression to a JSON string. + /// + /// # Examples + /// + /// ``` + /// use cql2::Expr; + /// + /// let expr = Expr::BoolConst(true); + /// let s = expr.to_json().unwrap(); + /// ``` + pub fn to_json(&self) -> Result { + serde_json::to_string(&self) } - pub fn as_json_pretty(&self) -> String { - serde_json::to_string_pretty(&self).unwrap() + + /// Converts this expression to a pretty JSON string. + /// + /// # Examples + /// + /// ``` + /// use cql2::Expr; + /// + /// let expr = Expr::BoolConst(true); + /// let s = expr.to_json_pretty().unwrap(); + /// ``` + pub fn to_json_pretty(&self) -> Result { + serde_json::to_string_pretty(&self) } - pub fn validate(&self) -> bool { - Validator::new().validate_str(&self.as_json()) + + /// Converts this expression to a [serde_json::Value]. + /// + /// # Examples + /// + /// ``` + /// use cql2::Expr; + /// + /// let expr = Expr::BoolConst(true); + /// let value = expr.to_value().unwrap(); + /// ``` + pub fn to_value(&self) -> Result { + serde_json::to_value(self) + } + + /// Returns true if this expression is valid CQL2. + /// + /// For detailed error reporting, use [Validator::validate] in conjunction with [Expr::to_value]. + /// + /// # Examples + /// + /// ``` + /// use cql2::Expr; + /// + /// let expr = Expr::BoolConst(true); + /// assert!(expr.is_valid()); + /// ``` + /// + /// # Panics + /// + /// Panics if the default validator can't be created. + pub fn is_valid(&self) -> bool { + serde_json::to_value(self) + .map(|value| { + Validator::new() + .expect("should be able to create the default validator") + .validate(&value) + .is_ok() + }) + .unwrap_or_default() } } diff --git a/src/main.rs b/src/main.rs index e3498cf..d8e3359 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,13 +1,37 @@ -use cql2::parse; -use std::io::{self, BufRead}; -fn main() -> io::Result<()> { - for line in io::stdin().lock().lines() { - let parsed = parse(&line?); +use cql2::Validator; +use std::io::BufRead; +fn main() { + let debug_level: u8 = std::env::var("CQL2_DEBUG_LEVEL") + .map(|s| { + s.parse() + .unwrap_or_else(|_| panic!("CQL2_DEBUG_LEVEL should be an integer: {}", s)) + }) + .unwrap_or(1); + let validator = Validator::new().unwrap(); + + let mut ok = true; + for line in std::io::stdin().lock().lines() { + let parsed = cql2::parse(&line.unwrap()); println!("Parsed: {:#?}", &parsed); - println!("{}", parsed.as_json()); + println!("{}", parsed.to_json().unwrap()); + let value = serde_json::to_value(parsed).unwrap(); + + if let Err(err) = validator.validate(&value) { + match debug_level { + 0 => println!("-----------\nCQL2 Is Invalid!\n---------------"), + 1 => println!("-----------\n{err}\n---------------"), + 2 => println!("-----------\n{err:#}\n---------------"), + _ => { + let detailed_output = err.detailed_output(); + println!("-----------\n{detailed_output:#}\n---------------"); + } + } + ok = false; + } + } - parsed.validate(); + if !ok { + std::process::exit(1); } - Ok(()) } diff --git a/tests/ogc_tests.rs b/tests/ogc_tests.rs index 40e7c73..c95c272 100644 --- a/tests/ogc_tests.rs +++ b/tests/ogc_tests.rs @@ -1,25 +1,19 @@ -use cql2::{parse, Validator}; +use cql2::{parse, Expr, Validator}; use rstest::rstest; use std::fs; use std::path::PathBuf; pub fn validate_file(f: &str) { - //println!("Current Directory: {:#?}", env::current_dir()); println!("File Path: {:#?}", f); let cql2 = fs::read_to_string(f).unwrap(); println!("CQL2: {}", cql2); - let expr: cql2::Expr = parse(&cql2); - println!("Expr: {}", expr.as_json_pretty()); - let valid = expr.validate(); - assert!(valid) -} + let expr: Expr = parse(&cql2); + println!("Expr: {}", expr.to_json_pretty().unwrap()); -#[rstest] -fn json_examples_are_valid(#[files("tests/fixtures/json/*.json")] path: PathBuf) { - let cql2 = fs::read_to_string(path).unwrap(); - let validator = Validator::new(); - let result = validator.validate_str(&cql2); - assert!(result) + Validator::new() + .unwrap() + .validate(&expr.to_value().unwrap()) + .unwrap(); } #[rstest] From d4c1bc326a38f7e616e50a919a0d2f59660577ff Mon Sep 17 00:00:00 2001 From: David W Bitner Date: Thu, 25 Jul 2024 14:27:17 -0500 Subject: [PATCH 2/6] format --- tests/ogc_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ogc_tests.rs b/tests/ogc_tests.rs index 4d64f8b..ee838fb 100644 --- a/tests/ogc_tests.rs +++ b/tests/ogc_tests.rs @@ -1,5 +1,5 @@ use assert_json_diff::assert_json_eq; -use cql2::{parse}; +use cql2::parse; use rstest::rstest; use serde_json::json; use std::fs; From c81559e0a460d5504203ff02a58c754a2ea4bc98 Mon Sep 17 00:00:00 2001 From: David W Bitner Date: Thu, 25 Jul 2024 14:44:16 -0500 Subject: [PATCH 3/6] remove features from cargo.toml --- Cargo.toml | 3 --- 1 file changed, 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2cf0e0c..e0d1504 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,9 +3,6 @@ name = "cql2" version = "0.1.0" edition = "2021" -[features] -defaults = ["bin"] - [dependencies] geozero = { git = "https://github.com/bitner/geozero.git", rev = "45828d7" } boon = "0.6.0" From 93c4d3602f02e7e181e6f79621341f3c6ea905bf Mon Sep 17 00:00:00 2001 From: David W Bitner Date: Thu, 25 Jul 2024 14:45:38 -0500 Subject: [PATCH 4/6] remove features from cargo.toml --- src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 492027b..d7be6a3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -613,4 +613,3 @@ pub fn parse_stdin() -> Expr { #[cfg(test)] use {assert_json_diff as _, rstest as _}; - From 6b8ca57a2591cbdf22e4e53664ea1df47bb00a0c Mon Sep 17 00:00:00 2001 From: David W Bitner Date: Thu, 25 Jul 2024 14:47:18 -0500 Subject: [PATCH 5/6] remove required features from bin entries --- Cargo.toml | 2 -- 1 file changed, 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e0d1504..0e7fc85 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,9 +21,7 @@ rstest = "0.21.0" [[bin]] name = "cql2json" path = "src/bin/cql2json.rs" -required-features = ["bin"] [[bin]] name = "cql2text" path = "src/bin/cql2text.rs" -required-features = ["bin"] From bab329a725792344cf4a693d9db63c4a5c70b600 Mon Sep 17 00:00:00 2001 From: David W Bitner Date: Thu, 25 Jul 2024 15:59:47 -0500 Subject: [PATCH 6/6] fixes per review --- src/bin/cql2json.rs | 2 +- src/lib.rs | 41 +++++++++++++++++++++++------------------ tests/ogc_tests.rs | 8 ++++---- 3 files changed, 28 insertions(+), 23 deletions(-) diff --git a/src/bin/cql2json.rs b/src/bin/cql2json.rs index 0cd2983..7382a2e 100644 --- a/src/bin/cql2json.rs +++ b/src/bin/cql2json.rs @@ -2,5 +2,5 @@ use cql2::parse_stdin; fn main() { let parsed = parse_stdin(); - println!("{}", parsed.as_json().unwrap()); + println!("{}", parsed.to_json().unwrap()); } diff --git a/src/lib.rs b/src/lib.rs index d7be6a3..9d3b30c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -146,7 +146,7 @@ impl Expr { } } - /// Converts this expression to a SqlQuery Struct + /// Converts this expression to a [SqlQuery] struct /// with parameters separated to use with parameter binding /// /// # Examples @@ -155,18 +155,18 @@ impl Expr { /// use cql2::Expr; /// /// let expr = Expr::Bool(true); - /// let s = expr.as_sql(); + /// let s = expr.to_sql(); /// ``` - pub fn as_sql(&self) -> SqlQuery { + pub fn to_sql(&self) -> SqlQuery { let params: &mut Vec = &mut vec![]; - let query = self.as_sql_inner(params); + let query = self.to_sql_inner(params); SqlQuery { query, params: params.to_vec(), } } - fn as_sql_inner(&self, params: &mut Vec) -> String { + fn to_sql_inner(&self, params: &mut Vec) -> String { match self { Expr::Bool(v) => { params.push(v.to_string()); @@ -180,11 +180,11 @@ impl Expr { params.push(v.to_string()); format!("${}", params.len()) } - Expr::Date { date } => date.as_sql_inner(params), - Expr::Timestamp { timestamp } => timestamp.as_sql_inner(params), + Expr::Date { date } => date.to_sql_inner(params), + Expr::Timestamp { timestamp } => timestamp.to_sql_inner(params), Expr::Interval { interval } => { - let a: Vec = interval.iter().map(|x| x.as_sql_inner(params)).collect(); + let a: Vec = interval.iter().map(|x| x.to_sql_inner(params)).collect(); format!("TSTZRANGE({},{})", a[0], a[1],) } Expr::Geometry(v) => { @@ -193,12 +193,12 @@ impl Expr { format!("${}", params.len()) } Expr::Array(v) => { - let array_els: Vec = v.iter().map(|a| a.as_sql_inner(params)).collect(); + let array_els: Vec = v.iter().map(|a| a.to_sql_inner(params)).collect(); format!("[{}]", array_els.join(", ")) } Expr::Property { property } => format!("\"{property}\""), Expr::Operation { op, args } => { - let a: Vec = args.iter().map(|x| x.as_sql_inner(params)).collect(); + let a: Vec = args.iter().map(|x| x.to_sql_inner(params)).collect(); match op.as_str() { "and" => format!("({})", a.join(" AND ")), "or" => format!("({})", a.join(" OR ")), @@ -212,7 +212,7 @@ impl Expr { } } Expr::BBox { bbox } => { - let array_els: Vec = bbox.iter().map(|a| a.as_sql_inner(params)).collect(); + let array_els: Vec = bbox.iter().map(|a| a.to_sql_inner(params)).collect(); format!("[{}]", array_els.join(", ")) } } @@ -226,9 +226,9 @@ impl Expr { /// use cql2::Expr; /// /// let expr = Expr::Bool(true); - /// let s = expr.as_json().unwrap(); + /// let s = expr.to_json().unwrap(); /// ``` - pub fn as_json(&self) -> Result { + pub fn to_json(&self) -> Result { serde_json::to_string(&self) } @@ -240,9 +240,9 @@ impl Expr { /// use cql2::Expr; /// /// let expr = Expr::Bool(true); - /// let s = expr.as_json_pretty().unwrap(); + /// let s = expr.to_json_pretty().unwrap(); /// ``` - pub fn as_json_pretty(&self) -> Result { + pub fn to_json_pretty(&self) -> Result { serde_json::to_string_pretty(&self) } @@ -277,9 +277,14 @@ impl Expr { /// /// Panics if the default validator can't be created. pub fn is_valid(&self) -> bool { - let value = serde_json::to_value(self).unwrap(); - let validator = Validator::new().unwrap(); - validator.validate(&value).is_ok() + let value = serde_json::to_value(self); + match &value { + Ok(value) => { + let validator = Validator::new().expect("Could not create default validator"); + validator.validate(value).is_ok() + } + _ => false, + } } } diff --git a/tests/ogc_tests.rs b/tests/ogc_tests.rs index ee838fb..145cc73 100644 --- a/tests/ogc_tests.rs +++ b/tests/ogc_tests.rs @@ -10,21 +10,21 @@ pub fn validate_str(cql2: &str) { let expr: cql2::Expr = parse(cql2); let outcql2: String = expr.as_cql2_text(); println!("Out CQL2: {}", outcql2); - println!("Expr: {}", expr.as_json().unwrap()); + println!("Expr: {}", expr.to_json().unwrap()); let valid = expr.is_valid(); assert!(valid); // make sure that the cql2 text generated by as_cql2 is valid as well let expr2: cql2::Expr = parse(&outcql2); - let js = expr2.as_json().unwrap(); + let js = expr2.to_json().unwrap(); println!("Expr2: {}", js); let valid2 = expr2.is_valid(); assert!(valid2); // make sure that when reparsing the output that the json is the same let expr3 = parse(&js); - println!("Expr3: {}", expr3.as_json().unwrap()); - assert_json_eq!(json!(js), json!(expr3.as_json().unwrap())); + println!("Expr3: {}", expr3.to_json().unwrap()); + assert_json_eq!(json!(js), json!(expr3.to_json().unwrap())); } pub fn validate_file(f: &str) {