Merge pull request #305 from rabbitmq/md/delete-reason

khepri_adv: Add `delete_reason` prop, accumulate keep-while expirations

src/khepri.erl

@@ -203,6 +203,22 @@
 -type child_list_length() :: non_neg_integer().
 %% Number of direct child nodes under a tree node.
 
+-type delete_reason() :: explicit | keep_while.
+%% The reason why a tree node was removed from the store.
+%%
+%% <ul>
+%% <li>`explicit' means that the tree node was removed from the store because a
+%% deletion command targeted that tree node or its ancestor. This reason is
+%% used when a tree node's path is provided to {@link khepri_adv:delete/3} for
+%% example.</li>
+%% <li>`keep_while': the tree node was removed because the keep-while condition
+%% set when the node was created became unsatisfied. Note that adding or
+%% updating a tree node can cause a keep-while condition to become unsatisfied,
+%% so a put operation may result in a tree node being deleted with this delete
+%% reason. See {@link khepri_condition:keep_while()} and {@link
+%% khepri:put_options()}.</li>
+%% </ul>
+
 -type node_props() ::
     #{data => khepri:data(),
       has_data => boolean(),
@@ -211,7 +227,8 @@
       payload_version => khepri:payload_version(),
       child_list_version => khepri:child_list_version(),
       child_list_length => khepri:child_list_length(),
-      child_names => [khepri_path:node_id()]}.
+      child_names => [khepri_path:node_id()],
+      delete_reason => khepri:delete_reason()}.
 %% Structure used to return properties, payload and child nodes for a specific
 %% tree node.
 %%
@@ -337,7 +354,8 @@
                                               child_names |
                                               payload |
                                               has_payload |
-                                              raw_payload],
+                                              raw_payload |
+                                              delete_reason],
                           include_root_props => boolean()}.
 %% Options used during tree traversal.
 %%
@@ -460,6 +478,7 @@
               payload_version/0,
               child_list_version/0,
               child_list_length/0,
+              delete_reason/0,
               node_props/0,
               trigger_id/0,
 

src/khepri_adv.erl

@@ -332,11 +332,18 @@ put(StoreId, PathPattern, Data) ->
 %% in the path pattern is not met, an error is returned and the tree structure
 %% is not modified.
 %%
-%% The returned `{ok, NodeProps}' tuple contains a map with the properties and
-%% payload (if any) of the targeted tree node: the payload was the one before
-%% the update, other properties like the payload version correspond to the
-%% updated node. If the targeted tree node didn't exist, `NodeProps' will be
-%% an empty map.
+%% The returned `{ok, NodePropsMap}' tuple contains a map where keys correspond
+%% to the path to a node affected by the put operation. Each key points to a
+%% map containing the properties and prior payload (if any) of a tree node
+%% created, updated or deleted by the put operation. If the put results in the
+%% creation of a tree node this props map will be empty. If the put updates an
+%% existing tree node then the props map will contain the payload of the tree
+%% node (if any) before the update while the other properties like the payload
+%% version correspond to the updated node. The `NodePropsMap' map might also
+%% contain deletions if the put operation leads to an existing tree node's
+%% keep-while condition becoming unsatisfied. The props map for any nodes
+%% deleted because of an expired keep-while condition will contain a
+%% `delete_reason' key set to `keep_while'.
 %%
 %% The payload must be one of the following form:
 %% <ul>
@@ -452,11 +459,18 @@ put_many(StoreId, PathPattern, Data) ->
 %% in the path pattern is not met, an error is returned and the tree structure
 %% is not modified.
 %%
