I'm not a huge fan of this but I'm not sure how best to configure the storage at the individual spec level.

This is where you'd use temp_config and put the spec inside of that block. Here's an example:

It could just be added to some helper method.

This code is a copy of the RemoteIP code. You can override the method in your action, but the default needs something and I think the IP makes sense. The specs pass in a HTTP::Request so I don't get context.remote_ip.

I think you should have access to request.remote_ip

This is actually patched in Lucky. context doesn't have a remote_ip method, but you may be thinking of context.request_id which is something different.

Now, with that said, it brings up the point that if you don't have the RemoteIpHandler in your middleware stack, that could cause issues... I wonder if there's a way that we could say you must have that handler in your stack in order to use this module? 🤔

This is where you'd use temp_config and put the spec inside of that block. Here's an example:

It could just be added to some helper method.

Maybe this can pull the header name from Lucky::RemoteIpHandler.settings.ip_header_name ?

I think you should have access to request.remote_ip

This is actually patched in Lucky. context doesn't have a remote_ip method, but you may be thinking of context.request_id which is something different.

Now, with that said, it brings up the point that if you don't have the RemoteIpHandler in your middleware stack, that could cause issues... I wonder if there's a way that we could say you must have that handler in your stack in order to use this module? 🤔

I'm not sure if this matters here, but if we build it at compile time, then it won't need to compute on each request.

This is actually a pretty slick interface. It feels consistent with some other Lucky modules.

Thanks so much for this !

I'd love to get a macro one liner with keeping the possibility to have a method coputing values at runtime like you proposed.

Example :

class RateLimitRoutes::Index < TestAction
  include Lucky::RateLimit
  rate_limit to: 1, within: 1.minute

  get "/rate_limit" do
    plain_text "hello"
  end
end

The dynamic setup still works as before :

class ComputedRateLimitRoutes::Index < TestAction
  include Lucky::RateLimit

  get "/computed_rate_limit" do
    plain_text "hello"
  end

  private def computed_rate_limit : NamedTuple(to: Int32, within: Time::Span)
    {to: 1, within: 1.minute}
  end
end

Here's what I got working locally :

module Lucky::RateLimit
  macro included
    before enforce_rate_limit
  end

  macro rate_limit(**tuple)
    private def computed_rate_limit : NamedTuple(to: Int32, within: Time::Span)
      {{tuple}}
    end
  end

  abstract def computed_rate_limit : NamedTuple(to: Int32, within: Time::Span)

  private def enforce_rate_limit
    cache = LuckyCache.settings.storage
    count = cache.fetch(rate_limit_key, as: Int32, expires_in: computed_rate_limit["within"]) { 0 }
    cache.write(rate_limit_key, expires_in: computed_rate_limit["within"]) { count + 1 }

    if count > computed_rate_limit["to"]
      context.response.status = HTTP::Status::TOO_MANY_REQUESTS
      context.response.headers["Retry-After"] = computed_rate_limit["within"].to_s
      plain_text("Rate limit exceeded")
    else
      continue
    end
  end

  private def rate_limit_key : String
    klass = self.class.to_s.downcase.gsub("::", ":")
    "ratelimit:#{klass}:#{rate_limit_identifier}"
  end

  private def rate_limit_identifier : Socket::Address | Nil
    request = context.request

    if x_forwarded = request.headers["X_FORWARDED_FOR"]?.try(&.split(',').first?).presence
      begin
        Socket::IPAddress.new(x_forwarded, 0)
      rescue Socket::Error
        # if the x_forwarded is not a valid ip address we fallback to request.remote_address
        request.remote_address
      end
    else
      request.remote_address
    end
  end
end

I'm sure there are still improvements macro wise but that's the idea.