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

Add global and channel rate limits #1065

Merged
merged 7 commits into from
May 19, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 41 additions & 12 deletions sopel/bot.py
Original file line number Diff line number Diff line change
Expand Up @@ -420,31 +420,60 @@ def reply(self, message, destination=None, reply_to=None, notice=False):

def call(self, func, sopel, trigger):
nick = trigger.nick
current_time = time.time()
if nick not in self._times:
self._times[nick] = dict()

if not trigger.admin and \
not func.unblockable and \
func.rate > 0 and \
func in self._times[nick]:
timediff = time.time() - self._times[nick][func]
if timediff < func.rate:
self._times[nick][func] = time.time()
if self.nick not in self._times:
self._times[self.nick] = dict()

if not trigger.admin and not func.unblockable:
usertimediff = current_time - self._times[nick][func]
if func.user_rate > 0 and usertimediff < func.user_rate and \
func in self._times[nick]:
self._times[nick][func] = current_time
LOGGER.info(
"%s prevented from using %s in %s due to user limit: %d < %d",
trigger.nick, func.__name__, trigger.sender, usertimediff,
func.user_rate
)
return
globaltimediff = current_time - self._times[self.nick][func]
if func.global_rate > 0 and globaltimediff < func.global_rate and \
func in self._times[self.nick]:
self._times[self.nick][func] = current_time
LOGGER.info(
"%s prevented from using %s in %s: %d < %d",
trigger.nick, func.__name__, trigger.sender, timediff,
func.rate
"%s prevented from using %s in %s due to global limit: %d < %d",
trigger.nick, func.__name__, trigger.sender, globaltimediff,
func.global_rate
)
return

if not trigger.is_privmsg:
chan = trigger.sender
if chan not in self._times:
self._times[chan] = dict()
chantimediff = current_time - self._times[chan][func]
if func.channel_rate > 0 and chantimediff < func.channel_rate and \
func in self._times[chan]:
self._times[chan][func] = current_time
LOGGER.info(
"%s prevented from using %s in %s due to channel limit: %d < %d",
trigger.nick, func.__name__, trigger.sender, timediff,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timediff is undefined, should be chantimediff

func.channel_rate
)
return

try:
exit_code = func(sopel, trigger)
except Exception:
exit_code = None
self.error(trigger)

if exit_code != NOLIMIT:
self._times[nick][func] = time.time()
self._times[nick][func] = current_time
self._times[self.nick][func] = current_time
if not trigger.is_privmsg:
self._times[trigger.sender][func] = current_time

def dispatch(self, pretrigger):
args = pretrigger.args
Expand Down
4 changes: 3 additions & 1 deletion sopel/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,9 @@ def clean_callable(func, config):
func.unblockable = getattr(func, 'unblockable', False)
func.priority = getattr(func, 'priority', 'medium')
func.thread = getattr(func, 'thread', True)
func.rate = getattr(func, 'rate', 0)
func.user_rate = getattr(func, 'user_rate', 0)
Copy link
Contributor

@maxpowa maxpowa Apr 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing rate will break backwards compatibility with jenni modules. Maybe alias rate to user_rate in the getattr?

func.channel_rate = getattr(func, 'channel_rate', 0)
func.global_rate = getattr(func, 'global_rate', 0)

if not hasattr(func, 'event'):
func.event = ['PRIVMSG']
Expand Down
10 changes: 7 additions & 3 deletions sopel/module.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,10 @@ def add_attribute(function):
return add_attribute


def rate(value):
"""Decorate a function to limit how often a single user may trigger it.
def rate(user_rate, channel_rate=0, global_rate=0):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason not to also specify a default value for user_rate? It's the first positional argument, so it should remain backward-compatible without requiring any particular limit to be specified in new modules.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No particular reason, other than that I don't know what a good default would be. If zero, that's the same as having no @Rate decorator at all.

Copy link
Member

@dgw dgw Apr 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would, however, allow using the decorator like @rate(channel_rate=300) to limit a command per-channel without having to specify a 0 for the first argument. IMO it's more explicit than @rate(0, 300).

I'd bet whoever owns the username "rate" on Github is going to get mighty tired of this thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't think of that; It was 3 in the morning, haha. Fixed now.

"""Decorate a function to limit how often it can be triggered. Without
arguments, the limit defaults to per-user. The global and channel parameters
are self-explanatory. A value of zero means no limit.

If a function is given a rate of 20, a single user may only use that
function once every 20 seconds. This limit applies to each user
Expand All @@ -260,7 +262,9 @@ def rate(value):
threading.Timer() instead of sched, or rate limiting will not work properly.
"""
def add_attribute(function):
function.rate = value
function.user_rate = user_rate
function.channel_rate = channel_rate
function.global_rate = global_rate
return function
return add_attribute

Expand Down