[#2250] #2250 forced refactoring: mixing of compile-time and runtime settings for RateLimiter (warning: increases complexity, doesn't provide any speed benefits at all).rate-limiter-compile-time-and-runtime-settings-mix-with-increased-complexity

2 files changed, 78 insertions, 65 deletions

diff --git a/lib/pleroma/plugs/rate_limiter/rate_limiter.ex b/lib/pleroma/plugs/rate_limiter/rate_limiter.ex
index 9c362a392..b4a9ec2ce 100644
--- a/lib/pleroma/plugs/rate_limiter/rate_limiter.ex
+++ b/lib/pleroma/plugs/rate_limiter/rate_limiter.ex
@@ -74,14 +74,25 @@ defmodule Pleroma.Plugs.RateLimiter do
 
   @doc false
   def init(plug_opts) do
-    plug_opts
+    with limiter_name when is_atom(limiter_name) <- plug_opts[:name] do
+      bucket_name_root = Keyword.get(plug_opts, :bucket_name, limiter_name)
+
+      %{
+        name: bucket_name_root,
+        opts: plug_opts
+      }
+    else
+      _ -> nil
+    end
   end
 
-  def call(conn, plug_opts) do
+  def call(conn, nil), do: conn
+
+  def call(conn, action_static_settings) do
     if disabled?() do
       handle_disabled(conn)
     else
-      action_settings = action_settings(plug_opts)
+      action_settings = action_settings(action_static_settings)
       handle(conn, action_settings)
     end
   end
@@ -123,38 +134,38 @@ defmodule Pleroma.Plugs.RateLimiter do
 
   @inspect_bucket_not_found {:error, :not_found}
 
-  def inspect_bucket(conn, bucket_name_root, plug_opts) do
-    with %{name: _} = action_settings <- action_settings(plug_opts) do
-      action_settings = incorporate_conn_info(action_settings, conn)
-      bucket_name = make_bucket_name(%{action_settings | name: bucket_name_root})
-      key_name = make_key_name(action_settings)
-      limit = get_limits(action_settings)
+  def inspect_bucket(conn, bucket_name_root, %{name: _} = action_static_settings) do
+    action_settings =
+      action_static_settings
+      |> action_settings()
+      |> incorporate_conn_info(conn)
+
+    bucket_name = make_bucket_name(%{action_settings | name: bucket_name_root})
+    key_name = make_key_name(action_settings)
+    limit = get_limits(action_settings)
 
-      case Cachex.get(bucket_name, key_name) do
-        {:error, :no_cache} ->
-          @inspect_bucket_not_found
+    case Cachex.get(bucket_name, key_name) do
+      {:error, :no_cache} ->
+        @inspect_bucket_not_found
 
-        {:ok, nil} ->
-          {0, limit}
+      {:ok, nil} ->
+        {0, limit}
 
-        {:ok, value} ->
-          {value, limit - value}
-      end
-    else
-      _ -> @inspect_bucket_not_found
+      {:ok, value} ->
+        {value, limit - value}
     end
   end
 
-  def action_settings(plug_opts) do
-    with limiter_name when is_atom(limiter_name) <- plug_opts[:name],
-         limits when not is_nil(limits) <- Config.get([:rate_limit, limiter_name]) do
-      bucket_name_root = Keyword.get(plug_opts, :bucket_name, limiter_name)
+  def inspect_bucket(_conn, _bucket_name_root, _action_static_settings),
+    do: @inspect_bucket_not_found
 
-      %{
-        name: bucket_name_root,
-        limits: limits,
-        opts: plug_opts
-      }
+  def action_settings(nil), do: nil
+
+  def action_settings(%{opts: opts} = action_static_settings) do
+    with limits when not is_nil(limits) <- Config.get([:rate_limit, opts[:name]]) do
+      Map.put(action_static_settings, :limits, limits)
+    else
+      _ -> nil
     end
   end
 
@@ -237,9 +248,9 @@ defmodule Pleroma.Plugs.RateLimiter do
   defp make_bucket_name(%{mode: :anon, name: bucket_name_root}),
     do: anon_bucket_name(bucket_name_root)
 
-  defp attach_selected_params(input, %{conn_params: conn_params, opts: plug_opts}) do
+  defp attach_selected_params(input, %{conn_params: conn_params, opts: action_static_settings}) do
     params_string =
-      plug_opts
+      action_static_settings
       |> Keyword.get(:params, [])
       |> Enum.sort()
       |> Enum.map(&Map.get(conn_params, &1, ""))
diff --git a/test/plugs/rate_limiter_test.exs b/test/plugs/rate_limiter_test.exs
index 104d67611..33162a6b8 100644
--- a/test/plugs/rate_limiter_test.exs
+++ b/test/plugs/rate_limiter_test.exs
@@ -62,23 +62,23 @@ defmodule Pleroma.Plugs.RateLimiterTest do
     end
 
     test "it restricts based on config values" do
-      limiter_name = :test_plug_opts
+      limiter_name = :test_action_static_settings
       scale = 80
       limit = 5
 
       Config.put([Pleroma.Web.Endpoint, :http, :ip], {8, 8, 8, 8})
       Config.put([:rate_limit, limiter_name], {scale, limit})
 
-      plug_opts = RateLimiter.init(name: limiter_name)
+      action_static_settings = RateLimiter.init(name: limiter_name)
       conn = conn(:get, "/")
 
       for i <- 1..5 do
-        conn = RateLimiter.call(conn, plug_opts)
-        assert {^i, _} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts)
+        conn = RateLimiter.call(conn, action_static_settings)
+        assert {^i, _} = RateLimiter.inspect_bucket(conn, limiter_name, action_static_settings)
         Process.sleep(10)
       end
 
