✨ Config v6 (#133)

* 🎨 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

Author: randycoulman

.credo.exs

@@ -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, []},

config/config.exs

@@ -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

lib/config_cat/cache_policy/auto.ex

@@ -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
@@ -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)

lib/config_cat/cache_policy/behaviour.ex

@@ -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

lib/config_cat/cache_policy/helpers.ex

@@ -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
@@ -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
 

lib/config_cat/cache_policy/lazy.ex

@@ -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
 

lib/config_cat/cache_policy/manual.ex

@@ -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

lib/config_cat/client.ex

@@ -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
@@ -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(
@@ -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.",
@@ -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
@@ -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}'."
@@ -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

lib/config_cat/config.ex

@@ -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()
@@ -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