-%% The returned `{ok, NodePropsMap}' tuple contains a map where keys
-%% correspond to the path to a tree node matching the path pattern. Each key
-%% then points to a map containing the properties and payload (if any) of the
-%% targeted tree node: the payload was the one before the update, other
-%% properties like the payload version correspond to the updated node.
+%% The returned `{ok, NodePropsMap}' tuple contains a map where keys correspond
+%% to the path to a node affected by the put operation. Each key points to a
+%% map containing the properties and prior payload (if any) of a tree node
+%% created, updated or deleted by the put operation. If the put results in the
+%% creation of a tree node this props map will be empty. If the put updates an
+%% existing tree node then the props map will contain the payload of the tree
+%% node (if any) before the update while the other properties like the payload
+%% version correspond to the updated node. The `NodePropsMap' map might also
+%% contain deletions if the put operation leads to an existing tree node's
+%% keep-while condition becoming unsatisfied. The props map for any nodes
+%% deleted because of an expired keep-while condition will contain a
+%% `delete_reason' key set to `keep_while'.
 %%
 %% The payload must be one of the following form:
 %% <ul>
@@ -831,9 +845,15 @@ delete(PathPattern, Options) when is_map(Options) ->
 %% one tree node would match at the time. If you want to delete multiple nodes
 %% at once, use {@link delete_many/3}.
 %%
-%% The returned `{ok, NodeProps}' tuple contains a map with the properties and
-%% payload (if any) of the targeted tree node as they were before the delete.
-%% If the targeted tree node didn't exist, `NodeProps' will be an empty map.
+%% The returned `{ok, NodePropsMap}' tuple contains a map where keys
+%% correspond to the path to a deleted tree node. Each key then points to a
+%% map containing the properties and payload (if any) of that deleted tree
+%% node as they were before the delete. Tree nodes in this map which were
+%% the target of the delete command will have a `delete_reason' key set to
+%% `explicit' in their associated props map. Any tree nodes deleted because
+%% their keep-while condition became unsatisfied due to the deletion will have
+%% the `delete_reason' key set to `keep_while' instead. (See {@link
+%% khepri_condition:keep_while()}.)
 %%
 %% When doing an asynchronous update, the {@link handle_async_ret/1}
 %% function should be used to handle the message received from Ra.
@@ -842,7 +862,8 @@ delete(PathPattern, Options) when is_map(Options) ->
 %% ```
 %% %% Delete the tree node at `/:foo/:bar'.
 %% {ok, #{data := value,
-%%        payload_version := 1}} = khepri_adv:delete(StoreId, [foo, bar]).
+%%        payload_version := 1,
+%%        delete_reason := explicit}} = khepri_adv:delete(StoreId, [foo, bar]).
 %% '''
 %%
 %% @param StoreId the name of the Khepri store.
@@ -925,18 +946,25 @@ delete_many(PathPattern, Options) when is_map(Options) ->
 %% The returned `{ok, NodePropsMap}' tuple contains a map where keys
 %% correspond to the path to a deleted tree node. Each key then points to a
 %% map containing the properties and payload (if any) of that deleted tree
-%% node as they were before the delete.
+%% node as they were before the delete. Tree nodes in this map which were
+%% the target of the delete command will have a `delete_reason' key set to
+%% `explicit' in their associated props map. Any tree nodes deleted because
+%% their keep-while condition became unsatisfied due to the deletion will have
+%% the `delete_reason' key set to `keep_while' instead. (See {@link
+%% khepri_condition:keep_while()}.)
 %%
 %% When doing an asynchronous update, the {@link handle_async_ret/1}
 %% function should be used to handle the message received from Ra.
 %%
 %% Example:
 %% ```
-%% %% Delete the tree node at `/:foo/:bar'.
+%% %% Delete all tree nodes matching `/*/:bar'.
 %% {ok, #{[foo, bar] := #{data := value,
-%%                        payload_version := 1},
-%%        [baz, bar] := #{payload_version := 1}}} = khepri_adv:delete_many(
-%%                                                    StoreId, [foo, bar]).
+%%                        payload_version := 1,
+%%                        delete_reason := explicit},
+%%        [baz, bar] := #{payload_version := 1,
+%%                        delete_reason := explicit}}} =
+%% khepri_adv:delete_many(StoreId, [?KHEPRI_WILDCARD_STAR, bar]).
 %% '''
 %%
 %% @param StoreId the name of the Khepri store.

