diff --git a/lib/req/rate_limiter.ex b/lib/req/rate_limiter.ex index c0e1795e..00e053fd 100644 --- a/lib/req/rate_limiter.ex +++ b/lib/req/rate_limiter.ex @@ -12,15 +12,22 @@ defmodule Req.RateLimiter do |> Req.RateLimiter.attach(name: :music_brainz, cooldown: 500) When `cooldown` is 0, the step is a no-op. + + ## Clock + + Time operations are delegated to a clock module implementing + `Req.RateLimiter.Clock`. Defaults to `Req.RateLimiter.SystemClock`. + Pass `:clock` in `attach/2` opts to override (useful in tests). """ @table __MODULE__ + @default_clock Req.RateLimiter.SystemClock @doc """ Creates the ETS table used to track request timestamps. Call once at application startup. """ - @type attach_opts :: [name: atom(), cooldown: non_neg_integer()] + @type attach_opts :: [name: atom(), cooldown: non_neg_integer(), clock: module()] @spec new() :: :ets.table() def new do @@ -34,16 +41,19 @@ defmodule Req.RateLimiter do * `:name` - atom identifying the API (e.g. `:music_brainz`) * `:cooldown` - minimum milliseconds between requests + * `:clock` - module implementing `Req.RateLimiter.Clock` (default: `Req.RateLimiter.SystemClock`) """ @spec attach(Req.Request.t(), attach_opts()) :: Req.Request.t() def attach(request, opts) do name = Keyword.fetch!(opts, :name) cooldown = Keyword.fetch!(opts, :cooldown) + clock = Keyword.get(opts, :clock, @default_clock) request |> Req.Request.put_private(:rate_limiter_name, name) |> Req.Request.put_private(:rate_limiter_cooldown, cooldown) + |> Req.Request.put_private(:rate_limiter_clock, clock) |> Req.Request.prepend_request_steps(rate_limiter: &throttle/1) end @@ -52,7 +62,8 @@ defmodule Req.RateLimiter do if cooldown > 0 do name = Req.Request.get_private(request, :rate_limiter_name) - now = System.monotonic_time(:millisecond) + clock = Req.Request.get_private(request, :rate_limiter_clock) + now = clock.now() case :ets.lookup(@table, name) do [{^name, last_at}] -> @@ -66,14 +77,14 @@ defmodule Req.RateLimiter do %{name: name} ) - Process.sleep(remaining) + clock.sleep(remaining) end [] -> :ok end - :ets.insert(@table, {name, System.monotonic_time(:millisecond)}) + :ets.insert(@table, {name, clock.now()}) end request diff --git a/lib/req/rate_limiter/clock.ex b/lib/req/rate_limiter/clock.ex new file mode 100644 index 00000000..a2e009f8 --- /dev/null +++ b/lib/req/rate_limiter/clock.ex @@ -0,0 +1,10 @@ +defmodule Req.RateLimiter.Clock do + @moduledoc """ + Behaviour for time operations used by `Req.RateLimiter`. + + Allows injecting a fake clock in tests for deterministic throttle assertions. + """ + + @callback now() :: integer() + @callback sleep(non_neg_integer()) :: :ok +end diff --git a/lib/req/rate_limiter/system_clock.ex b/lib/req/rate_limiter/system_clock.ex new file mode 100644 index 00000000..36438c74 --- /dev/null +++ b/lib/req/rate_limiter/system_clock.ex @@ -0,0 +1,13 @@ +defmodule Req.RateLimiter.SystemClock do + @moduledoc """ + Real clock implementation using `System.monotonic_time/1` and `Process.sleep/1`. + """ + + @behaviour Req.RateLimiter.Clock + + @impl true + def now, do: System.monotonic_time(:millisecond) + + @impl true + def sleep(ms), do: Process.sleep(ms) +end diff --git a/test/req/rate_limiter_test.exs b/test/req/rate_limiter_test.exs index a3c4826e..d1e07589 100644 --- a/test/req/rate_limiter_test.exs +++ b/test/req/rate_limiter_test.exs @@ -2,15 +2,28 @@ defmodule Req.RateLimiterTest do use ExUnit.Case, async: true alias Req.RateLimiter + alias Req.RateLimiter.FakeClock + + setup do + FakeClock.set(1000) + :ok + end + + defp adapter do + fn request -> + {request, Req.Response.new(status: 200, body: "ok")} + end + end describe "attach/2" do test "attaches rate_limiter step and sets private fields" do request = Req.new(url: "https://example.com") - |> RateLimiter.attach(name: :test_attach, cooldown: 100) + |> RateLimiter.attach(name: :test_attach, cooldown: 100, clock: FakeClock) assert Req.Request.get_private(request, :rate_limiter_name) == :test_attach assert Req.Request.get_private(request, :rate_limiter_cooldown) == 100 + assert Req.Request.get_private(request, :rate_limiter_clock) == FakeClock step_names = Enum.map(request.request_steps, &elem(&1, 0)) assert :rate_limiter in step_names @@ -20,7 +33,7 @@ defmodule Req.RateLimiterTest do request = Req.new(url: "https://example.com") |> Req.Request.append_request_steps(log_attempt: fn req -> req end) - |> RateLimiter.attach(name: :test_order, cooldown: 100) + |> RateLimiter.attach(name: :test_order, cooldown: 100, clock: FakeClock) step_names = Enum.map(request.request_steps, &elem(&1, 0)) rate_limiter_index = Enum.find_index(step_names, &(&1 == :rate_limiter)) @@ -34,76 +47,80 @@ defmodule Req.RateLimiterTest do test "does not sleep when cooldown is 0" do name = :"test_no_sleep_#{System.unique_integer([:positive])}" - adapter = fn request -> - {request, Req.Response.new(status: 200, body: "ok")} - end - request = - Req.new(url: "https://example.com", adapter: adapter) - |> RateLimiter.attach(name: name, cooldown: 0) + Req.new(url: "https://example.com", adapter: adapter()) + |> RateLimiter.attach(name: name, cooldown: 0, clock: FakeClock) - start = System.monotonic_time(:millisecond) {:ok, _} = Req.get(request) {:ok, _} = Req.get(request) - elapsed = System.monotonic_time(:millisecond) - start - assert elapsed < 200 + # Clock should not have advanced since cooldown is 0 + assert FakeClock.now() == 1000 end test "enforces cooldown between rapid consecutive requests" do name = :"test_cooldown_#{System.unique_integer([:positive])}" - cooldown = 100 - - adapter = fn request -> - {request, Req.Response.new(status: 200, body: "ok")} - end + cooldown = 500 request = - Req.new(url: "https://example.com", adapter: adapter) - |> RateLimiter.attach(name: name, cooldown: cooldown) + Req.new(url: "https://example.com", adapter: adapter()) + |> RateLimiter.attach(name: name, cooldown: cooldown, clock: FakeClock) {:ok, _} = Req.get(request) - start = System.monotonic_time(:millisecond) + # No time has passed, so second request must sleep the full cooldown {:ok, _} = Req.get(request) - elapsed = System.monotonic_time(:millisecond) - start - assert elapsed >= cooldown - 50 + # Clock advanced exactly by the cooldown amount + assert FakeClock.now() == 1000 + cooldown + end + + test "sleeps only the remaining cooldown time" do + name = :"test_partial_#{System.unique_integer([:positive])}" + cooldown = 500 + + request = + Req.new(url: "https://example.com", adapter: adapter()) + |> RateLimiter.attach(name: name, cooldown: cooldown, clock: FakeClock) + + {:ok, _} = Req.get(request) + + # Simulate 200ms passing + FakeClock.advance(200) + + {:ok, _} = Req.get(request) + + # Should have slept the remaining 300ms (500 - 200) + assert FakeClock.now() == 1000 + 200 + 300 end test "does not sleep when enough time has elapsed" do name = :"test_elapsed_#{System.unique_integer([:positive])}" - cooldown = 50 - - adapter = fn request -> - {request, Req.Response.new(status: 200, body: "ok")} - end + cooldown = 500 request = - Req.new(url: "https://example.com", adapter: adapter) - |> RateLimiter.attach(name: name, cooldown: cooldown) + Req.new(url: "https://example.com", adapter: adapter()) + |> RateLimiter.attach(name: name, cooldown: cooldown, clock: FakeClock) {:ok, _} = Req.get(request) - Process.sleep(cooldown + 10) - start = System.monotonic_time(:millisecond) + # Simulate more than cooldown passing + FakeClock.advance(cooldown + 100) + + before_second = FakeClock.now() {:ok, _} = Req.get(request) - elapsed = System.monotonic_time(:millisecond) - start - assert elapsed < 200 + # Clock should not have advanced (no sleep needed) + assert FakeClock.now() == before_second end test "emits telemetry event when throttling" do name = :"test_telemetry_#{System.unique_integer([:positive])}" - cooldown = 100 - - adapter = fn request -> - {request, Req.Response.new(status: 200, body: "ok")} - end + cooldown = 500 request = - Req.new(url: "https://example.com", adapter: adapter) - |> RateLimiter.attach(name: name, cooldown: cooldown) + Req.new(url: "https://example.com", adapter: adapter()) + |> RateLimiter.attach(name: name, cooldown: cooldown, clock: FakeClock) ref = :telemetry_test.attach_event_handlers(self(), [ @@ -113,23 +130,16 @@ defmodule Req.RateLimiterTest do {:ok, _} = Req.get(request) {:ok, _} = Req.get(request) - assert_received {[:req, :rate_limiter, :throttle], ^ref, %{sleep_ms: sleep_ms}, - %{name: ^name}} - - assert sleep_ms > 0 + assert_received {[:req, :rate_limiter, :throttle], ^ref, %{sleep_ms: 500}, %{name: ^name}} end test "does not emit telemetry event when no throttling needed" do name = :"test_no_telemetry_#{System.unique_integer([:positive])}" - cooldown = 50 - - adapter = fn request -> - {request, Req.Response.new(status: 200, body: "ok")} - end + cooldown = 500 request = - Req.new(url: "https://example.com", adapter: adapter) - |> RateLimiter.attach(name: name, cooldown: cooldown) + Req.new(url: "https://example.com", adapter: adapter()) + |> RateLimiter.attach(name: name, cooldown: cooldown, clock: FakeClock) ref = :telemetry_test.attach_event_handlers(self(), [ @@ -137,7 +147,7 @@ defmodule Req.RateLimiterTest do ]) {:ok, _} = Req.get(request) - Process.sleep(cooldown + 10) + FakeClock.advance(cooldown + 100) {:ok, _} = Req.get(request) refute_received {[:req, :rate_limiter, :throttle], ^ref, _, _} @@ -146,27 +156,23 @@ defmodule Req.RateLimiterTest do test "different API names are tracked independently" do name_a = :"test_api_a_#{System.unique_integer([:positive])}" name_b = :"test_api_b_#{System.unique_integer([:positive])}" - cooldown = 100 - - adapter = fn request -> - {request, Req.Response.new(status: 200, body: "ok")} - end + cooldown = 500 request_a = - Req.new(url: "https://example.com", adapter: adapter) - |> RateLimiter.attach(name: name_a, cooldown: cooldown) + Req.new(url: "https://example.com", adapter: adapter()) + |> RateLimiter.attach(name: name_a, cooldown: cooldown, clock: FakeClock) request_b = - Req.new(url: "https://example.com", adapter: adapter) - |> RateLimiter.attach(name: name_b, cooldown: cooldown) + Req.new(url: "https://example.com", adapter: adapter()) + |> RateLimiter.attach(name: name_b, cooldown: cooldown, clock: FakeClock) {:ok, _} = Req.get(request_a) - start = System.monotonic_time(:millisecond) + before_b = FakeClock.now() {:ok, _} = Req.get(request_b) - elapsed = System.monotonic_time(:millisecond) - start - assert elapsed < 200 + # API B has no prior request, so no sleep + assert FakeClock.now() == before_b end end end diff --git a/test/support/fake_clock.ex b/test/support/fake_clock.ex new file mode 100644 index 00000000..4229bad7 --- /dev/null +++ b/test/support/fake_clock.ex @@ -0,0 +1,34 @@ +defmodule Req.RateLimiter.FakeClock do + @moduledoc """ + Fake clock for deterministic rate limiter tests. + + Stores the current time in the process dictionary so each test process + has its own isolated clock state. Works with `async: true`. + """ + + @behaviour Req.RateLimiter.Clock + + @key :fake_clock_now + + @spec set(integer()) :: :ok + def set(now) do + Process.put(@key, now) + :ok + end + + @spec advance(non_neg_integer()) :: :ok + def advance(ms) do + Process.put(@key, now() + ms) + :ok + end + + @impl true + def now do + Process.get(@key, 0) + end + + @impl true + def sleep(ms) do + advance(ms) + end +end