Accept string-based paths in khepri_machine and khepri_tx

src/khepri_condition.erl

@@ -551,8 +551,6 @@ compare_numerical_values(_, _)                 -> false.
 
 is_valid(Component) when ?IS_PATH_COMPONENT(Component) ->
     true;
-is_valid(?THIS_NODE) ->
-    true;
 is_valid(#if_node_exists{exists = Exists}) ->
     is_boolean(Exists);
 is_valid(#if_name_matches{}) ->

src/khepri_machine.erl

@@ -435,9 +435,10 @@ put(StoreId, PathPattern, Payload, Options) ->
 
 put(StoreId, PathPattern, Payload, Extra, Options)
   when ?IS_KHEPRI_PAYLOAD(Payload) ->
-    khepri_path:ensure_is_valid(PathPattern),
+    PathPattern1 = khepri_path:from_string(PathPattern),
+    khepri_path:ensure_is_valid(PathPattern1),
     Payload1 = prepare_payload(Payload),
-    Command = #put{path = PathPattern,
+    Command = #put{path = PathPattern1,
                    payload = Payload1,
                    extra = Extra},
     process_command(StoreId, Command, Options);
@@ -500,9 +501,10 @@ get(StoreId, PathPattern) ->
 %% "error" tuple.
 
 get(StoreId, PathPattern, Options) ->
-    khepri_path:ensure_is_valid(PathPattern),
+    PathPattern1 = khepri_path:from_string(PathPattern),
+    khepri_path:ensure_is_valid(PathPattern1),
     Query = fun(#?MODULE{root = Root}) ->
-                    find_matching_nodes(Root, PathPattern, Options)
+                    find_matching_nodes(Root, PathPattern1, Options)
             end,
     process_query(StoreId, Query, Options).
 
@@ -555,8 +557,9 @@ delete(StoreId, PathPattern) ->
 %% message if a correlation ID was specified).
 
 delete(StoreId, PathPattern, Options) ->
-    khepri_path:ensure_is_valid(PathPattern),
-    Command = #delete{path = PathPattern},
+    PathPattern1 = khepri_path:from_string(PathPattern),
+    khepri_path:ensure_is_valid(PathPattern1),
+    Command = #delete{path = PathPattern1},
     process_command(StoreId, Command, Options).
 
 -spec transaction(StoreId, Fun) -> Ret when

src/khepri_path.erl

@@ -43,14 +43,14 @@
 
 -module(khepri_path).
 
+-include_lib("stdlib/include/assert.hrl").
+
 -include("include/khepri.hrl").
 
 -export([compile/1,
          from_string/1,
-         component_from_string/1,
          maybe_from_string/1,
          to_string/1,
-         component_to_string/1,
          combine_with_conditions/2,
          targets_specific_node/1,
          component_targets_specific_node/1,
@@ -60,6 +60,10 @@
          realpath/1,
          pattern_includes_root_node/1]).
 
+-ifdef(TEST).
+-export([component_to_string/1]).
+-endif.
+
 -type node_id() :: atom() | binary().
 %% A node name.
 
@@ -87,75 +91,120 @@
 compile(PathPattern) ->
     lists:map(fun khepri_condition:compile/1, PathPattern).
 
--spec from_string(string()) -> pattern().
+-spec from_string(MaybeString) -> PathPattern when
+      MaybeString :: string() | pattern(),
+      PathPattern :: pattern().
 
-from_string("") ->
-    [];
-from_string("/" ++ PathString) ->
-    from_string(PathString, []);
-from_string("./" ++ PathString) ->
-    from_string(PathString, [?THIS_NODE]);
-from_string(".") ->
-    [?THIS_NODE];
-from_string("../" ++ PathString) ->
-    from_string(PathString, [?PARENT_NODE]);
-from_string("..") ->
-    [?PARENT_NODE];
-from_string(PathString) ->
-    from_string(PathString, [?THIS_NODE]).
-
-from_string(PathString, ParentPath) ->
-    ReOpts = [{capture, all_but_first, list}, dotall],
-    case re:run(PathString, "^((?U)<<.*>>)(?:/(.*)|$)", ReOpts) of
-        {match, [ComponentString, Rest]} ->
-            Component = component_from_string(ComponentString),
-            from_string(Rest, [Component | ParentPath]);
-        {match, [ComponentString]} ->
-            Component = component_from_string(ComponentString),
-            lists:reverse([Component | ParentPath]);
-        nomatch ->
-            case re:run(PathString, "^([^/]*)(?:/(.*)|$)", ReOpts) of
-                {match, ["", Rest]} ->
-                    from_string(Rest, ParentPath);
-                {match, [ComponentString, Rest]} ->
-                    Component = component_from_string(ComponentString),
-                    from_string(Rest, [Component | ParentPath]);
-                {match, [""]} ->
-                    lists:reverse(ParentPath);
-                {match, [ComponentString]} ->
-                    Component = component_from_string(ComponentString),
-                    lists:reverse([Component | ParentPath])
+from_string("/" ++ MaybeString) ->
+    from_string(MaybeString, [?ROOT_NODE]);
+from_string(MaybeString) when is_list(MaybeString) ->
+    from_string(MaybeString, []);
+from_string(NotPath) ->
+    throw({invalid_path, #{path => NotPath}}).
+
+from_string([Component | _] = Rest, ReversedPath)
+  when ?IS_NODE_ID(Component) orelse
+       ?IS_CONDITION(Component) ->
+    finalize_path(Rest, ReversedPath);
+from_string([Char, Component | _] = Rest, ReversedPath)
+  when ?IS_SPECIAL_PATH_COMPONENT(Char) andalso
+       (?IS_NODE_ID(Component) orelse
+        ?IS_CONDITION(Component)) ->
+    finalize_path(Rest, ReversedPath);
+from_string([Char] = Rest, [] = ReversedPath)
+  when ?IS_SPECIAL_PATH_COMPONENT(Char) ->
+    finalize_path(Rest, ReversedPath);
+
+from_string([$/ | Rest], ReversedPath) ->
+    from_string(Rest, ReversedPath);
+
+from_string([$<, $< | Rest], ReversedPath) ->
+    ?assertNotEqual("", Rest),
+    parse_binary_from_string(Rest, ReversedPath);
+
+from_string([Char | _] = Rest, ReversedPath) when is_integer(Char) ->
+    parse_atom_from_string(Rest, ReversedPath);
+
+from_string([], ReversedPath) ->
+    finalize_path([], ReversedPath);
+
+from_string(Rest, ReversedPath) ->
+    NotPath = lists:reverse(ReversedPath) ++ Rest,
+    throw({invalid_path, #{path => NotPath,
+                           tail => Rest}}).
+
+parse_atom_from_string(Rest, ReversedPath) ->
+    parse_atom_from_string(Rest, "", ReversedPath).
+
+parse_atom_from_string([$/ | _] = Rest, Acc, ReversedPath) ->
+    Component = finalize_atom_component(Acc),
+    ReversedPath1 = prepend_component(Component, ReversedPath),
+    from_string(Rest, ReversedPath1);
+parse_atom_from_string([Char | Rest], Acc, ReversedPath)
+  when is_integer(Char) ->
+    Acc1 = [Char | Acc],
+    parse_atom_from_string(Rest, Acc1, ReversedPath);
+parse_atom_from_string([] = Rest, Acc, ReversedPath) ->
+    Component = finalize_atom_component(Acc),
+    ReversedPath1 = prepend_component(Component, ReversedPath),
+    from_string(Rest, ReversedPath1).
+
+finalize_atom_component(Acc) ->
+    Acc1 = lists:reverse(Acc),
+    case Acc1 of
+        "." ->
+            ?THIS_NODE;
+        ".." ->
+            ?PARENT_NODE;
+        "*" ->
+            ?STAR;
+        "**" ->
+            ?STAR_STAR;
+        _ ->
+            case re:run(Acc1, "\\*", [{capture, none}]) of
+                match ->
+                    ReOpts = [global, {return, list}],
+                    Regex = re:replace(Acc1, "\\*", ".*", ReOpts),
+                    #if_name_matches{regex = "^" ++ Regex ++ "$"};
+                nomatch ->
+                    erlang:list_to_atom(Acc1)
             end
     end.
 
--spec component_from_string(string()) -> pattern_component().
-
-component_from_string("/") ->
-    ?ROOT_NODE;
-component_from_string(".") ->
-    ?THIS_NODE;
-component_from_string("..") ->
-    ?PARENT_NODE;
-component_from_string("*") ->
-    ?STAR;
-component_from_string("**") ->
-    ?STAR_STAR;
-component_from_string(Component) ->
-    ReOpts1 = [{capture, all_but_first, list}, dotall],
-    Component1 = case re:run(Component, "^<<(.*)>>$", ReOpts1) of
-                     {match, [C]} -> C;
-                     nomatch      -> Component
-                 end,
-    ReOpts2 = [{capture, none}],
-    case re:run(Component1, "\\*", ReOpts2) of
-        match ->
-            ReOpts3 = [global, {return, list}],
-            Regex = re:replace(Component1, "\\*", ".*", ReOpts3),
-            #if_name_matches{regex = "^" ++ Regex ++ "$"};
-        nomatch when Component1 =:= Component ->
-            list_to_atom(Component);
-        nomatch ->
-            list_to_binary(Component1)
+parse_binary_from_string(Rest, ReversedPath) ->
+    parse_binary_from_string(Rest, "", ReversedPath).
+
+%% If a binary contains ">>" before its, we consider them to be part of the
+%% binary's content, not the end marker.
+parse_binary_from_string([$>, $> | Rest], Acc, ReversedPath)
+  when Rest =:= "" orelse hd(Rest) =:= $/ ->
+    Component = finalize_binary_componenent(Acc),
+    ReversedPath1 = prepend_component(Component, ReversedPath),
+    from_string(Rest, ReversedPath1);
+parse_binary_from_string([Char | Rest], Acc, ReversedPath)
+  when is_integer(Char) ->
+    Acc1 = [Char | Acc],
+    parse_binary_from_string(Rest, Acc1, ReversedPath);
+parse_binary_from_string([], Acc, ReversedPath) ->
+    %% This "binary" has no end marker. We consider it an atom then.
+    parse_atom_from_string([], Acc ++ "<<", ReversedPath).
+
+finalize_binary_componenent(Acc) ->
+    Acc1 = lists:reverse(Acc),
+    erlang:list_to_binary(Acc1).
+
+prepend_component(Component, []) when ?IS_NODE_ID(Component) ->
+    %% This is a relative path.
+    [Component, ?THIS_NODE];
+prepend_component(Component, ReversedPath) ->
+    [Component | ReversedPath].
+
+finalize_path(Rest, []) ->
+    Rest;
+finalize_path(Rest, ReversedPath) ->
+    case lists:reverse(ReversedPath) ++ Rest of
+        [?ROOT_NODE | Path] -> Path;
+        Path                -> Path
     end.
 
 -spec maybe_from_string(pattern() | string()) -> pattern().
@@ -309,8 +358,9 @@ is_valid(NotPathPattern) ->
 
 ensure_is_valid(PathPattern) ->
     case is_valid(PathPattern) of
-        true          -> ok;
-        {false, Path} -> throw({invalid_path, Path})
+        true               -> ok;
+        {false, Component} -> throw({invalid_path, #{path => PathPattern,
+                                                     component => Component}})
     end.
 
 -spec abspath(pattern(), pattern()) -> pattern().

src/khepri_tx.erl

@@ -95,11 +95,11 @@ put(PathPattern, Payload) ->
 %% @doc Creates or modifies a specific tree node in the tree structure.
 
 put(PathPattern, Payload, Extra) when ?IS_KHEPRI_PAYLOAD(Payload) ->
-    ensure_path_pattern_is_valid(PathPattern),
     ensure_updates_are_allowed(),
+    PathPattern1 = path_from_string(PathPattern),
     {State, SideEffects} = get_tx_state(),
     Ret = khepri_machine:insert_or_update_node(
-            State, PathPattern, Payload, Extra),
+            State, PathPattern1, Payload, Extra),
     case Ret of
         {NewState, Result, NewSideEffects} ->
             set_tx_state(NewState, SideEffects ++ NewSideEffects);
