-
Notifications
You must be signed in to change notification settings - Fork 3
Initial implementation of an in-memory version of the API #10
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
/target/ | ||
**/*.rs.bk | ||
Cargo.lock |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
[package] | ||
name = "as_storage" | ||
version = "0.1.0" | ||
authors = ["Thom Chiovoloni <[email protected]>"] | ||
lib = "as_storage" | ||
|
||
[dependencies] | ||
time = "0.1" | ||
url = "1.5.1" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,108 @@ | ||
|
||
extern crate time; | ||
extern crate url; | ||
|
||
use std::collections::HashMap; | ||
use time::Timespec; | ||
use url::Url; | ||
|
||
#[derive(Clone, Debug)] | ||
pub struct PlaceAction { | ||
pub url: Url, | ||
pub when: Timespec, | ||
pub event: String, | ||
// TODO: Think about more complex types -- Or do we want callers just | ||
// storing JSON here? Can we use std::any::Any and hope to serialize it? | ||
pub data: String | ||
} | ||
|
||
impl PlaceAction { | ||
pub fn new(url: Url, event: &str, data: &str) -> PlaceAction { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might need to jump through some hoops if we want to expose There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I figured String would honestly not necessarially be any better, given that it's still a complex type (although, I guess it's more likely to be already solved for String than for Url). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @fluffyemily's been looking at how to expose complex types over the FFI. |
||
PlaceAction { | ||
url: url, | ||
when: time::get_time(), | ||
event: String::from(event), | ||
data: String::from(data), | ||
} | ||
} | ||
} | ||
|
||
pub trait Store { | ||
fn record_place_action(&mut self, action: PlaceAction); | ||
fn query_place_actions<'a>(&'a self, url: &Url) -> &'a [PlaceAction]; | ||
} | ||
|
||
#[derive(Clone, Debug)] | ||
pub struct MemoryStore { | ||
data: HashMap<Url, Vec<PlaceAction>>, | ||
// This allows us to avoid needing to put Option<&'a [PlaceAction]> | ||
// as the return type for query_place_actions (where None would be used for | ||
// "no actions"). Instead, we just return a reference to a private sentinel | ||
// vector, which we ensure is always empty. | ||
sentinel: Vec<PlaceAction>, | ||
} | ||
|
||
impl MemoryStore { | ||
pub fn new() -> MemoryStore { | ||
MemoryStore { | ||
data: HashMap::new(), | ||
sentinel: Vec::new(), | ||
} | ||
} | ||
} | ||
|
||
impl Store for MemoryStore { | ||
fn record_place_action(&mut self, action: PlaceAction) { | ||
if let Some(v) = self.data.get_mut(&action.url) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Noted, I avoided it due to the extra clone it requires (rust-lang/rfcs#1769), but I'm probably overthinking it and that does not matter for Urls. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI: rust-lang/rfcs#1769 |
||
v.push(action); | ||
return; | ||
} | ||
// Can't use else since it would be 2 simultaneous mutable borrows | ||
self.data.insert(action.url.clone(), vec![action]); | ||
} | ||
|
||
fn query_place_actions<'a>(&'a self, url: &Url) -> &'a [PlaceAction] { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should return an owned copy of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My concern is that it could be an enormous number of records, since we do no filtering. Consider all the actions users might have taken on websites like facebook or reddit. Given that, I think even returning a slice is bad here. We probably should be returning an iterator of some sort (or a cursor)? But I don't know how we'd do that with FFI either. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Honestly, I wouldn't worry about slices and iterators and cloning and ownership at this stage. If you're just exploring concepts, you don't need efficiency. If you're not just exploring API modeling concepts, and you're worried about scale, then you don't want a string-based key-value store anyway. I'd be surprised if a consumer ever had a good reason to call You might be interested in how SQLite on Android uses cursor windows. |
||
self.data.get(url).map(|v| &v[..]).unwrap_or(&self.sentinel[..]) | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
#[test] | ||
fn memory_store() { | ||
let url1 = Url::parse("https://example.com/foo/bar?quux").unwrap(); | ||
let url2 = Url::parse("https://example.com/baz").unwrap(); | ||
let mut store = MemoryStore::new(); | ||
let start = time::get_time(); | ||
|
||
store.record_place_action(PlaceAction::new(url1.clone(), "visit", "")); | ||
store.record_place_action(PlaceAction::new(url1.clone(), "frobnicate", "quux")); | ||
store.record_place_action(PlaceAction::new(url1.clone(), "quank", "1002")); | ||
store.record_place_action(PlaceAction::new(url2.clone(), "asdf", "4321")); | ||
|
||
let actions = store.query_place_actions(&url1); | ||
assert_eq!(actions.len(), 3); | ||
|
||
for action in actions.iter() { | ||
assert_eq!(action.url, url1); | ||
assert!(action.when >= start); | ||
} | ||
|
||
let action = actions.iter().find(|&x| x.event == "visit").unwrap(); | ||
assert_eq!(action.data, ""); | ||
|
||
let action = actions.iter().find(|&x| x.event == "frobnicate").unwrap(); | ||
assert_eq!(action.data, "quux"); | ||
|
||
let actions = store.query_place_actions(&url2); | ||
assert_eq!(actions.len(), 1); | ||
|
||
assert_eq!(actions[0].event, "asdf"); | ||
assert_eq!(actions[0].data, "4321"); | ||
|
||
let actions = store.query_place_actions(&Url::parse("https://example.com").unwrap()); | ||
|
||
assert_eq!(actions.len(), 0); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can define our own trait for the data, and implement it for strings, numbers, timestamps, and so on? Though, I don't think that will work if we want to call the API from Swift or C++...without more type info, we wouldn't know how to interpret the
*c_void
that the FFI gives us. Leaving it as a string is fine for now.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that was my thought too. And since Any exists already, it seems like the trait we might want to use if we were going to go the route of allowing anything.