You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by tv...@apache.org on 2013/11/08 05:21:16 UTC
[6/6] git commit: [#6694] Refactoring and cleanup
[#6694] Refactoring and cleanup
Signed-off-by: Tim Van Steenburgh <tv...@gmail.com>
Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/7eb73bcc
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/7eb73bcc
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/7eb73bcc
Branch: refs/heads/tv/6694
Commit: 7eb73bcc5d76f9120a8c692df93e7a5fd86391dd
Parents: de17be5
Author: Tim Van Steenburgh <tv...@gmail.com>
Authored: Fri Nov 8 04:20:30 2013 +0000
Committer: Tim Van Steenburgh <tv...@gmail.com>
Committed: Fri Nov 8 04:20:30 2013 +0000
----------------------------------------------------------------------
.../user_profile/templates/send_message.html | 3 +-
.../ext/user_profile/templates/user_index.html | 37 ++++++------
Allura/allura/ext/user_profile/user_main.py | 59 +++++++++++---------
Allura/allura/lib/app_globals.py | 16 ++++++
Allura/allura/model/auth.py | 49 +++++++++++-----
.../tests/functional/test_user_profile.py | 19 ++++---
Allura/allura/tests/model/test_auth.py | 17 +++---
7 files changed, 125 insertions(+), 75 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7eb73bcc/Allura/allura/ext/user_profile/templates/send_message.html
----------------------------------------------------------------------
diff --git a/Allura/allura/ext/user_profile/templates/send_message.html b/Allura/allura/ext/user_profile/templates/send_message.html
index 44b64b0..eb99f41 100644
--- a/Allura/allura/ext/user_profile/templates/send_message.html
+++ b/Allura/allura/ext/user_profile/templates/send_message.html
@@ -36,7 +36,8 @@
{%if not expire_time%}
{{c.form.display(method = 'POST', action='send_user_message', user=user)}}
{% else %}
- <h2>Sorry, you can send an email after {{expire_time}}</h2>
+ <h2>Sorry, messaging is rate-limited.</h2>
+ <p>You can send another message in: {{expire_time}}</p>
{% endif %}
</div>
{% endblock %}
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7eb73bcc/Allura/allura/ext/user_profile/templates/user_index.html
----------------------------------------------------------------------
diff --git a/Allura/allura/ext/user_profile/templates/user_index.html b/Allura/allura/ext/user_profile/templates/user_index.html
index 88c54c7..e3f2833 100644
--- a/Allura/allura/ext/user_profile/templates/user_index.html
+++ b/Allura/allura/ext/user_profile/templates/user_index.html
@@ -104,7 +104,7 @@
<div class="grid-18">
{{user.get_pref('display_name')}}'s account(s):
<ul>
- {% for i in user.get_pref('socialnetworks') %}
+ {% for i in user.get_pref('socialnetworks') %}
<li>{{i.socialnetwork}}: <a href="{{i.accounturl}}">{{i.accounturl}}</a></li>
{% endfor %}
</ul>
@@ -118,7 +118,7 @@
<div class="grid-18">
{{user.get_pref('display_name')}}'s website(s):
<ul>
- {% for i in user.get_pref('webpages') %}
+ {% for i in user.get_pref('webpages') %}
<li><a href="{{i}}">{{i}}</a></li>
{% endfor %}
</ul>
@@ -132,7 +132,7 @@
<div class="grid-18">
{{user.get_pref('display_name')}}'s telephone number(s):
<ul>
- {% for i in user.get_pref('telnumbers') %}
+ {% for i in user.get_pref('telnumbers') %}
<li>{{i}}</li>
{% endfor %}
</ul>
@@ -153,14 +153,14 @@
{% if c.user.get_pref('timezone') %}
<div class="grid-18" id="timeslotsconverted" style="visibility: visible; display:none;">
- {{user.get_pref('display_name')}}'s availability time-slots.
+ {{user.get_pref('display_name')}}'s availability time-slots.
<div style="float:right;">
See timeslots in:
- <a href="JavaScript:void(0);" onclick="changeTimezone('utc')">UTC</a> |
- <a href="JavaScript:void(0);" onclick="changeTimezone('local')">
+ <a href="JavaScript:void(0);" onclick="changeTimezone('utc')">UTC</a> |
+ <a href="JavaScript:void(0);" onclick="changeTimezone('local')">
{{user.get_pref('display_name')}}'s local time
</a> |
- <b>Your local time</b>
+ <b>Your local time</b>
</div>
<ul>
{% for i in user.get_localized_availability(c.user.get_pref('timezone')) %}
@@ -171,13 +171,13 @@
{% endif %}
<div class="grid-18" id="timeslotsutc" style="visibility: visible; display:block;">
- {{user.get_pref('display_name')}}'s availability time-slots.
+ {{user.get_pref('display_name')}}'s availability time-slots.
<div style="float:right;">
See timeslots in:
- <b>UTC</b> |
- <a href="JavaScript:void(0);" onclick="changeTimezone('local')">
+ <b>UTC</b> |
+ <a href="JavaScript:void(0);" onclick="changeTimezone('local')">
{{user.get_pref('display_name')}}'s local time
- </a>
+ </a>
{% if c.user.get_pref('timezone') %} |
<a href="JavaScript:void(0);" onclick="changeTimezone('converted')">
Your local time
@@ -192,13 +192,13 @@
</div>
<div class="grid-18" id="timeslotslocal" style="visibility: visible; display:none;">
- {{user.get_pref('display_name')}}'s availability time-slots.
+ {{user.get_pref('display_name')}}'s availability time-slots.
<div style="float:right;">
See timeslots in:
- <a href="JavaScript:void(0);" onclick="changeTimezone('utc')">UTC</a> |
- <b>
+ <a href="JavaScript:void(0);" onclick="changeTimezone('utc')">UTC</a> |
+ <b>
{{user.get_pref('display_name')}}'s local time
- </b>
+ </b>
{% if c.user.get_pref('timezone') %} |
<a href="JavaScript:void(0);" onclick="changeTimezone('converted')">
Your local time
@@ -228,11 +228,12 @@
</div>
</div>
{% endif %}
- {% if has_email %}
+
+ {% if user.get_pref('email_address') and c.user.get_pref('email_address') %}
<div class="grid-24">
- <b><a href="send_message">Send me a message</a></b>
+ <b><a href="send_message">Send me a message</a></b>
</div>
- {% endif %}
+ {% endif %}
</div><!-- end of Personal data section -->
<div class="grid-24">
<b>Current {{user.get_pref('display_name')}}'s skills list</b>
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7eb73bcc/Allura/allura/ext/user_profile/user_main.py
----------------------------------------------------------------------
diff --git a/Allura/allura/ext/user_profile/user_main.py b/Allura/allura/ext/user_profile/user_main.py
index 76246c7..0dbfee1 100644
--- a/Allura/allura/ext/user_profile/user_main.py
+++ b/Allura/allura/ext/user_profile/user_main.py
@@ -92,14 +92,28 @@ class UserProfileController(BaseController, FeedController):
def _check_security(self):
require_access(c.project, 'read')
+ def _check_can_message(self, from_user, to_user):
+ if from_user is User.anonymous():
+ flash('You must be logged in to send user messages.', 'info')
+ redirect(request.referer)
+
+ if not (from_user and from_user.get_pref('email_address')):
+ flash('In order to send messages, you must have an email address '
+ 'associated with your account.', 'info')
+ redirect(request.referer)
+
+ if not (to_user and to_user.get_pref('email_address')):
+ flash('This user can not receive messages because they do not have '
+ 'an email address associated with their account.', 'info')
+ redirect(request.referer)
+
@expose('jinja:allura.ext.user_profile:templates/user_index.html')
def index(self, **kw):
user = c.project.user_project_of
if not user:
raise exc.HTTPNotFound()
provider = AuthenticationProvider.get(request)
- has_email = c.user.get_pref('email_address') is not None
- return dict(user=user, reg_date=provider.user_registration_date(user), has_email=has_email)
+ return dict(user=user, reg_date=provider.user_registration_date(user))
def get_feed(self, project, app, user):
"""Return a :class:`allura.controllers.feed.FeedArgs` object describing
@@ -116,41 +130,34 @@ class UserProfileController(BaseController, FeedController):
@expose('jinja:allura.ext.user_profile:templates/send_message.html')
def send_message(self):
- user = c.project.user_project_of
- both_have_emails = user and user.get_pref('email_address') and c.user.get_pref('email_address')
- if not both_have_emails:
- raise exc.HTTPNotFound()
+ """Render form for sending a message to another user.
- time_interval = config['user_message.time_interval']
- max_messages = config['user_message.max_messages']
- expire_time = None
+ """
+ self._check_can_message(c.user, c.project.user_project_of)
- if not c.user.check_send_emails_times(time_interval, max_messages):
- expire_seconds = c.user.send_emails_times[0] + timedelta(seconds=int(time_interval)) - datetime.utcnow()
- h, remainder = divmod(expire_seconds.total_seconds(), 3600)
- m, s = divmod(remainder, 60)
- expire_time = '%s:%s:%s' % (int(h), int(m), int(s))
+ delay = c.user.time_to_next_user_message()
+ expire_time = str(delay) if delay else None
c.form = F.send_message
- return dict(user=user, expire_time=expire_time)
+ return dict(user=c.project.user_project_of, expire_time=expire_time)
@require_post()
@expose()
@validate(dict(subject=validators.NotEmpty,
message=validators.NotEmpty))
def send_user_message(self, subject='', message='', cc=None):
- user = c.project.user_project_of
- both_have_emails = user and user.get_pref('email_address') and c.user.get_pref('email_address')
- if not both_have_emails:
- raise exc.HTTPNotFound()
+ """Handle POST for sending a message to another user.
+
+ """
+ self._check_can_message(c.user, c.project.user_project_of)
- time_interval = config['user_message.time_interval']
- max_messages = config['user_message.max_messages']
if cc:
cc = c.user.get_pref('email_address')
- user = c.project.user_project_of
- if c.user.check_send_emails_times(time_interval, max_messages):
- c.user.send_user_message(user, subject, message, cc)
+ if c.user.can_send_user_message():
+ c.user.send_user_message(c.project.user_project_of, subject, message, cc)
+ flash("Message sent.")
else:
- flash("You can't send more than %s messages per %s seconds" % (max_messages, time_interval), 'error')
- return redirect(user.url())
+ flash("You can't send more than %i messages per %i seconds" % (
+ c.user.user_message_max_messages,
+ c.user.user_message_time_interval), 'error')
+ return redirect(c.project.user_project_of.url())
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7eb73bcc/Allura/allura/lib/app_globals.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/app_globals.py b/Allura/allura/lib/app_globals.py
index 14fa5a1..88cb43b 100644
--- a/Allura/allura/lib/app_globals.py
+++ b/Allura/allura/lib/app_globals.py
@@ -433,6 +433,22 @@ class Globals(object):
return asbool(config.get('debug')) == False
@LazyProperty
+ def user_message_time_interval(self):
+ """The rolling window of time (in seconds) during which no more than
+ :meth:`user_message_max_messages` may be sent by any one user.
+
+ """
+ return int(config.get('user_message.time_interval', 3600))
+
+ @LazyProperty
+ def user_message_max_messages(self):
+ """The number of user messages that can be sent within
+ meth:`user_message_time_interval` before rate-limiting is enforced.
+
+ """
+ return int(config.get('user_message.max_messages', 20))
+
+ @LazyProperty
def server_name(self):
p1 = Popen(['hostname', '-s'], stdout=PIPE)
server_name = p1.communicate()[0].strip()
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7eb73bcc/Allura/allura/model/auth.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/auth.py b/Allura/allura/model/auth.py
index 228c3ed..a4be2cd 100644
--- a/Allura/allura/model/auth.py
+++ b/Allura/allura/model/auth.py
@@ -27,13 +27,12 @@ from email import header
from hashlib import sha256
import uuid
from pytz import timezone
-from datetime import timedelta, date, datetime, time
+from datetime import timedelta, datetime, time
import iso8601
import pymongo
from pylons import tmpl_context as c, app_globals as g
from pylons import request
-from tg import flash
from ming import schema as S
from ming import Field, collection
@@ -334,7 +333,7 @@ class User(MappedClass, ActivityNode, ActivityObject):
end_time=dict(h=int, m=int))])
localization=FieldProperty(dict(city=str,country=str))
timezone=FieldProperty(str)
- send_emails_times=FieldProperty([S.DateTime])
+ sent_user_message_times=FieldProperty([S.DateTime])
inactiveperiod=FieldProperty([dict(
start_date=S.DateTime,
end_date=S.DateTime)])
@@ -354,26 +353,46 @@ class User(MappedClass, ActivityNode, ActivityObject):
#Statistics
stats_id = FieldProperty(S.ObjectId, if_missing=None)
- def check_send_emails_times(self, time_interval, max_messages):
- times = []
- time_interval = timedelta(seconds=int(time_interval))
- for t in self.send_emails_times:
- if t + time_interval > datetime.utcnow():
- times.append(t)
- self.send_emails_times = times
- return len(times) < int(max_messages)
+ def can_send_user_message(self):
+ """Return true if User is permitted to send a mesage to another user.
+
+ Returns False if User has exceeded the user message rate limit, in
+ which case another message may not be sent until sufficient time has
+ passed to clear the limit.
+
+ """
+ now = datetime.utcnow()
+ time_interval = timedelta(seconds=g.user_message_time_interval)
+ self.sent_user_message_times = [t for t in self.sent_user_message_times
+ if t + time_interval > now]
+ return len(self.sent_user_message_times) < g.user_message_max_messages
+
+ def time_to_next_user_message(self):
+ """Return a timedelta of the time remaining before this user can send
+ another user message.
+
+ Returns zero if user message can be sent immediately.
+
+ """
+ if self.can_send_user_message():
+ return 0
+ return self.sent_user_message_times[0] + \
+ timedelta(seconds=g.user_message_time_interval) - \
+ datetime.utcnow()
def send_user_message(self, user, subject, message, cc):
+ """Send a user message (email) to ``user``.
+
+ """
allura.tasks.mail_tasks.sendsimplemail.post(
toaddr=user.get_pref('email_address'),
- fromaddr=c.user.get_pref('email_address'),
- reply_to=c.user.get_pref('email_address'),
+ fromaddr=self.get_pref('email_address'),
+ reply_to=self.get_pref('email_address'),
message_id=h.gen_message_id(),
subject=subject,
text=message,
cc=cc)
- self.send_emails_times.append(datetime.utcnow())
- flash("email sent")
+ self.sent_user_message_times.append(datetime.utcnow())
@property
def activity_name(self):
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7eb73bcc/Allura/allura/tests/functional/test_user_profile.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/functional/test_user_profile.py b/Allura/allura/tests/functional/test_user_profile.py
index 2727a07..7b17d28 100644
--- a/Allura/allura/tests/functional/test_user_profile.py
+++ b/Allura/allura/tests/functional/test_user_profile.py
@@ -18,6 +18,7 @@
from formencode.variabledecode import variable_encode
import mock
+from nose.tools import assert_equal
from allura.model import Project, User
from allura.tests import decorators as td
@@ -64,12 +65,12 @@ class TestUserProfile(TestController):
@td.with_user_project('test-user')
@mock.patch('allura.tasks.mail_tasks.sendsimplemail')
@mock.patch('allura.lib.helpers.gen_message_id')
- @mock.patch('allura.model.User.check_send_emails_times')
+ @mock.patch('allura.model.User.can_send_user_message')
def test_send_message(self, check, gen_message_id, sendsimplemail):
check.return_value = True
gen_message_id.return_value = 'id'
test_user = User.by_username('test-user')
- test_user.set_pref('email_address', 'test-user@sf.net')
+ test_user.set_pref('email_address', 'test-user@example.com')
response = self.app.get('/u/test-user/profile/send_message', status=200)
assert '<b>From:</b> "Test Admin" <test-admin@users.localhost>' in response
self.app.post('/u/test-user/profile/send_user_message',
@@ -101,23 +102,27 @@ class TestUserProfile(TestController):
check.return_value = False
response = self.app.get('/u/test-user/profile/send_message', status=200)
- assert 'Sorry, you can send an email after' in response
+ assert 'Sorry, messaging is rate-limited' in response
@td.with_user_project('test-user')
def test_send_message_for_anonymous(self):
- self.app.get('/u/test-user/profile/send_message',
+ r = self.app.get('/u/test-user/profile/send_message',
extra_environ={'username': '*anonymous'},
- status=404)
+ status=302)
+ assert 'You must be logged in to send user messages.' in self.webflash(r)
- self.app.post('/u/test-user/profile/send_user_message',
+ r = self.app.post('/u/test-user/profile/send_user_message',
params={'subject': 'test subject',
'message': 'test message',
'cc': 'on'},
extra_environ={'username': '*anonymous'},
- status=404)
+ status=302)
+ assert 'You must be logged in to send user messages.' in self.webflash(r)
@td.with_user_project('test-user')
def test_link_to_send_message_form(self):
+ User.by_username('test-admin').set_pref('email_address', 'admin@example.com')
+ User.by_username('test-user').set_pref('email_address', 'user@example.com')
r = self.app.get('/u/test-user/profile',
status=200)
assert '<a href="send_message">Send me a message</a>' in r
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7eb73bcc/Allura/allura/tests/model/test_auth.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/model/test_auth.py b/Allura/allura/tests/model/test_auth.py
index fe80fac..a2bb2da 100644
--- a/Allura/allura/tests/model/test_auth.py
+++ b/Allura/allura/tests/model/test_auth.py
@@ -24,8 +24,8 @@ from nose.tools import with_setup, assert_equal
from pylons import tmpl_context as c, app_globals as g
from webob import Request
from datetime import datetime, timedelta
-import mock
+from mock import patch
from pymongo.errors import DuplicateKeyError
from ming.orm.ormsession import ThreadLocalORMSession
@@ -33,7 +33,6 @@ from allura import model as M
from allura.lib import plugin
from allura.tests import decorators as td
from alluratest.controller import setup_basic_test, setup_global_objects
-from allura.tasks import mail_tasks
def setUp():
@@ -228,13 +227,15 @@ def test_user_projects_by_role():
assert_equal(set(p.shortname for p in c.user.my_projects()), set(['test', 'test2', 'u/test-admin', 'adobe-1', '--init--']))
assert_equal(set(p.shortname for p in c.user.my_projects('Admin')), set(['test', 'u/test-admin', 'adobe-1', '--init--']))
-
-def test_check_send_emails_times():
+@patch.object(g, 'user_message_max_messages', 3)
+def test_check_sent_user_message_times():
user1 = M.User.register(dict(username='test-user'), make_project=False)
time1 = datetime.utcnow() - timedelta(minutes=30)
time2 = datetime.utcnow() - timedelta(minutes=45)
time3 = datetime.utcnow() - timedelta(minutes=70)
- user1.send_emails_times = [time1, time2, time3]
- assert not user1.check_send_emails_times(3600, 1)
- assert_equal(len(user1.send_emails_times), 2)
- assert user1.check_send_emails_times(3600, 3)
+ user1.sent_user_message_times = [time1, time2, time3]
+ assert user1.can_send_user_message()
+ assert_equal(len(user1.sent_user_message_times), 2)
+ user1.sent_user_message_times.append(
+ datetime.utcnow() - timedelta(minutes=15))
+ assert not user1.can_send_user_message()