From 5e181d44c371dc1021255090bceab8151d7dfb76 Mon Sep 17 00:00:00 2001 From: Dov Alperin Date: Mon, 4 Dec 2023 16:04:26 -0500 Subject: [PATCH] fix: handle `already_started` when starting instance (#111) **Requirements** - [x] I have added test coverage for new or changed functionality - [x] I have followed the repository's [pull request submission guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests) - [ ] I have validated my changes against all supported platform versions **Related issues** https://github.com/launchdarkly/erlang-server-sdk/blob/242986ec2dd4e42cf603ef2005de1fd96444aa42/src/ldclient_instance.erl#L61 **Describe the solution you've provided** N/A **Describe alternatives you've considered** N/A **Additional context** Add any other context about the pull request here. --- src/ldclient_instance.erl | 57 +++++++++++++++++++++-------------- test/ldclient_start_SUITE.erl | 43 ++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 22 deletions(-) create mode 100644 test/ldclient_start_SUITE.erl diff --git a/src/ldclient_instance.erl b/src/ldclient_instance.erl index 2e7d77d..b29b7f7 100644 --- a/src/ldclient_instance.erl +++ b/src/ldclient_instance.erl @@ -58,7 +58,6 @@ -spec start(Tag :: atom(), SdkKey :: string(), Options :: options()) -> ok | {error, already_started, term()}. start(Tag, SdkKey, Options) -> - % TODO check if Tag already exists and return already_started error % Parse options into settings Settings = ldclient_config:parse_options(SdkKey, Options), ok = ldclient_config:register(Tag, Settings), @@ -67,14 +66,25 @@ start(Tag, SdkKey, Options) -> UpdateSupName = get_ref_from_tag(instance_stream, Tag), UpdateWorkerModule = get_update_processor(Settings), EventsSupName = get_ref_from_tag(instance_events, Tag), - {ok, _} = supervisor:start_child(ldclient_sup, ldclient_instance_sup:child_spec(SupName, [SupName, UpdateSupName, UpdateWorkerModule, EventsSupName, Tag])), - % Start storage backend - true = ldclient_update_processor_state:create_storage_initialized_state(Tag, false), - FeatureStore = maps:get(feature_store, Settings), - ok = FeatureStore:init(SupName, Tag, []), - % Start stream client - true = ldclient_update_processor_state:create_initialized_state(Tag, false), - start_updater(UpdateSupName, UpdateWorkerModule, Tag). + case + supervisor:start_child( + ldclient_sup, + ldclient_instance_sup:child_spec(SupName, [ + SupName, UpdateSupName, UpdateWorkerModule, EventsSupName, Tag + ]) + ) + of + {ok, _} -> + % Start storage backend + true = ldclient_update_processor_state:create_storage_initialized_state(Tag, false), + FeatureStore = maps:get(feature_store, Settings), + ok = FeatureStore:init(SupName, Tag, []), + % Start stream client + true = ldclient_update_processor_state:create_initialized_state(Tag, false), + start_updater(UpdateSupName, UpdateWorkerModule, Tag); + {error, {already_started, Pid}} -> + {error, already_started, Pid} + end. %% @doc Stop a client instance %% @@ -148,22 +158,25 @@ start_updater(UpdateSupName, UpdateWorkerModule, Tag) -> %% @private %% %% @end --spec get_update_processor(Settings :: #{ stream := boolean(), - offline := boolean(), - use_ldd := boolean(), - file_datasource := boolean(), - datasource := atom(), - _ => _ - }) -> atom(). -get_update_processor(#{ offline := true }) -> +-spec get_update_processor( + Settings :: #{ + stream := boolean(), + offline := boolean(), + use_ldd := boolean(), + file_datasource := boolean(), + datasource := atom(), + _ => _ + } +) -> atom(). +get_update_processor(#{offline := true}) -> ldclient_update_null_server; -get_update_processor(#{ use_ldd := true }) -> +get_update_processor(#{use_ldd := true}) -> ldclient_update_null_server; -get_update_processor(#{ datasource := DataSource }) when DataSource /= undefined -> +get_update_processor(#{datasource := DataSource}) when DataSource /= undefined -> list_to_atom("ldclient_update_" ++ atom_to_list(DataSource) ++ "_server"); -get_update_processor(#{ file_datasource := true }) -> +get_update_processor(#{file_datasource := true}) -> ldclient_update_file_server; -get_update_processor(#{ stream := false }) -> +get_update_processor(#{stream := false}) -> ldclient_update_poll_server; -get_update_processor(#{ stream := true }) -> +get_update_processor(#{stream := true}) -> ldclient_update_stream_server. diff --git a/test/ldclient_start_SUITE.erl b/test/ldclient_start_SUITE.erl new file mode 100644 index 0000000..a111d66 --- /dev/null +++ b/test/ldclient_start_SUITE.erl @@ -0,0 +1,43 @@ +-module(ldclient_start_SUITE). + +-include_lib("common_test/include/ct.hrl"). + +-compile(export_all). + +%%==================================================================== +%% ct functions +%%==================================================================== +all() -> + [ + test_already_running_client + ]. + +init_per_suite(Config) -> + {ok, _} = application:ensure_all_started(ldclient), + Config. + +end_per_suite(_) -> + ok = application:stop(ldclient). + +init_per_testcase(_, Config) -> + Options = #{ + send_events => false, + feature_store => ldclient_storage_map, + datasource => testdata + }, + ldclient:start_instance("", Options), + Config. + +end_per_testcase(_, _Config) -> + ldclient:stop_all_instances(). + +%%==================================================================== +%% Tests +%%==================================================================== + +test_already_running_client(_) -> + {error, already_started, _} = ldclient:start_instance("", #{ + send_events => false, + feature_store => ldclient_storage_map, + datasource => testdata + }).