Merge pull request #20 from rabbitmq/use-records-for-payload

Replace macros by records to represent payloads

Author: dumbbell

README.md

@@ -146,7 +146,9 @@ khepri:transaction(
                 %% There is less than 100 pieces of wood, or there is none
                 %% at all (the node does not exist in Khepri). We need to
                 %% request a new order.
-                {ok, _} = khepri_tx:put([order, wood], ?DATA_PAYLOAD(1000)),
+                {ok, _} = khepri_tx:put(
+                            [order, wood],
+                            #kpayload_data{data = 1000}),
                 true
         end
     end).

doc/overview.edoc

@@ -91,8 +91,8 @@ the future.
 
 Payloads are represented using macros or helper functions:
 <ul>
-<li>`?NO_PAYLOAD' and {@link khepri:no_payload/0}</li>
-<li>`?DATA_PAYLOAD(Term)' and {@link khepri:data_payload/1}</li>
+<li>`none' and {@link khepri:no_payload/0}</li>
+<li>`#kpayload_data{data = Term}' and {@link khepri:data_payload/1}</li>
 </ul>
 
 Functions in {@link khepri_machine} have no assumption on the type of the
@@ -222,7 +222,9 @@ specific use cases may need to rely on that low-level API.
 %% Unlike the high-level API's `khepri:insert/2' function, this low-level %
 %% insert returns whatever it replaced (if anything). In this case, there was
 %% nothing before, so the returned value is pretty empty.
-Ret1 = khepri_machine:put(StoreId, [stock, wood, <<"lime tree">>], ?DATA_PAYLOAD(150)),
+Ret1 = khepri_machine:put(
+         StoreId, [stock, wood, <<"lime tree">>],
+         #kpayload_data{data = 150}),
 {ok, #{}} = Ret1,
 
 Ret2 = khepri_machine:get(StoreId, [stock, wood, <<"lime tree">>]),

include/khepri.hrl

@@ -33,12 +33,10 @@
 -define(IS_PATH_PATTERN(Path),
         (Path =:= [] orelse ?IS_PATH_CONDITION(hd(Path)))).
 
--define(NO_PAYLOAD, none).
--define(DATA_PAYLOAD(Data), {data, Data}).
--define(IS_PAYLOAD(Payload), (Payload =:= none orelse
-                              (is_tuple(Payload) andalso
-                               size(Payload) =:= 2 andalso
-                               element(1, Payload) =:= data))).
+-record(kpayload_data, {data :: khepri_machine:data()}).
+
+-define(IS_KHEPRI_PAYLOAD(Payload), (Payload =:= none orelse
+                                     is_record(Payload, kpayload_data))).
 
 %% -------------------------------------------------------------------
 %% Path conditions.

src/khepri.erl

@@ -632,7 +632,7 @@ compare_and_swap(StoreId, Path, DataPattern, Data) ->
 %% @private
 
 do_put(StoreId, Path, Data) ->
-    case khepri_machine:put(StoreId, Path, ?DATA_PAYLOAD(Data)) of
+    case khepri_machine:put(StoreId, Path, #kpayload_data{data = Data}) of
         {ok, _} -> ok;
         Error   -> Error
     end.
@@ -654,7 +654,7 @@ clear_payload(Path) ->
       Path :: khepri_path:pattern() | string().
 %% @doc Clears the payload of an existing specific tree node in the tree structure.
 %%
-%% In other words, the payload is set to `?NO_PAYLOAD'.
+%% In other words, the payload is set to `none'.
 %%
 %% The `Path' can be provided as a list of node names and conditions or as a
 %% string. See {@link khepri_path:from_string/1}.
@@ -669,7 +669,7 @@ clear_payload(Path) ->
 
 clear_payload(StoreId, Path) ->
     Path1 = khepri_path:maybe_from_string(Path),
-    case khepri_machine:put(StoreId, Path1, ?NO_PAYLOAD) of
+    case khepri_machine:put(StoreId, Path1, none) of
         {ok, _} -> ok;
         Error   -> Error
     end.
@@ -967,23 +967,24 @@ clear_store(StoreId) ->
     delete(StoreId, [?STAR]).
 
 -spec no_payload() -> none.
-%% @doc Returns `?NO_PAYLOAD'.
+%% @doc Returns `none'.
 %%
-%% This is a helper for cases where using macros is inconvenient, like in an
+%% This is a helper for cases where using records is inconvenient, like in an
 %% Erlang shell.
 
 no_payload() ->
-    ?NO_PAYLOAD.
+    none.
 
--spec data_payload(Term) -> {data, Term} when
-      Term :: khepri_machine:data().
-%% @doc Returns `?DATA_PAYLOAD(Term)'.
+-spec data_payload(Term) -> Payload when
+      Term :: khepri_machine:data(),
+      Payload :: #kpayload_data{}.
+%% @doc Returns `#kpayload_data{data = Term}'.
 %%
 %% This is a helper for cases where using macros is inconvenient, like in an
 %% Erlang shell.
 
 data_payload(Term) ->
-    ?DATA_PAYLOAD(Term).
+    #kpayload_data{data = Term}.
 
 %% -------------------------------------------------------------------
 %% Public helpers.

src/khepri_condition.erl

@@ -345,17 +345,17 @@ is_met(
   _Child) ->
     eval_regex(Cond, SourceRegex, CompiledRegex, ChildName);
 is_met(#if_has_data{has_data = true},
-       _ChildName, #node{payload = {data, _}}) ->
+       _ChildName, #node{payload = #kpayload_data{data = _}}) ->
     true;
 is_met(#if_has_data{has_data = false} = Cond,
-       _ChildName, #node{payload = {data, _}}) ->
+       _ChildName, #node{payload = #kpayload_data{data = _}}) ->
     {false, Cond};
 is_met(#if_has_data{has_data = true} = Cond, _ChildName, _Child) ->
     {false, Cond};
 is_met(#if_has_data{has_data = false}, _ChildName, _Child) ->
     true;
 is_met(#if_data_matches{compiled = CompMatchSpec} = Cond,
-       _ChildName, #node{payload = {data, Data}}) ->
+       _ChildName, #node{payload = #kpayload_data{data = Data}}) ->
     case term_matches(Data, CompMatchSpec) of
         true  -> true;
         false -> {false, Cond}

src/khepri_machine.erl

@@ -152,7 +152,7 @@
                   child_list_version := child_list_version()}.
 %% Stats attached to each node in the tree structure.
 
--type payload() :: ?NO_PAYLOAD | ?DATA_PAYLOAD(data()).
+-type payload() :: none | #kpayload_data{}.
 %% All types of payload stored in the nodes of the tree structure.
 %%
 %% Beside the absence of payload, the only type of payload supported is data.
@@ -281,16 +281,16 @@ put(StoreId, PathPattern, Payload) ->
 %%
 %% The payload must be one of the following form:
 %% <ul>
-%% <li>`?NO_PAYLOAD', meaning there will be no payload attached to the
+%% <li>`none', meaning there will be no payload attached to the node</li>
+%% <li>`#kpayload_data{data = Term}' to store any type of term in the
 %% node</li>
-%% <li>`?DATA_PAYLOAD(Term)' to store any type of term in the node</li>
 %% </ul>
 %%
 %% Example:
 %% ```
 %% %% Insert a node at `/foo/bar', overwriting the previous value.
 %% Result = khepri_machine:put(
-%%            ra_cluster_name, [foo, bar], ?DATA_PAYLOAD(new_value)),
+%%            ra_cluster_name, [foo, bar], #kpayload_data{data = new_value}),
 %%
 %% %% Here is the content of `Result'.
 %% {ok, #{[foo, bar] => #{data => old_value,
@@ -307,7 +307,7 @@ put(StoreId, PathPattern, Payload) ->
 %%
 %% @returns an "ok" tuple with a map with one entry, or an "error" tuple.
 
-put(StoreId, PathPattern, Payload, Extra) when ?IS_PAYLOAD(Payload) ->
+put(StoreId, PathPattern, Payload, Extra) when ?IS_KHEPRI_PAYLOAD(Payload) ->
     khepri_path:ensure_is_valid(PathPattern),
     Command = #put{path = PathPattern,
                    payload = Payload,
@@ -728,11 +728,13 @@ set_node_payload(#node{stat = #{payload_version := DVersion} = Stat} = Node,
 -spec remove_node_payload(tree_node()) -> tree_node().
 %% @private
 
-remove_node_payload(#node{payload = ?NO_PAYLOAD} = Node) ->
+remove_node_payload(
+  #node{payload = none} = Node) ->
     Node;
-remove_node_payload(#node{stat = #{payload_version := DVersion} = Stat} = Node) ->
+remove_node_payload(
+  #node{stat = #{payload_version := DVersion} = Stat} = Node) ->
     Stat1 = Stat#{payload_version => DVersion + 1},
-    Node#node{stat = Stat1, payload = ?NO_PAYLOAD}.
+    Node#node{stat = Stat1, payload = none}.
 
 -spec add_node_child(tree_node(), khepri_path:component(), tree_node()) ->
     tree_node().
@@ -790,8 +792,8 @@ gather_node_props(#node{stat = #{payload_version := DVersion,
                       Result0
               end,
     case Payload of
-        ?DATA_PAYLOAD(Data) -> Result1#{data => Data};
-        _                   -> Result1
+        #kpayload_data{data = Data} -> Result1#{data => Data};
+        _                           -> Result1
     end.
 
 -spec to_absolute_keep_while(BasePath, KeepWhile) -> KeepWhile when

src/khepri_tx.erl

@@ -94,7 +94,7 @@ put(PathPattern, Payload) ->
       Result :: khepri_machine:result().
 %% @doc Creates or modifies a specific tree node in the tree structure.
 
-put(PathPattern, Payload, Extra) when ?IS_PAYLOAD(Payload) ->
+put(PathPattern, Payload, Extra) when ?IS_KHEPRI_PAYLOAD(Payload) ->
     ensure_path_pattern_is_valid(PathPattern),
     ensure_updates_are_allowed(),
     State = get_tx_state(),

test/conditions.erl

@@ -180,11 +180,11 @@ if_has_data_matching_test() ->
        {false, #if_has_data{has_data = false}},
        khepri_condition:is_met(
          khepri_condition:compile(#if_has_data{has_data = false}),
-         foo, #node{payload = {data, foo}})),
+         foo, #node{payload = #kpayload_data{data = foo}})),
     ?assert(
        khepri_condition:is_met(
          khepri_condition:compile(#if_has_data{has_data = true}),
-         foo, #node{payload = {data, foo}})).
+         foo, #node{payload = #kpayload_data{data = foo}})).
 
 if_data_matches_matching_test() ->
     CompiledCond1 = khepri_condition:compile(
@@ -195,7 +195,7 @@ if_data_matches_matching_test() ->
          CompiledCond1, foo, #node{})),
     ?assert(
        khepri_condition:is_met(
-         CompiledCond1, foo, #node{payload = {data, {a, b}}})),
+         CompiledCond1, foo, #node{payload = #kpayload_data{data = {a, b}}})),
 
     CompiledCond2 = khepri_condition:compile(
                       #if_data_matches{pattern = {a, '_'}}),
@@ -213,18 +213,18 @@ if_data_matches_matching_test() ->
 
     ?assert(
        khepri_condition:is_met(
-         CompiledCond2, foo, #node{payload = {data, {a, b}}})),
+         CompiledCond2, foo, #node{payload = #kpayload_data{data = {a, b}}})),
     ?assert(
        khepri_condition:is_met(
-         CompiledCond2, foo, #node{payload = {data, {a, c}}})),
+         CompiledCond2, foo, #node{payload = #kpayload_data{data = {a, c}}})),
     ?assertEqual(
        {false, CompiledCond2},
        khepri_condition:is_met(
-         CompiledCond2, foo, #node{payload = {data, {b, c}}})),
+         CompiledCond2, foo, #node{payload = #kpayload_data{data = {b, c}}})),
     ?assertEqual(
        {false, CompiledCond2},
        khepri_condition:is_met(
-         CompiledCond2, foo, #node{payload = {data, other}})).
+         CompiledCond2, foo, #node{payload = #kpayload_data{data = other}})).
 
 if_payload_version_matching_test() ->
     ?assert(

test/db_info.erl

@@ -112,7 +112,7 @@ get_store_info_with_keep_while_conds_test_() ->
      [?_assertEqual(
          {ok, #{[foo] => #{}}},
          khepri_machine:put(
-           ?FUNCTION_NAME, [foo], ?DATA_PAYLOAD(foo_value),
+           ?FUNCTION_NAME, [foo], #kpayload_data{data = foo_value},
            #{keep_while => KeepWhile})),
       ?_assertEqual(
          "\n"