[Proposal]: Add `purpose:` option to `rate_limit` method

rubyonrails-core
1

Context

A few weeks ago I tried to use the rate_limit method so that the count could be shared between several controllers, but when I read the source code I noticed that it didn’t seem possible to do that.

More precisely because of this line:

        cache_key = ["rate-limit", controller_path, name, instance_exec(&by)].compact.join(":")
#                                  ^^^^^^^^^^^^^^^

Here’s an example of what I’ve described above:

# app/controllers/api_controller.rb
class ApiController < ApplicationController
  rate_limit to: 10, within: 3.minutes
end

# app/controllers/api/messages_controller.rb
class Api::MessagesController < ApiController
  def index; end
end

# app/controllers/api/users_controller.rb
class Api::UsersController < ApiController
  def index; end
end

It seems that currently there is no way to have rate limit count shared between several controllers since, the count is separately counted for each controller.

This is the repository for the reproduction of the problem described above.

Proposal

I therefore propose to add a new option called purpose:, to the rate_limit method. This option could be used, for instance, to create a shared count for classes inherited from the same class.

Here’s an example of a potential use:

# app/controllers/api_controller.rb
class ApiController < ApplicationController
  rate_limit to: 10, within: 3.minutes, purpose: "api"
end

# app/controllers/api/messages_controller.rb
class Api::MessagesController < ApiController
  def index; end
end

# app/controllers/api/users_controller.rb
class Api::UsersController < ApiController
  def index; end
end

Now, using the purpose: option, we’re able to limit all classes that inherit of ApiController to the same rate limit count.

Thank you in advance for your time and potential feedback.

(If this proposal is accepted, I would be happy to work on that.)

2

I’m not 100% sure I understand your need, but it could it be solved with the by: option? There’s also a name options, which might be handy.

3

Sorry if I wasn’t clear enough, in the initial post.

Anyway, basically the problem I had initially was that I wanted to use the rate_limit method, to be able to limit the number of requests on an API.

This means that if someone reaches the maximum number of requests on one controller, the same person will be blocked on the other controllers, which inherit of the base API controller (ApiController).

So I tried to implement the following code:

# app/controllers/api_controller.rb
class ApiController < ApplicationController
  rate_limit to: 10, within: 3.minutes
end

# app/controllers/api/messages_controller.rb
class Api::MessagesController < ApiController
  def index; end
end

# app/controllers/api/users_controller.rb
class Api::UsersController < ApiController
  def index; end
end

Then I realized that if, for example, you reach the maximum number of requests on the Api::MessagesController, it’s still possible for the same person to access the Api::UsersController.

After I went to the source code and found out how the keys were created to be able to increment the request count each time a request was made:

    cache_key = ["rate-limit", controller_path, name, instance_exec(&by)].compact.join(":")

And the problem I have with the above code is that, by default, the value of the cache_key variable will be different for each controller (caused by the use of controller_path), and using the by or name options won’t change anything (but maybe I misunderstood).

And at the moment, it seems that there is no option in the rate_limit method to create a shared count for several controllers.

I’m not 100% sure I understand your need, but it could it be solved with the by: option? There’s also a name options, which might be handy.

I hope that the rewording of my initial post has helped you to understand the problem I have.

Let me know if there are any parts of my explanation that you still don’t understand.

2 Likes
5

Here are the changes I think would be necessary to implement this proposal.

6

Here’s the submitted PR that implements the changes previously shared above.