From 2340242d431224360ca0ff97bc12fe582f0c9282 Mon Sep 17 00:00:00 2001 From: Esteban Castro Borsani Date: Fri, 20 May 2016 01:41:22 +0200 Subject: [PATCH] Feature/rate limit own cache 120 (#121) * core.ratelimit validate cache config * tests clear all caches * settings set ST_RATELIMIT_CACHE to spirit_rl_cache * core.utils deprecations * tests deprecations * core.ratelimit get_fixed window depreaction warning when period is 0 * core.ratelimit tests * topic.private renamed test --- example/project/settings/base.py | 3 + example/project/settings/dev.py | 5 + spirit/admin/tests.py | 5 +- spirit/category/admin/tests.py | 5 +- spirit/category/tests.py | 5 +- spirit/comment/bookmark/tests.py | 9 +- spirit/comment/flag/tests.py | 5 +- spirit/comment/history/tests.py | 5 +- spirit/comment/like/tests.py | 7 +- spirit/comment/poll/tests.py | 11 +-- spirit/comment/tests.py | 11 +-- spirit/core/tests/tests_markdown.py | 11 +-- spirit/core/tests/tests_utils.py | 7 +- spirit/core/tests/tests_utils_deprecations.py | 23 +++++ spirit/core/tests/tests_utils_paginator.py | 11 +-- spirit/core/tests/tests_utils_ratelimit.py | 96 ++++++++++++++++--- spirit/core/tests/utils.py | 8 ++ spirit/core/utils/deprecations.py | 22 +++++ spirit/core/utils/ratelimit/ratelimit.py | 49 ++++++---- spirit/search/tests.py | 9 +- spirit/settings.py | 9 ++ spirit/settings_tests.py | 9 +- spirit/topic/favorite/tests.py | 5 +- spirit/topic/moderate/tests.py | 3 +- spirit/topic/notification/tests.py | 11 +-- spirit/topic/poll/tests.py | 11 +-- spirit/topic/private/tests.py | 13 ++- spirit/topic/tests.py | 9 +- spirit/topic/unread/tests.py | 7 +- spirit/user/auth/tests/tests.py | 7 +- spirit/user/tests.py | 9 +- 31 files changed, 268 insertions(+), 132 deletions(-) create mode 100644 spirit/core/tests/tests_utils_deprecations.py create mode 100644 spirit/core/utils/deprecations.py diff --git a/example/project/settings/base.py b/example/project/settings/base.py index d676f1c0c..87f65bb79 100644 --- a/example/project/settings/base.py +++ b/example/project/settings/base.py @@ -19,6 +19,9 @@ # Application definition +# This will be the default in next version +ST_RATELIMIT_CACHE = 'st_rate_limit' + # Extend the Spirit installed apps. # Check out the spirit.settings.py so you do not end up with duplicated apps. INSTALLED_APPS.extend([ diff --git a/example/project/settings/dev.py b/example/project/settings/dev.py index 53374ed7e..fee06e9aa 100644 --- a/example/project/settings/dev.py +++ b/example/project/settings/dev.py @@ -38,6 +38,11 @@ 'default': { 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', }, + 'st_rate_limit': { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + 'LOCATION': 'spirit_rl_cache', + 'TIMEOUT': None + } }) PASSWORD_HASHERS = [ diff --git a/spirit/admin/tests.py b/spirit/admin/tests.py index 627d0d33e..66fd2f301 100644 --- a/spirit/admin/tests.py +++ b/spirit/admin/tests.py @@ -4,7 +4,6 @@ from django.test import TestCase, RequestFactory from django.core.urlresolvers import reverse -from django.core.cache import cache from django.core.exceptions import PermissionDenied from django.contrib.auth import get_user_model @@ -27,7 +26,7 @@ class AdminViewTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.user.st.is_administrator = True self.user.st.save() @@ -343,7 +342,7 @@ def test_flag_detail_paginate(self): class AdminFormTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(self.category) diff --git a/spirit/category/admin/tests.py b/spirit/category/admin/tests.py index 23b1bd4b6..da794929d 100644 --- a/spirit/category/admin/tests.py +++ b/spirit/category/admin/tests.py @@ -4,7 +4,6 @@ from django.test import TestCase, RequestFactory from django.core.urlresolvers import reverse -from django.core.cache import cache from django.core.exceptions import PermissionDenied from django.contrib.auth import get_user_model @@ -19,7 +18,7 @@ class AdminViewTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.user.st.is_administrator = True self.user.st.save() @@ -87,7 +86,7 @@ def test_category_form_color(self): class AdminFormTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(self.category) diff --git a/spirit/category/tests.py b/spirit/category/tests.py index 4f3bd9375..3e245bbc4 100644 --- a/spirit/category/tests.py +++ b/spirit/category/tests.py @@ -6,7 +6,6 @@ from django.test import TestCase from django.core.urlresolvers import reverse -from django.core.cache import cache from django.utils import timezone from django.conf import settings @@ -21,7 +20,7 @@ class CategoryViewTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category_1 = utils.create_category(title="cat1") self.subcategory_1 = utils.create_subcategory(self.category_1) @@ -152,7 +151,7 @@ def test_category_detail_view_paginate(self): class CategoryMigrationTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() def test_uncategorized_category(self): """ diff --git a/spirit/comment/bookmark/tests.py b/spirit/comment/bookmark/tests.py index cf01154de..0c3b044fb 100644 --- a/spirit/comment/bookmark/tests.py +++ b/spirit/comment/bookmark/tests.py @@ -2,10 +2,9 @@ from __future__ import unicode_literals -from django.test import TestCase, RequestFactory +from django.test import TestCase from django.core.urlresolvers import reverse from django.template import Template, Context -from django.core.cache import cache from djconfig import config @@ -17,7 +16,7 @@ class CommentBookmarkViewTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) @@ -38,7 +37,7 @@ def test_bookmark_create(self): class CommentBookmarkModelsTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) @@ -91,7 +90,7 @@ def test_form(self): class CommentBookmarkTemplateTagsTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(self.category) diff --git a/spirit/comment/flag/tests.py b/spirit/comment/flag/tests.py index 225e28c5f..a067251d6 100644 --- a/spirit/comment/flag/tests.py +++ b/spirit/comment/flag/tests.py @@ -4,7 +4,6 @@ from django.test import TestCase from django.core.urlresolvers import reverse -from django.core.cache import cache from ...core.tests import utils from .models import Flag, CommentFlag @@ -14,7 +13,7 @@ class FlagViewTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) @@ -36,7 +35,7 @@ def test_flag_create(self): class FlagFormTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) diff --git a/spirit/comment/history/tests.py b/spirit/comment/history/tests.py index eba9a0a4c..e07ed33ef 100644 --- a/spirit/comment/history/tests.py +++ b/spirit/comment/history/tests.py @@ -5,7 +5,6 @@ from django.test import TestCase from django.core.urlresolvers import reverse -from django.core.cache import cache from djconfig.utils import override_djconfig @@ -17,7 +16,7 @@ class CommentHistoryViewTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) @@ -113,7 +112,7 @@ def test_comment_history_detail_denied_to_non_logged_users(self): class CommentHistoryModelsTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) diff --git a/spirit/comment/like/tests.py b/spirit/comment/like/tests.py index 6ee96cae9..4cd320eca 100644 --- a/spirit/comment/like/tests.py +++ b/spirit/comment/like/tests.py @@ -5,7 +5,6 @@ from django.test import TestCase from django.core.urlresolvers import reverse from django.template import Template, Context -from django.core.cache import cache from ...core.tests import utils from ..models import Comment @@ -17,7 +16,7 @@ class LikeViewTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) @@ -106,7 +105,7 @@ def test_like_delete_comment_decrease_likes_count(self): class LikeFormTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) @@ -137,7 +136,7 @@ def test_like_create_invalid(self): class LikeTemplateTagsTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) diff --git a/spirit/comment/poll/tests.py b/spirit/comment/poll/tests.py index 76f6a6154..cf9dcdd74 100644 --- a/spirit/comment/poll/tests.py +++ b/spirit/comment/poll/tests.py @@ -4,7 +4,6 @@ from django.test import TestCase, RequestFactory from django.core.urlresolvers import reverse -from django.core.cache import cache from django.contrib.auth import get_user_model from django.utils import timezone from django.template import Template, Context @@ -22,7 +21,7 @@ class PollViewTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.user2 = utils.create_user() self.category = utils.create_category() @@ -234,7 +233,7 @@ def test_poll_voters_secret_closed(self): class PollFormTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.user2 = utils.create_user() self.category = utils.create_category() @@ -384,7 +383,7 @@ def test_update_vote_single(self): class CommentPollTemplateTagsTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category) @@ -536,7 +535,7 @@ def test_render_polls_secret_closed(self): class PollModelsTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) @@ -775,7 +774,7 @@ def test_poll_choice_update_or_create_many_removed_poll(self): class PollUtilsTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) diff --git a/spirit/comment/tests.py b/spirit/comment/tests.py index a4cd74039..4336d0bae 100644 --- a/spirit/comment/tests.py +++ b/spirit/comment/tests.py @@ -7,7 +7,6 @@ import shutil from django.test import TestCase, RequestFactory -from django.core.cache import cache from django.core.urlresolvers import reverse from django.template import Template, Context from django.core.exceptions import PermissionDenied @@ -39,7 +38,7 @@ class CommentViewTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) @@ -402,7 +401,7 @@ def test_comment_image_upload_invalid(self): class CommentModelsTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) @@ -442,7 +441,7 @@ def test_comment_create_moderation_action(self): class CommentTemplateTagTests(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) @@ -477,7 +476,7 @@ def test_get_action_text(self): class CommentFormTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category) @@ -601,7 +600,7 @@ def test_comment_max_len_invalid(self): class CommentUtilsTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) diff --git a/spirit/core/tests/tests_markdown.py b/spirit/core/tests/tests_markdown.py index 67fce4612..9480621a4 100644 --- a/spirit/core/tests/tests_markdown.py +++ b/spirit/core/tests/tests_markdown.py @@ -2,13 +2,12 @@ from __future__ import unicode_literals -from django.core.cache import cache from django.test import TestCase from django.test.utils import override_settings from django.utils import translation from django.utils import timezone -from ..tests import utils as test_utils +from . import utils from ..utils.markdown import Markdown, quotify @@ -18,10 +17,10 @@ class UtilsMarkdownTests(TestCase): def setUp(self): - cache.clear() - self.user = test_utils.create_user(username="nitely") - self.user2 = test_utils.create_user(username="esteban") - self.user3 = test_utils.create_user(username="áéíóú") + utils.cache_clear() + self.user = utils.create_user(username="nitely") + self.user2 = utils.create_user(username="esteban") + self.user3 = utils.create_user(username="áéíóú") def test_markdown_escape(self): """ diff --git a/spirit/core/tests/tests_utils.py b/spirit/core/tests/tests_utils.py index 98d0c9925..54b2fc463 100644 --- a/spirit/core/tests/tests_utils.py +++ b/spirit/core/tests/tests_utils.py @@ -6,7 +6,6 @@ import json import os -from django.core.cache import cache from django.test import TestCase, RequestFactory from django.test.utils import override_settings from django.template import Template, Context @@ -28,7 +27,7 @@ from ..utils.timezone import TIMEZONE_CHOICES from ..utils.decorators import moderator_required, administrator_required from ..tags import time as ttags_utils -from ..tests import utils as test_utils +from . import utils as test_utils from ..tags.messages import render_messages User = get_user_model() @@ -37,7 +36,7 @@ class UtilsTests(TestCase): def setUp(self): - cache.clear() + test_utils.cache_clear() def test_render_form_errors(self): """ @@ -268,7 +267,7 @@ def test_timezone(self): class UtilsDecoratorsTests(TestCase): def setUp(self): - cache.clear() + test_utils.cache_clear() self.user = test_utils.create_user() def test_moderator_required(self): diff --git a/spirit/core/tests/tests_utils_deprecations.py b/spirit/core/tests/tests_utils_deprecations.py new file mode 100644 index 000000000..08ae2e533 --- /dev/null +++ b/spirit/core/tests/tests_utils_deprecations.py @@ -0,0 +1,23 @@ +# -*- coding: utf-8 -*- + +from __future__ import unicode_literals +import warnings + +from django.test import TestCase + +from ..utils import deprecations + + +class UtilsDeprecations(TestCase): + + def setUp(self): + pass + + def test_warn(self): + with warnings.catch_warnings(record=True) as w: + deprecations.warn("foo") + self.assertEqual(len(w), 1) + self.assertTrue(issubclass( + w[-1].category, + deprecations.RemovedInNextVersionWarning)) + self.assertTrue('foo' in str(w[-1].message)) diff --git a/spirit/core/tests/tests_utils_paginator.py b/spirit/core/tests/tests_utils_paginator.py index a671bc0fa..fa4fc46e2 100644 --- a/spirit/core/tests/tests_utils_paginator.py +++ b/spirit/core/tests/tests_utils_paginator.py @@ -2,7 +2,6 @@ from __future__ import unicode_literals -from django.core.cache import cache from django.test import TestCase, RequestFactory from django.template import Template, Context from django.test.utils import override_settings @@ -21,7 +20,7 @@ class UtilsPaginatorTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() def test_paginator_page(self): per_page = 15 @@ -52,7 +51,7 @@ def test_paginator_url(self): class UtilsInfinitePaginatorTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.topic = utils.create_topic(utils.create_category()) @@ -96,7 +95,7 @@ def test_paginate(self): class UtilsYTPaginatorTests(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.topic = utils.create_topic(utils.create_category()) @@ -168,7 +167,7 @@ def test_yt_paginator_page_range(self): class UtilsYTPaginatorTemplateTagsTests(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() def tests_yt_paginate(self): # first page @@ -233,7 +232,7 @@ def mock_render(template, context): class UtilsPaginatorTemplateTagsTests(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() def tests_paginate(self): # first page diff --git a/spirit/core/tests/tests_utils_ratelimit.py b/spirit/core/tests/tests_utils_ratelimit.py index a694e17c2..1183bf4bd 100644 --- a/spirit/core/tests/tests_utils_ratelimit.py +++ b/spirit/core/tests/tests_utils_ratelimit.py @@ -1,14 +1,15 @@ # -*- coding: utf-8 -*- from __future__ import unicode_literals +import warnings -from django.core.cache import cache -from django.test import TestCase, RequestFactory +from django.test import TestCase, RequestFactory, override_settings from django.contrib.auth.models import AnonymousUser, User from django.contrib.messages.storage.fallback import FallbackStorage from django.conf import settings from django.core.cache import caches +from . import utils from ..utils.ratelimit import ratelimit as rl_module from ..utils.ratelimit import RateLimit from ..utils.ratelimit.decorators import ratelimit @@ -26,7 +27,7 @@ def setup_request_factory_messages(req): class UtilsRateLimitTests(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() def test_rate_limit_split_rate(self): req = RequestFactory().post('/') @@ -200,21 +201,94 @@ def fixed_time_future(seconds): finally: rl_module.time.time = org_time_time - def test_rate_limit_timeout_too_low(self): + def test_rate_limit_pruned_too_frequently(self): """ - Should not limit when the timeout is\ - too low to increase the rate counter + Should not limit when the cache\ + is pruned too frequently """ req = RequestFactory().post('/') setup_request_factory_messages(req) req.user = User() req.user.pk = 1 - # The key is removed immediately - # when the timeout is 0 - @ratelimit(rate='1/0s') + @ratelimit(rate='1/m') def one(request): return request.is_limited - self.assertFalse(one(req)) - self.assertFalse(one(req)) + foo_cache = { + 'default': { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache' + }, + 'foo': { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + 'LOCATION': 'foo', + 'TIMEOUT': 0 # Faking cache pruned too frequently + }} + + with override_settings(CACHES=foo_cache, ST_RATELIMIT_CACHE='foo'): + with warnings.catch_warnings(record=True): # Ignore warnings + caches['foo'].clear() + self.assertFalse(one(req)) + self.assertFalse(one(req)) + + +class UtilsRateLimitDeprecationsTests(TestCase): + + def setUp(self): + utils.cache_clear() + + def test_validator_conf(self): + """ + Should create a deprecation\ + warning when cache has no timeout + """ + req = RequestFactory().post('/') + req.user = User() + req.user.pk = 1 + + @ratelimit(rate='1/m') + def one(_): + pass + + foo_cache = { + 'default': { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache' + }, + 'foo': { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + 'LOCATION': 'foo'}} + + with override_settings(CACHES=foo_cache, ST_RATELIMIT_CACHE='foo'): + with warnings.catch_warnings(record=True) as w: + utils.cache_clear() + one(req) + self.assertEqual(len(w), 1) + self.assertEqual( + str(w[-1].message), + 'settings.ST_RATELIMIT_CACHE cache\'s TIMEOUT ' + 'must be None (never expire) and it may ' + 'be other than the default cache. ' + 'To skip this check, for example when using ' + 'a third-party backend with no TIMEOUT option, set ' + 'settings.ST_RATELIMIT_SKIP_TIMEOUT_CHECK to True. ' + 'This will raise an exception in next version.') + + foo_cache['foo']['TIMEOUT'] = None + + with override_settings(CACHES=foo_cache, ST_RATELIMIT_CACHE='foo'): + with warnings.catch_warnings(record=True) as w: + caches['foo'].clear() + one(req) + self.assertEqual(len(w), 0) + + def test_get_fixed_window(self): + """ + Should create a deprecation\ + warning when period is zero + """ + with warnings.catch_warnings(record=True) as w: + RateLimit.get_fixed_window(period=0) + self.assertEqual(len(w), 1) + self.assertEqual( + str(w[-1].message), + 'Period must be greater than 0.') diff --git a/spirit/core/tests/utils.py b/spirit/core/tests/utils.py index 408be6f73..e724fea44 100644 --- a/spirit/core/tests/utils.py +++ b/spirit/core/tests/utils.py @@ -4,6 +4,7 @@ from django.contrib.auth import get_user_model from django.conf import settings +from django.core.cache import caches, cache from ...topic.models import Topic from ...category.models import Category @@ -76,3 +77,10 @@ def login(test_case_instance, user=None, password=None): password = password or "bar" login_successful = test_case_instance.client.login(username=user.username, password=password) test_case_instance.assertTrue(login_successful) + + +def cache_clear(): + cache.clear() # Default one + + for c in caches.all(): + c.clear() diff --git a/spirit/core/utils/deprecations.py b/spirit/core/utils/deprecations.py new file mode 100644 index 000000000..9597bbe77 --- /dev/null +++ b/spirit/core/utils/deprecations.py @@ -0,0 +1,22 @@ +# -*- coding: utf-8 -*- + +from __future__ import unicode_literals +import warnings + +__all__ = [ + 'RemovedInNextVersionWarning', + 'warn'] + + +class RemovedInNextVersionWarning(DeprecationWarning): + """""" + + +def warn(message, stacklevel=3): + warnings.warn( + message, + category=RemovedInNextVersionWarning, + stacklevel=stacklevel) + + +warnings.simplefilter("default", RemovedInNextVersionWarning) diff --git a/spirit/core/utils/ratelimit/ratelimit.py b/spirit/core/utils/ratelimit/ratelimit.py index 4df85f5dc..acd24ab27 100644 --- a/spirit/core/utils/ratelimit/ratelimit.py +++ b/spirit/core/utils/ratelimit/ratelimit.py @@ -8,19 +8,39 @@ from django.conf import settings from django.core.cache import caches +from ..deprecations import warn + TIME_DICT = { 's': 1, 'm': 60} -class RateLimitError(Exception): - """""" +def validate_cache_config(): + try: + cache = settings.CACHES[settings.ST_RATELIMIT_CACHE] + except KeyError: + # Django will raise later when using + # this cache so we do nothing + return + + if (not settings.ST_RATELIMIT_SKIP_TIMEOUT_CHECK and + cache.get('TIMEOUT', 1) is not None): + # todo: ConfigurationError in next version + warn( + 'settings.ST_RATELIMIT_CACHE cache\'s TIMEOUT ' + 'must be None (never expire) and it may ' + 'be other than the default cache. ' + 'To skip this check, for example when using ' + 'a third-party backend with no TIMEOUT option, set ' + 'settings.ST_RATELIMIT_SKIP_TIMEOUT_CHECK to True. ' + 'This will raise an exception in next version.') class RateLimit: def __init__(self, request, uid, method=None, field=None, rate='5/5m'): + validate_cache_config() self.request = request self.uid = uid self.method = method or ['POST'] @@ -51,7 +71,8 @@ def split_rate(rate): @staticmethod def get_fixed_window(period): if not period: # todo: assert on Spirit 0.5 - return 0 + warn('Period must be greater than 0.') + return time.time() # Closer to no period timestamp = int(time.time()) return timestamp - timestamp % period @@ -83,31 +104,17 @@ def _get_keys(self, field=None): return [self._make_key(k) for k in keys] - def __incr(self, key): + def _incr(self, key): cache = caches[settings.ST_RATELIMIT_CACHE] - cache.add(key, 0, timeout=self.time) + cache.add(key, 0) try: # This resets the timeout to # default, see Django ticket #26619 return cache.incr(key) except ValueError: # Key does not exists - raise RateLimitError - - def _incr(self, key): - try: - return self.__incr(key) - except RateLimitError: - pass - - try: - # Retry in case the key - # has just timed-out - return self.__incr(key) - except RateLimitError: - # The timeout is too low - # or the cache is being pruned - # too frequently + # The cache is being + # pruned too frequently return 1 def _incr_all(self): diff --git a/spirit/search/tests.py b/spirit/search/tests.py index 86e33a010..24821b672 100644 --- a/spirit/search/tests.py +++ b/spirit/search/tests.py @@ -3,7 +3,6 @@ from __future__ import unicode_literals from django.test import TestCase -from django.core.cache import cache from django.core.urlresolvers import reverse from django.template import Template, Context from django.conf import settings @@ -28,7 +27,7 @@ class SearchTopicIndexTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() def test_index_queryset_excludes_private_topics(self): """ @@ -61,7 +60,7 @@ def setUp(self): # self.connections = haystack.connections # haystack.connections = haystack.loading.ConnectionHandler(HAYSTACK_TEST) - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user, title="spirit search test foo") @@ -123,7 +122,7 @@ def test_advanced_search_in_category(self): class SearchFormTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() def test_basic_search(self): data = {'q': 'foobar', } @@ -149,7 +148,7 @@ def test_advanced_search_invalid_too_short(self): class SearchTemplateTagTests(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() def test_render_search_form(self): """ diff --git a/spirit/settings.py b/spirit/settings.py index 1cbc8e856..a1e2c0f3a 100644 --- a/spirit/settings.py +++ b/spirit/settings.py @@ -11,6 +11,7 @@ ST_RATELIMIT_ENABLE = True ST_RATELIMIT_CACHE_PREFIX = 'srl' ST_RATELIMIT_CACHE = 'default' +ST_RATELIMIT_SKIP_TIMEOUT_CHECK = False ST_NOTIFICATIONS_PER_PAGE = 20 @@ -88,6 +89,14 @@ }, } +CACHES.update({ + 'st_rate_limit': { + 'BACKEND': CACHES['default']['BACKEND'], + 'LOCATION': 'spirit_rl_cache', + 'TIMEOUT': None + } +}) + AUTHENTICATION_BACKENDS = [ 'spirit.user.auth.backends.UsernameAuthBackend', 'spirit.user.auth.backends.EmailAuthBackend', diff --git a/spirit/settings_tests.py b/spirit/settings_tests.py index 96be4822c..ef22a0974 100644 --- a/spirit/settings_tests.py +++ b/spirit/settings_tests.py @@ -42,6 +42,11 @@ 'default': { 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', }, + 'st_rate_limit': { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + 'LOCATION': 'spirit_rl_cache', + 'TIMEOUT': None + } }) # speedup tests requiring login @@ -60,4 +65,6 @@ )), ] -TEMPLATES[0]['OPTIONS']['debug'] = True \ No newline at end of file +TEMPLATES[0]['OPTIONS']['debug'] = True + +ST_RATELIMIT_CACHE = 'st_rate_limit' diff --git a/spirit/topic/favorite/tests.py b/spirit/topic/favorite/tests.py index f0367d062..0923a159f 100644 --- a/spirit/topic/favorite/tests.py +++ b/spirit/topic/favorite/tests.py @@ -4,7 +4,6 @@ from django.test import TestCase from django.core.urlresolvers import reverse -from django.core.cache import cache from ...core.tests import utils from .models import TopicFavorite @@ -15,7 +14,7 @@ class FavoriteViewTest(TestCase): # TODO: templatetags test def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) @@ -81,7 +80,7 @@ def test_favorite_delete_next(self): class FavoriteFormTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) diff --git a/spirit/topic/moderate/tests.py b/spirit/topic/moderate/tests.py index 9fdef64b1..52aa49f6e 100644 --- a/spirit/topic/moderate/tests.py +++ b/spirit/topic/moderate/tests.py @@ -3,7 +3,6 @@ from __future__ import unicode_literals from django.test import TestCase -from django.core.cache import cache from django.core.urlresolvers import reverse from ...core.tests import utils @@ -14,7 +13,7 @@ class TopicViewTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() def test_topic_moderate_delete(self): diff --git a/spirit/topic/notification/tests.py b/spirit/topic/notification/tests.py index 8b00e405e..d38d72560 100644 --- a/spirit/topic/notification/tests.py +++ b/spirit/topic/notification/tests.py @@ -4,10 +4,9 @@ import json import datetime -from django.test import TestCase, RequestFactory +from django.test import TestCase from django.test.utils import override_settings from django.core.urlresolvers import reverse -from django.core.cache import cache from django.template import Template, Context from django.utils import timezone @@ -23,7 +22,7 @@ class TopicNotificationViewTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.user2 = utils.create_user() self.category = utils.create_category() @@ -310,7 +309,7 @@ def test_topic_notification_update_invalid_user(self): class TopicNotificationFormTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() def test_notification_creation(self): @@ -355,7 +354,7 @@ def test_notification(self): class TopicNotificationModelsTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.user2 = utils.create_user() self.category = utils.create_category() @@ -480,7 +479,7 @@ def test_topic_notification_notify_new_mentions_unactive(self): class TopicNotificationTemplateTagsTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(self.category) diff --git a/spirit/topic/poll/tests.py b/spirit/topic/poll/tests.py index f9b579a05..403c479d5 100644 --- a/spirit/topic/poll/tests.py +++ b/spirit/topic/poll/tests.py @@ -4,7 +4,6 @@ from django.test import TestCase from django.core.urlresolvers import reverse -from django.core.cache import cache from django.contrib.auth import get_user_model from django.template import Template, Context @@ -20,7 +19,7 @@ class TopicPollViewTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.user2 = utils.create_user() self.category = utils.create_category() @@ -208,7 +207,7 @@ def topic_poll_post_vote_handler(sender, poll, user, **kwargs): class TopicPollFormTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.user2 = utils.create_user() self.category = utils.create_category() @@ -294,7 +293,7 @@ def test_update_poll_choices_filled_but_deleted(self): class TopicPollVoteManyFormTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.user2 = utils.create_user() self.category = utils.create_category() @@ -400,7 +399,7 @@ def test_update_vote_single(self): class TopicPollSignalTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) @@ -438,7 +437,7 @@ def test_topic_poll_post_vote_handler(self): class TopicPollTemplateTagsTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) diff --git a/spirit/topic/private/tests.py b/spirit/topic/private/tests.py index 3ff72466c..918646368 100644 --- a/spirit/topic/private/tests.py +++ b/spirit/topic/private/tests.py @@ -4,7 +4,6 @@ import datetime from django.test import TestCase -from django.core.cache import cache from django.core.urlresolvers import reverse from django.template import Template, Context from django.conf import settings @@ -29,7 +28,7 @@ class TopicPrivateViewTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.user2 = utils.create_user() @@ -259,7 +258,7 @@ def test_private_list_bookmarks(self): self.assertEqual(response.context['topics'][0].bookmark, bookmark) @override_djconfig(topics_per_page=1) - def test_private_list(self): + def test_private_list_paginated(self): """ private topic list paginated """ @@ -393,7 +392,7 @@ def test_private_created_list_paginate(self): class TopicPrivateFormTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.user2 = utils.create_user() @@ -456,7 +455,7 @@ def test_private_join(self): class TopicTemplateTagsTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = Category.objects.get(pk=settings.ST_TOPIC_PRIVATE_CATEGORY_PK) self.topic = utils.create_topic(category=self.category, user=self.user) @@ -479,7 +478,7 @@ def test_render_invite_form(self): class TopicPrivateUtilsTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.user2 = utils.create_user() self.category = utils.create_category() @@ -496,4 +495,4 @@ def test_topic_private_notify_access(self): notification = TopicNotification.objects.get(user=private.user, topic=private.topic) self.assertTrue(notification.is_active) self.assertFalse(notification.is_read) - self.assertEqual(notification.comment, comment) \ No newline at end of file + self.assertEqual(notification.comment, comment) diff --git a/spirit/topic/tests.py b/spirit/topic/tests.py index 3416045ff..227b39b4b 100644 --- a/spirit/topic/tests.py +++ b/spirit/topic/tests.py @@ -4,7 +4,6 @@ import datetime from django.test import TestCase, RequestFactory -from django.core.cache import cache from django.core.urlresolvers import reverse from django.utils import timezone from django.core.exceptions import ObjectDoesNotExist @@ -25,7 +24,7 @@ class TopicViewTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() def test_topic_publish(self): @@ -323,7 +322,7 @@ def test_topic_active_view_paginate(self): class TopicFormTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() def test_topic_publish(self): @@ -375,7 +374,7 @@ def test_topic_update(self): class TopicUtilsTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() def test_topic_viewed(self): @@ -403,7 +402,7 @@ def test_topic_viewed(self): class TopicModelsTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.category = utils.create_category() self.topic = utils.create_topic(category=self.category, user=self.user) diff --git a/spirit/topic/unread/tests.py b/spirit/topic/unread/tests.py index 3fad757a0..a470dce97 100644 --- a/spirit/topic/unread/tests.py +++ b/spirit/topic/unread/tests.py @@ -2,8 +2,7 @@ from __future__ import unicode_literals -from django.core.cache import cache -from django.test import TestCase, RequestFactory +from django.test import TestCase from django.core.urlresolvers import reverse from ...core.tests import utils @@ -14,7 +13,7 @@ class TopicUnreadViewTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.user2 = utils.create_user() self.category = utils.create_category() @@ -125,7 +124,7 @@ def test_topic_unread_list_bookmarks(self): class TopicUnreadModelsTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.user2 = utils.create_user() self.category = utils.create_category() diff --git a/spirit/user/auth/tests/tests.py b/spirit/user/auth/tests/tests.py index 61258ef53..926383150 100644 --- a/spirit/user/auth/tests/tests.py +++ b/spirit/user/auth/tests/tests.py @@ -4,7 +4,6 @@ from django.test import TestCase from django.core.urlresolvers import reverse -from django.core.cache import cache from django.contrib.auth import get_user_model from django.core import mail from django.utils.translation import ugettext as _ @@ -24,7 +23,7 @@ class UserViewTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.user2 = utils.create_user() self.category = utils.create_category() @@ -304,7 +303,7 @@ def test_logout_anonymous_redirect(self): class UserFormTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() def test_registration(self): @@ -543,7 +542,7 @@ def test_login_username_invalid(self): class UserBackendTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user(email="foobar@bar.com", password="bar") def test_email_auth_backend(self): diff --git a/spirit/user/tests.py b/spirit/user/tests.py index 64f342a57..cf3f31afc 100644 --- a/spirit/user/tests.py +++ b/spirit/user/tests.py @@ -5,7 +5,6 @@ from django.test import TestCase, RequestFactory from django.core.urlresolvers import reverse -from django.core.cache import cache from django.contrib.auth import get_user_model, HASH_SESSION_KEY from django.core import mail from django.utils.translation import ugettext as _ @@ -30,7 +29,7 @@ class UserViewTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() self.user2 = utils.create_user() self.category = utils.create_category() @@ -442,7 +441,7 @@ def test_profile_email_change(self): class UserFormTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() def test_profile(self): @@ -570,7 +569,7 @@ def test_email_check_case_sensitive(self): class UserModelTest(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() def test_user_superuser(self): """ @@ -597,7 +596,7 @@ def test_user_administrator(self): class UtilsUserTests(TestCase): def setUp(self): - cache.clear() + utils.cache_clear() self.user = utils.create_user() def test_user_activation_token_generator(self):