Finish up the support for the SET command and its options

This includes a number of tidying up in the acceptance tests. Along with the
cleaning up, I discovered some logic errors in the code handling requests, etc

acceptance/helpers.py

@@ -4,14 +4,17 @@ import time
 import unittest
 
 class MozzoniTest(unittest.TestCase):
-    def setUp(self):
+    @classmethod
+    def setUpClass(self):
         self.server = subprocess.Popen(['./obj/mozzonid'])
         time.sleep(1)
         self.r = redis.Redis(host='localhost',
                 port=6379,
                 db=0,
                 socket_timeout=5)
-    def tearDown(self):
+
+    @classmethod
+    def tearDownClass(self):
         if self.server:
             self.server.terminate()
 

acceptance/test_set.py

@@ -1,20 +1,55 @@
 #!/usr/bin/env python
 
 import helpers
+import random
 import time
+import sys
 
 class SetTest(helpers.MozzoniTest):
+    def setUp(self):
+        # Generate a random key for each test case to avoid potential
+        # collisions
+        self.key = random.randint(1, sys.maxint)
+
     def test_set(self):
-        self.assertTrue(self.r.set('key', 'value'))
-        self.assertEqual(self.r.get('key'), 'value')
+        self.assertTrue(self.r.set(self.key, 'value'))
+        self.assertEqual(self.r.get(self.key), 'value')
+
+    # Replace the given key
+    def test_set_replace(self):
+        self.assertTrue(self.r.set(self.key, 'value'))
+        self.assertEqual(self.r.get(self.key), 'value')
+        self.assertTrue(self.r.set(self.key, 'another'))
+        self.assertEqual(self.r.get(self.key), 'another')
 
     def test_set_ex(self):
         self.assertTrue(
-                self.r.set('keyex', 'value', ex=1)
+                self.r.set(self.key, 'value', ex=1)
                 )
-        self.assertEqual(self.r.get('keyex'), 'value')
+        self.assertEqual(self.r.get(self.key), 'value')
         time.sleep(2)
-        self.assertFalse(self.r.get('keyex'), 'value')
+        self.assertFalse(self.r.get(self.key), 'value')
+
+    def test_set_px(self):
+        self.assertTrue(
+                self.r.set(self.key, 'value', px=1500)
+                )
+        self.assertEqual(self.r.get(self.key), 'value')
+        time.sleep(2)
+        self.assertFalse(self.r.get(self.key), 'value')
+
+    def test_set_nx(self):
+        self.assertTrue(self.r.set(self.key, 'value'))
+        self.assertEqual(self.r.get(self.key), 'value')
+
+        # Ensure that we cannot set the key which already existsj
+        self.assertEqual(None, self.r.set(self.key, 'another', nx=True))
+        self.assertTrue(self.r.get(self.key), 'value')
+
+    def test_set_xx(self):
+        self.assertEqual(None, self.r.set(self.key, 'value', xx=True))
+        self.assertEqual(None, self.r.get(self.key));
 
 if __name__ == '__main__':
+    random.seed()
     unittest.main()

src/mozzoni-client.adb

@@ -127,7 +127,7 @@ package body Mozzoni.Client is
    begin
 
       loop
