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

This commit is contained in:
Ivan Tashkinov 2020-02-29 22:10:32 +03:00
parent c747260989
commit 2acaecca20
2 changed files with 78 additions and 65 deletions

View File

@ -74,14 +74,25 @@ defmodule Pleroma.Plugs.RateLimiter do
@doc false @doc false
def init(plug_opts) do 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 end
def call(conn, plug_opts) do def call(conn, nil), do: conn
def call(conn, action_static_settings) do
if disabled?() do if disabled?() do
handle_disabled(conn) handle_disabled(conn)
else else
action_settings = action_settings(plug_opts) action_settings = action_settings(action_static_settings)
handle(conn, action_settings) handle(conn, action_settings)
end end
end end
@ -123,38 +134,38 @@ defmodule Pleroma.Plugs.RateLimiter do
@inspect_bucket_not_found {:error, :not_found} @inspect_bucket_not_found {:error, :not_found}
def inspect_bucket(conn, bucket_name_root, plug_opts) do def inspect_bucket(conn, bucket_name_root, %{name: _} = action_static_settings) do
with %{name: _} = action_settings <- action_settings(plug_opts) do action_settings =
action_settings = incorporate_conn_info(action_settings, conn) action_static_settings
bucket_name = make_bucket_name(%{action_settings | name: bucket_name_root}) |> action_settings()
key_name = make_key_name(action_settings) |> incorporate_conn_info(conn)
limit = get_limits(action_settings)
case Cachex.get(bucket_name, key_name) do bucket_name = make_bucket_name(%{action_settings | name: bucket_name_root})
{:error, :no_cache} -> key_name = make_key_name(action_settings)
@inspect_bucket_not_found limit = get_limits(action_settings)
{:ok, nil} -> case Cachex.get(bucket_name, key_name) do
{0, limit} {:error, :no_cache} ->
@inspect_bucket_not_found
{:ok, value} -> {:ok, nil} ->
{value, limit - value} {0, limit}
end
else {:ok, value} ->
_ -> @inspect_bucket_not_found {value, limit - value}
end end
end end
def action_settings(plug_opts) do def inspect_bucket(_conn, _bucket_name_root, _action_static_settings),
with limiter_name when is_atom(limiter_name) <- plug_opts[:name], do: @inspect_bucket_not_found
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 action_settings(nil), do: nil
name: bucket_name_root,
limits: limits, def action_settings(%{opts: opts} = action_static_settings) do
opts: plug_opts 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
end end
@ -237,9 +248,9 @@ defmodule Pleroma.Plugs.RateLimiter do
defp make_bucket_name(%{mode: :anon, name: bucket_name_root}), defp make_bucket_name(%{mode: :anon, name: bucket_name_root}),
do: anon_bucket_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 = params_string =
plug_opts action_static_settings
|> Keyword.get(:params, []) |> Keyword.get(:params, [])
|> Enum.sort() |> Enum.sort()
|> Enum.map(&Map.get(conn_params, &1, "")) |> Enum.map(&Map.get(conn_params, &1, ""))

View File