-      conn = RateLimiter.call(conn, plug_opts)
+      conn = RateLimiter.call(conn, action_static_settings)
       assert %{"error" => "Throttled"} = Phoenix.ConnTest.json_response(conn, :too_many_requests)
       assert conn.halted
 
@@ -86,8 +86,8 @@ defmodule Pleroma.Plugs.RateLimiterTest do
 
       conn = conn(:get, "/")
 
-      conn = RateLimiter.call(conn, plug_opts)
-      assert {1, 4} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts)
+      conn = RateLimiter.call(conn, action_static_settings)
+      assert {1, 4} = RateLimiter.inspect_bucket(conn, limiter_name, action_static_settings)
 
       refute conn.status == Plug.Conn.Status.code(:too_many_requests)
       refute conn.resp_body
@@ -103,13 +103,15 @@ defmodule Pleroma.Plugs.RateLimiterTest do
       Config.put([Pleroma.Web.Endpoint, :http, :ip], {8, 8, 8, 8})
 
       base_bucket_name = "#{limiter_name}:group1"
-      plug_opts = RateLimiter.init(name: limiter_name, bucket_name: base_bucket_name)
+      action_static_settings = RateLimiter.init(name: limiter_name, bucket_name: base_bucket_name)
 
       conn = conn(:get, "/")
 
-      RateLimiter.call(conn, plug_opts)
-      assert {1, 4} = RateLimiter.inspect_bucket(conn, base_bucket_name, plug_opts)
-      assert {:error, :not_found} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts)
+      RateLimiter.call(conn, action_static_settings)
+      assert {1, 4} = RateLimiter.inspect_bucket(conn, base_bucket_name, action_static_settings)
+
+      assert {:error, :not_found} =
+               RateLimiter.inspect_bucket(conn, limiter_name, action_static_settings)
     end
 
     test "`params` option allows different queries to be tracked independently" do
@@ -117,15 +119,15 @@ defmodule Pleroma.Plugs.RateLimiterTest do
       Config.put([:rate_limit, limiter_name], {1000, 5})
       Config.put([Pleroma.Web.Endpoint, :http, :ip], {8, 8, 8, 8})
 
-      plug_opts = RateLimiter.init(name: limiter_name, params: ["id"])
+      action_static_settings = RateLimiter.init(name: limiter_name, params: ["id"])
 
       conn = conn(:get, "/?id=1")
       conn = Plug.Conn.fetch_query_params(conn)
       conn_2 = conn(:get, "/?id=2")
 
-      RateLimiter.call(conn, plug_opts)
-      assert {1, 4} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts)
-      assert {0, 5} = RateLimiter.inspect_bucket(conn_2, limiter_name, plug_opts)
+      RateLimiter.call(conn, action_static_settings)
+      assert {1, 4} = RateLimiter.inspect_bucket(conn, limiter_name, action_static_settings)
+      assert {0, 5} = RateLimiter.inspect_bucket(conn_2, limiter_name, action_static_settings)
     end
 
     test "it supports combination of options modifying bucket name" do
@@ -135,7 +137,7 @@ defmodule Pleroma.Plugs.RateLimiterTest do
 
       base_bucket_name = "#{limiter_name}:group1"
 
-      plug_opts =
+      action_static_settings =
         RateLimiter.init(name: limiter_name, bucket_name: base_bucket_name, params: ["id"])
 
       id = "100"
@@ -144,9 +146,9 @@ defmodule Pleroma.Plugs.RateLimiterTest do
       conn = Plug.Conn.fetch_query_params(conn)
       conn_2 = conn(:get, "/?id=#{101}")
 
-      RateLimiter.call(conn, plug_opts)
-      assert {1, 4} = RateLimiter.inspect_bucket(conn, base_bucket_name, plug_opts)
-      assert {0, 5} = RateLimiter.inspect_bucket(conn_2, base_bucket_name, plug_opts)
+      RateLimiter.call(conn, action_static_settings)
+      assert {1, 4} = RateLimiter.inspect_bucket(conn, base_bucket_name, action_static_settings)
+      assert {0, 5} = RateLimiter.inspect_bucket(conn_2, base_bucket_name, action_static_settings)
     end
   end
 
