From 5d22ff40e24d0b169d097a55c77b6ac9712b67c1 Mon Sep 17 00:00:00 2001 From: ruslandoga <67764432+ruslandoga@users.noreply.github.com> Date: Thu, 14 Nov 2024 18:10:56 +0700 Subject: [PATCH 01/12] update redis backend to new hammer api --- .formatter.exs | 2 +- .github/workflows/ci.yml | 45 ++--- .tool-versions | 1 - CHANGELOG.md | 6 + README.md | 56 +++---- compose.yml | 5 + config/config.exs | 15 -- docker-compose.yml | 7 - guides/Overview.md | 37 ----- lib/hammer/redis.ex | 206 +++++++++++++++++++++++ lib/hammer_backend_redis.ex | 256 ----------------------------- mix.exs | 27 ++- mix.lock | 4 +- test/hammer/redis_test.exs | 170 +++++++++++++++++++ test/hammer_backend_redis_test.exs | 158 ------------------ test/test_helper.exs | 23 ++- 16 files changed, 471 insertions(+), 547 deletions(-) delete mode 100644 .tool-versions create mode 100644 compose.yml delete mode 100644 config/config.exs delete mode 100644 docker-compose.yml delete mode 100644 guides/Overview.md create mode 100644 lib/hammer/redis.ex delete mode 100644 lib/hammer_backend_redis.ex create mode 100644 test/hammer/redis_test.exs delete mode 100644 test/hammer_backend_redis_test.exs diff --git a/.formatter.exs b/.formatter.exs index d2cda26..7f59b1f 100644 --- a/.formatter.exs +++ b/.formatter.exs @@ -1,4 +1,4 @@ # Used by "mix format" [ - inputs: ["{mix,.formatter}.exs", "{config,lib,test}/**/*.{ex,exs}"] + inputs: ["{mix,.formatter}.exs", "{lib,test}/**/*.{ex,exs}"] ] diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ea7e96f..8864bf8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,12 +9,11 @@ on: - master jobs: - setup: + test: runs-on: ubuntu-latest env: MIX_ENV: test - # The hostname used to communicate with the Redis service container - REDIS_HOST: localhost + services: redis: # Docker Hub image @@ -29,41 +28,45 @@ jobs: --health-retries 5 strategy: - fail-fast: false + # https://hexdocs.pm/elixir/compatibility-and-deprecations.html#between-elixir-and-erlang-otp matrix: - os: [ubuntu-22.04, ubuntu-20.04] - elixir_version: [1.12.3, 1.13.3, 1.14.1] - otp_version: [24, 25] - exclude: - - otp_version: 25 - elixir_version: 1.12.3 + elixir: [1.15, 1.16, 1.17] + otp: [25, 26] + include: + - elixir: 1.17 + otp: 27 + steps: - uses: actions/checkout@v4 - uses: erlef/setup-beam@v1 with: - otp-version: ${{matrix.otp_version}} - elixir-version: ${{matrix.elixir_version}} + otp-version: ${{matrix.otp}} + elixir-version: ${{matrix.elixir}} - uses: actions/cache@v4 with: path: | deps _build - key: deps-${{ runner.os }}-${{ matrix.otp_version }}-${{ matrix.elixir_version }}-${{ hashFiles('**/mix.lock') }} + key: test-otp-${{ matrix.otp }}-elixir-${{ matrix.elixir }}-ref-${{ github.head_ref || github.ref }}-mix-${{ hashFiles('**/mix.lock') }} restore-keys: | - deps-${{ runner.os }}-${{ matrix.otp_version }}-${{ matrix.elixir_version }} + test-otp-${{ matrix.otp }}-elixir-${{ matrix.elixir }}-ref-${{ github.head_ref || github.ref }}-mix- + test-otp-${{ matrix.otp }}-elixir-${{ matrix.elixir }}-ref-refs/heads/master-mix- - run: mix deps.get - - - run: mix format --check-formatted - - run: mix deps.unlock --check-unused - - run: mix deps.compile - - run: mix compile --warnings-as-errors - - run: mix credo --strict --format=oneline + - run: mix test --warnings-as-errors --cover --include redis --include slow - - run: mix test --warnings-as-errors --cover + format: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: erlef/setup-beam@v1 + with: + elixir-version: 1 + otp-version: 27 + - run: mix format --check-formatted diff --git a/.tool-versions b/.tool-versions deleted file mode 100644 index b6551eb..0000000 --- a/.tool-versions +++ /dev/null @@ -1 +0,0 @@ -elixir 1.13.1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e2bb4e..b191964 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## 7.0.0-rc.0 (2024-12-06) + +### Changed + +- Conform to new Hammer API + ## 6.2.0 (2024-12-04) ### Changed diff --git a/README.md b/README.md index ec29d6a..343098e 100644 --- a/README.md +++ b/README.md @@ -1,11 +1,18 @@ -# HammerBackendRedis +# Hammer.Redis -[![Build Status](https://github.com/ExHammer/hammer-backend-redis/actions/workflows/ci.yml/badge.svg)](https://github.com/ExHammer/hammer-backend-redis/actions/workflows/ci.yml) [![Hex.pm](https://img.shields.io/hexpm/v/hammer_backend_redis.svg)](https://hex.pm/packages/hammer_backend_redis) [![Documentation](https://img.shields.io/badge/documentation-gray)](https://hexdocs.pm/hammer_backend_redis) +[![Build Status](https://github.com/ExHammer/hammer-backend-redis/actions/workflows/ci.yml/badge.svg)](https://github.com/ExHammer/hammer-backend-redis/actions/workflows/ci.yml) +[![Hex.pm](https://img.shields.io/hexpm/v/hammer_backend_redis.svg)](https://hex.pm/packages/hammer_backend_redis) +[![Documentation](https://img.shields.io/badge/documentation-gray)](https://hexdocs.pm/hammer_backend_redis) [![Total Download](https://img.shields.io/hexpm/dt/hammer_backend_redis.svg)](https://hex.pm/packages/hammer_backend_redis) [![License](https://img.shields.io/hexpm/l/hammer_backend_redis.svg)](https://github.com/ExHammer/hammer-backend-redis/blob/master/LICENSE.md) A Redis backend for the [Hammer](https://github.com/ExHammer/hammer) rate-limiter. +This backend uses the [Redix](https://hex.pm/packages/redix) library to connect to Redis. And [poolboy](https://hex.pm/packages/poolboy) to pool the connections. + +The algorithm we are using is the first method described (called "bucketing") in [Rate Limiting with Redis](https://youtu.be/CRGPbCbRTHA?t=753). +In other sources it's sometimes called a "fixed window counter". + ## Installation Hammer-backend-redis @@ -14,52 +21,37 @@ can be installed by adding `hammer_backend_redis` to your list of dependencies i ```elixir def deps do - [{:hammer_backend_redis, "~> 6.1"}, - {:hammer, "~> 6.0"}] + [ + {:hammer_backend_redis, "~> 7.0"} + ] end ``` ## Usage -Configure the `:hammer` application to use the Redis backend: +Define the rate limiter: ```elixir -config :hammer, - backend: {Hammer.Backend.Redis, [delete_buckets_timeout: 10_0000, - key_prefix: "my_application:rate_limiter", - expiry_ms: 60_000 * 60 * 2, - redix_config: [host: "localhost", - port: 6379]]} +defmodule MyApp.RateLimit do + use Hammer, backend: Hammer.Redis +end ``` -(the `redix_config` arg is a keyword-list which is passed to -[Redix](https://hex.pm/packages/redix), it's also aliased to `redis_config`, -with an `s`) - -Another option to configure Redis is to use the Redis Url format (see https://hexdocs.pm/redix/Redix.html#start_link/1-using-a-redis-uri) to configure Redis. If both options are specified -the redis_url will be used first. +And add it to your app's supervision tree: ```elixir -config :hammer, - backend: {Hammer.Backend.Redis, [delete_buckets_timeout: 10_0000, - key_prefix: "my_application:rate_limiter", - expiry_ms: 60_000 * 60 * 2, - redis_url: "redis://HOST:PORT"]} +children = [ + {MyApp.RateLimit, url: "redis://localhost:6379"} +] ``` -And that's it, calls to `Hammer.check_rate/3` and so on will use Redis to store -the rate-limit counters. - -See the [Hammer Tutorial](https://hexdocs.pm/hammer/tutorial.html) for more. - -## Documentation - -On hexdocs: [https://hexdocs.pm/hammer_backend_redis/](https://hexdocs.pm/hammer_backend_redis/) +And that's it, calls to `MyApp.RateLimit.hit/3` and so on will use Redis to store +the rate-limit counters. See the [documentation](https://hexdocs.pm/hammer_backend_redis/Hammer.Redis.html) for more details. ## Run tests locally -You need a running Redis instance. One can be started locally using `docker-compose up -d`. -See the docker-compose.yml for more details on. +You need a running Redis instance. One can be started locally using `docker compose up -d redis`. +See the [compose.yml](./compose.yml) for more details. ## Getting Help diff --git a/compose.yml b/compose.yml new file mode 100644 index 0000000..d21f08e --- /dev/null +++ b/compose.yml @@ -0,0 +1,5 @@ +services: + redis: + image: redis:latest + ports: + - 6379:6379 diff --git a/config/config.exs b/config/config.exs deleted file mode 100644 index d971332..0000000 --- a/config/config.exs +++ /dev/null @@ -1,15 +0,0 @@ -# This file is responsible for configuring your application -# and its dependencies with the aid of the Mix.Config module. -import Config - -config :hammer, - backend: - {Hammer.Backend.Redis, - [ - expiry_ms: 60_000 * 60 * 2, - delete_buckets_timeout: 5000, - redix_config: [ - host: System.get_env("REDIS_HOST", "localhost"), - port: "REDIS_PORT" |> System.get_env("6379") |> String.to_integer() - ] - ]} diff --git a/docker-compose.yml b/docker-compose.yml deleted file mode 100644 index 453d7ce..0000000 --- a/docker-compose.yml +++ /dev/null @@ -1,7 +0,0 @@ -version: '3' - -services: - redis-server: - image: redis:6.2 - ports: - - "6379" \ No newline at end of file diff --git a/guides/Overview.md b/guides/Overview.md deleted file mode 100644 index 411a9bc..0000000 --- a/guides/Overview.md +++ /dev/null @@ -1,37 +0,0 @@ -# Overview - -A Redis backend for the Hammer rate-limiter - -[Hammer](https://github.com/ExHammer/hammer) is a rate-limiter for -the [Elixir](https://elixir-lang.org/) language. It's killer feature is a -pluggable backend system, allowing you to use whichever storage suits your -needs. - -This package provides a Redis backend for Hammer, using -the [Redix](https://github.com/whatyouhide/redix) library to connect to the -Redis server. - -To get started, read -the [Hammer Tutorial](https://hexdocs.pm/hammer/tutorial.html) first, then add -the `hammer_backend_redis` dependency: - -```elixir - def deps do - [{:hammer_backend_redis, "~> 6.1"}, - {:hammer, "~> 6.0"}] - end -``` - -... then configure the `:hammer` application to use the Redis backend: - -```elixir -config :hammer, - backend: {Hammer.Backend.Redis, [expiry_ms: 60_000 * 60 * 2, - redix_config: [host: "some.host"]]} -``` - -(the `redix_config` arg is a keyword-list which is passed to Redix, it's also -aliased to `redis_config`, with an `s`) - -And that's it, calls to `Hammer.check_rate/3` and so on will use Redis to store -the rate-limit counters. diff --git a/lib/hammer/redis.ex b/lib/hammer/redis.ex new file mode 100644 index 0000000..8557705 --- /dev/null +++ b/lib/hammer/redis.ex @@ -0,0 +1,206 @@ +defmodule Hammer.Redis do + @moduledoc """ + This backend uses the [Redix](https://hex.pm/packages/redix) library to connect to Redis. + And [poolboy](https://hex.pm/packages/poolboy) to manage the connections. + + defmodule MyApp.RateLimit do + # the default prefix is "MyApp.RateLimit:" + # the default timeout is :infinity + use Hammer, backend: Hammer.Redis, prefix: "MyApp.RateLimit:", timeout: :infinity + end + + redix_opts = [url: "redis://localhost:6379"] + poolboy_opts = [size: 10, max_overflow: 2] + MyApp.RateLimit.start_link(redix_opts ++ poolboy_opts) + + # increment and timeout arguments are optional + # by default increment is 1 and timeout is as defined in the module + {:allow, _count} = MyApp.RateLimit.hit(key, scale, limit) + {:allow, _count} = MyApp.RateLimit.hit(key, scale, limit, _increment = 1, _timeout = :infinity) + + """ + + defmacro __before_compile__(%{module: module}) do + hammer_opts = Module.get_attribute(module, :hammer_opts) + + prefix = String.trim_leading(Atom.to_string(module), "Elixir.") + prefix = Keyword.get(hammer_opts, :prefix, prefix) + timeout = Keyword.get(hammer_opts, :timeout, :infinity) + + unless is_binary(prefix) do + raise ArgumentError, """ + Expected `:prefix` value to be a string, got: #{inspect(prefix)} + """ + end + + case timeout do + :infinity -> + :ok + + _ when is_integer(timeout) and timeout > 0 -> + :ok + + _ -> + raise ArgumentError, """ + Expected `:timeout` value to be a positive integer or `:infinity`, got: #{inspect(timeout)} + """ + end + + quote do + @pool unquote(module) + @prefix unquote(prefix) + @timeout unquote(timeout) + + def child_spec(opts) do + %{ + id: @pool, + start: {__MODULE__, :start_link, [opts]}, + type: :worker + } + end + + def start_link(opts) do + opts = Keyword.put(opts, :name, @pool) + Hammer.Redis.start_link(opts) + end + + def hit(key, scale, limit, increment \\ 1, timeout \\ @timeout) do + Hammer.Redis.hit(@pool, @prefix, key, scale, limit, increment, timeout) + end + + def inc(key, scale, increment \\ 1, timeout \\ @timeout) do + Hammer.Redis.inc(@pool, @prefix, key, scale, increment, timeout) + end + + def set(key, scale, count, timeout \\ @timeout) do + Hammer.Redis.set(@pool, @prefix, key, scale, count, timeout) + end + + def get(key, scale, timeout \\ @timeout) do + Hammer.Redis.get(@pool, @prefix, key, scale, timeout) + end + end + end + + @doc false + def start_link(opts) do + {name, opts} = Keyword.pop!(opts, :name) + {pool_opts, redix_opts} = Keyword.split(opts, [:size, :max_overflow]) + + pool_args = [ + name: {:local, name}, + worker_module: Redix, + size: Keyword.get(pool_opts, :size, 5), + max_overflow: Keyword.get(opts, :max_overflow, 2) + ] + + {url, redix_opts} = Keyword.pop(redix_opts, :url) + + redix_opts = + if url do + url_opts = Redix.URI.to_start_options(url) + Keyword.merge(url_opts, redix_opts) + else + redix_opts + end + + :poolboy.start_link(pool_args, redix_opts) + end + + @doc false + def hit(pool, prefix, key, scale, limit, increment, timeout) do + now = now() + window = div(now, scale) + full_key = redis_key(prefix, key, window) + expires_at = (window + 1) * scale + + commands = [ + ["MULTI"], + ["INCRBY", full_key, increment], + ["EXPIREAT", full_key, div(expires_at, 1000), "NX"], + ["EXEC"] + ] + + ["OK", "QUEUED", "QUEUED", [count, _]] = + :poolboy.transaction(pool, fn conn -> Redix.pipeline!(conn, commands) end, timeout) + + if count <= limit do + {:allow, count} + else + {:deny, expires_at - now} + end + end + + @doc false + def inc(pool, prefix, key, scale, increment, timeout) do + now = now() + window = div(now, scale) + full_key = redis_key(prefix, key, window) + expires_at = (window + 1) * scale + + commands = [ + ["MULTI"], + ["INCRBY", full_key, increment], + ["EXPIREAT", full_key, div(expires_at, 1000), "NX"], + ["EXEC"] + ] + + ["OK", "QUEUED", "QUEUED", [count, _]] = + :poolboy.transaction(pool, fn conn -> Redix.pipeline!(conn, commands) end, timeout) + + count + end + + @doc false + def set(pool, prefix, key, scale, count, timeout) do + now = now() + window = div(now, scale) + full_key = redis_key(prefix, key, window) + expires_at = (window + 1) * scale + + commands = [ + ["MULTI"], + ["SET", full_key, count], + ["EXPIREAT", full_key, div(expires_at, 1000), "NX"], + ["EXEC"] + ] + + ["OK", "QUEUED", "QUEUED", [_, _]] = + :poolboy.transaction( + pool, + fn conn -> Redix.pipeline!(conn, commands) end, + timeout + ) + + count + end + + @doc false + def get(pool, prefix, key, scale, timeout) do + now = now() + window = div(now, scale) + full_key = redis_key(prefix, key, window) + + count = + :poolboy.transaction( + pool, + fn conn -> Redix.command!(conn, ["GET", full_key]) end, + timeout + ) + + case count do + nil -> 0 + count -> String.to_integer(count) + end + end + + @compile inline: [redis_key: 3] + defp redis_key(prefix, key, window) do + "#{prefix}:#{key}:#{window}" + end + + @compile inline: [now: 0] + defp now do + System.system_time(:millisecond) + end +end diff --git a/lib/hammer_backend_redis.ex b/lib/hammer_backend_redis.ex deleted file mode 100644 index 2198423..0000000 --- a/lib/hammer_backend_redis.ex +++ /dev/null @@ -1,256 +0,0 @@ -defmodule Hammer.Backend.Redis do - @moduledoc """ - Documentation for Hammer.Backend.Redis - - This backend uses the [Redix](https://hex.pm/packages/redix) library to connect to Redis. - - The backend process is started by calling `start_link`: - - Hammer.Backend.Redis.start_link( - expiry_ms: 60_000 * 10, - redix_config: [host: "example.com", port: 5050] - ) - - Options are: - - - `expiry_ms`: Expiry time of buckets in milliseconds, - used to set TTL on Redis keys. This configuration is mandatory. - - `redix_config`: Keyword list of options to the `Redix` redis client, - also aliased to `redis_config` - - `key_prefix`: The prefix to use for all the redis keys (defaults to "Hammer:Redis:") - - `redis_url`: String url of redis server to connect to - (optional, invokes Redix.start_link/2) - """ - - @behaviour Hammer.Backend - - use GenServer - - @type bucket_key :: {bucket :: integer, id :: String.t()} - @type bucket_info :: - {key :: bucket_key, count :: integer, created :: integer, updated :: integer} - ## Public API - - @spec start :: :ignore | {:error, any} | {:ok, pid} - def start do - start([]) - end - - @spec start(keyword()) :: :ignore | {:error, any} | {:ok, pid} - def start(args) do - GenServer.start(__MODULE__, args) - end - - @spec start_link :: :ignore | {:error, any} | {:ok, pid} - def start_link do - start_link([]) - end - - @spec start_link(keyword()) :: :ignore | {:error, any} | {:ok, pid} - def start_link(args) do - GenServer.start_link(__MODULE__, args) - end - - @spec stop :: any - def stop do - GenServer.call(__MODULE__, :stop) - end - - @doc """ - Record a hit in the bucket identified by `key` - """ - @impl Hammer.Backend - def count_hit(pid, key, now) do - GenServer.call(pid, {:count_hit, key, now, 1}) - end - - @doc """ - Record a hit in the bucket identified by `key`, with a custom increment - """ - @impl Hammer.Backend - def count_hit(pid, key, now, increment) do - GenServer.call(pid, {:count_hit, key, now, increment}) - end - - @doc """ - Retrieve information about the bucket identified by `key` - """ - @impl Hammer.Backend - def get_bucket(pid, key) do - GenServer.call(pid, {:get_bucket, key}) - end - - @doc """ - Delete all buckets associated with `id`. - """ - @impl Hammer.Backend - def delete_buckets(pid, id) do - delete_buckets_timeout = GenServer.call(pid, {:get_delete_buckets_timeout}) - GenServer.call(pid, {:delete_buckets, id}, delete_buckets_timeout) - end - - ## GenServer Callbacks - - @impl GenServer - def init(args) do - expiry_ms = Keyword.get(args, :expiry_ms) - - if !expiry_ms do - raise RuntimeError, "Missing required config: expiry_ms" - end - - key_prefix = Keyword.get(args, :key_prefix, "Hammer:Redis:") - - redix_config = - Keyword.get( - args, - :redix_config, - Keyword.get(args, :redis_config, []) - ) - - redis_url = Keyword.get(args, :redis_url, nil) - - {:ok, redix} = - if is_binary(redis_url) && byte_size(redis_url) > 0 do - Redix.start_link(redis_url, redix_config) - else - Redix.start_link(redix_config) - end - - delete_buckets_timeout = Keyword.get(args, :delete_buckets_timeout, 5000) - - {:ok, - %{ - redix: redix, - expiry_ms: expiry_ms, - delete_buckets_timeout: delete_buckets_timeout, - key_prefix: key_prefix - }} - end - - @impl GenServer - def handle_call(:stop, _from, state) do - {:stop, :normal, :ok, state} - end - - def handle_call({:count_hit, key, now, increment}, _from, state) do - expiry = get_expiry(state) - - result = do_count_hit(state, key, now, increment, expiry) - {:reply, result, state} - end - - def handle_call({:get_bucket, key}, _from, %{redix: r} = state) do - redis_key = make_redis_key(state, key) - command = ["HMGET", redis_key, "count", "created", "updated"] - - result = - case Redix.command(r, command) do - {:ok, [nil, nil, nil]} -> - {:ok, nil} - - {:ok, [count, created, updated]} -> - count = String.to_integer(count) - created = String.to_integer(created) - updated = String.to_integer(updated) - {:ok, {key, count, created, updated}} - - {:error, reason} -> - {:error, reason} - end - - {:reply, result, state} - end - - def handle_call({:delete_buckets, id}, _from, %{redix: r} = state) do - redis_key_pattern = make_redis_key_pattern(state, id) - result = do_delete_buckets(r, redis_key_pattern, 0, 0) - - {:reply, result, state} - end - - def handle_call( - {:get_delete_buckets_timeout}, - _from, - %{delete_buckets_timeout: delete_buckets_timeout} = state - ) do - {:reply, delete_buckets_timeout, state} - end - - defp do_delete_buckets(r, redis_key_pattern, cursor, count_deleted) do - case Redix.command(r, ["SCAN", cursor, "MATCH", redis_key_pattern]) do - {:ok, ["0", []]} -> - {:ok, count_deleted} - - {:ok, [next_cursor, []]} -> - do_delete_buckets(r, redis_key_pattern, next_cursor, count_deleted) - - {:ok, ["0", keys]} -> - {:ok, deleted} = Redix.command(r, ["DEL" | keys]) - {:ok, deleted + count_deleted} - - {:ok, [next_cursor, keys]} -> - {:ok, deleted} = Redix.command(r, ["DEL" | keys]) - do_delete_buckets(r, redis_key_pattern, next_cursor, count_deleted + deleted) - - {:error, reason} -> - {:error, reason} - end - end - - # we are using the first method described (called bucketing) - # in https://www.youtube.com/watch?v=CRGPbCbRTHA - # but we add the 'created' and 'updated' meta information fields. - defp do_count_hit(%{redix: r} = state, key, now, increment, expiry) do - redis_key = make_redis_key(state, key) - - cmds = [ - ["MULTI"], - [ - "HINCRBY", - redis_key, - "count", - increment - ], - [ - "HSETNX", - redis_key, - "created", - now - ], - [ - "HSET", - redis_key, - "updated", - now - ], - [ - "EXPIRE", - redis_key, - expiry - ], - ["EXEC"] - ] - - case Redix.pipeline(r, cmds) do - {:ok, ["OK", "QUEUED", "QUEUED", "QUEUED", "QUEUED", [new_count, _, _, 1]]} -> - {:ok, new_count} - - {:error, reason} -> - {:error, reason} - end - end - - defp make_redis_key(%{key_prefix: key_prefix}, {bucket, id}) do - "#{key_prefix}#{id}:#{bucket}" - end - - defp make_redis_key_pattern(%{key_prefix: key_prefix}, id) do - "#{key_prefix}#{id}:*" - end - - defp get_expiry(state) do - %{expiry_ms: expiry_ms} = state - round(expiry_ms / 1000 + 1) - end -end diff --git a/mix.exs b/mix.exs index fc7fb76..afb31ec 100644 --- a/mix.exs +++ b/mix.exs @@ -1,7 +1,7 @@ -defmodule HammerBackendRedis.Mixfile do +defmodule Hammer.Redis.MixProject do use Mix.Project - @version "6.2.0" + @version "7.0.0-rc.0" def project do [ @@ -9,14 +9,12 @@ defmodule HammerBackendRedis.Mixfile do description: "Redis backend for Hammer rate-limiter", source_url: "https://github.com/ExHammer/hammer-backend-redis", homepage_url: "https://github.com/ExHammer/hammer-backend-redis", - version: "#{@version}", - elixir: "~> 1.13", - build_embedded: Mix.env() == :prod, - start_permanent: Mix.env() == :prod, + version: @version, + elixir: "~> 1.15", deps: deps(), docs: docs(), package: package(), - test_coverage: [summary: [threshold: 75]] + test_coverage: [summary: [threshold: 90]] ] end @@ -26,11 +24,10 @@ defmodule HammerBackendRedis.Mixfile do def docs do [ - main: "overview", - extras: ["guides/Overview.md", "CHANGELOG.md"], - source_ref: "v#{@version}", - main: "overview", - formatters: ["html", "epub"] + main: "readme", + extras: ["README.md", "CHANGELOG.md"], + skip_undefined_reference_warnings_on: ["README.md", "CHANGELOG.md"], + source_ref: "v#{@version}" ] end @@ -39,9 +36,9 @@ defmodule HammerBackendRedis.Mixfile do {:credo, "~> 1.6", only: [:dev, :test]}, {:dialyxir, "~> 1.1", only: [:dev], runtime: false}, {:ex_doc, "~> 0.28", only: :dev}, - {:hammer, "~> 6.0"}, - {:mock, "~> 0.3.7", only: :test}, - {:redix, "~> 1.1"} + {:hammer, github: "ruslandoga/hammer", branch: "just-use"}, + {:redix, "~> 1.1"}, + {:poolboy, "~> 1.5"} ] end diff --git a/mix.lock b/mix.lock index d339681..493cf1b 100644 --- a/mix.lock +++ b/mix.lock @@ -6,13 +6,11 @@ "erlex": {:hex, :erlex, "0.2.7", "810e8725f96ab74d17aac676e748627a07bc87eb950d2b83acd29dc047a30595", [:mix], [], "hexpm", "3ed95f79d1a844c3f6bf0cea61e0d5612a42ce56da9c03f01df538685365efb0"}, "ex_doc": {:hex, :ex_doc, "0.35.1", "de804c590d3df2d9d5b8aec77d758b00c814b356119b3d4455e4b8a8687aecaf", [:mix], [{:earmark_parser, "~> 1.4.39", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "2121c6402c8d44b05622677b761371a759143b958c6c19f6558ff64d0aed40df"}, "file_system": {:hex, :file_system, "1.0.1", "79e8ceaddb0416f8b8cd02a0127bdbababe7bf4a23d2a395b983c1f8b3f73edd", [:mix], [], "hexpm", "4414d1f38863ddf9120720cd976fce5bdde8e91d8283353f0e31850fa89feb9e"}, - "hammer": {:hex, :hammer, "6.1.0", "f263e3c3e9946bd410ea0336b2abe0cb6260af4afb3a221e1027540706e76c55", [:make, :mix], [{:poolboy, "~> 1.5", [hex: :poolboy, repo: "hexpm", optional: false]}], "hexpm", "b47e415a562a6d072392deabcd58090d8a41182cf9044cdd6b0d0faaaf68ba57"}, + "hammer": {:git, "https://github.com/ruslandoga/hammer.git", "3565989023a530efc4e36272fead5d702345092f", [branch: "just-use"]}, "jason": {:hex, :jason, "1.4.4", "b9226785a9aa77b6857ca22832cffa5d5011a667207eb2a0ad56adb5db443b8a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "c5eb0cab91f094599f94d55bc63409236a8ec69a21a67814529e8d5f6cc90b3b"}, "makeup": {:hex, :makeup, "1.2.1", "e90ac1c65589ef354378def3ba19d401e739ee7ee06fb47f94c687016e3713d1", [:mix], [{:nimble_parsec, "~> 1.4", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "d36484867b0bae0fea568d10131197a4c2e47056a6fbe84922bf6ba71c8d17ce"}, "makeup_elixir": {:hex, :makeup_elixir, "1.0.0", "74bb8348c9b3a51d5c589bf5aebb0466a84b33274150e3b6ece1da45584afc82", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "49159b7d7d999e836bedaf09dcf35ca18b312230cf901b725a64f3f42e407983"}, "makeup_erlang": {:hex, :makeup_erlang, "1.0.1", "c7f58c120b2b5aa5fd80d540a89fdf866ed42f1f3994e4fe189abebeab610839", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "8a89a1eeccc2d798d6ea15496a6e4870b75e014d1af514b1b71fa33134f57814"}, - "meck": {:hex, :meck, "0.9.2", "85ccbab053f1db86c7ca240e9fc718170ee5bda03810a6292b5306bf31bae5f5", [:rebar3], [], "hexpm", "81344f561357dc40a8344afa53767c32669153355b626ea9fcbc8da6b3045826"}, - "mock": {:hex, :mock, "0.3.8", "7046a306b71db2488ef54395eeb74df0a7f335a7caca4a3d3875d1fc81c884dd", [:mix], [{:meck, "~> 0.9.2", [hex: :meck, repo: "hexpm", optional: false]}], "hexpm", "7fa82364c97617d79bb7d15571193fc0c4fe5afd0c932cef09426b3ee6fe2022"}, "nimble_options": {:hex, :nimble_options, "1.1.1", "e3a492d54d85fc3fd7c5baf411d9d2852922f66e69476317787a7b2bb000a61b", [:mix], [], "hexpm", "821b2470ca9442c4b6984882fe9bb0389371b8ddec4d45a9504f00a66f650b44"}, "nimble_parsec": {:hex, :nimble_parsec, "1.4.0", "51f9b613ea62cfa97b25ccc2c1b4216e81df970acd8e16e8d1bdc58fef21370d", [:mix], [], "hexpm", "9c565862810fb383e9838c1dd2d7d2c437b3d13b267414ba6af33e50d2d1cf28"}, "poolboy": {:hex, :poolboy, "1.5.2", "392b007a1693a64540cead79830443abf5762f5d30cf50bc95cb2c1aaafa006b", [:rebar3], [], "hexpm", "dad79704ce5440f3d5a3681c8590b9dc25d1a561e8f5a9c995281012860901e3"}, diff --git a/test/hammer/redis_test.exs b/test/hammer/redis_test.exs new file mode 100644 index 0000000..0a98ddc --- /dev/null +++ b/test/hammer/redis_test.exs @@ -0,0 +1,170 @@ +defmodule Hammer.RedisTest do + use ExUnit.Case, async: true + + @moduletag :redis + + defmodule RateLimit do + use Hammer, backend: Hammer.Redis + end + + setup do + start_supervised!({RateLimit, url: "redis://localhost:6379"}) + "OK" = :poolboy.transaction(RateLimit, &Redix.command!(&1, ["FLUSHALL"])) + :ok + end + + defp redis_all(pool \\ RateLimit) do + :poolboy.transaction(pool, fn conn -> + keys = Redix.command!(conn, ["KEYS", "*"]) + + Enum.map(keys, fn key -> + {key, Redix.command!(conn, ["GET", key])} + end) + end) + end + + test "key prefix is set to the module name by default" do + key = "key" + scale = :timer.seconds(10) + limit = 5 + + RateLimit.hit(key, scale, limit) + + assert [{"Hammer.RedisTest.RateLimit:" <> _, "1"}] = redis_all() + end + + test "key has expirytime set" do + key = "key" + scale = :timer.seconds(10) + limit = 5 + + RateLimit.hit(key, scale, limit) + [{redis_key, "1"}] = redis_all() + + expected_expiretime = div(System.system_time(:second), 10) * 10 + 10 + + assert :poolboy.transaction(RateLimit, &Redix.command!(&1, ["EXPIRETIME", redis_key])) == + expected_expiretime + end + + describe "hit" do + test "returns {:allow, 1} tuple on first access" do + key = "key" + scale = :timer.seconds(10) + limit = 10 + + assert {:allow, 1} = RateLimit.hit(key, scale, limit) + end + + test "returns {:allow, 4} tuple on in-limit checks" do + key = "key" + scale = :timer.minutes(10) + limit = 10 + + assert {:allow, 1} = RateLimit.hit(key, scale, limit) + assert {:allow, 2} = RateLimit.hit(key, scale, limit) + assert {:allow, 3} = RateLimit.hit(key, scale, limit) + assert {:allow, 4} = RateLimit.hit(key, scale, limit) + end + + test "returns expected tuples on mix of in-limit and out-of-limit checks" do + key = "key" + scale = :timer.minutes(10) + limit = 2 + + assert {:allow, 1} = RateLimit.hit(key, scale, limit) + assert {:allow, 2} = RateLimit.hit(key, scale, limit) + assert {:deny, _wait} = RateLimit.hit(key, scale, limit) + assert {:deny, _wait} = RateLimit.hit(key, scale, limit) + end + + @tag :slow + test "returns expected tuples after waiting for the next window" do + key = "key" + scale = :timer.seconds(1) + limit = 2 + + assert {:allow, 1} = RateLimit.hit(key, scale, limit) + assert {:allow, 2} = RateLimit.hit(key, scale, limit) + assert {:deny, wait} = RateLimit.hit(key, scale, limit) + + :timer.sleep(wait) + + assert {:allow, 1} = RateLimit.hit(key, scale, limit) + assert {:allow, 2} = RateLimit.hit(key, scale, limit) + assert {:deny, _wait} = RateLimit.hit(key, scale, limit) + end + + test "with custom increment" do + key = "cost-key" + scale = :timer.seconds(1) + limit = 10 + + assert {:allow, 4} = RateLimit.hit(key, scale, limit, 4) + assert {:allow, 9} = RateLimit.hit(key, scale, limit, 5) + assert {:deny, _wait} = RateLimit.hit(key, scale, limit, 3) + end + + test "mixing default and custom increment" do + key = "cost-key" + scale = :timer.seconds(1) + limit = 10 + + assert {:allow, 3} = RateLimit.hit(key, scale, limit, 3) + assert {:allow, 4} = RateLimit.hit(key, scale, limit) + assert {:allow, 5} = RateLimit.hit(key, scale, limit) + assert {:allow, 9} = RateLimit.hit(key, scale, limit, 4) + assert {:allow, 10} = RateLimit.hit(key, scale, limit) + assert {:deny, _wait} = RateLimit.hit(key, scale, limit, 2) + end + end + + describe "inc" do + test "increments the count for the given key and scale" do + key = "key" + scale = :timer.seconds(10) + + assert RateLimit.get(key, scale) == 0 + + assert RateLimit.inc(key, scale) == 1 + assert RateLimit.get(key, scale) == 1 + + assert RateLimit.inc(key, scale) == 2 + assert RateLimit.get(key, scale) == 2 + + assert RateLimit.inc(key, scale) == 3 + assert RateLimit.get(key, scale) == 3 + + assert RateLimit.inc(key, scale) == 4 + assert RateLimit.get(key, scale) == 4 + end + end + + describe "get/set" do + test "get returns the count set for the given key and scale" do + key = "key" + scale = :timer.seconds(10) + count = 10 + + assert RateLimit.get(key, scale) == 0 + assert RateLimit.set(key, scale, count) == count + assert RateLimit.get(key, scale) == count + end + end + + describe "reset" do + test "resets the count for the given key and scale" do + key = "key" + scale = :timer.seconds(10) + count = 10 + + assert RateLimit.get(key, scale) == 0 + + assert RateLimit.set(key, scale, count) == count + assert RateLimit.get(key, scale) == count + + assert RateLimit.reset(key, scale) == 0 + assert RateLimit.get(key, scale) == 0 + end + end +end diff --git a/test/hammer_backend_redis_test.exs b/test/hammer_backend_redis_test.exs deleted file mode 100644 index 8ffa0be..0000000 --- a/test/hammer_backend_redis_test.exs +++ /dev/null @@ -1,158 +0,0 @@ -defmodule HammerBackendRedisTest do - use ExUnit.Case - - alias Hammer.Backend - - setup do - {Backend.Redis, config} = Application.get_env(:hammer, :backend) - {:ok, pid} = Backend.Redis.start_link(config) - %{redix: redix} = :sys.get_state(pid) - - assert {:ok, "OK"} = Redix.command(redix, ["FLUSHALL"]) - {:ok, [pid: pid, redix: redix, key_prefix: Keyword.get(config, :key_prefix, "Hammer:Redis:")]} - end - - test "count_hit, insert", %{pid: pid, redix: redix, key_prefix: key_prefix} do - bucket = 1 - id = "one" - bucket_key = {bucket, id} - now = 123 - now_str = Integer.to_string(now) - - assert {:ok, 0} = Redix.command(redix, ["EXISTS", make_redis_key(key_prefix, bucket_key)]) - assert {:ok, 1} == Backend.Redis.count_hit(pid, bucket_key, now) - assert {:ok, 1} = Redix.command(redix, ["EXISTS", make_redis_key(key_prefix, bucket_key)]) - - assert {:ok, ["count", "1", "created", ^now_str, "updated", ^now_str]} = - Redix.command(redix, ["HGETALL", make_redis_key(key_prefix, bucket_key)]) - end - - test "count_hit, insert, with custom increment", %{ - pid: pid, - redix: redix, - key_prefix: key_prefix - } do - bucket = 1 - id = "one" - bucket_key = {bucket, id} - now = 123 - now_str = Integer.to_string(now) - inc = Enum.random(1..100) - inc_str = Integer.to_string(inc) - - assert {:ok, inc} == Backend.Redis.count_hit(pid, bucket_key, now, inc) - assert {:ok, 1} = Redix.command(redix, ["EXISTS", make_redis_key(key_prefix, bucket_key)]) - - assert {:ok, ["count", ^inc_str, "created", ^now_str, "updated", ^now_str]} = - Redix.command(redix, ["HGETALL", make_redis_key(key_prefix, bucket_key)]) - end - - test "count_hit, update", %{pid: pid, redix: redix, key_prefix: key_prefix} do - # 1. set-up - bucket = 1 - id = "one" - bucket_key = {bucket, id} - now_before = 123 - now_before_str = Integer.to_string(now_before) - now_after = 456 - now_after_str = Integer.to_string(now_after) - - assert {:ok, 1} == Backend.Redis.count_hit(pid, bucket_key, now_before) - assert {:ok, 1} = Redix.command(redix, ["EXISTS", make_redis_key(key_prefix, bucket_key)]) - - # 2. function call under test: count == 2 - assert {:ok, 2} == Backend.Redis.count_hit(pid, bucket_key, now_after) - assert {:ok, 1} = Redix.command(redix, ["EXISTS", make_redis_key(key_prefix, bucket_key)]) - - assert {:ok, ["count", "2", "created", ^now_before_str, "updated", ^now_after_str]} = - Redix.command(redix, ["HGETALL", make_redis_key(key_prefix, bucket_key)]) - end - - test "get_bucket", %{pid: pid} do - # 1. set-up - bucket = 1 - id = "one" - bucket_key = {bucket, id} - - now_before = 123 - inc_before = Enum.random(1..100) - - now_after = 456 - inc_after = Enum.random(1..100) - - inc_total = inc_before + inc_after - - assert {:ok, ^inc_before} = Backend.Redis.count_hit(pid, bucket_key, now_before, inc_before) - - assert {:ok, ^inc_total} = Backend.Redis.count_hit(pid, bucket_key, now_after, inc_after) - - # 2. function call under test - assert {:ok, {^bucket_key, ^inc_total, ^now_before, ^now_after}} = - Backend.Redis.get_bucket(pid, bucket_key) - end - - test "delete buckets", %{pid: pid, redix: redix} do - bucket = 1 - id = "one" - bucket_key = {bucket, id} - now = 123 - - {:ok, _} = Backend.Redis.count_hit(pid, bucket_key, now, 1) - assert {:ok, 1} = Backend.Redis.delete_buckets(pid, id) - assert {:ok, []} = Redix.command(redix, ["KEYS", "*"]) - end - - test "delete buckets with no keys matching", %{pid: pid, redix: redix} do - id = "one" - now = 123 - - for bucket <- 1..10 do - bucket_key = {bucket, id} - {:ok, _} = Backend.Redis.count_hit(pid, bucket_key, now, 1) - end - - assert {:ok, 0} = Backend.Redis.delete_buckets(pid, "foobar") - - # Previous keys remain untouched - {:ok, keys} = Redix.command(redix, ["KEYS", "*"]) - assert 10 == length(keys) - end - - test "delete buckets when many buckets exist", %{pid: pid, redix: redix} do - id = "one" - now = 123 - - for bucket <- 1..1000 do - bucket_key = {bucket, id} - {:ok, _} = Backend.Redis.count_hit(pid, bucket_key, now, 1) - end - - assert {:ok, 1_000} = Backend.Redis.delete_buckets(pid, id) - assert {:ok, []} = Redix.command(redix, ["KEYS", "*"]) - end - - test "delete buckets when scan returns empty list but valid cursor", %{pid: pid, redix: redix} do - # By writing only one key to the keyspace, most iterations of the SCAN call - # will return a valid cursor but an empty list. The valid cursor should - # make the delete_buckets call iterate until the intended key is found - now = 123 - id = "one" - bucket_key = {1, id} - {:ok, _} = Backend.Redis.count_hit(pid, bucket_key, now, 1) - - another_id = "another" - - for bucket <- 1..1000 do - bucket_key = {bucket, another_id} - {:ok, _} = Backend.Redis.count_hit(pid, bucket_key, now, 1) - end - - assert {:ok, 1} = Backend.Redis.delete_buckets(pid, id) - {:ok, keys} = Redix.command(redix, ["KEYS", "*"]) - assert 1_000 == length(keys) - end - - defp make_redis_key(prefix, {bucket, id}) do - "#{prefix}#{id}:#{bucket}" - end -end diff --git a/test/test_helper.exs b/test/test_helper.exs index 869559e..0f8d409 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -1 +1,22 @@ -ExUnit.start() +task = + Task.async(fn -> + {:ok, redix} = Redix.start_link("redis://localhost:6379") + {:ok, "PONG"} == Redix.command(redix, ["PING"]) + end) + +redis_available? = Task.await(task) + +exclude = + if redis_available? do + [] + else + Mix.shell().error(""" + To enable Redis tests, start the local container with the following command: + + docker compose up -d redis + """) + + [:redis] + end + +ExUnit.start(exclude: exclude) From 79f39334135dcb99b0e33e470f089d8badb6661d Mon Sep 17 00:00:00 2001 From: ruslandoga <67764432+ruslandoga@users.noreply.github.com> Date: Sat, 16 Nov 2024 15:22:56 +0700 Subject: [PATCH 02/12] eh --- lib/hammer/redis.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/hammer/redis.ex b/lib/hammer/redis.ex index 8557705..10bc669 100644 --- a/lib/hammer/redis.ex +++ b/lib/hammer/redis.ex @@ -117,6 +117,7 @@ defmodule Hammer.Redis do commands = [ ["MULTI"], ["INCRBY", full_key, increment], + # TODO document time issues ["EXPIREAT", full_key, div(expires_at, 1000), "NX"], ["EXEC"] ] From 87fde8c883c073f024b2622d28c184e9ac84288d Mon Sep 17 00:00:00 2001 From: ruslandoga <67764432+ruslandoga@users.noreply.github.com> Date: Sat, 16 Nov 2024 01:44:41 +0700 Subject: [PATCH 03/12] no pool --- README.md | 2 +- lib/hammer/redis.ex | 68 ++++++++++++++------------------------ mix.exs | 3 +- mix.lock | 1 - test/hammer/redis_test.exs | 12 +++---- 5 files changed, 31 insertions(+), 55 deletions(-) diff --git a/README.md b/README.md index 343098e..f8ee213 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ A Redis backend for the [Hammer](https://github.com/ExHammer/hammer) rate-limiter. -This backend uses the [Redix](https://hex.pm/packages/redix) library to connect to Redis. And [poolboy](https://hex.pm/packages/poolboy) to pool the connections. +This backend uses the [Redix](https://hex.pm/packages/redix) library to connect to Redis. A single connection is used per rate-limiter. It should be enough for most use-cases since packets for rate limiting requests are short (i.e. no head of line blocking) and Redis is OK with pipelined requests (i.e. we don't block awaiting replies). Consider benchmarking before introducing more connections since TCP performance might be unintuitive. The algorithm we are using is the first method described (called "bucketing") in [Rate Limiting with Redis](https://youtu.be/CRGPbCbRTHA?t=753). In other sources it's sometimes called a "fixed window counter". diff --git a/lib/hammer/redis.ex b/lib/hammer/redis.ex index 10bc669..b8fd445 100644 --- a/lib/hammer/redis.ex +++ b/lib/hammer/redis.ex @@ -1,7 +1,6 @@ defmodule Hammer.Redis do @moduledoc """ This backend uses the [Redix](https://hex.pm/packages/redix) library to connect to Redis. - And [poolboy](https://hex.pm/packages/poolboy) to manage the connections. defmodule MyApp.RateLimit do # the default prefix is "MyApp.RateLimit:" @@ -9,9 +8,7 @@ defmodule Hammer.Redis do use Hammer, backend: Hammer.Redis, prefix: "MyApp.RateLimit:", timeout: :infinity end - redix_opts = [url: "redis://localhost:6379"] - poolboy_opts = [size: 10, max_overflow: 2] - MyApp.RateLimit.start_link(redix_opts ++ poolboy_opts) + MyApp.RateLimit.start_link(url: "redis://localhost:6379") # increment and timeout arguments are optional # by default increment is 1 and timeout is as defined in the module @@ -27,6 +24,9 @@ defmodule Hammer.Redis do prefix = Keyword.get(hammer_opts, :prefix, prefix) timeout = Keyword.get(hammer_opts, :timeout, :infinity) + # TODO + name = module + unless is_binary(prefix) do raise ArgumentError, """ Expected `:prefix` value to be a string, got: #{inspect(prefix)} @@ -47,68 +47,58 @@ defmodule Hammer.Redis do end quote do - @pool unquote(module) + @name unquote(name) @prefix unquote(prefix) @timeout unquote(timeout) def child_spec(opts) do %{ - id: @pool, + id: __MODULE__, start: {__MODULE__, :start_link, [opts]}, type: :worker } end def start_link(opts) do - opts = Keyword.put(opts, :name, @pool) + opts = Keyword.put(opts, :name, @name) Hammer.Redis.start_link(opts) end def hit(key, scale, limit, increment \\ 1, timeout \\ @timeout) do - Hammer.Redis.hit(@pool, @prefix, key, scale, limit, increment, timeout) + Hammer.Redis.hit(@name, @prefix, key, scale, limit, increment, timeout) end def inc(key, scale, increment \\ 1, timeout \\ @timeout) do - Hammer.Redis.inc(@pool, @prefix, key, scale, increment, timeout) + Hammer.Redis.inc(@name, @prefix, key, scale, increment, timeout) end def set(key, scale, count, timeout \\ @timeout) do - Hammer.Redis.set(@pool, @prefix, key, scale, count, timeout) + Hammer.Redis.set(@name, @prefix, key, scale, count, timeout) end def get(key, scale, timeout \\ @timeout) do - Hammer.Redis.get(@pool, @prefix, key, scale, timeout) + Hammer.Redis.get(@name, @prefix, key, scale, timeout) end end end @doc false def start_link(opts) do - {name, opts} = Keyword.pop!(opts, :name) - {pool_opts, redix_opts} = Keyword.split(opts, [:size, :max_overflow]) - - pool_args = [ - name: {:local, name}, - worker_module: Redix, - size: Keyword.get(pool_opts, :size, 5), - max_overflow: Keyword.get(opts, :max_overflow, 2) - ] + {url, opts} = Keyword.pop(opts, :url) - {url, redix_opts} = Keyword.pop(redix_opts, :url) - - redix_opts = + opts = if url do url_opts = Redix.URI.to_start_options(url) - Keyword.merge(url_opts, redix_opts) + Keyword.merge(url_opts, opts) else - redix_opts + opts end - :poolboy.start_link(pool_args, redix_opts) + Redix.start_link(opts) end @doc false - def hit(pool, prefix, key, scale, limit, increment, timeout) do + def hit(name, prefix, key, scale, limit, increment, timeout) do now = now() window = div(now, scale) full_key = redis_key(prefix, key, window) @@ -123,7 +113,7 @@ defmodule Hammer.Redis do ] ["OK", "QUEUED", "QUEUED", [count, _]] = - :poolboy.transaction(pool, fn conn -> Redix.pipeline!(conn, commands) end, timeout) + Redix.pipeline!(name, commands, timeout: timeout) if count <= limit do {:allow, count} @@ -133,7 +123,7 @@ defmodule Hammer.Redis do end @doc false - def inc(pool, prefix, key, scale, increment, timeout) do + def inc(name, prefix, key, scale, increment, timeout) do now = now() window = div(now, scale) full_key = redis_key(prefix, key, window) @@ -147,13 +137,13 @@ defmodule Hammer.Redis do ] ["OK", "QUEUED", "QUEUED", [count, _]] = - :poolboy.transaction(pool, fn conn -> Redix.pipeline!(conn, commands) end, timeout) + Redix.pipeline!(name, commands, timeout: timeout) count end @doc false - def set(pool, prefix, key, scale, count, timeout) do + def set(name, prefix, key, scale, count, timeout) do now = now() window = div(now, scale) full_key = redis_key(prefix, key, window) @@ -167,27 +157,17 @@ defmodule Hammer.Redis do ] ["OK", "QUEUED", "QUEUED", [_, _]] = - :poolboy.transaction( - pool, - fn conn -> Redix.pipeline!(conn, commands) end, - timeout - ) + Redix.pipeline!(name, commands, timeout: timeout) count end @doc false - def get(pool, prefix, key, scale, timeout) do + def get(name, prefix, key, scale, timeout) do now = now() window = div(now, scale) full_key = redis_key(prefix, key, window) - - count = - :poolboy.transaction( - pool, - fn conn -> Redix.command!(conn, ["GET", full_key]) end, - timeout - ) + count = Redix.command!(name, ["GET", full_key], timeout: timeout) case count do nil -> 0 diff --git a/mix.exs b/mix.exs index afb31ec..57c1516 100644 --- a/mix.exs +++ b/mix.exs @@ -37,8 +37,7 @@ defmodule Hammer.Redis.MixProject do {:dialyxir, "~> 1.1", only: [:dev], runtime: false}, {:ex_doc, "~> 0.28", only: :dev}, {:hammer, github: "ruslandoga/hammer", branch: "just-use"}, - {:redix, "~> 1.1"}, - {:poolboy, "~> 1.5"} + {:redix, "~> 1.1"} ] end diff --git a/mix.lock b/mix.lock index 493cf1b..9742d5f 100644 --- a/mix.lock +++ b/mix.lock @@ -13,7 +13,6 @@ "makeup_erlang": {:hex, :makeup_erlang, "1.0.1", "c7f58c120b2b5aa5fd80d540a89fdf866ed42f1f3994e4fe189abebeab610839", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "8a89a1eeccc2d798d6ea15496a6e4870b75e014d1af514b1b71fa33134f57814"}, "nimble_options": {:hex, :nimble_options, "1.1.1", "e3a492d54d85fc3fd7c5baf411d9d2852922f66e69476317787a7b2bb000a61b", [:mix], [], "hexpm", "821b2470ca9442c4b6984882fe9bb0389371b8ddec4d45a9504f00a66f650b44"}, "nimble_parsec": {:hex, :nimble_parsec, "1.4.0", "51f9b613ea62cfa97b25ccc2c1b4216e81df970acd8e16e8d1bdc58fef21370d", [:mix], [], "hexpm", "9c565862810fb383e9838c1dd2d7d2c437b3d13b267414ba6af33e50d2d1cf28"}, - "poolboy": {:hex, :poolboy, "1.5.2", "392b007a1693a64540cead79830443abf5762f5d30cf50bc95cb2c1aaafa006b", [:rebar3], [], "hexpm", "dad79704ce5440f3d5a3681c8590b9dc25d1a561e8f5a9c995281012860901e3"}, "redix": {:hex, :redix, "1.5.2", "ab854435a663f01ce7b7847f42f5da067eea7a3a10c0a9d560fa52038fd7ab48", [:mix], [{:castore, "~> 0.1.0 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:nimble_options, "~> 0.5.0 or ~> 1.0", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.0 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "78538d184231a5d6912f20567d76a49d1be7d3fca0e1aaaa20f4df8e1142dcb8"}, "telemetry": {:hex, :telemetry, "1.3.0", "fedebbae410d715cf8e7062c96a1ef32ec22e764197f70cda73d82778d61e7a2", [:rebar3], [], "hexpm", "7015fc8919dbe63764f4b4b87a95b7c0996bd539e0d499be6ec9d7f3875b79e6"}, } diff --git a/test/hammer/redis_test.exs b/test/hammer/redis_test.exs index 0a98ddc..4dc82b4 100644 --- a/test/hammer/redis_test.exs +++ b/test/hammer/redis_test.exs @@ -9,17 +9,15 @@ defmodule Hammer.RedisTest do setup do start_supervised!({RateLimit, url: "redis://localhost:6379"}) - "OK" = :poolboy.transaction(RateLimit, &Redix.command!(&1, ["FLUSHALL"])) + "OK" = Redix.command!(RateLimit, ["FLUSHALL"]) :ok end - defp redis_all(pool \\ RateLimit) do - :poolboy.transaction(pool, fn conn -> - keys = Redix.command!(conn, ["KEYS", "*"]) + defp redis_all(conn \\ RateLimit) do + keys = Redix.command!(conn, ["KEYS", "*"]) - Enum.map(keys, fn key -> - {key, Redix.command!(conn, ["GET", key])} - end) + Enum.map(keys, fn key -> + {key, Redix.command!(conn, ["GET", key])} end) end From 2cdb90eed7ec75eefa3dc66fcd3505c30ae4318c Mon Sep 17 00:00:00 2001 From: ruslandoga <67764432+ruslandoga@users.noreply.github.com> Date: Sat, 16 Nov 2024 01:46:32 +0700 Subject: [PATCH 04/12] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b191964..9926d2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Changed - Conform to new Hammer API +- Remove Poolboy ## 6.2.0 (2024-12-04) From f459bb11b5e16ea78e663f6af080c81eebd7df93 Mon Sep 17 00:00:00 2001 From: ruslandoga <67764432+ruslandoga@users.noreply.github.com> Date: Sat, 16 Nov 2024 01:57:36 +0700 Subject: [PATCH 05/12] throw more info in, edit later --- CHANGELOG.md | 2 +- README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9926d2d..81109ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ ### Changed - Conform to new Hammer API -- Remove Poolboy +- Remove Poolboy as it introduces unnecessary blocking. ## 6.2.0 (2024-12-04) diff --git a/README.md b/README.md index f8ee213..eb41c1b 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ A Redis backend for the [Hammer](https://github.com/ExHammer/hammer) rate-limiter. -This backend uses the [Redix](https://hex.pm/packages/redix) library to connect to Redis. A single connection is used per rate-limiter. It should be enough for most use-cases since packets for rate limiting requests are short (i.e. no head of line blocking) and Redis is OK with pipelined requests (i.e. we don't block awaiting replies). Consider benchmarking before introducing more connections since TCP performance might be unintuitive. +This backend is a thin [Redix](https://hex.pm/packages/redix) wrapper. A single connection is used per rate-limiter. It should be enough for most use-cases since packets for rate limiting requests are short (i.e. no head of line blocking) and Redis is OK with [pipelining](https://redis.io/learn/operate/redis-at-scale/talking-to-redis/client-performance-improvements#pipelining) (i.e. we don't block awaiting replies). Consider benchmarking before introducing more connections since TCP performance might be unintuitive. For possible pooling approaches, see Redix docs on [pooling](https://hexdocs.pm/redix/real-world-usage.html#name-based-pool) and also [PartitionSupervisor.](https://hexdocs.pm/elixir/1.17.3/PartitionSupervisor.html) Do not use poolboy or db_connection-like pools since they practically disable pipelining which leads to worse connection utilisation and worse performance. The algorithm we are using is the first method described (called "bucketing") in [Rate Limiting with Redis](https://youtu.be/CRGPbCbRTHA?t=753). In other sources it's sometimes called a "fixed window counter". From 564c3f09af303371ca755f73408d2b525a14dd4c Mon Sep 17 00:00:00 2001 From: ruslandoga <67764432+ruslandoga@users.noreply.github.com> Date: Sat, 16 Nov 2024 15:24:23 +0700 Subject: [PATCH 06/12] eh --- README.md | 2 ++ lib/hammer/redis.ex | 13 +++---------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index eb41c1b..d62a400 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,8 @@ This backend is a thin [Redix](https://hex.pm/packages/redix) wrapper. A single The algorithm we are using is the first method described (called "bucketing") in [Rate Limiting with Redis](https://youtu.be/CRGPbCbRTHA?t=753). In other sources it's sometimes called a "fixed window counter". +**TODO:** document ttl issues if servers are misconfigured + ## Installation Hammer-backend-redis diff --git a/lib/hammer/redis.ex b/lib/hammer/redis.ex index b8fd445..ed7d9e8 100644 --- a/lib/hammer/redis.ex +++ b/lib/hammer/redis.ex @@ -105,11 +105,8 @@ defmodule Hammer.Redis do expires_at = (window + 1) * scale commands = [ - ["MULTI"], ["INCRBY", full_key, increment], - # TODO document time issues - ["EXPIREAT", full_key, div(expires_at, 1000), "NX"], - ["EXEC"] + ["EXPIREAT", full_key, div(expires_at, 1000), "NX"] ] ["OK", "QUEUED", "QUEUED", [count, _]] = @@ -130,10 +127,8 @@ defmodule Hammer.Redis do expires_at = (window + 1) * scale commands = [ - ["MULTI"], ["INCRBY", full_key, increment], - ["EXPIREAT", full_key, div(expires_at, 1000), "NX"], - ["EXEC"] + ["EXPIREAT", full_key, div(expires_at, 1000), "NX"] ] ["OK", "QUEUED", "QUEUED", [count, _]] = @@ -150,10 +145,8 @@ defmodule Hammer.Redis do expires_at = (window + 1) * scale commands = [ - ["MULTI"], ["SET", full_key, count], - ["EXPIREAT", full_key, div(expires_at, 1000), "NX"], - ["EXEC"] + ["EXPIREAT", full_key, div(expires_at, 1000), "NX"] ] ["OK", "QUEUED", "QUEUED", [_, _]] = From 69f06987c3f5e12230e821c0935715ba1bc9f48f Mon Sep 17 00:00:00 2001 From: ruslandoga <67764432+ruslandoga@users.noreply.github.com> Date: Sat, 16 Nov 2024 15:25:14 +0700 Subject: [PATCH 07/12] no multi --- lib/hammer/redis.ex | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/hammer/redis.ex b/lib/hammer/redis.ex index ed7d9e8..af9ce39 100644 --- a/lib/hammer/redis.ex +++ b/lib/hammer/redis.ex @@ -109,7 +109,7 @@ defmodule Hammer.Redis do ["EXPIREAT", full_key, div(expires_at, 1000), "NX"] ] - ["OK", "QUEUED", "QUEUED", [count, _]] = + [count, _] = Redix.pipeline!(name, commands, timeout: timeout) if count <= limit do @@ -131,7 +131,7 @@ defmodule Hammer.Redis do ["EXPIREAT", full_key, div(expires_at, 1000), "NX"] ] - ["OK", "QUEUED", "QUEUED", [count, _]] = + [count, _] = Redix.pipeline!(name, commands, timeout: timeout) count @@ -149,8 +149,7 @@ defmodule Hammer.Redis do ["EXPIREAT", full_key, div(expires_at, 1000), "NX"] ] - ["OK", "QUEUED", "QUEUED", [_, _]] = - Redix.pipeline!(name, commands, timeout: timeout) + Redix.pipeline!(name, commands, timeout: timeout) count end From bd281cdafd94327b278bec92ef64ce6722efd8d1 Mon Sep 17 00:00:00 2001 From: "emmanuel.pinault" Date: Fri, 6 Dec 2024 14:01:07 -0800 Subject: [PATCH 08/12] adds a ci publisher. Fix pipeline --- .github/workflows/ci.yml | 2 ++ publish.yml | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 publish.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8864bf8..89777e3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,6 +33,8 @@ jobs: elixir: [1.15, 1.16, 1.17] otp: [25, 26] include: + - elixir: 1.14 + otp: 25 - elixir: 1.17 otp: 27 diff --git a/publish.yml b/publish.yml new file mode 100644 index 0000000..ff2854c --- /dev/null +++ b/publish.yml @@ -0,0 +1,18 @@ +name: Publish + +on: + push: + tags: + - "*" + +jobs: + publish: + runs-on: ubuntu-latest + steps: + - name: Check out + uses: actions/checkout@v4 + + - name: Publish package to hex.pm + uses: hipcall/github_action_publish_hex@v1 + env: + HEX_API_KEY: ${{ secrets.HEX_API_KEY }} From f350215db115dd16e5b888652c859af0cfc44a91 Mon Sep 17 00:00:00 2001 From: "emmanuel.pinault" Date: Fri, 6 Dec 2024 14:05:57 -0800 Subject: [PATCH 09/12] update deps --- .tool-versions | 2 ++ mix.exs | 12 ++++++------ mix.lock | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) create mode 100644 .tool-versions diff --git a/.tool-versions b/.tool-versions new file mode 100644 index 0000000..d8df771 --- /dev/null +++ b/.tool-versions @@ -0,0 +1,2 @@ +elixir 1.17.3-otp-27 +erlang 27.1.2 diff --git a/mix.exs b/mix.exs index 57c1516..47c2b49 100644 --- a/mix.exs +++ b/mix.exs @@ -10,7 +10,7 @@ defmodule Hammer.Redis.MixProject do source_url: "https://github.com/ExHammer/hammer-backend-redis", homepage_url: "https://github.com/ExHammer/hammer-backend-redis", version: @version, - elixir: "~> 1.15", + elixir: "~> 1.14", deps: deps(), docs: docs(), package: package(), @@ -33,11 +33,11 @@ defmodule Hammer.Redis.MixProject do defp deps do [ - {:credo, "~> 1.6", only: [:dev, :test]}, - {:dialyxir, "~> 1.1", only: [:dev], runtime: false}, - {:ex_doc, "~> 0.28", only: :dev}, - {:hammer, github: "ruslandoga/hammer", branch: "just-use"}, - {:redix, "~> 1.1"} + {:credo, "~> 1.7", only: [:dev, :test]}, + {:dialyxir, "~> 1.4", only: [:dev], runtime: false}, + {:ex_doc, "~> 0.34", only: :dev}, + {:hammer, "7.0.0-rc.0"}, + {:redix, "~> 1.5"} ] end diff --git a/mix.lock b/mix.lock index 9742d5f..cae398b 100644 --- a/mix.lock +++ b/mix.lock @@ -6,7 +6,7 @@ "erlex": {:hex, :erlex, "0.2.7", "810e8725f96ab74d17aac676e748627a07bc87eb950d2b83acd29dc047a30595", [:mix], [], "hexpm", "3ed95f79d1a844c3f6bf0cea61e0d5612a42ce56da9c03f01df538685365efb0"}, "ex_doc": {:hex, :ex_doc, "0.35.1", "de804c590d3df2d9d5b8aec77d758b00c814b356119b3d4455e4b8a8687aecaf", [:mix], [{:earmark_parser, "~> 1.4.39", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "2121c6402c8d44b05622677b761371a759143b958c6c19f6558ff64d0aed40df"}, "file_system": {:hex, :file_system, "1.0.1", "79e8ceaddb0416f8b8cd02a0127bdbababe7bf4a23d2a395b983c1f8b3f73edd", [:mix], [], "hexpm", "4414d1f38863ddf9120720cd976fce5bdde8e91d8283353f0e31850fa89feb9e"}, - "hammer": {:git, "https://github.com/ruslandoga/hammer.git", "3565989023a530efc4e36272fead5d702345092f", [branch: "just-use"]}, + "hammer": {:hex, :hammer, "7.0.0-rc.0", "ef988621b95614e4e8c340513fdd4824521f1206314c057bef04c85612c74e8e", [:mix], [], "hexpm", "bb3034112923cc9d2bdf4bba86a3356f5bdde13633e0c679a67759403b11516d"}, "jason": {:hex, :jason, "1.4.4", "b9226785a9aa77b6857ca22832cffa5d5011a667207eb2a0ad56adb5db443b8a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "c5eb0cab91f094599f94d55bc63409236a8ec69a21a67814529e8d5f6cc90b3b"}, "makeup": {:hex, :makeup, "1.2.1", "e90ac1c65589ef354378def3ba19d401e739ee7ee06fb47f94c687016e3713d1", [:mix], [{:nimble_parsec, "~> 1.4", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "d36484867b0bae0fea568d10131197a4c2e47056a6fbe84922bf6ba71c8d17ce"}, "makeup_elixir": {:hex, :makeup_elixir, "1.0.0", "74bb8348c9b3a51d5c589bf5aebb0466a84b33274150e3b6ece1da45584afc82", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "49159b7d7d999e836bedaf09dcf35ca18b312230cf901b725a64f3f42e407983"}, From ca2a70c15b7bba0ff32530dbec89b51d36d088e0 Mon Sep 17 00:00:00 2001 From: "emmanuel.pinault" Date: Fri, 6 Dec 2024 14:50:46 -0800 Subject: [PATCH 10/12] adds typespec and fix implementation of behavior --- lib/hammer/redis.ex | 58 ++++++++++++++++++++++++++++++++------ test/hammer/redis_test.exs | 19 +------------ 2 files changed, 51 insertions(+), 26 deletions(-) diff --git a/lib/hammer/redis.ex b/lib/hammer/redis.ex index af9ce39..7391434 100644 --- a/lib/hammer/redis.ex +++ b/lib/hammer/redis.ex @@ -16,6 +16,8 @@ defmodule Hammer.Redis do {:allow, _count} = MyApp.RateLimit.hit(key, scale, limit, _increment = 1, _timeout = :infinity) """ + # Redix does not define a type for its start options, so we define our own so hopefully redix will be updated to provide a type + @type redis_opts :: {:url, String.t()} | Keyword.t() defmacro __before_compile__(%{module: module}) do hammer_opts = Module.get_attribute(module, :hammer_opts) @@ -51,6 +53,7 @@ defmodule Hammer.Redis do @prefix unquote(prefix) @timeout unquote(timeout) + @spec child_spec(Keyword.t()) :: Supervisor.child_spec() def child_spec(opts) do %{ id: __MODULE__, @@ -59,30 +62,36 @@ defmodule Hammer.Redis do } end + @spec start_link(Hammer.Redis.redis_opts()) :: {:ok, Redix.connection()} def start_link(opts) do opts = Keyword.put(opts, :name, @name) Hammer.Redis.start_link(opts) end - def hit(key, scale, limit, increment \\ 1, timeout \\ @timeout) do - Hammer.Redis.hit(@name, @prefix, key, scale, limit, increment, timeout) + @impl Hammer + def hit(key, scale, limit, increment \\ 1) do + Hammer.Redis.hit(@name, @prefix, key, scale, limit, increment, @timeout) end - def inc(key, scale, increment \\ 1, timeout \\ @timeout) do - Hammer.Redis.inc(@name, @prefix, key, scale, increment, timeout) + @impl Hammer + def inc(key, scale, increment \\ 1) do + Hammer.Redis.inc(@name, @prefix, key, scale, increment, @timeout) end - def set(key, scale, count, timeout \\ @timeout) do - Hammer.Redis.set(@name, @prefix, key, scale, count, timeout) + @impl Hammer + def set(key, scale, count) do + Hammer.Redis.set(@name, @prefix, key, scale, count, @timeout) end - def get(key, scale, timeout \\ @timeout) do - Hammer.Redis.get(@name, @prefix, key, scale, timeout) + @impl Hammer + def get(key, scale) do + Hammer.Redis.get(@name, @prefix, key, scale, @timeout) end end end @doc false + @spec start_link(Hammer.Redis.redis_opts()) :: {:ok, Redix.connection()} def start_link(opts) do {url, opts} = Keyword.pop(opts, :url) @@ -98,6 +107,16 @@ defmodule Hammer.Redis do end @doc false + @spec hit( + Redix.connection(), + String.t(), + String.t(), + non_neg_integer(), + non_neg_integer(), + non_neg_integer(), + non_neg_integer() + ) :: + {:allow, non_neg_integer()} | {:deny, non_neg_integer()} def hit(name, prefix, key, scale, limit, increment, timeout) do now = now() window = div(now, scale) @@ -120,6 +139,14 @@ defmodule Hammer.Redis do end @doc false + @spec inc( + Redix.connection(), + String.t(), + String.t(), + non_neg_integer(), + non_neg_integer(), + non_neg_integer() + ) :: non_neg_integer() def inc(name, prefix, key, scale, increment, timeout) do now = now() window = div(now, scale) @@ -138,6 +165,14 @@ defmodule Hammer.Redis do end @doc false + @spec set( + Redix.connection(), + String.t(), + String.t(), + non_neg_integer(), + non_neg_integer(), + non_neg_integer() + ) :: non_neg_integer() def set(name, prefix, key, scale, count, timeout) do now = now() window = div(now, scale) @@ -155,6 +190,13 @@ defmodule Hammer.Redis do end @doc false + @spec get( + Redix.connection(), + String.t(), + String.t(), + non_neg_integer(), + non_neg_integer() + ) :: non_neg_integer() def get(name, prefix, key, scale, timeout) do now = now() window = div(now, scale) diff --git a/test/hammer/redis_test.exs b/test/hammer/redis_test.exs index 4dc82b4..4bf8390 100644 --- a/test/hammer/redis_test.exs +++ b/test/hammer/redis_test.exs @@ -41,8 +41,7 @@ defmodule Hammer.RedisTest do expected_expiretime = div(System.system_time(:second), 10) * 10 + 10 - assert :poolboy.transaction(RateLimit, &Redix.command!(&1, ["EXPIRETIME", redis_key])) == - expected_expiretime + assert Redix.command!(RateLimit, ["EXPIRETIME", redis_key]) == expected_expiretime end describe "hit" do @@ -149,20 +148,4 @@ defmodule Hammer.RedisTest do assert RateLimit.get(key, scale) == count end end - - describe "reset" do - test "resets the count for the given key and scale" do - key = "key" - scale = :timer.seconds(10) - count = 10 - - assert RateLimit.get(key, scale) == 0 - - assert RateLimit.set(key, scale, count) == count - assert RateLimit.get(key, scale) == count - - assert RateLimit.reset(key, scale) == 0 - assert RateLimit.get(key, scale) == 0 - end - end end From 98d3ce66f300986f5f500289d2527e145a825876 Mon Sep 17 00:00:00 2001 From: "emmanuel.pinault" Date: Fri, 6 Dec 2024 14:54:55 -0800 Subject: [PATCH 11/12] remove TODO as unclear what needs to happen --- lib/hammer/redis.ex | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/hammer/redis.ex b/lib/hammer/redis.ex index 7391434..182d4b7 100644 --- a/lib/hammer/redis.ex +++ b/lib/hammer/redis.ex @@ -26,7 +26,6 @@ defmodule Hammer.Redis do prefix = Keyword.get(hammer_opts, :prefix, prefix) timeout = Keyword.get(hammer_opts, :timeout, :infinity) - # TODO name = module unless is_binary(prefix) do From 3cce309b57fdd7cd5abefdd3ab79583a0b01e007 Mon Sep 17 00:00:00 2001 From: "emmanuel.pinault" Date: Fri, 6 Dec 2024 15:11:09 -0800 Subject: [PATCH 12/12] fix lint and long line issue --- lib/hammer/redis.ex | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/hammer/redis.ex b/lib/hammer/redis.ex index 182d4b7..7f3db8a 100644 --- a/lib/hammer/redis.ex +++ b/lib/hammer/redis.ex @@ -16,9 +16,11 @@ defmodule Hammer.Redis do {:allow, _count} = MyApp.RateLimit.hit(key, scale, limit, _increment = 1, _timeout = :infinity) """ - # Redix does not define a type for its start options, so we define our own so hopefully redix will be updated to provide a type + # Redix does not define a type for its start options, so we define our + # own so hopefully redix will be updated to provide a type @type redis_opts :: {:url, String.t()} | Keyword.t() + # credo:disable-for-next-line Credo.Check.Refactor.CyclomaticComplexity defmacro __before_compile__(%{module: module}) do hammer_opts = Module.get_attribute(module, :hammer_opts)