Skip to content

Commit

Permalink
fix: rewrote settings logic to mimic aw-server-python behavior (#428)
Browse files Browse the repository at this point in the history
* fix: rewrote settings logic to mimic aw-server-python behavior

* refactor: removed redundant KeyValue struct

* format: applied cargo fmt

* format: applied cargo clippy --fix
  • Loading branch information
ErikBjare committed Oct 19, 2023
1 parent 7538163 commit 728c00e
Show file tree
Hide file tree
Showing 11 changed files with 179 additions and 237 deletions.
47 changes: 21 additions & 26 deletions aw-datastore/src/datastore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use serde_json::value::Value;
use aw_models::Bucket;
use aw_models::BucketMetadata;
use aw_models::Event;
use aw_models::KeyValue;

use rusqlite::params;
use rusqlite::types::ToSql;
Expand Down Expand Up @@ -897,7 +896,7 @@ impl DatastoreInstance {
Ok(())
}

pub fn get_key_value(&self, conn: &Connection, key: &str) -> Result<KeyValue, DatastoreError> {
pub fn get_key_value(&self, conn: &Connection, key: &str) -> Result<String, DatastoreError> {
let mut stmt = match conn.prepare(
"
SELECT * FROM key_value WHERE KEY = ?1",
Expand All @@ -910,16 +909,7 @@ impl DatastoreInstance {
}
};

match stmt.query_row([key], |row| {
Ok(KeyValue {
key: row.get(0)?,
value: row.get(1)?,
timestamp: Some(DateTime::from_utc(
NaiveDateTime::from_timestamp_opt(row.get(2)?, 0).unwrap(),
Utc,
)),
})
}) {
match stmt.query_row([key], |row| row.get(1)) {
Ok(result) => Ok(result),
Err(err) => match err {
rusqlite::Error::QueryReturnedNoRows => {
Expand All @@ -932,12 +922,12 @@ impl DatastoreInstance {
}
}

pub fn get_keys_starting(
pub fn get_key_values(
&self,
conn: &Connection,
pattern: &str,
) -> Result<Vec<String>, DatastoreError> {
let mut stmt = match conn.prepare("SELECT key FROM key_value WHERE key LIKE ?") {
) -> Result<HashMap<String, String>, DatastoreError> {
let mut stmt = match conn.prepare("SELECT key, value FROM key_value WHERE key LIKE ?") {
Ok(stmt) => stmt,
Err(err) => {
return Err(DatastoreError::InternalError(format!(
Expand All @@ -946,25 +936,30 @@ impl DatastoreInstance {
}
};

let mut output = Vec::<String>::new();
let mut output = HashMap::<String, String>::new();
// Rusqlite's get wants index and item type as parameters.
let result = stmt.query_map([pattern], |row| row.get::<usize, String>(0));
let result = stmt.query_map([pattern], |row| {
Ok((row.get::<usize, String>(0)?, row.get::<usize, String>(1)?))
});
match result {
Ok(keys) => {
for row in keys {
Ok(settings) => {
for row in settings {
// Unwrap to String or panic on SQL row if type is invalid. Can't happen with a
// properly initialized table.
output.push(row.unwrap());
let (key, value) = row.unwrap();
// Only return keys starting with "settings.".
if !key.starts_with("settings.") {
continue;
}
output.insert(key, value);
}
Ok(output)
}
Err(err) => match err {
rusqlite::Error::QueryReturnedNoRows => {
Err(DatastoreError::NoSuchKey(pattern.to_string()))
}
_ => Err(DatastoreError::InternalError(format!(
"Failed to get key_value rows starting with pattern {pattern}"
))),
rusqlite::Error::QueryReturnedNoRows => Ok(output),
_ => Err(DatastoreError::InternalError(
"Failed to get settings".to_string(),
)),
},
}
}
Expand Down
59 changes: 29 additions & 30 deletions aw-datastore/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use rusqlite::TransactionBehavior;

use aw_models::Bucket;
use aw_models::Event;
use aw_models::KeyValue;

use crate::DatastoreError;
use crate::DatastoreInstance;
Expand Down Expand Up @@ -51,8 +50,8 @@ pub enum Response {
Event(Event),
EventList(Vec<Event>),
Count(i64),
KeyValue(KeyValue),
StringVec(Vec<String>),
KeyValue(String),
KeyValues(HashMap<String, String>),
}

#[allow(clippy::large_enum_variant)]
Expand All @@ -74,9 +73,9 @@ pub enum Command {
GetEventCount(String, Option<DateTime<Utc>>, Option<DateTime<Utc>>),
DeleteEventsById(String, Vec<i64>),
ForceCommit(),
InsertKeyValue(String, String),
GetKeyValues(String),
GetKeyValue(String),
GetKeysStarting(String),
SetKeyValue(String, String),
DeleteKeyValue(String),
Close(),
}
Expand Down Expand Up @@ -275,18 +274,18 @@ impl DatastoreWorker {
self.commit = true;
Ok(Response::Empty())
}
Command::InsertKeyValue(key, data) => match ds.insert_key_value(tx, &key, &data) {
Command::GetKeyValues(pattern) => match ds.get_key_values(tx, pattern.as_str()) {
Ok(result) => Ok(Response::KeyValues(result)),
Err(e) => Err(e),
},
Command::SetKeyValue(key, data) => match ds.insert_key_value(tx, &key, &data) {
Ok(()) => Ok(Response::Empty()),
Err(e) => Err(e),
},
Command::GetKeyValue(key) => match ds.get_key_value(tx, &key) {
Ok(result) => Ok(Response::KeyValue(result)),
Err(e) => Err(e),
},
Command::GetKeysStarting(pattern) => match ds.get_keys_starting(tx, &pattern) {
Ok(result) => Ok(Response::StringVec(result)),
Err(e) => Err(e),
},
Command::DeleteKeyValue(key) => match ds.delete_key_value(tx, &key) {
Ok(()) => Ok(Response::Empty()),
Err(e) => Err(e),
Expand Down Expand Up @@ -475,46 +474,46 @@ impl Datastore {
}
}

pub fn insert_key_value(&self, key: &str, data: &str) -> Result<(), DatastoreError> {
let cmd = Command::InsertKeyValue(key.to_string(), data.to_string());
let receiver = self.requester.request(cmd).unwrap();

_unwrap_response(receiver)
}

pub fn delete_key_value(&self, key: &str) -> Result<(), DatastoreError> {
let cmd = Command::DeleteKeyValue(key.to_string());
let receiver = self.requester.request(cmd).unwrap();

_unwrap_response(receiver)
}

pub fn get_key_value(&self, key: &str) -> Result<KeyValue, DatastoreError> {
let cmd = Command::GetKeyValue(key.to_string());
pub fn get_key_values(&self, pattern: &str) -> Result<HashMap<String, String>, DatastoreError> {
let cmd = Command::GetKeyValues(pattern.to_string());
let receiver = self.requester.request(cmd).unwrap();

match receiver.collect().unwrap() {
Ok(r) => match r {
Response::KeyValue(value) => Ok(value),
Response::KeyValues(value) => Ok(value),
_ => panic!("Invalid response"),
},
Err(e) => Err(e),
}
}

pub fn get_keys_starting(&self, pattern: &str) -> Result<Vec<String>, DatastoreError> {
let cmd = Command::GetKeysStarting(pattern.to_string());
pub fn get_key_value(&self, key: &str) -> Result<String, DatastoreError> {
let cmd = Command::GetKeyValue(key.to_string());
let receiver = self.requester.request(cmd).unwrap();

match receiver.collect().unwrap() {
Ok(r) => match r {
Response::StringVec(value) => Ok(value),
Response::KeyValue(kv) => Ok(kv),
_ => panic!("Invalid response"),
},
Err(e) => Err(e),
}
}

pub fn set_key_value(&self, key: &str, data: &str) -> Result<(), DatastoreError> {
let cmd = Command::SetKeyValue(key.to_string(), data.to_string());
let receiver = self.requester.request(cmd).unwrap();

_unwrap_response(receiver)
}

pub fn delete_key_value(&self, key: &str) -> Result<(), DatastoreError> {
let cmd = Command::DeleteKeyValue(key.to_string());
let receiver = self.requester.request(cmd).unwrap();

_unwrap_response(receiver)
}

// Should block until worker has stopped
pub fn close(&self) {
info!("Sending close request to database");
Expand Down
6 changes: 3 additions & 3 deletions aw-datastore/tests/datastore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ mod datastore_tests {

// Get all events
let fetched_events_all = ds.get_events(&bucket.id, None, None, None).unwrap();
let expected_fetched_events = vec![&e2, &e1];
let expected_fetched_events = [&e2, &e1];
assert_eq!(fetched_events_all.len(), 2);
for i in 0..fetched_events_all.len() {
let expected = &expected_fetched_events[i];
Expand Down Expand Up @@ -268,7 +268,7 @@ mod datastore_tests {

// Get all events
let fetched_events_all = ds.get_events(&bucket.id, None, None, None).unwrap();
let expected_fetched_events = vec![&e2, &e1];
let expected_fetched_events = [&e2, &e1];
assert_eq!(fetched_events_all.len(), 2);
for i in 0..fetched_events_all.len() {
let expected = &expected_fetched_events[i];
Expand All @@ -286,7 +286,7 @@ mod datastore_tests {

// Get all events
let fetched_events_all = ds.get_events(&bucket.id, None, None, None).unwrap();
let expected_fetched_events = vec![e2];
let expected_fetched_events = [e2];
assert_eq!(fetched_events_all.len(), 1);
for i in 0..fetched_events_all.len() {
let expected = &expected_fetched_events[i];
Expand Down
30 changes: 0 additions & 30 deletions aw-models/src/key_value.rs

This file was deleted.

3 changes: 0 additions & 3 deletions aw-models/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ mod bucket;
mod duration;
mod event;
mod info;
mod key_value;
mod query;
mod timeinterval;
mod tryvec;
Expand All @@ -30,8 +29,6 @@ pub use self::bucket::BucketMetadata;
pub use self::bucket::BucketsExport;
pub use self::event::Event;
pub use self::info::Info;
pub use self::key_value::Key;
pub use self::key_value::KeyValue;
pub use self::query::Query;
pub use self::timeinterval::TimeInterval;
pub use self::tryvec::TryVec;
14 changes: 7 additions & 7 deletions aw-query/tests/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ mod query_tests {

let code = String::from("True;False;a=True;return True;");
match aw_query::query(&code, &interval, &ds).unwrap() {
aw_query::DataType::Bool(b) => assert_eq!(b, true),
aw_query::DataType::Bool(b) => assert!(b),
ref data => panic!("Wrong datatype, {data:?}"),
};
}
Expand All @@ -129,42 +129,42 @@ mod query_tests {
// number comparison true
let code = String::from("return 1==1;");
match aw_query::query(&code, &interval, &ds).unwrap() {
aw_query::DataType::Bool(b) => assert_eq!(b, true),
aw_query::DataType::Bool(b) => assert!(b),
ref data => panic!("Wrong datatype, {data:?}"),
};

// number comparison false
let code = String::from("return 2==1;");
match aw_query::query(&code, &interval, &ds).unwrap() {
aw_query::DataType::Bool(b) => assert_eq!(b, false),
aw_query::DataType::Bool(b) => assert!(!b),
ref data => panic!("Wrong datatype, {data:?}"),
};

// string comparison true
let code = String::from(r#"return "a"=="a";"#);
match aw_query::query(&code, &interval, &ds).unwrap() {
aw_query::DataType::Bool(b) => assert_eq!(b, true),
aw_query::DataType::Bool(b) => assert!(b),
ref data => panic!("Wrong datatype, {data:?}"),
};

// string comparison false
let code = String::from(r#"return "a"=="b";"#);
match aw_query::query(&code, &interval, &ds).unwrap() {
aw_query::DataType::Bool(b) => assert_eq!(b, false),
aw_query::DataType::Bool(b) => assert!(!b),
ref data => panic!("Wrong datatype, {data:?}"),
};

// bool comparison true
let code = String::from("return True==True;");
match aw_query::query(&code, &interval, &ds).unwrap() {
aw_query::DataType::Bool(b) => assert_eq!(b, true),
aw_query::DataType::Bool(b) => assert!(b),
ref data => panic!("Wrong datatype, {data:?}"),
};

// bool comparison false
let code = String::from("return False==True;");
match aw_query::query(&code, &interval, &ds).unwrap() {
aw_query::DataType::Bool(b) => assert_eq!(b, false),
aw_query::DataType::Bool(b) => assert!(!b),
ref data => panic!("Wrong datatype, {data:?}"),
};

Expand Down
4 changes: 2 additions & 2 deletions aw-server/src/endpoints/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,9 @@ pub fn build_rocket(server_state: ServerState, config: AWConfig) -> rocket::Rock
"/api/0/settings",
routes![
settings::setting_get,
settings::settings_list_get,
settings::setting_set,
settings::setting_delete
settings::setting_delete,
settings::settings_get,
],
)
.mount("/", rocket_cors::catch_all_options_routes());
Expand Down
Loading

0 comments on commit 728c00e

Please sign in to comment.