From fa2a4c26119fca19d717e34fe83d29ead36ddf21 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Wed, 11 Aug 2021 20:51:37 -0700 Subject: [PATCH] Add the `mpop` action and update the `pop` action --- CHANGELOG.md | 3 +- server/src/actions/mod.rs | 1 + server/src/actions/mpop.rs | 63 +++++++++++++++++++++++++++++++++++ server/src/actions/pop.rs | 41 ++++++++--------------- server/src/queryengine/mod.rs | 3 +- server/src/storage/sengine.rs | 1 - server/src/tests/kvengine.rs | 36 +++++++++++++++++--- 7 files changed, 113 insertions(+), 35 deletions(-) create mode 100644 server/src/actions/mpop.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f7f7700..47fa57e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -105,7 +105,8 @@ All changes in this project will be noted in this file. - Non-interactive TLS private key passphrase input: Just save your password in some file and then pass `--tlspassin /path/to/passfile.txt`. You can do the same by using the `tlspassin` key under SSL in the configuration file - +- `MPOP` now replaces `POP` to accept multiple keys while `POP` will accept a single key + to follow the `MGET`/`GET` naming convention - TLS port can now be set to a custom port via CLI arguments ### Fixes diff --git a/server/src/actions/mod.rs b/server/src/actions/mod.rs index 1379d30a..caf2e6fc 100644 --- a/server/src/actions/mod.rs +++ b/server/src/actions/mod.rs @@ -39,6 +39,7 @@ pub mod jget; pub mod keylen; pub mod lskeys; pub mod mget; +pub mod mpop; pub mod mset; pub mod mupdate; pub mod pop; diff --git a/server/src/actions/mpop.rs b/server/src/actions/mpop.rs new file mode 100644 index 00000000..de0f10b8 --- /dev/null +++ b/server/src/actions/mpop.rs @@ -0,0 +1,63 @@ +/* + * Created on Wed Aug 11 2021 + * + * This file is a part of Skytable + * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source + * NoSQL database written by Sayan Nandan ("the Author") with the + * vision to provide flexibility in data modelling without compromising + * on performance, queryability or scalability. + * + * Copyright (c) 2021, Sayan Nandan + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * +*/ + +use crate::corestore; +use crate::dbnet::connection::prelude::*; +use crate::protocol::responses; +use crate::queryengine::ActionIter; +use crate::resp::BytesWrapper; + +action!( + /// Run an MPOP action + fn mpop(handle: &corestore::Corestore, con: &mut T, act: ActionIter) { + err_if_len_is!(act, con, eq 0); + if registry::state_okay() { + con.write_array_length(act.len()).await?; + for key in act { + if !registry::state_okay() { + // we keep this check just in case the server fails in-between running a + // pop operation + con.write_response(responses::groups::SERVER_ERR).await?; + } else { + match kve!(con, handle).pop(key) { + Ok(Some((_key, val))) => { + con.write_response(BytesWrapper(val.into_inner())).await? + } + Ok(None) => con.write_response(responses::groups::NIL).await?, + Err(_) => { + con.write_response(responses::groups::ENCODING_ERROR) + .await? + } + } + } + } + } else { + // don't begin the operation at all if the database is poisoned + return con.write_response(responses::groups::SERVER_ERR).await; + } + Ok(()) + } +); diff --git a/server/src/actions/pop.rs b/server/src/actions/pop.rs index 195f2b2d..2999a8c6 100644 --- a/server/src/actions/pop.rs +++ b/server/src/actions/pop.rs @@ -24,40 +24,25 @@ * */ -use crate::corestore; use crate::dbnet::connection::prelude::*; -use crate::protocol::responses; -use crate::queryengine::ActionIter; use crate::resp::BytesWrapper; -action!( - /// Run a POP action - fn pop(handle: &corestore::Corestore, con: &mut T, act: ActionIter) { - err_if_len_is!(act, con, eq 0); +action! { + fn pop(handle: &Corestore, con: &mut T, mut act: ActionIter) { + err_if_len_is!(act, con, not 1); + let key = unsafe { + // SAFETY: We have checked for there to be one arg + act.next().unsafe_unwrap() + }; if registry::state_okay() { - con.write_array_length(act.len()).await?; - for key in act { - if !registry::state_okay() { - // we keep this check just in case the server fails in-between running a - // pop operation - con.write_response(responses::groups::SERVER_ERR).await?; - } else { - match kve!(con, handle).pop(key) { - Ok(Some((_key, val))) => { - con.write_response(BytesWrapper(val.into_inner())).await? - } - Ok(None) => con.write_response(responses::groups::NIL).await?, - Err(_) => { - con.write_response(responses::groups::ENCODING_ERROR) - .await? - } - } - } + match kve!(con, handle).pop(key) { + Ok(Some((_key, val))) => conwrite!(con, BytesWrapper(val.into_inner()))?, + Ok(None) => conwrite!(con, groups::NIL)?, + Err(()) => conwrite!(con, groups::ENCODING_ERROR)?, } } else { - // don't begin the operation at all if the database is poisoned - return con.write_response(responses::groups::SERVER_ERR).await; + conwrite!(con, groups::SERVER_ERR)?; } Ok(()) } -); +} diff --git a/server/src/queryengine/mod.rs b/server/src/queryengine/mod.rs index f346f7cd..d0dd3576 100644 --- a/server/src/queryengine/mod.rs +++ b/server/src/queryengine/mod.rs @@ -133,7 +133,8 @@ where CREATE => ddl::create, DROP => ddl::ddl_drop, USE => self::entity_swap, - INSPECT => inspect::inspect + INSPECT => inspect::inspect, + MPOP => actions::mpop::mpop ); Ok(()) } diff --git a/server/src/storage/sengine.rs b/server/src/storage/sengine.rs index 6165101b..2e73778f 100644 --- a/server/src/storage/sengine.rs +++ b/server/src/storage/sengine.rs @@ -131,7 +131,6 @@ impl SnapshotEngine { } pub fn parse_dir(&self) -> SnapshotResult<()> { parse_dir!(self.local_queue, DIR_SNAPROOT); - println!("Queue: {:?}", self.local_queue); Ok(()) } /// Generate the snapshot name diff --git a/server/src/tests/kvengine.rs b/server/src/tests/kvengine.rs index aa7a4cd6..f8c52ee9 100644 --- a/server/src/tests/kvengine.rs +++ b/server/src/tests/kvengine.rs @@ -1141,14 +1141,15 @@ mod __private { Response::Item(Element::RespCode(RespCode::ActionError)) ); } - async fn test_pop_syntax_error() { - query.push("pop"); + async fn test_mpop_syntax_error() { + query.push("mpop"); assert_eq!( con.run_simple_query(&query).await.unwrap(), Response::Item(Element::RespCode(RespCode::ActionError)) ); } - async fn test_pop_all_success() { + + async fn test_mpop_all_success() { setkeys!( con, "x":100, @@ -1165,7 +1166,7 @@ mod __private { ])) ) } - async fn test_pop_mixed() { + async fn test_mpop_mixed() { setkeys!( con, "x":100, @@ -1185,4 +1186,31 @@ mod __private { ])) ); } + async fn test_pop_syntax_error() { + query.push("pop"); + assert_eq!( + con.run_simple_query(&query).await.unwrap(), + Response::Item(Element::RespCode(RespCode::ActionError)) + ); + } + async fn test_pop_okay() { + setkeys!( + con, + "x":100 + ); + query.push("pop"); + query.push("x"); + assert_eq!( + con.run_simple_query(&query).await.unwrap(), + Response::Item(Element::String("100".to_owned())) + ); + } + async fn test_pop_nil() { + query.push("pop"); + query.push("x"); + assert_eq!( + con.run_simple_query(&query).await.unwrap(), + Response::Item(Element::String("100".to_owned())) + ); + } }