Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,5 +138,5 @@ For the specification of the custom protocol used for communicating with a Cache
- [x] add graceful shutdown
- [x] add command line flag handler
- [x] built CLI client
- [x] add INCR/DECR commands for INT type

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please add an entry to the Commands table in the README for the new INC command : )

- [ ] add persistance
- [ ] add INCR/DECR commands for INT type
25 changes: 25 additions & 0 deletions cachew/src/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,31 @@ impl Database {
database_error!(DatabaseErrorType::KeyNotFound(key.to_string()))
}

/// Increments a value by its key. Creates the key with a default value of 1 if it does not exist.
///
/// # Arguments:
/// * `key`: The query key.
///
/// # Returns:
/// INC_OK enum on success or an error.
pub fn inc(&mut self, key: &str) -> Result<QueryResponseType, String> {
if let Some(serialized_value) = self.storage.get(key) {
let mut deserialized_value: ValueType = deserialize(serialized_value).unwrap();

if let ValueType::Int(ref mut value) = deserialized_value {
*value += 1;
self.storage.insert(key.to_owned(), serialize(&deserialized_value).unwrap());
return Ok(QueryResponseType::INC_OK(deserialized_value));
} else {
return database_error!(DatabaseErrorType::WrongValueType);
}
}

let default_value = ValueType::Int(1);
self.storage.insert(key.to_owned(), serialize(&default_value).unwrap());
return Ok(QueryResponseType::INC_OK(default_value));
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Nice! Two points here:

  1. We need a guard here at the beginning of the function that checks if the value-type is INT. Because if it the type is STR and we do INC key for a non-exisiting key, it will create a key-value pair "key: 1" where 1 is an integer and not a STR.
  2. Could you add also tests here in the database.rs for inc?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

How do I check if the value is int? I thought about it but it's not very clear to me.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Sorry, I meant check the database-type, so it only works when the database stores INTs, e.g like this:

if self.database_type != DatabaseType::Int {
    return database_error!(DatabaseErrorType::WrongValueType);
}


/// Gets values from a range of keys.
///
/// # Arguments:
Expand Down
20 changes: 20 additions & 0 deletions cachew/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,21 @@ fn parse_get(query: &str) -> Result<QueryRequest, String> {
Ok(QueryRequest::GET(key.to_owned()))
}

/// Parses the parameters of a INC query.
///
/// # Arguments:
/// * `query`: A string containing the parameters of the query, e.g if the query was "INC key" the the parameters are everything after "INC ".
///
/// # Returns:
/// An instance of `QueryRequest`, variants: INC or ERROR (if the parse failed).
fn parse_inc(query: &str) -> Result<QueryRequest, String> {
let key = match validate_key(query) {
Ok(key) => key,
Err(error) => return Err(error)
};

Ok(QueryRequest::INC(key.to_owned()))
}

/// Parses the parameters of a DEL query.
///
Expand Down Expand Up @@ -345,6 +360,9 @@ pub fn parse<'a>(request: &'a str, database_type: &DatabaseType) -> Result<Query
if request.starts_with("GET ") {
return parse_get(request.strip_prefix("GET ").unwrap());
}
else if request.starts_with("INC ") {
return parse_inc(request.strip_prefix("INC ").unwrap());
}
else if request.starts_with("DEL ") {
return parse_del(request.strip_prefix("DEL ").unwrap());
}
Expand Down Expand Up @@ -416,6 +434,8 @@ mod tests {
assert_eq!(get_query, parser_error!(ParserErrorType::UnexpectedCharacter));
}

#[test]

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please add some tests here similiar to the others.


#[test]
fn test_parse_get_range() {
let get_range_query = parse_get("RANGE key0 key1");
Expand Down
3 changes: 3 additions & 0 deletions cachew/src/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ impl QueryResponse {
QueryResponseType::GET_OK(value) => {
Self::build_ok_response("GET".to_string(), Some(Self::handle_value_types(&value)), Some(database_type))
},
QueryResponseType::INC_OK(value) => {
Self::build_ok_response("INC".to_string(), Some(Self::handle_value_types(&value)), Some(database_type))
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Also add tests for inc in this file pls

QueryResponseType::GET_RANGE_OK(values) => {
let mut content: String = String::new();
for (idx, value) in values.iter().enumerate() {
Expand Down
2 changes: 2 additions & 0 deletions cachew/src/schemas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ pub struct KeyValuePair {
#[derive(Debug, PartialEq)]
pub enum QueryRequest<'a> {
GET(String),
INC(String),
SET(KeyValuePair),
SET_MANY(Vec<KeyValuePair>),
GET_RANGE { key_lower: String, key_upper: String},
Expand Down Expand Up @@ -43,6 +44,7 @@ pub enum ValueType {
#[derive(Debug, PartialEq)]
pub enum QueryResponseType {
GET_OK(ValueType),
INC_OK(ValueType),
GET_RANGE_OK(Vec<ValueType>),
GET_MANY_OK(Vec<ValueType>),
DEL_OK,
Expand Down
4 changes: 4 additions & 0 deletions cachew/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ impl State {

match request {
QueryRequest::GET(key) => self.db.get(&key),
QueryRequest::INC(key) => self.db.inc(&key),
QueryRequest::GET_RANGE { key_lower, key_upper } => self.db.get_range(key_lower, key_upper),
QueryRequest::GET_MANY(keys) => self.db.get_many(keys),
QueryRequest::DEL(key) => self.db.del(&key),
Expand Down Expand Up @@ -150,6 +151,9 @@ mod tests {
]));
assert_eq!(response_set_many, Ok(QueryResponseType::SET_MANY_OK));

let response_inc = state.execute_request(client_address, QueryRequest::INC("key_inc".to_string()));
assert_eq!(response_inc, Ok(QueryResponseType::INC_OK(ValueType::Int(1))));

Comment on lines +154 to +156

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Coud you add the same query again to test if the value was incremented to 2?

let response_get = state.execute_request(client_address, QueryRequest::GET("key1".to_string()));
assert_eq!(response_get, Ok(QueryResponseType::GET_OK(ValueType::Str("value1".to_string()))));

Expand Down