Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

interval.rb allowed? not calling super #4

Open
morgz opened this issue Oct 4, 2011 · 2 comments
Open

interval.rb allowed? not calling super #4

morgz opened this issue Oct 4, 2011 · 2 comments
Assignees
Labels

Comments

@morgz
Copy link

morgz commented Oct 4, 2011

I may be missing something but it seems strange that

 def allowed?(request)
      t1 = request_start_time(request)
      t0 = cache_get(key = cache_key(request)) rescue nil
      allowed = !t0 || (dt = t1 - t0.to_f) >= minimum_interval
      begin
        cache_set(key, t1)
        allowed
      rescue => e
        # If an error occurred while trying to update the timestamp stored
        # in the cache, we will fall back to allowing the request through.
        # This prevents the Rack application blowing up merely due to a
        # backend cache server (Memcached, Redis, etc.) being offline.
        allowed = true
      end
    end

in interval.rb doesn't call the super implementation in limiter.rb

def allowed?(request)
  case
    when whitelisted?(request) then true
    when blacklisted?(request) then false
    else true # override in subclasses
  end
end
@artob artob self-assigned this Nov 14, 2014
@FreekingDean FreekingDean assigned FreekingDean and unassigned artob Oct 5, 2016
@GuyPaddock
Copy link

GuyPaddock commented Oct 7, 2016

The tricky part here is that you can't really just call super because it handles both the whitelist and blacklist cases. If the user is whitelisted then they shouldn't be subject to any limiting whatsoever, whereas if they're blacklisted, they should always be limited. It's almost like the superclass needs some method you can call that takes a block that is only executed if the user is neither whitelisted nor blacklisted, to avoid the sub-class having to duplicate the superclass logic.

@FreekingDean
Copy link
Collaborator

Right, I believe the block implementation would be best, but I am looking into other options that might be more straightforward :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

4 participants