src/khepri_node.hrl

@@ -16,7 +16,8 @@
                            child_list_version => ?INIT_CHILD_LIST_VERSION}).
 
 -define(DEFAULT_PROPS_TO_RETURN, [payload,
-                                  payload_version]).
+                                  payload_version,
+                                  delete_reason]).
 
 -record(node, {props = ?INIT_NODE_PROPS :: khepri_machine:props(),
                payload = ?NO_PAYLOAD :: khepri_payload:payload(),

src/khepri_tree.erl

@@ -233,7 +233,9 @@ gather_node_props(#node{props = #{payload_version := PVersion,
                   _                   -> Acc
               end;
           (raw_payload, Acc) ->
-              Acc#{raw_payload => Payload}
+              Acc#{raw_payload => Payload};
+          (delete_reason, Acc) ->
+              Acc
       end, #{}, WantedProps);
 gather_node_props(#node{}, _Options) ->
     #{}.
@@ -528,22 +530,41 @@ find_matching_nodes_cb(_, {interrupted, _, _}, _, Acc, _) ->
 
 delete_matching_nodes(Tree, PathPattern, AppliedChanges, TreeOptions) ->
     Fun = fun(Path, Node, Result) ->
-                  delete_matching_nodes_cb(Path, Node, TreeOptions, Result)
+                  delete_matching_nodes_cb(
+                    Path, Node, TreeOptions, explicit, Result)
           end,
     walk_down_the_tree(
       Tree, PathPattern, TreeOptions, AppliedChanges, Fun, #{}).
 
-delete_matching_nodes_cb([] = Path, #node{} = Node, TreeOptions, Result) ->
+delete_matching_nodes_cb(
+  [] = Path, #node{} = Node, TreeOptions, DeleteReason, Result) ->
     Node1 = remove_node_payload(Node),
     Node2 = remove_node_child_nodes(Node1),
-    NodeProps = gather_node_props(Node, TreeOptions),
-    {ok, Node2, Result#{Path => NodeProps}};
-delete_matching_nodes_cb(Path, #node{} = Node, TreeOptions, Result) ->
-    NodeProps = gather_node_props(Node, TreeOptions),
-    {ok, delete, Result#{Path => NodeProps}};
-delete_matching_nodes_cb(_, {interrupted, _, _}, _Options, Result) ->
+    NodeProps1 = gather_node_props(Node, TreeOptions),
+    NodeProps2 = maybe_add_delete_reason_prop(
+                   NodeProps1, TreeOptions, DeleteReason),
+    {ok, Node2, Result#{Path => NodeProps2}};
+delete_matching_nodes_cb(
+  Path, #node{} = Node, TreeOptions, DeleteReason, Result) ->
+    NodeProps1 = gather_node_props(Node, TreeOptions),
+    NodeProps2 = maybe_add_delete_reason_prop(
+                   NodeProps1, TreeOptions, DeleteReason),
+    {ok, delete, Result#{Path => NodeProps2}};
+delete_matching_nodes_cb(
+  _, {interrupted, _, _}, _Options, _DeleteReason, Result) ->
     {ok, keep, Result}.
 
+maybe_add_delete_reason_prop(
+  NodeProps, #{props_to_return := WantedProps}, DeleteReason) ->
+    case lists:member(delete_reason, WantedProps) of
+        true ->
+            NodeProps#{delete_reason => DeleteReason};
+        false ->
+            NodeProps
+    end;
+maybe_add_delete_reason_prop(NodeProps, _TreeOptions, _DeleteReason) ->
+    NodeProps.
+
 %% -------------------------------------------------------------------
 %% Insert or update a tree node.
 %% -------------------------------------------------------------------
@@ -1322,7 +1343,9 @@ handle_keep_while_for_parent_update(
 handle_keep_while_for_parent_update(
   #walk{tree = Tree,
         node = ParentNode,
-        reversed_path = ReversedPath} = Walk,
+        reversed_path = ReversedPath,
+        tree_options = TreeOptions,
+        fun_acc = Acc} = Walk,
   AppliedChangesAcc) ->
     ParentPath = lists:reverse(ReversedPath),
     IsMet = is_keep_while_condition_met_on_self(
@@ -1335,7 +1358,11 @@ handle_keep_while_for_parent_update(
             %% This parent node must be removed because it doesn't meet its
             %% own keep_while condition. keep_while conditions for nodes
             %% depending on this one will be evaluated with the recursion.
-            Walk1 = Walk#walk{node = delete},
+            {ok, delete, Acc1} = delete_matching_nodes_cb(
+                                   ParentPath, ParentNode,
+                                   TreeOptions, keep_while, Acc),
+            Walk1 = Walk#walk{node = delete,
+                              fun_acc = Acc1},
             walk_back_up_the_tree(Walk1, AppliedChangesAcc)
     end.
 
@@ -1519,14 +1546,27 @@ remove_expired_nodes([], Walk) ->
     {ok, Walk};
 remove_expired_nodes(
   [PathToDelete | Rest],
-  #walk{tree = Tree, applied_changes = AppliedChanges} = Walk) ->
-    case delete_matching_nodes(Tree, PathToDelete, AppliedChanges, #{}) of
-        {ok, Tree1, AppliedChanges1, _Acc} ->
+  #walk{tree = Tree,
+        applied_changes = AppliedChanges,
+        tree_options = TreeOptions,
+        fun_acc = Acc} = Walk) ->
+    %% See `delete_matching_nodes/4'. This is the same except that the
+    %% accumulator is passed through and the `DeleteReason' is provided as
+    %% `keep_while'.
+    Fun = fun(Path, Node, Result) ->
+                  delete_matching_nodes_cb(
+                    Path, Node, TreeOptions, keep_while, Result)
+          end,
+    Result = walk_down_the_tree(
+               Tree, PathToDelete, TreeOptions, AppliedChanges, Fun, Acc),
+    case Result of
+        {ok, Tree1, AppliedChanges1, Acc1} ->
             AppliedChanges2 = merge_applied_changes(
                                 AppliedChanges, AppliedChanges1),
             Walk1 = Walk#walk{tree = Tree1,
                               node = Tree1#tree.root,
-                              applied_changes = AppliedChanges2},
+                              applied_changes = AppliedChanges2,
+                              fun_acc = Acc1},
             remove_expired_nodes(Rest, Walk1)
     end.
 

test/advanced_delete.erl

@@ -36,7 +36,8 @@ delete_existing_node_test_() ->
          khepri_adv:create(?FUNCTION_NAME, [foo], foo_value)),
       ?_assertEqual(
          {ok, #{[foo] => #{data => foo_value,
-                           payload_version => 1}}},
+                           payload_version => 1,
+                           delete_reason => explicit}}},
          khepri_adv:delete(?FUNCTION_NAME, [foo])),
       ?_assertEqual(
          {error, ?khepri_error(node_not_found, #{node_name => foo,
@@ -77,7 +78,8 @@ delete_many_on_existing_node_with_condition_true_test_() ->
          khepri_adv:create(?FUNCTION_NAME, [foo], foo_value)),
       ?_assertEqual(
          {ok, #{[foo] => #{data => foo_value,
-                           payload_version => 1}}},
+                           payload_version => 1,
+                           delete_reason => explicit}}},
          khepri_adv:delete_many(
            ?FUNCTION_NAME, [#if_name_matches{regex = "foo"}])),
       ?_assertEqual(

test/advanced_tx_delete.erl

@@ -50,7 +50,8 @@ delete_existing_node_test_() ->
       ?_assertEqual(
          {ok,
           {ok, #{[foo] => #{data => foo_value,
-                            payload_version => 1}}}},
+                            payload_version => 1,
+                            delete_reason => explicit}}}},
          begin
              Fun = fun() ->
                            khepri_tx_adv:delete([foo])
@@ -116,7 +117,8 @@ delete_many_on_existing_node_with_condition_true_test_() ->
       ?_assertEqual(
          {ok,
           {ok, #{[foo] => #{data => foo_value,
-                            payload_version => 1}}}},
+                            payload_version => 1,
+                            delete_reason => explicit}}}},
          begin
              Fun = fun() ->
                            khepri_tx_adv:delete_many(

test/async_option.erl

@@ -289,7 +289,8 @@ async_with_correlation_and_priority_in_delete_test_() ->
              RaEvent = receive {ra_event, _, _} = Event -> Event end,
              ?assertEqual(
                [{Correlation, {ok, #{[foo] =>
-                                     #{payload_version => 1}}}}],
+                                     #{payload_version => 1,
+                                       delete_reason => explicit}}}}],
                khepri:handle_async_ret(?FUNCTION_NAME, RaEvent)),
              ?assertEqual(
                 {error,

test/cluster_SUITE.erl

@@ -1693,7 +1693,8 @@ can_use_default_store_on_single_node(_Config) ->
        {ok, #{[bar] => #{payload_version => 2}}},
        khepri_adv:clear_many_payloads([bar])),
     ?assertEqual(
-       {ok, #{[bar] => #{payload_version => 2}}},
+       {ok, #{[bar] => #{payload_version => 2,
+                         delete_reason => explicit}}},
        khepri_adv:delete([bar])),
     ?assertMatch(
        {ok, #{}},

test/delete_command.erl

@@ -127,7 +127,8 @@ delete_a_node_deep_into_the_tree_test() ->
                       options = #{props_to_return => [payload,
                                                       payload_version,
                                                       child_list_version,
-                                                      child_list_length]}},
+                                                      child_list_length,
+                                                      delete_reason]}},
     {S1, Ret, SE} = khepri_machine:apply(?META, Command, S0),
     Root = khepri_machine:get_root(S1),
 
@@ -140,7 +141,16 @@ delete_a_node_deep_into_the_tree_test() ->
        Root),
     ?assertEqual({ok, #{[foo, bar, baz] => #{payload_version => 1,
                                              child_list_version => 1,
-                                             child_list_length => 1}}}, Ret),
+                                             child_list_length => 1,
+                                             delete_reason => explicit},
+                        [foo, bar] => #{payload_version => 1,
+                                        child_list_version => 2,
+                                        child_list_length => 0,
+                                        delete_reason => keep_while},
+                        [foo] => #{payload_version => 1,
+                                   child_list_version => 2,
+                                   child_list_length => 0,
+                                   delete_reason => keep_while}}}, Ret),
     ?assertEqual([], SE).
 
 delete_existing_node_with_condition_true_test() ->
@@ -275,7 +285,8 @@ delete_many_nodes_at_once_test() ->
                       options = #{props_to_return => [payload,
                                                       payload_version,
                                                       child_list_version,
-                                                      child_list_length]}},
+                                                      child_list_length,
+                                                      delete_reason]}},
     {S1, Ret, SE} = khepri_machine:apply(?META, Command, S0),
     Root = khepri_machine:get_root(S1),
 
@@ -292,11 +303,13 @@ delete_many_nodes_at_once_test() ->
     ?assertEqual({ok, #{[bar] => #{data => bar_value,
                                    payload_version => 1,
                                    child_list_version => 1,
-                                   child_list_length => 0},
+                                   child_list_length => 0,
+                                   delete_reason => explicit},
                         [baz] => #{data => baz_value,
                                    payload_version => 1,
                                    child_list_version => 1,
-                                   child_list_length => 0}}}, Ret),
+                                   child_list_length => 0,
+                                   delete_reason => explicit}}}, Ret),
     ?assertEqual([], SE).
 
 delete_command_bumps_applied_command_count_test() ->