Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Makefile
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When tests failed it would leave redis running, and then running tests again would require cleaning redis. This makes that a little more tolerable.

Note it would be better to try to do some trap exit like thing to always run a cleanup. It seems a little less than ideal to do directly in the makefile, but potentially we could move this step into a script and that would make it a bit better.

Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ run:
doc:
@$(REBAR3) edoc

tests:
docker run --name ld-test-redis -p 6379:6379 -d redis
tests: clean-redis
docker run --name ld-test-redis -p 6379:6379 -d redis:7.2
@$(REBAR3) ct --dir="test,test-redis" --logdir logs/ct
docker rm --force ld-test-redis

Expand All @@ -40,7 +40,7 @@ tls-tests:

#This is for local debugging if your tests fail and the Redis Docker container is not torn down properly
clean-redis:
docker rm --force ld-test-redis
docker rm --force ld-test-redis 2> /dev/null || true

colon := :
build-contract-tests:
Expand Down
2 changes: 1 addition & 1 deletion rebar.config
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
{test, [
{deps, [
{bookish_spork, "0.3.5"},
{cowboy, "2.8.0"},
{cowboy, "2.10.0"},
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First, this is only for tests. But the very old version we were using was not compiling correct in OTP 28.

The latest version has dependency constraint problems with rebar3. You can use overrides to fix that, but then it isn't compatible with our tests. So updating this dependency to something newer will need to be its own work.

{meck, "0.9.2"}
]},
{extra_src_dirs, [{"test", [{recursive, true}]}]}
Expand Down
65 changes: 44 additions & 21 deletions src/ldclient_storage_redis_server.erl
Original file line number Diff line number Diff line change
Expand Up @@ -236,11 +236,16 @@ empty_bucket(true, Bucket, Client, Prefix) ->
all_items(false, Bucket, _Client, _Prefix) ->
{error, bucket_not_found, "Redis hash " ++ atom_to_list(Bucket) ++ " does not exist."};
all_items(true, Bucket, Client, Prefix) ->
{ok, Values} = eredis:q(Client, ["HGETALL", bucket_name(Prefix, Bucket)]),
NullFilter = [<<"null">>],
NewValues = lists:filter(fun(Elem) ->
not lists:member(Elem, NullFilter) end, Values), %This removes the initial null key and value
pairs(NewValues, Bucket).
case eredis:q(Client, ["HGETALL", bucket_name(Prefix, Bucket)]) of
{ok, Values} ->
NullFilter = [<<"null">>],
NewValues = lists:filter(fun(Elem) ->
not lists:member(Elem, NullFilter) end, Values), %This removes the initial null key and value
pairs(NewValues, Bucket);
{error, Reason} ->
error_logger:error_msg("Redis error listing all items in bucket ~p: ~s", [Bucket, format_error(Reason)]),
[]
end.

pairs([A, B | L], Bucket) ->
Decoded = jsx:decode(B, [return_maps]),
Expand All @@ -265,19 +270,24 @@ pairs([], _Bucket) -> [].
lookup_key(false, _Key, Bucket, _Client, _Prefix) ->
{error, bucket_not_found, "Redis hash " ++ atom_to_list(Bucket) ++ " does not exist."};
lookup_key(true, Key, Bucket, Client, Prefix) ->
{ok, Value} = eredis:q(Client, ["HGET", bucket_name(Prefix, Bucket), Key]),
if
(Value == undefined) -> [];
true ->
Decoded = jsx:decode(Value, [return_maps]),
case eredis:q(Client, ["HGET", bucket_name(Prefix, Bucket), Key]) of
{ok, Value} ->
if
Bucket == features ->
Parsed = ldclient_flag:new(Decoded),
[{Key, Parsed}];
Bucket == segments ->
Parsed = ldclient_segment:new(Decoded),
[{Key, Parsed}]
end
(Value == undefined) -> [];
true ->
Decoded = jsx:decode(Value, [return_maps]),
if
Bucket == features ->
Parsed = ldclient_flag:new(Decoded),
[{Key, Parsed}];
Bucket == segments ->
Parsed = ldclient_segment:new(Decoded),
[{Key, Parsed}]
end
end;
{error, Reason} ->
error_logger:error_msg("Redis error looking up key ~p in bucket ~p: ~s", [Key, Bucket, format_error(Reason)]),
[]
end.

%% @doc Upsert key value pairs in bucket
Expand Down Expand Up @@ -330,15 +340,28 @@ delete_key(true, Key, Bucket, Client, Prefix) ->
bucket_name(Prefix, Bucket) ->
lists:concat([Prefix, ":", Bucket]).

%% @doc Format Redis error for logging
%% @private
%%
%% @end
-spec format_error(Reason :: no_connection | binary()) -> string().
format_error(no_connection) -> "no connection to Redis";
format_error(Reason) when is_binary(Reason) -> binary_to_list(Reason).

-spec set_init(Client :: client_pid(), Prefix :: string()) -> ok.
set_init(Client, Prefix) ->
{ok, _} = eredis:q(Client, ["SET", lists:concat([Prefix, ":$inited"]), ""]),
ok.

-spec get_init(Client :: client_pid(), Prefix :: string()) -> boolean().
get_init(Client, Prefix) ->
{ok, Value} = eredis:q(Client, ["GET", lists:concat([Prefix, ":$inited"])]),
case Value of
undefined -> false;
_ -> true
case eredis:q(Client, ["GET", lists:concat([Prefix, ":$inited"])]) of
{ok, Value} ->
case Value of
undefined -> false;
_ -> true
end;
{error, Reason} ->
error_logger:error_msg("Redis error getting init flag: ~s", [format_error(Reason)]),
false
end.
4 changes: 3 additions & 1 deletion src/ldclient_update_null_server.erl
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,11 @@ start_link(Tag) ->
-spec init(Args :: term()) ->
{ok, State :: state()} | {ok, State :: state(), timeout() | hibernate} |
{stop, Reason :: term()} | ignore.
init([_Tag]) ->
init([Tag]) ->
% Need to trap exit so supervisor:terminate_child calls terminate callback
process_flag(trap_exit, true),
% When using the null update processor we are always initialized.
ldclient_update_processor_state:set_initialized_state(Tag, true),
State = #{},
{ok, State}.

Expand Down