@@ -156,24 +158,24 @@ defmodule Pleroma.Plugs.RateLimiterTest do
       Config.put([:rate_limit, limiter_name], [{1000, 5}, {1, 10}])
       Config.put([Pleroma.Web.Endpoint, :http, :ip], {8, 8, 8, 8})
 
-      plug_opts = RateLimiter.init(name: limiter_name)
+      action_static_settings = RateLimiter.init(name: limiter_name)
 
       conn = %{conn(:get, "/") | remote_ip: {127, 0, 0, 2}}
       conn_2 = %{conn(:get, "/") | remote_ip: {127, 0, 0, 3}}
 
       for i <- 1..5 do
-        conn = RateLimiter.call(conn, plug_opts)
-        assert {^i, _} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts)
+        conn = RateLimiter.call(conn, action_static_settings)
+        assert {^i, _} = RateLimiter.inspect_bucket(conn, limiter_name, action_static_settings)
         refute conn.halted
       end
 
-      conn = RateLimiter.call(conn, plug_opts)
+      conn = RateLimiter.call(conn, action_static_settings)
 
       assert %{"error" => "Throttled"} = Phoenix.ConnTest.json_response(conn, :too_many_requests)
       assert conn.halted
 
-      conn_2 = RateLimiter.call(conn_2, plug_opts)
-      assert {1, 4} = RateLimiter.inspect_bucket(conn_2, limiter_name, plug_opts)
+      conn_2 = RateLimiter.call(conn_2, action_static_settings)
+      assert {1, 4} = RateLimiter.inspect_bucket(conn_2, limiter_name, action_static_settings)
 
       refute conn_2.status == Plug.Conn.Status.code(:too_many_requests)
       refute conn_2.resp_body
@@ -196,18 +198,18 @@ defmodule Pleroma.Plugs.RateLimiterTest do
       Config.put([Pleroma.Web.Endpoint, :http, :ip], {8, 8, 8, 8})
       Config.put([:rate_limit, limiter_name], [{1000, 1}, {scale, limit}])
 
-      plug_opts = RateLimiter.init(name: limiter_name)
+      action_static_settings = RateLimiter.init(name: limiter_name)
 
       user = insert(:user)
       conn = conn(:get, "/") |> assign(:user, user)
 
       for i <- 1..5 do
-        conn = RateLimiter.call(conn, plug_opts)
-        assert {^i, _} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts)
+        conn = RateLimiter.call(conn, action_static_settings)
+        assert {^i, _} = RateLimiter.inspect_bucket(conn, limiter_name, action_static_settings)
         refute conn.halted
       end
 
-      conn = RateLimiter.call(conn, plug_opts)
+      conn = RateLimiter.call(conn, action_static_settings)
 
       assert %{"error" => "Throttled"} = Phoenix.ConnTest.json_response(conn, :too_many_requests)
       assert conn.halted
@@ -218,7 +220,7 @@ defmodule Pleroma.Plugs.RateLimiterTest do
       Config.put([:rate_limit, limiter_name], [{1, 10}, {1000, 5}])
       Config.put([Pleroma.Web.Endpoint, :http, :ip], {8, 8, 8, 8})
 
-      plug_opts = RateLimiter.init(name: limiter_name)
+      action_static_settings = RateLimiter.init(name: limiter_name)
 
       user = insert(:user)
       conn = conn(:get, "/") |> assign(:user, user)
@@ -227,16 +229,16 @@ defmodule Pleroma.Plugs.RateLimiterTest do
       conn_2 = conn(:get, "/") |> assign(:user, user_2)
 
       for i <- 1..5 do
-        conn = RateLimiter.call(conn, plug_opts)
-        assert {^i, _} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts)
+        conn = RateLimiter.call(conn, action_static_settings)
+        assert {^i, _} = RateLimiter.inspect_bucket(conn, limiter_name, action_static_settings)
       end
 
-      conn = RateLimiter.call(conn, plug_opts)
+      conn = RateLimiter.call(conn, action_static_settings)
       assert %{"error" => "Throttled"} = Phoenix.ConnTest.json_response(conn, :too_many_requests)
       assert conn.halted
 
-      conn_2 = RateLimiter.call(conn_2, plug_opts)
-      assert {1, 4} = RateLimiter.inspect_bucket(conn_2, limiter_name, plug_opts)
+      conn_2 = RateLimiter.call(conn_2, action_static_settings)
+      assert {1, 4} = RateLimiter.inspect_bucket(conn_2, limiter_name, action_static_settings)
       refute conn_2.status == Plug.Conn.Status.code(:too_many_requests)
       refute conn_2.resp_body
       refute conn_2.halted