Skip to content

fix: rewrote settings logic to mimic aw-server-python behavior #428

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ErikBjare merged 4 commits into from
Oct 19, 2023
Merged

fix: rewrote settings logic to mimic aw-server-python behavior #428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2fb9a80
fix: rewrote settings logic to mimic aw-server-python behavior
ErikBjare Oct 19, 2023
1903d7b
refactor: removed redundant KeyValue struct
ErikBjare Oct 19, 2023
b1f7373
format: applied cargo fmt
ErikBjare Oct 19, 2023
55ddb98
format: applied cargo clippy --fix
ErikBjare Oct 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 21 additions & 26 deletions aw-datastore/src/datastore.rs
View file
Open in desktop
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,
)),
Comment on lines -917 to -920
Copy link
Copy Markdown
Member Author

@ErikBjare ErikBjare Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't see the point of timestamp. Not offered in the Python implementation, so removed.

The column will still exist in the table.

})
}) {
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions aw-models/src/lib.rs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,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