-         Bytes_Read := Integer (Read_Socket (Socket, Buffer'Address, Buffer'Length));
+         Bytes_Read := Integer (Read_Socket (Socket, Buffer'Address, Read_Buffer_Size));
          if Mozzoni.Error_Number /= 0 then
             Mozzoni.Log.Log_Message (Alog.Error, "Errno set while reading socket:" & Integer'Image (Mozzoni.Error_Number));
          end if;

src/mozzoni-commands-keys.adb

@@ -21,22 +21,73 @@ package body Mozzoni.Commands.Keys is
       Buffer : Unbounded_String := Command (3).Value;
       Value_Item : Value_Type;
 
-      function Convert_To_Secs (Item : in Command_Item) return Duration is
-      begin
-         return Duration'Value (To_String (Item.Value));
-      end Convert_To_Secs;
+      -- Set_Options is a helpful little structure for processing the options
+      -- sent on a SET request.
+      type Set_Options is record
+         If_Exists : Boolean := False;
+         If_Not_Exists : Boolean := False;
+         Expire_Seconds : Duration := 0.0;
+         Expire_Milliseconds : Duration := 0.0;
+      end record;
 
+      function Process_Options (C : Command_Array_Access) return Set_Options is
+         Options : Set_Options;
+         Previous_Value : Unbounded_String := Null_Unbounded_String;
+      begin
+         if C'Length = 4 then
+            -- Standard SET invocation with no options, return early
+            return Options;
+         end if;
+
+         -- Read the rest of the items in the Command for the potential values
+         for Item of C (4 .. C'Length) loop
+            if Item.Value = "NX" then
+               Options.If_Not_Exists := True;
+            elsif Item.Value = "XX" then
+               Options.If_Exists := True;
+            else
+               null;
+            end if;
+
+            if "EX" = Previous_Value then
+               Options.Expire_Seconds := Duration'Value (To_String (Item.Value));
+            elsif "PX" = Previous_Value then
+               Options.Expire_Milliseconds := Duration'Value (To_String (Item.Value)) / 1000.0;
+            end if;
+
+            Previous_Value := Item.Value;
+         end loop;
+         return Options;
+      end Process_Options;
+
+      Options : constant Set_Options := Process_Options (Command);
+      Now     : constant Ada.Calendar.Time := Ada.Calendar.Clock;
+      Exists  : constant Boolean := KeyValue.Exists (Key_Item.Value);
    begin
       Value_Item.Buffer := Buffer;
 
-      if Command'Length > 5 then
-         if Command (4).Value = "EX" then
-            Value_Item.Expiration := Ada.Calendar.Clock + Convert_To_Secs (Command (5));
-         end if;
+      Log.Log_Message (Alog.Warning,
+                               "Commands length: " & Integer'Image (Command'Length));
+
+      if Options.Expire_Seconds > 0.0 then
+         Value_Item.Expiration := Now + Options.Expire_Seconds;
+      elsif Options.Expire_Milliseconds > 0.0 then
+         Value_Item.Expiration := Now + Options.Expire_Milliseconds;
+      end if;
+
+      if Options.If_Not_Exists and Exists then
+         -- Return a bulk string null response if we cannot set this value
+         Client.Write (Prepare_Response ("$-1"));
+         return;
+      end if;
+
+      if Options.If_Exists and not Exists then
+         -- Return a bulk string null response if we cannot set this value
+         Client.Write (Prepare_Response ("$-1"));
+         return;
       end if;
 
       KeyValue.Set (Key_Item.Value, Value_Item);
-
       Client.Write (Prepare_Response ("+OK"));
    end Handle_Set;
 
@@ -50,7 +101,7 @@ package body Mozzoni.Commands.Keys is
    begin
       Client.Write ('$');
 
-      if KeyValue.Is_Expired (Key) then
+      if KeyValue.Exists (Key) = False then
          Client.Write (Prepare_Response ("-1"));
          return;
       end if;

src/mozzoni-store.adb

@@ -10,11 +10,14 @@ package body Mozzoni.Store is
 
       function Exists (Key : in Key_Type) return Boolean is
       begin
-         if Is_Expired (Key) then
-            return False;
+         if Hashed_Maps.Contains (Store, Key) then
+            if Is_Expired (Key) then
+               return False;
+            else
+               return True;
+            end if;
          end if;
-
-         return Hashed_Maps.Contains (Store, Key);
+         return False;
       end Exists;
 
 
@@ -23,7 +26,6 @@ package body Mozzoni.Store is
          Now   : constant Ada.Calendar.Time := Ada.Calendar.Clock;
 
          use type Ada.Calendar.Time;
-
       begin
          if Value.Expiration = No_Expiry then
             return False;