rack-throttle icon indicating copy to clipboard operation
rack-throttle copied to clipboard

interval.rb allowed? not calling super

Open morgz opened this issue 13 years ago • 2 comments

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

morgz avatar Oct 04 '11 15:10 morgz

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.

GuyPaddock avatar Oct 07 '16 02:10 GuyPaddock

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

FreekingDean avatar Oct 10 '16 16:10 FreekingDean