From 4f459465c100cf5459438a60571f3c85e4d82703 Mon Sep 17 00:00:00 2001 From: William Brown Date: Tue, 27 Nov 2018 20:48:21 +1000 Subject: [PATCH] Add support for working server integration test! --- CHECKLIST.md | 3 + src/core.rs | 35 +++++----- src/entry.rs | 24 +++++++ src/event.rs | 66 +++++++++++++++---- src/lib.rs | 13 ++-- src/proto.rs | 31 --------- src/proto_v1.rs | 66 +++++++++++++++++++ src/server.rs | 23 ++++--- .../{integration_test.rs => proto_v1_test.rs} | 26 +++++--- 9 files changed, 201 insertions(+), 86 deletions(-) create mode 100644 CHECKLIST.md delete mode 100644 src/proto.rs create mode 100644 src/proto_v1.rs rename tests/{integration_test.rs => proto_v1_test.rs} (78%) diff --git a/CHECKLIST.md b/CHECKLIST.md new file mode 100644 index 000000000..b28b04f64 --- /dev/null +++ b/CHECKLIST.md @@ -0,0 +1,3 @@ + + + diff --git a/src/core.rs b/src/core.rs index 4c9d0bc90..d01c18014 100644 --- a/src/core.rs +++ b/src/core.rs @@ -8,10 +8,10 @@ use bytes::BytesMut; use futures::{future, Future, Stream}; use super::config::Configuration; -use super::event::{CreateEvent, EventResult, SearchEvent}; +use super::event::{CreateEvent, SearchEvent, SearchResult}; use super::filter::Filter; use super::log; -use super::proto::{CreateRequest, SearchRequest}; +use super::proto_v1::{CreateRequest, SearchRequest, Response, SearchResponse}; use super::server; struct AppState { @@ -20,7 +20,7 @@ struct AppState { } macro_rules! json_event_decode { - ($req:expr, $state:expr, $event_type:ty, $message_type:ty) => {{ + ($req:expr, $state:expr, $event_type:ty, $response_type:ty, $message_type:ty) => {{ // This is copied every request. Is there a better way? // The issue is the fold move takes ownership of state if // we don't copy this here @@ -58,11 +58,13 @@ macro_rules! json_event_decode { .send( // Could make this a .into_inner() and move? // event::SearchEvent::new(obj.filter), - <($event_type)>::new(obj), + <($event_type)>::from_request(obj), ) .from_err() .and_then(|res| match res { - Ok(entries) => Ok(HttpResponse::Ok().json(entries)), + Ok(event_result) => Ok(HttpResponse::Ok().json( + event_result.response() + )), Err(e) => Ok(HttpResponse::InternalServerError().json(e)), }); @@ -101,15 +103,15 @@ fn class_list((_name, state): (Path, State)) -> FutureResponse // // FIXME: Don't use SEARCHEVENT here!!!! // - SearchEvent::new(SearchRequest::new(filt)), + SearchEvent::from_request(SearchRequest::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(EventResult::Search { entries }) => Ok(HttpResponse::Ok().json(entries)), - Ok(_) => Ok(HttpResponse::Ok().into()), + Ok(search_result) => Ok(HttpResponse::Ok().json( search_result.response() )), + // Ok(_) => Ok(HttpResponse::Ok().into()), // Can we properly report this? Err(_) => Ok(HttpResponse::InternalServerError().into()), }) @@ -120,13 +122,13 @@ fn class_list((_name, state): (Path, State)) -> FutureResponse fn create( (req, state): (HttpRequest, State), ) -> impl Future { - json_event_decode!(req, state, CreateEvent, CreateRequest) + json_event_decode!(req, state, CreateEvent, Response, CreateRequest) } fn search( (req, state): (HttpRequest, State), ) -> impl Future { - json_event_decode!(req, state, SearchEvent, SearchRequest) + json_event_decode!(req, state, SearchEvent, SearchResponse, SearchRequest) } pub fn create_server_core(config: Configuration) { @@ -153,19 +155,16 @@ pub fn create_server_core(config: Configuration) { // Connect all our end points here. .middleware(middleware::Logger::default()) .resource("/", |r| r.f(index)) - // curl --header "Content-Type: application/json" --request POST --data '{ "entries": [ {"attrs": {"class": ["group"], "name": ["testgroup"], "description": ["testperson"]}}]}' http://127.0.0.1:8080/create - .resource("/create", |r| { + // curl --header "Content-Type: application/json" --request POST --data '{ "entries": [ {"attrs": {"class": ["group"], "name": ["testgroup"], "description": ["testperson"]}}]}' http://127.0.0.1:8080/v1/create + .resource("/v1/create", |r| { r.method(http::Method::POST).with_async(create) }) - // curl --header "Content-Type: application/json" --request POST --data '{ "filter" : { "Eq": ["class", "user"] }}' http://127.0.0.1:8080/search - .resource("/search", |r| { + // curl --header "Content-Type: application/json" --request POST --data '{ "filter" : { "Eq": ["class", "user"] }}' http://127.0.0.1:8080/v1/search + .resource("/v1/search", |r| { r.method(http::Method::POST).with_async(search) }) // Add an ldap compat search function type? - .resource("/list/{class_list}", |r| { - r.method(http::Method::GET).with(class_list) - }) - .resource("/list/{class_list}/", |r| { + .resource("/v1/list/{class_list}", |r| { r.method(http::Method::GET).with(class_list) }) }) diff --git a/src/entry.rs b/src/entry.rs index 3eeb9ea69..6476e4e17 100644 --- a/src/entry.rs +++ b/src/entry.rs @@ -1,4 +1,5 @@ // use serde_json::{Error, Value}; +use super::proto_v1::Entry as ProtoEntry; use std::collections::btree_map::{Iter as BTreeIter, IterMut as BTreeIterMut}; use std::collections::BTreeMap; use std::slice::Iter as SliceIter; @@ -179,6 +180,29 @@ impl Entry { inner: self.attrs.iter_mut(), } } + + // FIXME: Can we consume protoentry? + pub fn from(e: &ProtoEntry) -> Self { + // Why not the trait? In the future we may want to extend + // this with server aware functions for changes of the + // incoming data. + Entry { + // For now, we do a straight move + attrs: e.attrs.clone(), + } + } + + pub fn into(&self) -> ProtoEntry { + // It's very likely that at this stage we'll need to apply + // access controls, dynamic attributes or more. + // As a result, this may not even be the right place + // for the conversion as algorithmically it may be + // better to do this from the outside view. This can + // of course be identified and changed ... + ProtoEntry { + attrs: self.attrs.clone(), + } + } } impl Clone for Entry { diff --git a/src/event.rs b/src/event.rs index 2c60fdd9c..45cf41c77 100644 --- a/src/event.rs +++ b/src/event.rs @@ -1,5 +1,6 @@ use super::filter::Filter; -use super::proto::{CreateRequest, SearchRequest}; +use super::proto_v1::Entry as ProtoEntry; +use super::proto_v1::{CreateRequest, SearchRequest, SearchResponse, Response}; use actix::prelude::*; use entry::Entry; use error::OperationError; @@ -7,12 +8,41 @@ use error::OperationError; // Should the event Result have the log items? // FIXME: Remove seralising here - each type should // have it's own result type! -#[derive(Serialize, Deserialize, Debug)] -pub enum EventResult { - Search { entries: Vec }, - Modify, - Delete, - Create, + +#[derive(Debug)] +pub struct OpResult { +} + +impl OpResult { + pub fn response(self) -> Response { + Response{} + } +} + +#[derive(Debug)] +pub struct SearchResult { + entries: Vec, +} + +impl SearchResult { + pub fn new(entries: Vec) -> Self { + SearchResult { + // FIXME: Can we consume this iter? + entries: entries.iter().map(|e| { + // FIXME: The issue here is this probably is applying transforms + // like access control ... May need to change. + e.into() + + }).collect() + } + } + + // Consume self into a search response + pub fn response(self) -> SearchResponse { + SearchResponse { + entries: self.entries + } + } } // At the top we get "event types" and they contain the needed @@ -25,11 +55,11 @@ pub struct SearchEvent { } impl Message for SearchEvent { - type Result = Result; + type Result = Result; } impl SearchEvent { - pub fn new(request: SearchRequest) -> Self { + pub fn from_request(request: SearchRequest) -> Self { SearchEvent { filter: request.filter, class: (), @@ -48,13 +78,25 @@ pub struct CreateEvent { } impl Message for CreateEvent { - type Result = Result; + type Result = Result; } +// FIXME: Should this actually be in createEvent handler? impl CreateEvent { - pub fn new(request: CreateRequest) -> Self { + pub fn from_request(request: CreateRequest) -> Self { CreateEvent { - entries: request.entries, + // From ProtoEntry -> Entry + // What is the correct consuming iterator here? Can we + // even do that? + entries: request.entries.iter().map(|e| + Entry::from(e) + ).collect(), + } + } + + pub fn from_vec(entries: Vec) -> Self { + CreateEvent { + entries: entries } } } diff --git a/src/lib.rs b/src/lib.rs index 9704d4fd1..891679f31 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,16 +24,17 @@ extern crate env_logger; // This has to be before be so the import order works #[macro_use] -pub mod log; +mod log; #[macro_use] mod audit; mod be; +mod entry; +mod event; +mod schema; +mod server; + pub mod config; pub mod core; -pub mod entry; pub mod error; -pub mod event; pub mod filter; -pub mod proto; -pub mod schema; -pub mod server; +pub mod proto_v1; diff --git a/src/proto.rs b/src/proto.rs deleted file mode 100644 index e6a612bc0..000000000 --- a/src/proto.rs +++ /dev/null @@ -1,31 +0,0 @@ -use super::entry::Entry; -use super::filter::Filter; - -// These proto implementations are here because they have public definitions - -// FIXME: We probably need a proto entry to transform our -// server core entry into. - -// FIXME: Proto Response as well here - -#[derive(Debug, Serialize, Deserialize)] -pub struct SearchRequest { - pub filter: Filter, -} - -impl SearchRequest { - pub fn new(filter: Filter) -> Self { - SearchRequest { filter: filter } - } -} - -#[derive(Debug, Serialize, Deserialize)] -pub struct CreateRequest { - pub entries: Vec, -} - -impl CreateRequest { - pub fn new(entries: Vec) -> Self { - CreateRequest { entries: entries } - } -} diff --git a/src/proto_v1.rs b/src/proto_v1.rs new file mode 100644 index 000000000..50a8afcd7 --- /dev/null +++ b/src/proto_v1.rs @@ -0,0 +1,66 @@ +// use super::entry::Entry; +use super::filter::Filter; +use std::collections::BTreeMap; + +// These proto implementations are here because they have public definitions + +// FIXME: We probably need a proto entry to transform our +// server core entry into. We also need to get from proto +// entry to our actual entry. +// +// There is agood future reason for this seperation. It allows changing +// the in memory server core entry type, without affecting the proto + +#[derive(Debug, Serialize, Deserialize, Clone)] +pub struct Entry { + pub attrs: BTreeMap>, +} + +// FIXME: Do I need proto filter? +// Probably yes, don't be shit william. + +#[derive(Debug, Serialize, Deserialize)] +pub struct Response { + // ? +} + +impl Response { + pub fn new(_: ()) -> Self { + Response{} + } +} + +#[derive(Debug, Serialize, Deserialize)] +pub struct SearchRequest { + pub filter: Filter, +} + +impl SearchRequest { + pub fn new(filter: Filter) -> Self { + SearchRequest { filter: filter } + } +} + +#[derive(Debug, Serialize, Deserialize)] +pub struct SearchResponse { + pub entries: Vec, +} + +impl SearchResponse { + pub fn new(entries: Vec) -> Self { + SearchResponse { + entries: entries + } + } +} + +#[derive(Debug, Serialize, Deserialize)] +pub struct CreateRequest { + pub entries: Vec, +} + +impl CreateRequest { + pub fn new(entries: Vec) -> Self { + CreateRequest { entries: entries } + } +} diff --git a/src/server.rs b/src/server.rs index bc85fcd8b..c7a9c3e0b 100644 --- a/src/server.rs +++ b/src/server.rs @@ -5,7 +5,7 @@ use be::{Backend, BackendError}; use entry::Entry; use error::OperationError; -use event::{CreateEvent, EventResult, SearchEvent}; +use event::{CreateEvent, SearchEvent, SearchResult, OpResult}; use log::EventLog; use schema::Schema; @@ -125,7 +125,7 @@ impl Actor for QueryServer { // at this point our just is just to route to do_ impl Handler for QueryServer { - type Result = Result; + type Result = Result; fn handle(&mut self, msg: SearchEvent, _: &mut Self::Context) -> Self::Result { let mut audit = AuditEvent::new(); @@ -139,7 +139,7 @@ impl Handler for QueryServer { // was this ok? let res = match self.search(&mut audit, &msg) { - Ok(entries) => Ok(EventResult::Search { entries: entries }), + Ok(entries) => Ok(SearchResult::new(entries)), Err(e) => Err(e), }; @@ -152,7 +152,7 @@ impl Handler for QueryServer { } impl Handler for QueryServer { - type Result = Result; + type Result = Result; fn handle(&mut self, msg: CreateEvent, _: &mut Self::Context) -> Self::Result { let mut audit = AuditEvent::new(); @@ -160,7 +160,7 @@ impl Handler for QueryServer { audit_log!(audit, "Begin create event {:?}", msg); let res = match self.create(&mut audit, &msg) { - Ok(()) => Ok(EventResult::Create), + Ok(()) => Ok(OpResult{}), Err(e) => Err(e), }; @@ -192,7 +192,8 @@ mod tests { use super::super::event::{CreateEvent, SearchEvent}; use super::super::filter::Filter; use super::super::log; - use super::super::proto::{CreateRequest, SearchRequest}; + use super::super::proto_v1::{CreateRequest, SearchRequest}; + use super::super::proto_v1::Entry as ProtoEntry; use super::super::schema::Schema; use super::super::server::QueryServer; @@ -226,8 +227,8 @@ mod tests { run_test!(|_log, mut server: QueryServer, audit: &mut AuditEvent| { let filt = Filter::Pres(String::from("name")); - let se1 = SearchEvent::new(SearchRequest::new(filt.clone())); - let se2 = SearchEvent::new(SearchRequest::new(filt)); + let se1 = SearchEvent::from_request(SearchRequest::new(filt.clone())); + let se2 = SearchEvent::from_request(SearchRequest::new(filt)); let e: Entry = serde_json::from_str( r#"{ @@ -243,7 +244,7 @@ mod tests { let expected = vec![e]; - let ce = CreateEvent::new(CreateRequest::new(expected.clone())); + let ce = CreateEvent::from_vec(expected.clone()); let r1 = server.search(audit, &se1).unwrap(); assert!(r1.len() == 0); @@ -260,4 +261,8 @@ mod tests { future::ok(()) }); } + + // Test Create Empty + + // } diff --git a/tests/integration_test.rs b/tests/proto_v1_test.rs similarity index 78% rename from tests/integration_test.rs rename to tests/proto_v1_test.rs index 2674110eb..28428639b 100644 --- a/tests/integration_test.rs +++ b/tests/proto_v1_test.rs @@ -4,12 +4,7 @@ use actix::prelude::*; extern crate rsidm; use rsidm::config::Configuration; use rsidm::core::create_server_core; -use rsidm::entry::Entry; -use rsidm::event::EventResult; -use rsidm::log::{self, EventLog, LogEvent}; -use rsidm::proto::{CreateRequest, SearchRequest}; -use rsidm::server::{self, QueryServer}; -// use be; +use rsidm::proto_v1::{CreateRequest, SearchRequest, SearchResponse, Response, Entry}; extern crate reqwest; @@ -21,7 +16,6 @@ use std::sync::mpsc; use std::thread; extern crate tokio; -// use tokio::executor::current_thread::CurrentThread; // Test external behaviorus of the service. @@ -62,18 +56,30 @@ fn test_server_proto() { run_test!(|| { let client = reqwest::Client::new(); + let e: Entry = serde_json::from_str( + r#"{ + "attrs": { + "class": ["person"], + "name": ["testperson"], + "description": ["testperson"], + "displayname": ["testperson"] + } + }"#, + ) + .unwrap(); + let c = CreateRequest { - entries: Vec::new(), + entries: vec![e] }; let mut response = client - .post("http://127.0.0.1:8080/create") + .post("http://127.0.0.1:8080/v1/create") .body(serde_json::to_string(&c).unwrap()) .send() .unwrap(); println!("{:?}", response); - let r: EventResult = serde_json::from_str(response.text().unwrap().as_str()).unwrap(); + let r: Response = serde_json::from_str(response.text().unwrap().as_str()).unwrap(); println!("{:?}", r);