Merge pull request #482 from openvstorage/asd-assert-small-value

test assert asd small value + fix bug it exposes
openvstorage · Dec 6, 2016 · ef4c2b8 · ef4c2b8
2 parents 486dfc9 + adac99e
commit ef4c2b8
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 8 deletions.
diff --git a/ocaml/src/asd_server.ml b/ocaml/src/asd_server.ml
@@ -919,10 +919,12 @@ let execute_update : type req res.
                    "Assertion failed, expected some but got None instead" >>= fun () ->
                  Error.(failwith (Assert_failed (Slice.get_string_unsafe key)))
                | Some value ->
-                 begin match snd value with
-                   | Value.Direct _ ->
-                     (* must be checked just before applying the transaction *)
-                     Lwt.return (`AssertSome (key, expected, value))
+                 begin
+                   match snd value with
+                   | Value.Direct actual ->
+                      if Blob.(equal (Slice actual) expected)
+                      then Lwt.return ()
+                      else Error.(failwith (Assert_failed (Slice.get_string_unsafe key)))
                    | Value.OnFs (loc, size) ->
                      (* check the blob now, and just before applying the transaction
                         check whether the key is still associated with the same blob
@@ -940,8 +942,11 @@ let execute_update : type req res.
                        Error.(failwith (Assert_failed (Slice.get_string_unsafe key)))
                      end else
                        (* could also be ok for other values ... *)
-                       Lwt.return (`AssertSome (key, expected, value))
-                 end)
+                       Lwt.return ()
+                 end >>= fun () ->
+                 (* must be checked just before applying the transaction *)
+                 Lwt.return (`AssertSome (key, value))
+            )
             some_asserts_with_values >>= fun some_asserts' ->
           Lwt.return (none_asserts', some_asserts')
         in
@@ -1057,15 +1062,15 @@ let execute_update : type req res.
 
           List.iter
             (function
-              | `AssertSome (key, expected, expected_value) ->
+              | `AssertSome (key, expected) ->
                  begin match get_value_option kv key with
                        | None ->
                           Lwt_log.ign_warning_f
                             "Assertion failed, key=%S expected some but got none"
                             ([%show : Slice.t] key);
                           Error.(failwith (Assert_failed (Slice.get_string_unsafe key)))
                        | Some value ->
-                          if value <> expected_value
+                          if value <> expected
                           then raise ConcurrentModification
                  end)
             some_asserts;

diff --git a/ocaml/src/osd_kvs_test.ml b/ocaml/src/osd_kvs_test.ml
@@ -165,6 +165,11 @@ let test_list_all client =
   Lwt.return ()
 
 let test_assert (client : kvs) =
+  apply
+    client
+    []
+    [ Osd.Update.set_string "key" "not this value" Checksum.Checksum.NoChecksum false; ]
+  >>= fun () ->
   Lwt.catch
     (fun () ->
      apply client