@ -62,23 +62,23 @@ defmodule Pleroma.Plugs.RateLimiterTest do
end end
test "it restricts based on config values" do test "it restricts based on config values" do
limiter_name = :test_plug_opts limiter_name = :test_action_static_settings
scale = 80 scale = 80
limit = 5 limit = 5
Config.put([Pleroma.Web.Endpoint, :http, :ip], {8, 8, 8, 8}) Config.put([Pleroma.Web.Endpoint, :http, :ip], {8, 8, 8, 8})
Config.put([:rate_limit, limiter_name], {scale, limit}) 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, "/") conn = conn(:get, "/")
for i <- 1..5 do for i <- 1..5 do
conn = RateLimiter.call(conn, plug_opts) conn = RateLimiter.call(conn, action_static_settings)
assert {^i, _} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts) assert {^i, _} = RateLimiter.inspect_bucket(conn, limiter_name, action_static_settings)
Process.sleep(10) Process.sleep(10)
end 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 %{"error" => "Throttled"} = Phoenix.ConnTest.json_response(conn, :too_many_requests)
assert conn.halted assert conn.halted
@ -86,8 +86,8 @@ defmodule Pleroma.Plugs.RateLimiterTest do
conn = conn(:get, "/") conn = conn(:get, "/")
conn = RateLimiter.call(conn, plug_opts) conn = RateLimiter.call(conn, action_static_settings)
assert {1, 4} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts) assert {1, 4} = RateLimiter.inspect_bucket(conn, limiter_name, action_static_settings)
refute conn.status == Plug.Conn.Status.code(:too_many_requests) refute conn.status == Plug.Conn.Status.code(:too_many_requests)
refute conn.resp_body refute conn.resp_body
@ -103,13 +103,15 @@ defmodule Pleroma.Plugs.RateLimiterTest do
Config.put([Pleroma.Web.Endpoint, :http, :ip], {8, 8, 8, 8}) Config.put([Pleroma.Web.Endpoint, :http, :ip], {8, 8, 8, 8})
base_bucket_name = "#{limiter_name}:group1" 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, "/") conn = conn(:get, "/")
RateLimiter.call(conn, plug_opts) RateLimiter.call(conn, action_static_settings)
assert {1, 4} = RateLimiter.inspect_bucket(conn, base_bucket_name, plug_opts) assert {1, 4} = RateLimiter.inspect_bucket(conn, base_bucket_name, action_static_settings)
assert {:error, :not_found} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts)
assert {:error, :not_found} =
RateLimiter.inspect_bucket(conn, limiter_name, action_static_settings)
end end
test "`params` option allows different queries to be tracked independently" do 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([:rate_limit, limiter_name], {1000, 5})
Config.put([Pleroma.Web.Endpoint, :http, :ip], {8, 8, 8, 8}) 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 = conn(:get, "/?id=1")
conn = Plug.Conn.fetch_query_params(conn) conn = Plug.Conn.fetch_query_params(conn)
conn_2 = conn(:get, "/?id=2") conn_2 = conn(:get, "/?id=2")
RateLimiter.call(conn, plug_opts) RateLimiter.call(conn, action_static_settings)
assert {1, 4} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts) assert {1, 4} = RateLimiter.inspect_bucket(conn, limiter_name, action_static_settings)
assert {0, 5} = RateLimiter.inspect_bucket(conn_2, limiter_name, plug_opts) assert {0, 5} = RateLimiter.inspect_bucket(conn_2, limiter_name, action_static_settings)
end end
test "it supports combination of options modifying bucket name" do 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" base_bucket_name = "#{limiter_name}:group1"
plug_opts = action_static_settings =
RateLimiter.init(name: limiter_name, bucket_name: base_bucket_name, params: ["id"]) RateLimiter.init(name: limiter_name, bucket_name: base_bucket_name, params: ["id"])
id = "100" id = "100"
@ -144,9 +146,9 @@ defmodule Pleroma.Plugs.RateLimiterTest do
conn = Plug.Conn.fetch_query_params(conn) conn = Plug.Conn.fetch_query_params(conn)
conn_2 = conn(:get, "/?id=#{101}") conn_2 = conn(:get, "/?id=#{101}")
RateLimiter.call(conn, plug_opts) RateLimiter.call(conn, action_static_settings)
assert {1, 4} = RateLimiter.inspect_bucket(conn, base_bucket_name, plug_opts) assert {1, 4} = RateLimiter.inspect_bucket(conn, base_bucket_name, action_static_settings)
assert {0, 5} = RateLimiter.inspect_bucket(conn_2, base_bucket_name, plug_opts) assert {0, 5} = RateLimiter.inspect_bucket(conn_2, base_bucket_name, action_static_settings)
end end
end end
@ -156,24 +158,24 @@ defmodule Pleroma.Plugs.RateLimiterTest do
Config.put([:rate_limit, limiter_name], [{1000, 5}, {1, 10}]) Config.put([:rate_limit, limiter_name], [{1000, 5}, {1, 10}])
Config.put([Pleroma.Web.Endpoint, :http, :ip], {8, 8, 8, 8}) 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 = %{conn(:get, "/") | remote_ip: {127, 0, 0, 2}}
conn_2 = %{conn(:get, "/") | remote_ip: {127, 0, 0, 3}} conn_2 = %{conn(:get, "/") | remote_ip: {127, 0, 0, 3}}
for i <- 1..5 do for i <- 1..5 do
conn = RateLimiter.call(conn, plug_opts) conn = RateLimiter.call(conn, action_static_settings)
assert {^i, _} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts) assert {^i, _} = RateLimiter.inspect_bucket(conn, limiter_name, action_static_settings)
refute conn.halted refute conn.halted
end 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 %{"error" => "Throttled"} = Phoenix.ConnTest.json_response(conn, :too_many_requests)
assert conn.halted assert conn.halted
conn_2 = RateLimiter.call(conn_2, plug_opts) conn_2 = RateLimiter.call(conn_2, action_static_settings)
assert {1, 4} = RateLimiter.inspect_bucket(conn_2, limiter_name, plug_opts) 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.status == Plug.Conn.Status.code(:too_many_requests)
refute conn_2.resp_body 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([Pleroma.Web.Endpoint, :http, :ip], {8, 8, 8, 8})
Config.put([:rate_limit, limiter_name], [{1000, 1}, {scale, limit}]) 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) user = insert(:user)
conn = conn(:get, "/") |> assign(:user, user) conn = conn(:get, "/") |> assign(:user, user)
for i <- 1..5 do for i <- 1..5 do
conn = RateLimiter.call(conn, plug_opts) conn = RateLimiter.call(conn, action_static_settings)
assert {^i, _} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts) assert {^i, _} = RateLimiter.inspect_bucket(conn, limiter_name, action_static_settings)
refute conn.halted refute conn.halted
end 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 %{"error" => "Throttled"} = Phoenix.ConnTest.json_response(conn, :too_many_requests)
assert conn.halted assert conn.halted
@ -218,7 +220,7 @@ defmodule Pleroma.Plugs.RateLimiterTest do
Config.put([:rate_limit, limiter_name], [{1, 10}, {1000, 5}]) Config.put([:rate_limit, limiter_name], [{1, 10}, {1000, 5}])
Config.put([Pleroma.Web.Endpoint, :http, :ip], {8, 8, 8, 8}) 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) user = insert(:user)
conn = conn(:get, "/") |> assign(:user, user) conn = conn(:get, "/") |> assign(:user, user)
@ -227,16 +229,16 @@ defmodule Pleroma.Plugs.RateLimiterTest do
conn_2 = conn(:get, "/") |> assign(:user, user_2) conn_2 = conn(:get, "/") |> assign(:user, user_2)
for i <- 1..5 do for i <- 1..5 do
conn = RateLimiter.call(conn, plug_opts) conn = RateLimiter.call(conn, action_static_settings)
assert {^i, _} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts) assert {^i, _} = RateLimiter.inspect_bucket(conn, limiter_name, action_static_settings)
end 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 %{"error" => "Throttled"} = Phoenix.ConnTest.json_response(conn, :too_many_requests)
assert conn.halted assert conn.halted
conn_2 = RateLimiter.call(conn_2, plug_opts) conn_2 = RateLimiter.call(conn_2, action_static_settings)
assert {1, 4} = RateLimiter.inspect_bucket(conn_2, limiter_name, plug_opts) 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.status == Plug.Conn.Status.code(:too_many_requests)
refute conn_2.resp_body refute conn_2.resp_body
refute conn_2.halted refute conn_2.halted