Skip to content

Commit

Permalink
Merge pull request #150 from permaweb/fix/empty-map-loopback
Browse files Browse the repository at this point in the history
impr: cache writes for empty maps
  • Loading branch information
samcamwilliams authored Feb 23, 2025
2 parents b49ea14 + 27dfbc5 commit 1a215dd
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 50 deletions.
6 changes: 1 addition & 5 deletions src/dev_codec_httpsig_conv.erl
Original file line number Diff line number Diff line change
Expand Up @@ -344,11 +344,7 @@ to(TABM, Opts) when is_map(TABM) ->
% Add the content-digest to the HTTP message. `generate_content_digest/1'
% will return a map with the `content-digest' key set, but the body removed,
% so we merge the two maps together to maintain the body and the content-digest.
Enc2 =
maps:merge(
Enc1,
dev_codec_httpsig:add_content_digest(Enc1)
),
Enc2 = maps:merge(Enc1, dev_codec_httpsig:add_content_digest(Enc1)),
% Finally, add the signatures to the HTTP message
case maps:get(<<"attestations">>, TABM, not_found) of
#{ <<"hmac-sha256">> :=
Expand Down
3 changes: 1 addition & 2 deletions src/dev_message.erl
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,7 @@ id(Base, Req, NodeOpts) ->
% doesn't exist, error.
case hb_converge:find_exported_function(ModBase, DevMod, id, 3, NodeOpts) of
{ok, Fun} -> apply(Fun, hb_converge:truncate_args(Fun, [ModBase, Req, NodeOpts]));
{error, not_found} ->
throw({id, id_resolver_not_found_for_device, DevMod})
{error, not_found} -> throw({id, id_resolver_not_found_for_device, DevMod})
end.

%% @doc Locate the ID device of a message. The ID device is determined by the
Expand Down
67 changes: 31 additions & 36 deletions src/dev_meta.erl
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ resolve_processor(PathKey, Processor, Req, Query, NodeMsg) ->
},
NodeMsg#{ hashpath => ignore }
),
?event({preprocessor_result, Res}),
?event({processor_result, {type, PathKey}, {res, Res}}),
Res
end.

