From f40fdee9cde0182b54251ecc23d17f906f5fc6a5 Mon Sep 17 00:00:00 2001 From: William Brown Date: Tue, 20 Nov 2018 16:51:10 +1000 Subject: [PATCH] Fixed a large number of error cases --- src/entry.rs | 18 ++++++---- src/error.rs | 12 +++---- src/filter.rs | 2 -- src/log.rs | 1 - src/main.rs | 98 ++++++++++++++++++++------------------------------- src/schema.rs | 82 ++++++++++++++++++++---------------------- 6 files changed, 94 insertions(+), 119 deletions(-) diff --git a/src/entry.rs b/src/entry.rs index 2d95312e0..3eeb9ea69 100644 --- a/src/entry.rs +++ b/src/entry.rs @@ -1,7 +1,6 @@ // use serde_json::{Error, Value}; use std::collections::btree_map::{Iter as BTreeIter, IterMut as BTreeIterMut}; use std::collections::BTreeMap; -use std::marker::PhantomData; use std::slice::Iter as SliceIter; // make a trait entry for everything to adhere to? @@ -116,10 +115,15 @@ impl Entry { .entry(attr) .and_modify(|v| { // Here we need to actually do a check/binary search ... - v.binary_search(&value).map_err(|idx| { - // This cloning is to fix a borrow issue ... - v.insert(idx, value.clone()) - }); + // FIXME: Because map_err is lazy, this won't do anything on release + match v.binary_search(&value) { + Ok(_) => {} + Err(idx) => { + // This cloning is to fix a borrow issue with the or_insert below. + // Is there a better way? + v.insert(idx, value.clone()) + } + } }) .or_insert(vec![value]); } @@ -281,7 +285,7 @@ mod tests { let u: User = User::new("william", "William Brown"); let d = serde_json::to_string_pretty(&u).unwrap(); - let u2: User = serde_json::from_str(d.as_str()).unwrap(); + let _u2: User = serde_json::from_str(d.as_str()).unwrap(); } #[test] @@ -290,7 +294,7 @@ mod tests { e.add_ava(String::from("userid"), String::from("william")); - let d = serde_json::to_string_pretty(&e).unwrap(); + let _d = serde_json::to_string_pretty(&e).unwrap(); } #[test] diff --git a/src/error.rs b/src/error.rs index 585a30c4a..60c45f1f7 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,10 +1,10 @@ #[derive(Debug, PartialEq)] pub enum SchemaError { - NOT_IMPLEMENTED, - INVALID_CLASS, + NotImplemented, + InvalidClass, // FIXME: Is there a way to say what we are missing on error? - MISSING_MUST_ATTRIBUTE, - INVALID_ATTRIBUTE, - INVALID_ATTRIBUTE_SYNTAX, - EMPTY_FILTER, + MissingMustAttribute, + InvalidAttribute, + InvalidAttributeSyntax, + EmptyFilter, } diff --git a/src/filter.rs b/src/filter.rs index e75585a71..e2ceeaa73 100644 --- a/src/filter.rs +++ b/src/filter.rs @@ -3,7 +3,6 @@ // entry to assert it matches. use super::entry::Entry; -use super::schema::Schema; use std::cmp::{Ordering, PartialOrd}; // Perhaps make these json serialisable. Certainly would make parsing @@ -128,7 +127,6 @@ impl PartialOrd for Filter { #[cfg(test)] mod tests { - use super::super::schema::Schema; use super::Filter; use serde_json; use std::cmp::{Ordering, PartialOrd}; diff --git a/src/log.rs b/src/log.rs index 1e2411247..1c30faeb5 100644 --- a/src/log.rs +++ b/src/log.rs @@ -19,7 +19,6 @@ macro_rules! log_event { }) } - // We need to pass in config for this later // Or we need to pass in the settings for it IE level and dest? // Is there an efficent way to set a log level filter in the macros diff --git a/src/main.rs b/src/main.rs index cbe2c67b6..214fba8e0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -12,7 +12,7 @@ extern crate uuid; // use actix::prelude::*; use actix_web::{ error, http, middleware, App, AsyncResponder, Error, FutureResponse, HttpMessage, HttpRequest, - HttpResponse, Json, Path, State + HttpResponse, Path, State, }; use bytes::BytesMut; @@ -72,68 +72,55 @@ fn class_list((_name, state): (Path, State)) -> FutureResponse } fn search( - (item, req): (Json, HttpRequest), -) -> Box> { - Box::new( - req.state() - .qe - .send( - // FIXME: Item is BORROWED, so we have to .clone it. - // We should treat this as immutable and let the caller - // clone when mutation is required ... - // however, that involves lifetime complexities. - event::SearchEvent::new(item.filter.clone()), - ) - .from_err() - .and_then(|res| match res { - // FIXME: entries should not be EventResult type - Ok(entries) => Ok(HttpResponse::Ok().json(entries)), - Err(_) => Ok(HttpResponse::InternalServerError().into()), - // Err(_) => Ok(error::ErrorInternalServerError("Test error").into()), - }), - ) -} - -// Based on actix web example json -fn search2(req: &HttpRequest) -> Box> { - println!("{:?}", req); + (req, state): (HttpRequest, State), +) -> impl Future { // HttpRequest::payload() is stream of Bytes objects req.payload() + // `Future::from_err` acts like `?` in that it coerces the error type from + // the future into the final error type .from_err() // `fold` will asynchronously read each chunk of the request body and // call supplied closure, then it resolves to result of closure .fold(BytesMut::new(), move |mut body, chunk| { // limit max size of in-memory payload if (body.len() + chunk.len()) > MAX_SIZE { - Err(error::ErrorBadRequest("Request size too large.")) + Err(error::ErrorBadRequest("overflow")) } else { body.extend_from_slice(&chunk); Ok(body) } }) - .and_then(|body| { - // body is loaded, now we can deserialize serde-json - // FIXME: THIS IS FUCKING AWFUL - let obj = serde_json::from_slice::(&body).unwrap(); - // Dispatch a search - println!("{:?}", obj); - // We have to resolve this NOW else we break everything :( - /* - req.state().qe.send( - event::SearchEvent::new(obj.filter) - ) - .from_err() - .and_then(|res| future::result(match res { - // What type is entry? - Ok(event::EventResult::Search { entries }) => Ok(HttpResponse::Ok().json(entries)), - Ok(_) => Ok(HttpResponse::Ok().into()), - // Can we properly report this? - Err(_) => Ok(HttpResponse::InternalServerError().into()), - })) - */ - Ok(HttpResponse::InternalServerError().into()) - }) - .responder() + // `Future::and_then` can be used to merge an asynchronous workflow with a + // synchronous workflow + .and_then( + move |body| -> Box> { + // body is loaded, now we can deserialize serde-json + let r_obj = serde_json::from_slice::(&body); + + // Send to the db for create + match r_obj { + Ok(obj) => { + let res = state + .qe + .send( + // Could make this a .into_inner() and move? + event::SearchEvent::new(obj.filter), + ) + .from_err() + .and_then(|res| match res { + Ok(entries) => Ok(HttpResponse::Ok().json(entries)), + Err(_) => Ok(HttpResponse::InternalServerError().into()), + }); + + Box::new(res) + } + Err(e) => Box::new(future::err(error::ErrorBadRequest(format!( + "Json Decode Failed: {:?}", + e + )))), + } + }, + ) } fn main() { @@ -162,7 +149,7 @@ fn main() { // Connect all our end points here. .middleware(middleware::Logger::default()) .resource("/", |r| r.f(index)) - // + // /* .resource("/create", |r| { r.method(http::Method::POST) @@ -171,15 +158,8 @@ fn main() { */ // curl --header "Content-Type: application/json" --request POST --data '{ "filter" : { "Eq": ["class", "user"] }}' http://127.0.0.1:8080/search .resource("/search", |r| { - r.method(http::Method::POST) - /* - .with_config(extract_item_limit, |cfg| { - cfg.0.limit(4096); // <- limit size of the payload - }) - */ - .with(search) + r.method(http::Method::POST).with_async(search) }) - .resource("/search2", |r| r.method(http::Method::POST).a(search2)) // Add an ldap compat search function type? .resource("/list/{class_list}", |r| { r.method(http::Method::GET).with(class_list) diff --git a/src/schema.rs b/src/schema.rs index d85c8f213..c49f802be 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -21,6 +21,7 @@ enum Ternary { False, } +#[allow(non_camel_case_types)] #[derive(Debug, Clone, PartialEq)] pub enum IndexType { EQUALITY, @@ -44,6 +45,7 @@ impl TryFrom<&str> for IndexType { } } +#[allow(non_camel_case_types)] #[derive(Debug, Clone, PartialEq)] pub enum SyntaxType { // We need an insensitive string type too ... @@ -94,19 +96,19 @@ impl SchemaAttribute { // Validation. fn validate_bool(&self, v: &String) -> Result<(), SchemaError> { bool::from_str(v.as_str()) - .map_err(|_| SchemaError::INVALID_ATTRIBUTE_SYNTAX) + .map_err(|_| SchemaError::InvalidAttributeSyntax) .map(|_| ()) } fn validate_syntax(&self, v: &String) -> Result<(), SchemaError> { SyntaxType::try_from(v.as_str()) - .map_err(|_| SchemaError::INVALID_ATTRIBUTE_SYNTAX) + .map_err(|_| SchemaError::InvalidAttributeSyntax) .map(|_| ()) } fn validate_index(&self, v: &String) -> Result<(), SchemaError> { IndexType::try_from(v.as_str()) - .map_err(|_| SchemaError::INVALID_ATTRIBUTE_SYNTAX) + .map_err(|_| SchemaError::InvalidAttributeSyntax) .map(|_| ()) } @@ -116,7 +118,7 @@ impl SchemaAttribute { if &t == v { Ok(()) } else { - Err(SchemaError::INVALID_ATTRIBUTE_SYNTAX) + Err(SchemaError::InvalidAttributeSyntax) } } @@ -133,7 +135,7 @@ impl SchemaAttribute { pub fn validate_ava(&self, ava: &Vec) -> Result<(), SchemaError> { // Check multivalue if self.multivalue == false && ava.len() > 1 { - return Err(SchemaError::INVALID_ATTRIBUTE_SYNTAX); + return Err(SchemaError::InvalidAttributeSyntax); }; // If syntax, check the type is correct match self.syntax { @@ -639,7 +641,7 @@ impl Schema { }); if c_valid != Ternary::True { - return Err(SchemaError::INVALID_CLASS); + return Err(SchemaError::InvalidClass); }; let classes: HashMap = entry @@ -666,6 +668,7 @@ impl Schema { .map(|s| (s.clone(), self.attributes.get(s).unwrap())) .collect(); + /* let may: HashMap = classes .iter() // Join our class systemmmust + must into one iter @@ -678,16 +681,18 @@ impl Schema { }) .map(|s| (s.clone(), self.attributes.get(s).unwrap())) .collect(); + */ // FIXME: Error needs to say what is missing // We need to return *all* missing attributes. // Check that all must are inplace // for each attr in must, check it's present on our ent - for (attr_name, attr) in must { + // FIXME: Could we iter over only the attr_name + for (attr_name, _attr) in must { let avas = entry.get_ava(&attr_name); if avas.is_none() { - return Err(SchemaError::MISSING_MUST_ATTRIBUTE); + return Err(SchemaError::MissingMustAttribute); } } @@ -706,7 +711,7 @@ impl Schema { } None => { if !extensible { - return Err(SchemaError::INVALID_ATTRIBUTE); + return Err(SchemaError::InvalidAttribute); } } } @@ -755,25 +760,25 @@ impl Schema { match filt { Filter::Eq(attr, value) => match self.attributes.get(attr) { Some(schema_a) => schema_a.validate_value(value), - None => Err(SchemaError::INVALID_ATTRIBUTE), + None => Err(SchemaError::InvalidAttribute), }, Filter::Sub(attr, value) => match self.attributes.get(attr) { Some(schema_a) => schema_a.validate_value(value), - None => Err(SchemaError::INVALID_ATTRIBUTE), + None => Err(SchemaError::InvalidAttribute), }, Filter::Pres(attr) => { // This could be better as a contains_key // because we never use the value match self.attributes.get(attr) { Some(_) => Ok(()), - None => Err(SchemaError::INVALID_ATTRIBUTE), + None => Err(SchemaError::InvalidAttribute), } } Filter::Or(filters) => { // This should never happen because // optimising should remove them as invalid parts? if filters.len() == 0 { - return Err(SchemaError::EMPTY_FILTER); + return Err(SchemaError::EmptyFilter); }; filters.iter().fold(Ok(()), |acc, filt| { if acc.is_ok() { @@ -787,7 +792,7 @@ impl Schema { // This should never happen because // optimising should remove them as invalid parts? if filters.len() == 0 { - return Err(SchemaError::EMPTY_FILTER); + return Err(SchemaError::EmptyFilter); }; filters.iter().fold(Ok(()), |acc, filt| { if acc.is_ok() { @@ -801,7 +806,7 @@ impl Schema { // This should never happen because // optimising should remove them as invalid parts? if filters.len() == 0 { - return Err(SchemaError::EMPTY_FILTER); + return Err(SchemaError::EmptyFilter); }; filters.iter().fold(Ok(()), |acc, filt| { if acc.is_ok() { @@ -867,17 +872,6 @@ mod tests { #[test] fn test_schema_attribute_simple() { - let class_attribute = SchemaAttribute { - // class: vec![String::from("attributetype")], - name: String::from("class"), - description: String::from("The set of classes defining an object"), - system: true, - secret: false, - multivalue: true, - index: vec![IndexType::EQUALITY], - syntax: SyntaxType::UTF8STRING_INSENSITIVE, - }; - // Test schemaAttribute validation of types. // Test single value string @@ -897,7 +891,7 @@ mod tests { let r2 = single_value_string.validate_ava(&vec![String::from("test1"), String::from("test2")]); - assert_eq!(r2, Err(SchemaError::INVALID_ATTRIBUTE_SYNTAX)); + assert_eq!(r2, Err(SchemaError::InvalidAttributeSyntax)); // test multivalue string, boolean @@ -929,7 +923,7 @@ mod tests { let r3 = multi_value_boolean.validate_ava(&vec![String::from("test1"), String::from("test2")]); - assert_eq!(r3, Err(SchemaError::INVALID_ATTRIBUTE_SYNTAX)); + assert_eq!(r3, Err(SchemaError::InvalidAttributeSyntax)); let r4 = multi_value_boolean.validate_ava(&vec![String::from("true"), String::from("false")]); @@ -951,7 +945,7 @@ mod tests { assert_eq!(r6, Ok(())); let r7 = single_value_syntax.validate_ava(&vec![String::from("thaeountaheu")]); - assert_eq!(r7, Err(SchemaError::INVALID_ATTRIBUTE_SYNTAX)); + assert_eq!(r7, Err(SchemaError::InvalidAttributeSyntax)); let single_value_index = SchemaAttribute { // class: vec![String::from("attributetype")], @@ -968,7 +962,7 @@ mod tests { assert_eq!(r8, Ok(())); let r9 = single_value_index.validate_ava(&vec![String::from("thaeountaheu")]); - assert_eq!(r9, Err(SchemaError::INVALID_ATTRIBUTE_SYNTAX)); + assert_eq!(r9, Err(SchemaError::InvalidAttributeSyntax)); } #[test] @@ -1002,7 +996,7 @@ mod tests { assert_eq!( schema.validate_entry(&e_no_class), - Err(SchemaError::INVALID_CLASS) + Err(SchemaError::InvalidClass) ); let e_bad_class: Entry = serde_json::from_str( @@ -1015,7 +1009,7 @@ mod tests { .unwrap(); assert_eq!( schema.validate_entry(&e_bad_class), - Err(SchemaError::INVALID_CLASS) + Err(SchemaError::InvalidClass) ); let e_attr_invalid: Entry = serde_json::from_str( @@ -1029,7 +1023,7 @@ mod tests { assert_eq!( schema.validate_entry(&e_attr_invalid), - Err(SchemaError::MISSING_MUST_ATTRIBUTE) + Err(SchemaError::MissingMustAttribute) ); let e_attr_invalid_may: Entry = serde_json::from_str( @@ -1050,7 +1044,7 @@ mod tests { assert_eq!( schema.validate_entry(&e_attr_invalid_may), - Err(SchemaError::INVALID_ATTRIBUTE) + Err(SchemaError::InvalidAttribute) ); let e_attr_invalid_syn: Entry = serde_json::from_str( @@ -1070,7 +1064,7 @@ mod tests { assert_eq!( schema.validate_entry(&e_attr_invalid_syn), - Err(SchemaError::INVALID_ATTRIBUTE_SYNTAX) + Err(SchemaError::InvalidAttributeSyntax) ); let e_ok: Entry = serde_json::from_str( @@ -1114,7 +1108,7 @@ mod tests { .unwrap(); assert_eq!( schema.validate_entry(&e_test), - Err(SchemaError::INVALID_ATTRIBUTE_SYNTAX) + Err(SchemaError::InvalidAttributeSyntax) ); let e_expect: Entry = serde_json::from_str( @@ -1154,7 +1148,7 @@ mod tests { assert_eq!( schema.validate_entry(&e_extensible_bad), - Err(SchemaError::INVALID_ATTRIBUTE_SYNTAX) + Err(SchemaError::InvalidAttributeSyntax) ); let e_extensible: Entry = serde_json::from_str( @@ -1210,7 +1204,7 @@ mod tests { #[test] fn test_schema_filter_validation() { - let mut schema = Schema::new(); + let schema = Schema::new(); // Test mixed case attr name let f_mixed: Filter = serde_json::from_str( r#"{ @@ -1222,7 +1216,7 @@ mod tests { .unwrap(); assert_eq!( schema.validate_filter(&f_mixed), - Err(SchemaError::INVALID_ATTRIBUTE) + Err(SchemaError::InvalidAttribute) ); // test syntax of bool let f_bool: Filter = serde_json::from_str( @@ -1235,7 +1229,7 @@ mod tests { .unwrap(); assert_eq!( schema.validate_filter(&f_bool), - Err(SchemaError::INVALID_ATTRIBUTE_SYNTAX) + Err(SchemaError::InvalidAttributeSyntax) ); // test insensitise values let f_insense: Filter = serde_json::from_str( @@ -1248,7 +1242,7 @@ mod tests { .unwrap(); assert_eq!( schema.validate_filter(&f_insense), - Err(SchemaError::INVALID_ATTRIBUTE_SYNTAX) + Err(SchemaError::InvalidAttributeSyntax) ); // Test the recursive structures validate let f_or_empty: Filter = serde_json::from_str( @@ -1259,7 +1253,7 @@ mod tests { .unwrap(); assert_eq!( schema.validate_filter(&f_or_empty), - Err(SchemaError::EMPTY_FILTER) + Err(SchemaError::EmptyFilter) ); let f_or: Filter = serde_json::from_str( r#"{ @@ -1271,7 +1265,7 @@ mod tests { .unwrap(); assert_eq!( schema.validate_filter(&f_or), - Err(SchemaError::INVALID_ATTRIBUTE_SYNTAX) + Err(SchemaError::InvalidAttributeSyntax) ); let f_or_mult: Filter = serde_json::from_str( r#"{ @@ -1284,7 +1278,7 @@ mod tests { .unwrap(); assert_eq!( schema.validate_filter(&f_or_mult), - Err(SchemaError::INVALID_ATTRIBUTE_SYNTAX) + Err(SchemaError::InvalidAttributeSyntax) ); let f_or_ok: Filter = serde_json::from_str( r#"{