@@ -114,9 +114,9 @@ get(PathPattern) ->
     get(PathPattern, #{}).
 
 get(PathPattern, Options) ->
-    ensure_path_pattern_is_valid(PathPattern),
+    PathPattern1 = path_from_string(PathPattern),
     {#khepri_machine{root = Root}, _SideEffects} = get_tx_state(),
-    khepri_machine:find_matching_nodes(Root, PathPattern, Options).
+    khepri_machine:find_matching_nodes(Root, PathPattern1, Options).
 
 -spec exists(Path) -> Exists when
       Path :: khepri_path:pattern(),
@@ -151,10 +151,10 @@ find(Path, Condition) ->
     get(Path1).
 
 delete(PathPattern) ->
-    ensure_path_pattern_is_valid(PathPattern),
     ensure_updates_are_allowed(),
+    PathPattern1 = path_from_string(PathPattern),
     {State, SideEffects} = get_tx_state(),
-    Ret = khepri_machine:delete_matching_nodes(State, PathPattern),
+    Ret = khepri_machine:delete_matching_nodes(State, PathPattern1),
     case Ret of
         {NewState, Result, NewSideEffects} ->
             set_tx_state(NewState, SideEffects ++ NewSideEffects);
@@ -602,13 +602,22 @@ set_tx_state(#khepri_machine{} = NewState, SideEffects) ->
 get_tx_props() ->
     erlang:get(?TX_PROPS).
 
--spec ensure_path_pattern_is_valid(PathPattern) -> ok | no_return() when
+-spec path_from_string(PathPattern) -> PathPattern | no_return() when
       PathPattern :: khepri_path:pattern().
+%% @doc Converts a string to a path (if necessary) and validates it.
+%%
+%% This is the same as calling {@link khepri_path:from_string/1} then {@link
+%% khepri_path:is_valid/1}, but the exception is caught to abort the
+%% transaction instead.
 
-ensure_path_pattern_is_valid(PathPattern) ->
-    case khepri_path:is_valid(PathPattern) of
-        true          -> ok;
-        {false, Path} -> abort({invalid_path, Path})
+path_from_string(PathPattern) ->
+    try
+        PathPattern1 = khepri_path:from_string(PathPattern),
+        khepri_path:ensure_is_valid(PathPattern1),
+        PathPattern1
+    catch
+        throw:{invalid_path, _} = Reason ->
+            abort(Reason)
     end.
 
 -spec ensure_updates_are_allowed() -> ok | no_return().

test/machine_code_called_from_ra.erl

@@ -86,13 +86,14 @@ use_an_invalid_path_test_() ->
      fun() -> test_ra_server_helpers:setup(?FUNCTION_NAME) end,
      fun(Priv) -> test_ra_server_helpers:cleanup(Priv) end,
      [?_assertThrow(
-         {invalid_path, not_a_list},
+         {invalid_path, #{path := not_a_list}},
          khepri_machine:put(
            ?FUNCTION_NAME,
            not_a_list,
            none)),
       ?_assertThrow(
-         {invalid_path, "not_a_component"},
+         {invalid_path, #{path := ["not_a_component"],
+                          tail := ["not_a_component"]}},
          khepri_machine:put(
            ?FUNCTION_NAME,
            ["not_a_component"],