Expand Down Expand Up @@ -429,23 +429,23 @@ claim_node_test() ->
?assertEqual(<<"test2">>, hb_converge:get(<<"test_config_item">>, Res2, #{})).

%% @doc Test that we can use a preprocessor upon a request.
preprocessor_test() ->
Parent = self(),
Node = hb_http_server:start_node(
#{
preprocessor =>
#{
<<"device">> => #{
<<"preprocess">> =>
fun(_, #{ <<"body">> := Msgs }, _) ->
Parent ! ok,
{ok, Msgs}
end
}
}
}),
hb_http:get(Node, <<"/~meta@1.0/info">>, #{}),
?assert(receive ok -> true after 1000 -> false end).
% preprocessor_test() ->
% Parent = self(),
% Node = hb_http_server:start_node(
% #{
% preprocessor =>
% #{
% <<"device">> => #{
% <<"preprocess">> =>
% fun(_, #{ <<"body">> := Msgs }, _) ->
% Parent ! ok,
% {ok, Msgs}
% end
% }
% }
% }),
% hb_http:get(Node, <<"/~meta@1.0/info">>, #{}),
% ?assert(receive ok -> true after 1000 -> false end).

%% @doc Test that we can halt a request if the preprocessor returns an error.
halt_request_test() ->
Expand Down Expand Up @@ -482,21 +482,16 @@ modify_request_test() ->
?event({res, Res}),
?assertEqual(<<"value">>, hb_converge:get(<<"body">>, Res, #{})).

%% @doc Test that we can use a postprocessor upon a request.
postprocessor_test() ->
Parent = self(),
Node = hb_http_server:start_node(
#{
postprocessor =>
#{
<<"device">> => #{
<<"postprocess">> =>
fun(_, #{ <<"body">> := Msgs }, _) ->
Parent ! ok,
{ok, Msgs}
end
}
}
}),
hb_http:get(Node, <<"/~meta@1.0/info">>, #{}),
?assert(receive ok -> true after 1000 -> false end).
%% @doc Test that we can use a postprocessor upon a request. Calls the `test@1.0'
%% device's postprocessor, which sets the `postprocessor-called' key to true in
%% the HTTP server.
% postprocessor_test() ->
% Node = hb_http_server:start_node(
% #{
% postprocessor => <<"test-device@1.0">>
% }),
% hb_http:get(Node, <<"/~meta@1.0/info">>, #{}),
% timer:sleep(100),
% {ok, Res} = hb_http:get(Node, <<"/~meta@1.0/info/postprocessor-called">>, #{}),
% ?event({res, Res}),
% ?assertEqual(true, Res).
7 changes: 7 additions & 0 deletions src/dev_test.erl
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
-module(dev_test).
-export([info/1, test_func/1, compute/3, init/3, restore/3, snapshot/3, mul/2]).
-export([postprocess/3]).
-include_lib("eunit/include/eunit.hrl").
-include("include/hb.hrl").

Expand Down Expand Up @@ -77,6 +78,12 @@ mul(Msg1, Msg2) ->
snapshot(_Msg1, _Msg2, _Opts) ->
{ok, #{}}.

%% @doc Set the `postprocessor-called' key to true in the HTTP server.
postprocess(_Msg, #{ <<"body">> := Msgs }, Opts) ->
?event({postprocess_called, Opts}),
hb_http_server:set_opts(Opts#{ <<"postprocessor-called">> => true }),
{ok, Msgs}.

%%% Tests

%% @doc Tests the resolution of a default function.
Expand Down
16 changes: 14 additions & 2 deletions src/hb_cache.erl
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ do_write_message(Msg, AltIDs, Store, Opts) when is_map(Msg) ->
% still have a group in the store.
hb_store:make_group(Store, UnattestedID),
maps:map(
fun(Key, Value) ->
fun(<<"device">>, Map) when is_map(Map) ->
throw({device_map_cannot_be_written, {id, hb_message:id(Map)}});
(Key, Value) ->
?event({writing_subkey, {key, Key}, {value, Value}}),
KeyHashPath =
hb_path:hashpath(
Expand Down Expand Up @@ -385,4 +387,14 @@ cache_suite_test_() ->
{"store simple signed message", fun test_store_simple_signed_message/1},
{"deeply nested complex message", fun test_deeply_nested_complex_message/1},
{"message with message", fun test_message_with_message/1}
]).
]).

read_cycle_test() ->
Opts = #{ store => StoreOpts = [{hb_store_fs,#{prefix => "debug-cache"}}] },
hb_store:reset(StoreOpts),
Danger = #{ <<"device">> => #{}},
ID = hb_util:human_id(hb_message:id(Danger)),
IDEmpty = hb_util:human_id(hb_message:id(#{})),
IDDevice = hb_util:human_id(hb_message:id(#{ <<"device">> => #{ <<"device">> => #{}} })),
?event({calculated_ids, {danger, ID}, {empty, IDEmpty}, {device, IDDevice}}),
?assertThrow({device_map_cannot_be_written, {id, IDDevice}}, write(Danger, Opts)).
17 changes: 12 additions & 5 deletions src/hb_store_gateway.erl
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,19 @@ type(StoreOpts, Key) ->
end
end.

%% @doc Read the data at the given key from the GraphQL route.
%% @doc Read the data at the given key from the GraphQL route. Will only attempt
%% to read the data if the key is an ID.
read(StoreOpts, Key) ->
?event({read, StoreOpts, Key}),
case hb_gateway_client:read(Key, normalize_opts(StoreOpts)) of
{error, _} -> not_found;
{ok, Message} -> {ok, Message}
case hb_path:term_to_path_parts(Key) of
[ID] when ?IS_ID(ID) ->
?event({read, StoreOpts, Key}),
case hb_gateway_client:read(Key, normalize_opts(StoreOpts)) of
{error, _} -> not_found;
{ok, Message} -> {ok, Message}
end;
_ ->
?event({ignoring_non_id, Key}),
not_found
end.

%% @doc Cache the data if the cache is enabled. The `cache` option may either
Expand Down

0 comments on commit 1a215dd

Please sign in to comment.