Skip to content

Commit

Permalink
✨ Config v6 (#133)
Browse files Browse the repository at this point in the history
* 🎨 Adopt Styler

Adopt the [Styler](https://github.com/adobe/elixir-styler) library, which is an Elixir formatter plugin that imposes some opinionated formatting on the code in place of a number of Credo rules.

While I don't always prefer its opinions, I really like have it auto-fix a number of issues, and I find that it's worth the tradeoff to adopt it.

* ♻️ Encapsulate config format (#114)

* ♻️ Rename settings -> feature flags

This is a start at encapsulating the format of the Config map.

* ♻️ Introduce Config.Preferences module

Refactor existing use of preferences to use new module.

* ♻️ Encapsulate EvaluationFormula and rules

* ♻️ Move entry matching to config modules
  • Loading branch information
randycoulman authored Feb 2, 2024
1 parent cf572a3 commit b4110dd
Show file tree
Hide file tree
Showing 153 changed files with 8,215 additions and 1,179 deletions.
2 changes: 1 addition & 1 deletion .credo.exs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@
{Credo.Check.Refactor.IoPuts, []},
{Credo.Check.Refactor.LongQuoteBlocks, []},
{Credo.Check.Refactor.MatchInCondition, []},
{Credo.Check.Refactor.Nesting, []},
{Credo.Check.Refactor.Nesting, [max_nesting: 3]},
{Credo.Check.Refactor.PassAsyncInTestCases, []},
{Credo.Check.Refactor.FilterFilter, []},
{Credo.Check.Refactor.RejectReject, []},
Expand Down
19 changes: 19 additions & 0 deletions config/config.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import Config

if config_env() == :dev do
config :mix_test_interactive, clear: true
end

if config_env() == :test do
config :logger, level: :warning

if Version.compare(System.version(), "1.15.0") == :lt do
config :logger, :console,
colors: [enabled: false],
format: "$level $message\n"
else
config :logger, :default_formatter,
colors: [enabled: false],
format: "$level $message\n"
end
end
6 changes: 3 additions & 3 deletions lib/config_cat/cache_policy/auto.ex
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ defmodule ConfigCat.CachePolicy.Auto do

@impl GenServer
def handle_call(:get, _from, %State{} = state) do
{:reply, Helpers.cached_settings(state), state}
{:reply, Helpers.cached_config(state), state}
end

@impl GenServer
Expand Down Expand Up @@ -204,10 +204,10 @@ defmodule ConfigCat.CachePolicy.Auto do
defp be_initialized(%State{} = state) when initialized?(state), do: state

defp be_initialized(%State{} = state) do
settings = Helpers.cached_settings(state)
config = Helpers.cached_config(state)

for caller <- state.policy_state.callers do
GenServer.reply(caller, settings)
GenServer.reply(caller, config)
end

Helpers.on_client_ready(state)
Expand Down
2 changes: 1 addition & 1 deletion lib/config_cat/cache_policy/behaviour.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ defmodule ConfigCat.CachePolicy.Behaviour do
alias ConfigCat.FetchTime

@callback get(ConfigCat.instance_id()) ::
{:ok, Config.settings(), FetchTime.t()} | {:error, :not_found}
{:ok, Config.t(), FetchTime.t()} | {:error, :not_found}
@callback offline?(ConfigCat.instance_id()) :: boolean()
@callback set_offline(ConfigCat.instance_id()) :: :ok
@callback set_online(ConfigCat.instance_id()) :: :ok
Expand Down
21 changes: 3 additions & 18 deletions lib/config_cat/cache_policy/helpers.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ defmodule ConfigCat.CachePolicy.Helpers do
alias ConfigCat.Cache
alias ConfigCat.CachePolicy
alias ConfigCat.Config
alias ConfigCat.ConfigCache
alias ConfigCat.ConfigEntry
alias ConfigCat.ConfigFetcher.FetchError
alias ConfigCat.FetchTime
Expand Down Expand Up @@ -66,25 +65,11 @@ defmodule ConfigCat.CachePolicy.Helpers do
Hooks.invoke_on_client_ready(state.instance_id)
end

@spec cached_settings(State.t()) ::
{:ok, Config.settings(), FetchTime.t()} | {:error, :not_found}
def cached_settings(%State{} = state) do
with {:ok, %ConfigEntry{} = entry} <- cached_entry(state),
{:ok, settings} <- Config.fetch_settings(entry.config) do
{:ok, settings, entry.fetch_time_ms}
else
:error ->
{:error, :not_found}

error ->
error
end
end

@spec cached_config(State.t()) :: ConfigCache.result()
@spec cached_config(State.t()) ::
{:ok, Config.t(), FetchTime.t()} | {:error, :not_found}
def cached_config(%State{} = state) do
with {:ok, %ConfigEntry{} = entry} <- cached_entry(state) do
{:ok, entry.config}
{:ok, entry.config, entry.fetch_time_ms}
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/config_cat/cache_policy/lazy.ex
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ defmodule ConfigCat.CachePolicy.Lazy do
@impl GenServer
def handle_call(:get, _from, %State{} = state) do
with {:ok, new_state} <- maybe_refresh(state) do
{:reply, Helpers.cached_settings(new_state), new_state}
{:reply, Helpers.cached_config(new_state), new_state}
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/config_cat/cache_policy/manual.ex
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ defmodule ConfigCat.CachePolicy.Manual do

@impl GenServer
def handle_call(:get, _from, %State{} = state) do
{:reply, Helpers.cached_settings(state), state}
{:reply, Helpers.cached_config(state), state}
end

@impl GenServer
Expand Down
95 changes: 65 additions & 30 deletions lib/config_cat/client.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,18 @@ defmodule ConfigCat.Client do
use GenServer

alias ConfigCat.CachePolicy
alias ConfigCat.Config
alias ConfigCat.Config.Setting
alias ConfigCat.EvaluationDetails
alias ConfigCat.EvaluationLogger
alias ConfigCat.FetchTime
alias ConfigCat.Hooks
alias ConfigCat.OverrideDataSource
alias ConfigCat.Rollout
alias ConfigCat.User

require ConfigCat.Config.SettingType, as: SettingType
require ConfigCat.ConfigCatLogger, as: ConfigCatLogger
require ConfigCat.Constants, as: Constants

defmodule State do
@moduledoc false
Expand Down Expand Up @@ -96,9 +99,12 @@ defmodule ConfigCat.Client do

@impl GenServer
def handle_call({:get_key_and_value, variation_id}, _from, %State{} = state) do
case cached_settings(state) do
{:ok, settings, _fetch_time_ms} ->
result = Enum.find_value(settings, nil, &entry_matching(&1, variation_id))
case cached_config(state) do
{:ok, config, _fetch_time_ms} ->
result =
config
|> Config.settings()
|> Enum.find_value(nil, &entry_matching(&1, variation_id))

if is_nil(result) do
ConfigCatLogger.error(
Expand Down Expand Up @@ -183,9 +189,9 @@ defmodule ConfigCat.Client do
end

defp do_get_all_keys(%State{} = state) do
case cached_settings(state) do
{:ok, settings, _fetch_time_ms} ->
Map.keys(settings)
case cached_config(state) do
{:ok, config, _fetch_time_ms} ->
config |> Config.settings() |> Map.keys()

_ ->
ConfigCatLogger.error("Config JSON is not present. Returning empty result.",
Expand All @@ -197,29 +203,26 @@ defmodule ConfigCat.Client do
end

defp entry_matching({key, setting}, variation_id) do
value_matching(key, setting, variation_id) ||
value_matching(key, Map.get(setting, Constants.rollout_rules()), variation_id) ||
value_matching(key, Map.get(setting, Constants.percentage_rules()), variation_id)
end

defp value_matching(key, value, variation_id) when is_list(value) do
Enum.find_value(value, nil, &value_matching(key, &1, variation_id))
end

defp value_matching(key, value, variation_id) do
if Map.get(value, Constants.variation_id(), nil) == variation_id do
{key, Map.get(value, Constants.value())}
case Setting.variation_value(setting, variation_id) do
nil -> nil
value -> {key, value}
end
end

defp evaluate(key, user, default_value, default_variation_id, %State{} = state) do
user = if user != nil, do: user, else: state.default_user

details =
case cached_settings(state) do
{:ok, settings, fetch_time_ms} ->
%EvaluationDetails{} =
details =
with {:ok, config, fetch_time_ms} <- cached_config(state),
{:ok, _settings} <- Config.fetch_settings(config),
{:ok, logger} <- EvaluationLogger.start() do
try do
%EvaluationDetails{} =
details = Rollout.evaluate(key, user, default_value, default_variation_id, settings)
details =
Rollout.evaluate(key, user, default_value, default_variation_id, config, logger)

check_type_mismatch(details.value, default_value)

fetch_time =
case FetchTime.to_datetime(fetch_time_ms) do
Expand All @@ -228,7 +231,14 @@ defmodule ConfigCat.Client do
end

%{details | fetch_time: fetch_time}
after
logger
|> EvaluationLogger.result()
|> ConfigCatLogger.debug(event_id: 5000)

EvaluationLogger.stop(logger)
end
else
_ ->
message =
"Config JSON is not present when evaluating setting '#{key}'. Returning the `default_value` parameter that you specified in your application: '#{default_value}'."
Expand All @@ -249,23 +259,48 @@ defmodule ConfigCat.Client do
details
end

defp cached_settings(%State{} = state) do
defp cached_config(%State{} = state) do
%{cache_policy: policy, flag_overrides: flag_overrides, instance_id: instance_id} = state
local_settings = OverrideDataSource.overrides(flag_overrides)
local_config = OverrideDataSource.overrides(flag_overrides)

case OverrideDataSource.behaviour(flag_overrides) do
:local_only ->
{:ok, local_settings, 0}
{:ok, local_config, 0}

:local_over_remote ->
with {:ok, remote_settings, fetch_time_ms} <- policy.get(instance_id) do
{:ok, Map.merge(remote_settings, local_settings), fetch_time_ms}
with {:ok, remote_config, fetch_time_ms} <- policy.get(instance_id) do
{:ok, Config.merge(remote_config, local_config), fetch_time_ms}
end

:remote_over_local ->
with {:ok, remote_settings, fetch_time_ms} <- policy.get(instance_id) do
{:ok, Map.merge(local_settings, remote_settings), fetch_time_ms}
with {:ok, remote_config, fetch_time_ms} <- policy.get(instance_id) do
merged = Config.merge(local_config, remote_config)
{:ok, merged, fetch_time_ms}
end
end
end

defp check_type_mismatch(_value, nil), do: :ok

defp check_type_mismatch(value, default_value) do
value_type = SettingType.from_value(value)
default_type = SettingType.from_value(default_value)
number_types = [SettingType.double(), SettingType.int()]

cond do
value_type == default_type ->
:ok

value_type in number_types and default_type in number_types ->
:ok

true ->
ConfigCatLogger.warning(
"The type of a setting does not match the type of the specified default value (#{default_value}). " <>
"Setting's type was #{value_type} but the default value's type was #{default_type}. " <>
"Please make sure that using a default value not matching the setting's type was intended.",
event_id: 4002
)
end
end
end
81 changes: 56 additions & 25 deletions lib/config_cat/config.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,27 @@ defmodule ConfigCat.Config do
@moduledoc """
Defines configuration-related types used in the rest of the library.
"""
alias ConfigCat.RedirectMode
alias ConfigCat.Config.Preferences
alias ConfigCat.Config.Segment
alias ConfigCat.Config.Setting

@typedoc false
@type comparator :: non_neg_integer()

@typedoc "The name of a configuration setting."
@type key :: String.t()

@typedoc "The configuration settings within a Config."
@type settings :: map()
@typedoc false
@type opt :: {:preferences, Preferences.t()} | {:settings, settings()}

@typedoc false
@type salt :: String.t()

@typedoc false
@type settings :: %{String.t() => Setting.t()}

@typedoc "A collection of configuration settings and preferences."
@type t :: map()
@type t :: %{String.t() => map()}

@typedoc false
@type url :: String.t()
Expand All @@ -25,43 +33,66 @@ defmodule ConfigCat.Config do
@typedoc "The name of a variation being tested."
@type variation_id :: String.t()

@feature_flags "f"
@settings "f"
@preferences "p"
@preferences_base_url "u"
@redirect_mode "r"
@segments "s"

@doc false
@spec new([opt]) :: t()
def new(opts \\ []) do
settings = Keyword.get(opts, :settings, %{})
preferences = Keyword.get_lazy(opts, :preferences, &Preferences.new/0)

%{@settings => settings, @preferences => preferences}
end

@doc false
@spec preferences(t()) :: Preferences.t()
def preferences(config) do
Map.get_lazy(config, @preferences, &Preferences.new/0)
end

@doc false
@spec new_with_preferences(url(), RedirectMode.t()) :: t()
def new_with_preferences(base_url, redirect_mode) do
%{
@preferences => %{
@preferences_base_url => base_url,
@redirect_mode => redirect_mode
}
}
@spec segments(t()) :: [Segment.t()]
def segments(config) do
Map.get(config, @segments, [])
end

@doc false
@spec new_with_settings(settings()) :: t()
def new_with_settings(settings) do
%{@feature_flags => settings}
@spec settings(t()) :: settings()
def settings(config) do
Map.get(config, @settings, %{})
end

@doc false
@spec fetch_settings(t()) :: {:ok, settings()} | {:error, :not_found}
def fetch_settings(config) do
case Map.fetch(config, @feature_flags) do
case Map.fetch(config, @settings) do
{:ok, settings} -> {:ok, settings}
:error -> {:error, :not_found}
end
end

@doc false
@spec preferences(t()) :: {url() | nil, RedirectMode.t() | nil}
def preferences(config) do
case config[@preferences] do
nil -> {nil, nil}
preferences -> {preferences[@preferences_base_url], preferences[@redirect_mode]}
end
@spec merge(left :: t(), right :: t()) :: t()
def merge(left, right) do
left_flags = settings(left)
right_flags = settings(right)

Map.put(left, @settings, Map.merge(left_flags, right_flags))
end

@doc false
@spec inline_salt_and_segments(t()) :: t()
def inline_salt_and_segments(config) do
salt = config |> preferences() |> Preferences.salt()
segments = segments(config)

Map.update(
config,
@settings,
%{},
&Map.new(&1, fn {key, setting} -> {key, Setting.inline_salt_and_segments(setting, salt, segments)} end)
)
end
end
Loading

0 comments on commit b4110dd

Please sign in to comment.