Add the `mpop` action and update the `pop` action

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

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;

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 <ohsayan@outlook.com>
+ *
+ * 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 <https://www.gnu.org/licenses/>.
+ *
+*/
+
+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(())
+    }
+);

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(())
     }
-);
+}

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(())
 }

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

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()))
+        );
+    }
 }