From 56e4809c28765f4cfb3da7ed14a7cc2fa121a7a8 Mon Sep 17 00:00:00 2001 From: William Brown Date: Sun, 11 Nov 2018 13:39:11 +1300 Subject: [PATCH] Add partialeq and partialcmp allowing tests to work --- src/be/mod.rs | 32 +++++---- src/entry.rs | 18 ++++- src/event.rs | 16 +++-- src/filter.rs | 144 +++++++++++++++++++++++++++++++++++++- src/lib.rs | 10 +-- src/main.rs | 34 +++++---- src/server.rs | 72 ++++++++++++++----- tests/integration_test.rs | 9 +-- 8 files changed, 272 insertions(+), 63 deletions(-) diff --git a/src/be/mod.rs b/src/be/mod.rs index d4b322769..b1406d17a 100644 --- a/src/be/mod.rs +++ b/src/be/mod.rs @@ -84,7 +84,8 @@ impl Backend { ) ", NO_PARAMS, - ).unwrap(); + ) + .unwrap(); // Create a version table for migration indication @@ -98,7 +99,7 @@ impl Backend { } } - pub fn create(&mut self, entries: Vec) -> Result { + pub fn create(&mut self, entries: &Vec) -> Result { log_event!(self.log, "Begin create"); let be_audit = BackendAuditEvent::new(); @@ -119,7 +120,8 @@ impl Backend { .map(|val| { // TODO: Should we do better than unwrap? serde_json::to_string(&val).unwrap() - }).collect(); + }) + .collect(); log_event!(self.log, "serialising: {:?}", ser_entries); @@ -134,7 +136,8 @@ impl Backend { conn.execute( "INSERT INTO id2entry (data) VALUES (?1)", &[&ser_entry as &ToSql], - ).unwrap(); + ) + .unwrap(); } // TODO: update indexes (as needed) @@ -148,7 +151,7 @@ impl Backend { } // Take filter, and AuditEvent ref? - pub fn search(&self, filt: Filter) -> Vec { + pub fn search(&self, filt: &Filter) -> Result, ()> { // Do things // Alloc a vec for the entries. // FIXME: Make this actually a good size for the result set ... @@ -172,7 +175,8 @@ impl Backend { .query_map(NO_PARAMS, |row| IdEntry { id: row.get(0), data: row.get(1), - }).unwrap(); + }) + .unwrap(); for row in id2entry_iter { println!("{:?}", row); // FIXME: Handle this properly. @@ -193,9 +197,10 @@ impl Backend { } else { None } - }).collect(); + }) + .collect(); - entries + Ok(entries) } pub fn modify() {} @@ -222,14 +227,13 @@ mod tests { extern crate futures; use futures::future; - use futures::future::lazy; use futures::future::Future; extern crate tokio; use super::super::entry::Entry; use super::super::filter::Filter; - use super::super::log::{self, EventLog, LogEvent}; + use super::super::log::{self, EventLog}; use super::{Backend, BackendError}; macro_rules! run_test { @@ -237,7 +241,7 @@ mod tests { System::run(|| { let test_log = log::start(); - let mut be = Backend::new(test_log.clone(), ""); + let be = Backend::new(test_log.clone(), ""); // Could wrap another future here for the future::ok bit... let fut = $test_fn(test_log, be); @@ -257,7 +261,7 @@ mod tests { run_test!(|log: actix::Addr, mut be: Backend| { log_event!(log, "Simple Create"); - let empty_result = be.create(Vec::new()); + let empty_result = be.create(&Vec::new()); log_event!(log, "{:?}", empty_result); assert_eq!(empty_result, Err(BackendError::EmptyRequest)); @@ -266,13 +270,13 @@ mod tests { .unwrap(); assert!(e.validate()); - let single_result = be.create(vec![e]); + let single_result = be.create(&vec![e]); assert!(single_result.is_ok()); // Construct a filter let filt = Filter::Pres(String::from("userid")); - let entries = be.search(filt); + let entries = be.search(&filt).unwrap(); println!("{:?}", entries); // There should only be one entry so is this enough? diff --git a/src/entry.rs b/src/entry.rs index 7aa4d82b2..8e33e9d08 100644 --- a/src/entry.rs +++ b/src/entry.rs @@ -1,4 +1,4 @@ -use serde_json::{Error, Value}; +// use serde_json::{Error, Value}; use std::collections::BTreeMap; // make a trait entry for everything to adhere to? @@ -62,6 +62,22 @@ impl Entry { } } +impl Clone for Entry { + fn clone(&self) -> Entry { + Entry { + attrs: self.attrs.clone(), + } + } +} + +impl PartialEq for Entry { + fn eq(&self, rhs: &Entry) -> bool { + // FIXME: This is naive. Later it should be schema + // aware checking. + self.attrs == rhs.attrs + } +} + // pub trait Entry { //fn to_json_str(&self) -> String; // fn to_index_diff -> ??? diff --git a/src/event.rs b/src/event.rs index 2247e13b5..fbffcbfa9 100644 --- a/src/event.rs +++ b/src/event.rs @@ -1,3 +1,4 @@ +use super::filter::Filter; use actix::prelude::*; use entry::Entry; @@ -10,12 +11,15 @@ pub enum EventResult { Create, } +#[derive(Debug)] +pub struct RawSearchEvent {} + // At the top we get "event types" and they contain the needed // actions, and a generic event component. #[derive(Debug)] pub struct SearchEvent { - filter: (), + pub filter: Filter, class: (), // String // It could be better to box this later ... event: AuditEvent, @@ -26,9 +30,9 @@ impl Message for SearchEvent { } impl SearchEvent { - pub fn new() -> Self { + pub fn new(filter: Filter) -> Self { SearchEvent { - filter: (), + filter: filter, class: (), event: AuditEvent { time_start: (), @@ -44,7 +48,7 @@ impl SearchEvent { pub struct CreateEvent { // This may still actually change to handle the *raw* nature of the // input that we plan to parse. - entries: Vec, + pub entries: Vec, // It could be better to box this later ... event: AuditEvent, } @@ -54,9 +58,9 @@ impl Message for CreateEvent { } impl CreateEvent { - pub fn new() -> Self { + pub fn new(entries: Vec) -> Self { CreateEvent { - entries: Vec::new(), + entries: entries, event: AuditEvent { time_start: (), time_end: (), diff --git a/src/filter.rs b/src/filter.rs index ed4bd755e..d7323c859 100644 --- a/src/filter.rs +++ b/src/filter.rs @@ -3,6 +3,7 @@ // entry to assert it matches. use super::entry::Entry; +use std::cmp::{Ordering, PartialOrd}; // Perhaps make these json serialisable. Certainly would make parsing // simpler ... @@ -21,11 +22,22 @@ pub enum Filter { impl Filter { fn optimise(mut self) -> Self { // Apply optimisations to the filter + // An easy way would be imple partialOrd + // then do sort on the or/and/not + // as the general conditions we want + // to optimise on are in those ... + // + // The other big one is folding redundant + // terms down. + // + // If an or/not/and condition has no items, remove it + // + // If its the root item? self } // In the future this will probably be used with schema ... - fn validate(mut self) -> Result<(), ()> { + fn validate(&self) -> Result<(), ()> { Ok(()) } @@ -38,6 +50,9 @@ impl Filter { // What other parse types do we need? + // FIXME: This check should be in ENTRY not here, because it's up to others + // to interpret filter meaning and application!!! + // Assert if this filter matches the entry (no index) pub fn entry_match_no_index(&self, e: &Entry) -> bool { // Go through the filter components and check them in the entry. @@ -56,10 +71,68 @@ impl Filter { } } +impl Clone for Filter { + fn clone(&self) -> Self { + // I think we only need to match self then new + clone? + match self { + Filter::Eq(a, v) => Filter::Eq(a.clone(), v.clone()), + Filter::Sub(a, v) => Filter::Sub(a.clone(), v.clone()), + Filter::Pres(a) => Filter::Pres(a.clone()), + Filter::Or(l) => Filter::Or(l.clone()), + Filter::And(l) => Filter::And(l.clone()), + Filter::Not(l) => Filter::Not(l.clone()), + } + } +} + +impl PartialEq for Filter { + fn eq(&self, rhs: &Filter) -> bool { + match (self, rhs) { + (Filter::Eq(a1, v1), Filter::Eq(a2, v2)) => a1 == a2 && v1 == v2, + (Filter::Sub(a1, v1), Filter::Sub(a2, v2)) => a1 == a2 && v1 == v2, + (Filter::Pres(a1), Filter::Pres(a2)) => a1 == a2, + (Filter::Or(l1), Filter::Or(l2)) => l1 == l2, + (Filter::And(l1), Filter::And(l2)) => l1 == l2, + (Filter::Not(l1), Filter::Not(l2)) => l1 == l2, + (_, _) => false, + } + } +} + +// remember, this isn't ordering by alphanumeric, this is ordering of +// optimisation preference! +// +impl PartialOrd for Filter { + fn partial_cmp(&self, rhs: &Filter) -> Option { + match (self, rhs) { + (Filter::Eq(a1, _), Filter::Eq(a2, _)) => { + // Order attr name, then value + // Later me may add rules to put certain attrs ahead due + // to optimisation rules + a1.partial_cmp(a2) + } + (Filter::Sub(a1, _), Filter::Sub(a2, _)) => a1.partial_cmp(a2), + (Filter::Pres(a1), Filter::Pres(a2)) => a1.partial_cmp(a2), + (Filter::Eq(_, _), _) => { + // Always higher prefer Eq over all else, as these will have + // the best indexes and return smallest candidates. + Some(Ordering::Less) + } + (_, Filter::Eq(_, _)) => Some(Ordering::Greater), + (Filter::Pres(_), _) => Some(Ordering::Less), + (_, Filter::Pres(_)) => Some(Ordering::Greater), + (Filter::Sub(_, _), _) => Some(Ordering::Greater), + (_, Filter::Sub(_, _)) => Some(Ordering::Less), + (_, _) => Some(Ordering::Equal), + } + } +} + #[cfg(test)] mod tests { use super::Filter; use serde_json; + use std::cmp::{Ordering, PartialOrd}; #[test] fn test_filter_simple() { @@ -82,4 +155,73 @@ mod tests { fn test_filter_optimise() { // Given sets of "optimisable" filters, optimise them. } + + #[test] + fn test_filter_eq() { + let f_t1a = Filter::Pres(String::from("userid")); + let f_t1b = Filter::Pres(String::from("userid")); + let f_t1c = Filter::Pres(String::from("zzzz")); + + assert_eq!(f_t1a == f_t1b, true); + assert_eq!(f_t1a == f_t1c, false); + assert_eq!(f_t1b == f_t1c, false); + + let f_t2a = Filter::And(vec![f_t1a]); + let f_t2b = Filter::And(vec![f_t1b]); + let f_t2c = Filter::And(vec![f_t1c]); + assert_eq!(f_t2a == f_t2b, true); + assert_eq!(f_t2a == f_t2c, false); + assert_eq!(f_t2b == f_t2c, false); + + assert_eq!(f_t2c == Filter::Pres(String::from("test")), false); + } + + #[test] + fn test_filter_ord() { + // Test that we uphold the rules of partialOrd + // Basic equality + // Test the two major paths here (str vs list) + let f_t1a = Filter::Pres(String::from("userid")); + let f_t1b = Filter::Pres(String::from("userid")); + + assert_eq!(f_t1a.partial_cmp(&f_t1b), Some(Ordering::Equal)); + assert_eq!(f_t1b.partial_cmp(&f_t1a), Some(Ordering::Equal)); + + let f_t2a = Filter::And(vec![]); + let f_t2b = Filter::And(vec![]); + assert_eq!(f_t2a.partial_cmp(&f_t2b), Some(Ordering::Equal)); + assert_eq!(f_t2b.partial_cmp(&f_t2a), Some(Ordering::Equal)); + + // antisymmetry: if a < b then !(a > b), as well as a > b implying !(a < b); and + let f_t3b = Filter::Eq(String::from("userid"), String::from("")); + assert_eq!(f_t1a.partial_cmp(&f_t3b), Some(Ordering::Greater)); + assert_eq!(f_t3b.partial_cmp(&f_t1a), Some(Ordering::Less)); + + // transitivity: a < b and b < c implies a < c. The same must hold for both == and >. + let f_t4b = Filter::Sub(String::from("userid"), String::from("")); + assert_eq!(f_t1a.partial_cmp(&f_t4b), Some(Ordering::Less)); + assert_eq!(f_t3b.partial_cmp(&f_t4b), Some(Ordering::Less)); + + assert_eq!(f_t4b.partial_cmp(&f_t1a), Some(Ordering::Greater)); + assert_eq!(f_t4b.partial_cmp(&f_t3b), Some(Ordering::Greater)); + } + + #[test] + fn test_filter_clone() { + // Test that cloning filters yields the same result regardless of + // complexity. + let f_t1a = Filter::Pres(String::from("userid")); + let f_t1b = f_t1a.clone(); + let f_t1c = Filter::Pres(String::from("zzzz")); + + assert_eq!(f_t1a == f_t1b, true); + assert_eq!(f_t1a == f_t1c, false); + + let f_t2a = Filter::And(vec![f_t1a]); + let f_t2b = f_t2a.clone(); + let f_t2c = Filter::And(vec![f_t1c]); + + assert_eq!(f_t2a == f_t2b, true); + assert_eq!(f_t2a == f_t2c, false); + } } diff --git a/src/lib.rs b/src/lib.rs index 0bae7e0c9..219d3be32 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,12 +10,12 @@ extern crate r2d2_sqlite; extern crate rusqlite; extern crate uuid; -use actix::prelude::*; -use actix_web::{ - http, middleware, App, AsyncResponder, FutureResponse, HttpRequest, HttpResponse, Path, State, -}; +// use actix::prelude::*; +// use actix_web::{ +// http, middleware, App, AsyncResponder, FutureResponse, HttpRequest, HttpResponse, Path, State, +// }; -use futures::Future; +// use futures::Future; // This has to be before be so the import order works #[macro_use] diff --git a/src/main.rs b/src/main.rs index c5f6a2ad2..9512e26c9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,24 +1,24 @@ extern crate serde; extern crate serde_json; -#[macro_use] -extern crate serde_derive; +// #[macro_use] extern crate actix; extern crate actix_web; extern crate futures; +extern crate serde_derive; extern crate uuid; -use actix::prelude::*; +// use actix::prelude::*; use actix_web::{ - http, middleware, App, AsyncResponder, FutureResponse, HttpRequest, HttpResponse, Path, State, + http, App, AsyncResponder, FutureResponse, HttpRequest, HttpResponse, Path, State, }; use futures::Future; #[macro_use] extern crate rsidm; -use rsidm::be; use rsidm::event; -use rsidm::log::{self, EventLog}; +use rsidm::filter::Filter; +use rsidm::log; use rsidm::server; struct AppState { @@ -34,8 +34,10 @@ fn index(req: &HttpRequest) -> HttpResponse { HttpResponse::Ok().body("Hello\n") } -fn class_list((name, state): (Path, State)) -> FutureResponse { +fn class_list((_name, state): (Path, State)) -> FutureResponse { // println!("request to class_list"); + let filt = Filter::Pres(String::from("objectclass")); + state .qe .send( @@ -43,14 +45,17 @@ fn class_list((name, state): (Path, State)) -> FutureResponse< // LONG TERM // Make a search REQUEST, and create the audit struct here, then // pass it to the server - event::SearchEvent::new() + // + // FIXME: Don't use SEARCHEVENT here!!!! + // + event::SearchEvent::new(filt), ) // TODO: How to time this part of the code? // What does this do? .from_err() .and_then(|res| match res { // What type is entry? - Ok(event::EventResult::Search{ entries }) => Ok(HttpResponse::Ok().json(entries)), + 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()), @@ -84,9 +89,14 @@ fn main() { // Connect all our end points here. // .middleware(middleware::Logger::default()) .resource("/", |r| r.f(index)) - .resource("/{class_list}", |r| r.method(http::Method::GET).with(class_list)) - .resource("/{class_list}/", |r| r.method(http::Method::GET).with(class_list)) - }).bind("127.0.0.1:8080") + .resource("/{class_list}", |r| { + r.method(http::Method::GET).with(class_list) + }) + .resource("/{class_list}/", |r| { + r.method(http::Method::GET).with(class_list) + }) + }) + .bind("127.0.0.1:8080") .unwrap() .start(); diff --git a/src/server.rs b/src/server.rs index 4840bfc01..5bc6bc68d 100644 --- a/src/server.rs +++ b/src/server.rs @@ -46,15 +46,26 @@ impl QueryServer { // Actually conduct a search request // This is the core of the server, as it processes the entire event // applies all parts required in order and more. - pub fn search(&mut self) -> Result, ()> { - Ok(Vec::new()) + pub fn search(&mut self, se: &SearchEvent) -> Result, ()> { + match self.be.search(&se.filter) { + Ok(r) => Ok(r), + Err(_) => Err(()), + } } // What should this take? // This should probably take raw encoded entries? Or sohuld they // be handled by fe? - pub fn create(&mut self) -> Result<(), ()> { - Ok(()) + pub fn create(&mut self, ce: &CreateEvent) -> Result<(), ()> { + // Start a txn + // Run any pre checks + // We may change from ce.entries later to something else? + match self.be.create(&ce.entries) { + Ok(_) => Ok(()), + Err(_) => Err(()), + } + // Run and post checks + // Commit/Abort the txn } } @@ -75,8 +86,10 @@ impl Handler for QueryServer { // Parse what we need from the event? // What kind of event is it? + // In the future we'll likely change search event ... + // was this ok? - let res = match self.search() { + let res = match self.search(&msg) { Ok(entries) => Ok(EventResult::Search { entries: entries }), Err(e) => Err(e), }; @@ -92,13 +105,20 @@ impl Handler for QueryServer { fn handle(&mut self, msg: CreateEvent, _: &mut Self::Context) -> Self::Result { log_event!(self.log, "Begin event {:?}", msg); - Err(()) + + let res = match self.create(&msg) { + Ok(()) => Ok(EventResult::Create), + Err(e) => Err(e), + }; + + log_event!(self.log, "End event {:?}", msg); + // At the end of the event we send it for logging. + res } } // Auth requests? How do we structure these ... - #[cfg(test)] mod tests { extern crate actix; @@ -106,26 +126,28 @@ mod tests { extern crate futures; use futures::future; - use futures::future::lazy; use futures::future::Future; extern crate tokio; - use super::super::server::QueryServer; use super::super::be::Backend; - use super::super::log::{self, EventLog, LogEvent}; + use super::super::entry::Entry; + use super::super::event::{CreateEvent, SearchEvent}; + use super::super::filter::Filter; + use super::super::log; + use super::super::server::QueryServer; macro_rules! run_test { ($test_fn:expr) => {{ System::run(|| { let test_log = log::start(); - let mut be = Backend::new(test_log.clone(), ""); - let mut test_server = QueryServer::new(test_log.clone(), be); + let be = Backend::new(test_log.clone(), ""); + let test_server = QueryServer::new(test_log.clone(), be); // Could wrap another future here for the future::ok bit... let fut = $test_fn(test_log, test_server); - let comp_fut = fut.map_err(|()| ()).and_then(|r| { + let comp_fut = fut.map_err(|()| ()).and_then(|_r| { println!("Stopping actix ..."); actix::System::current().stop(); future::result(Ok(())) @@ -136,19 +158,33 @@ mod tests { }}; } - #[test] fn test_be_create_user() { - run_test!(|log, mut server: QueryServer| { - let r1 = server.search().unwrap(); + run_test!(|_log, mut server: QueryServer| { + let filt = Filter::Pres(String::from("userid")); + + let se1 = SearchEvent::new(filt.clone()); + let se2 = SearchEvent::new(filt); + + let mut e: Entry = Entry::new(); + e.add_ava(String::from("userid"), String::from("william")) + .unwrap(); + + let expected = vec![e]; + + let ce = CreateEvent::new(expected.clone()); + + let r1 = server.search(&se1).unwrap(); assert!(r1.len() == 0); - let cr = server.create(); + let cr = server.create(&ce); assert!(cr.is_ok()); - let r2 = server.search().unwrap(); + let r2 = server.search(&se2).unwrap(); assert!(r2.len() == 1); + assert_eq!(r2, expected); + future::ok(()) }); } diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 6db88daa8..9558c67e8 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -1,8 +1,6 @@ extern crate actix; use actix::prelude::*; -use std::panic; - extern crate rsidm; use rsidm::log::{self, EventLog, LogEvent}; use rsidm::server::{self, QueryServer}; @@ -10,11 +8,10 @@ use rsidm::server::{self, QueryServer}; extern crate futures; use futures::future; -use futures::future::lazy; use futures::future::Future; extern crate tokio; -use tokio::executor::current_thread::CurrentThread; +// use tokio::executor::current_thread::CurrentThread; // Test external behaviorus of the service. @@ -41,7 +38,7 @@ macro_rules! run_test { // Now chain them ... // Now append the server shutdown. - let comp_fut = fut.map_err(|_| ()).and_then(|r| { + let comp_fut = fut.map_err(|_| ()).and_then(|_r| { println!("Stopping actix ..."); actix::System::current().stop(); future::result(Ok(())) @@ -58,7 +55,7 @@ macro_rules! run_test { #[test] fn test_schema() { run_test!( - |log: actix::Addr, server: actix::Addr| log.send(LogEvent { + |log: actix::Addr, _server: actix::Addr| log.send(LogEvent { msg: String::from("Test log event") }) );