Merge branch 'refactor/config-get' into 'develop'

Refactor Pleroma.Config.get to return consistent results for nil values

Closes #2001

See merge request pleroma/pleroma!2853

3 files changed, 88 insertions, 13 deletions

diff --git a/lib/pleroma/config.ex b/lib/pleroma/config.ex
index cc80deff5..a8329cc1e 100644
--- a/lib/pleroma/config.ex
+++ b/lib/pleroma/config.ex
@@ -11,12 +11,10 @@ defmodule Pleroma.Config do
 
   def get([key], default), do: get(key, default)
 
-  def get([parent_key | keys], default) do
-    case :pleroma
-         |> Application.get_env(parent_key)
-         |> get_in(keys) do
-      nil -> default
-      any -> any
+  def get([_ | _] = path, default) do
+    case fetch(path) do
+      {:ok, value} -> value
+      :error -> default
     end
   end
 
@@ -34,6 +32,24 @@ defmodule Pleroma.Config do
     end
   end
 
+  def fetch(key) when is_atom(key), do: fetch([key])
+
+  def fetch([root_key | keys]) do
+    Enum.reduce_while(keys, Application.fetch_env(:pleroma, root_key), fn
+      key, {:ok, config} when is_map(config) or is_list(config) ->
+        case Access.fetch(config, key) do
+          :error ->
+            {:halt, :error}
+
+          value ->
+            {:cont, value}
+        end
+
+      _key, _config ->
+        {:halt, :error}
+    end)
+  end
+
   def put([key], value), do: put(key, value)
 
   def put([parent_key | keys], value) do
@@ -50,12 +66,15 @@ defmodule Pleroma.Config do
 
   def delete([key]), do: delete(key)
 
-  def delete([parent_key | keys]) do
-    {_, parent} =
-      Application.get_env(:pleroma, parent_key)
-      |> get_and_update_in(keys, fn _ -> :pop end)
+  def delete([parent_key | keys] = path) do
+    with {:ok, _} <- fetch(path) do
+      {_, parent} =
+        parent_key
+        |> get()
+        |> get_and_update_in(keys, fn _ -> :pop end)
 
-    Application.put_env(:pleroma, parent_key, parent)
+      Application.put_env(:pleroma, parent_key, parent)
+    end
   end
 
   def delete(key) do
diff --git a/test/config_test.exs b/test/config_test.exs
index a46ab4302..1556e4237 100644
--- a/test/config_test.exs
+++ b/test/config_test.exs
@@ -28,6 +28,34 @@ defmodule Pleroma.ConfigTest do
     assert Pleroma.Config.get([:azerty, :uiop], true) == true
   end
 
+  describe "nil values" do
+    setup do
+      Pleroma.Config.put(:lorem, nil)
+      Pleroma.Config.put(:ipsum, %{dolor: [sit: nil]})
+      Pleroma.Config.put(:dolor, sit: %{amet: nil})
+
+      on_exit(fn -> Enum.each(~w(lorem ipsum dolor)a, &Pleroma.Config.delete/1) end)
+    end
+
+    test "get/1 with an atom for nil value" do
+      assert Pleroma.Config.get(:lorem) == nil
+    end
+
+    test "get/2 with an atom for nil value" do
+      assert Pleroma.Config.get(:lorem, true) == nil
+    end
+
+    test "get/1 with a list of keys for nil value" do
+      assert Pleroma.Config.get([:ipsum, :dolor, :sit]) == nil
+      assert Pleroma.Config.get([:dolor, :sit, :amet]) == nil
+    end
+
+    test "get/2 with a list of keys for nil value" do
+      assert Pleroma.Config.get([:ipsum, :dolor, :sit], true) == nil
+      assert Pleroma.Config.get([:dolor, :sit, :amet], true) == nil
+    end
+  end
+
   test "get/1 when value is false" do
     Pleroma.Config.put([:instance, :false_test], false)
     Pleroma.Config.put([:instance, :nested], [])
@@ -89,5 +117,23 @@ defmodule Pleroma.ConfigTest do
     Pleroma.Config.put([:delete_me, :delete_me], hello: "world", world: "Hello")
     Pleroma.Config.delete([:delete_me, :delete_me, :world])
     assert Pleroma.Config.get([:delete_me, :delete_me]) == [hello: "world"]
+
+    assert Pleroma.Config.delete([:this_key_does_not_exist])
+    assert Pleroma.Config.delete([:non, :existing, :key])
+  end
+
+  test "fetch/1" do
+    Pleroma.Config.put([:lorem], :ipsum)
+    Pleroma.Config.put([:ipsum], dolor: :sit)
+
+    assert Pleroma.Config.fetch([:lorem]) == {:ok, :ipsum}
+    assert Pleroma.Config.fetch(:lorem) == {:ok, :ipsum}
+    assert Pleroma.Config.fetch([:ipsum, :dolor]) == {:ok, :sit}
+    assert Pleroma.Config.fetch([:lorem, :ipsum]) == :error
+    assert Pleroma.Config.fetch([:loremipsum]) == :error
+    assert Pleroma.Config.fetch(:loremipsum) == :error
+
+    Pleroma.Config.delete([:lorem])
+    Pleroma.Config.delete([:ipsum])
   end
 end
diff --git a/test/support/helpers.ex b/test/support/helpers.ex
index 5cbf2e291..ecd4b1e18 100644
--- a/test/support/helpers.ex
+++ b/test/support/helpers.ex
@@ -17,9 +17,19 @@ defmodule Pleroma.Tests.Helpers do
 
   defmacro clear_config(config_path, do: yield) do
     quote do
-      initial_setting = Config.get(unquote(config_path))
+      initial_setting = Config.fetch(unquote(config_path))
       unquote(yield)
-      on_exit(fn -> Config.put(unquote(config_path), initial_setting) end)
+
+      on_exit(fn ->
+        case initial_setting do
+          :error ->
+            Config.delete(unquote(config_path))
+
+          {:ok, value} ->
+            Config.put(unquote(config_path), value)
+        end
+      end)
+
       :ok
     end
   end