Mailing-List: contact allura-commits-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: allura-dev@incubator.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: brondsem@apache.org
To: allura-commits@incubator.apache.org
Date: Thu, 30 May 2013 14:25:09 -0000
Message-Id: <cda5bcef8b0d42c0abcade241e018de5@git.apache.org>
In-Reply-To: <c0aa5f2d0e294a2dbc4c73a5d1985d7d@git.apache.org>
References: <c0aa5f2d0e294a2dbc4c73a5d1985d7d@git.apache.org>
Subject: [2/4] git commit: [#4994] Fixed bug in direct notification
 accumulation that could cause missing notifications

[#4994] Fixed bug in direct notification accumulation that could cause missing notifications

Signed-off-by: Cory Johns <cjohns@slashdotmedia.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/9bc64c9b
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/9bc64c9b
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/9bc64c9b

Branch: refs/heads/master
Commit: 9bc64c9bf99bc272e7b9862c8ff50432e2eb02db
Parents: aef20b8
Author: Cory Johns <cjohns@slashdotmedia.com>
Authored: Wed May 29 16:38:28 2013 +0000
Committer: Cory Johns <cjohns@slashdotmedia.com>
Committed: Wed May 29 23:10:23 2013 +0000

----------------------------------------------------------------------
 Allura/allura/model/notification.py            |    2 +-
 Allura/allura/tests/model/test_notification.py |   40 +++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/9bc64c9b/Allura/allura/model/notification.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/notification.py b/Allura/allura/model/notification.py
index 61a5657..1ec530a 100644
--- a/Allura/allura/model/notification.py
+++ b/Allura/allura/model/notification.py
@@ -549,7 +549,7 @@ class Mailbox(MappedClass):
             for (subject, from_address, reply_to_address, author_id), ns in ngroups.iteritems():
                 try:
                     if len(ns) == 1:
-                        n.send_direct(self.user_id)
+                        ns[0].send_direct(self.user_id)
                     else:
                         Notification.send_digest(
                             self.user_id, from_address, subject, ns, reply_to_address)

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/9bc64c9b/Allura/allura/tests/model/test_notification.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/model/test_notification.py b/Allura/allura/tests/model/test_notification.py
index a894e20..6cdfbbb 100644
--- a/Allura/allura/tests/model/test_notification.py
+++ b/Allura/allura/tests/model/test_notification.py
@@ -17,10 +17,13 @@
 
 import unittest
 from datetime import timedelta
+import collections
 
 from pylons import tmpl_context as c, app_globals as g
 from nose.tools import assert_equal, assert_in
 from ming.orm import ThreadLocalORMSession
+import mock
+import bson
 
 from alluratest.controller import setup_basic_test, setup_global_objects, REGISTRY
 from allura import model as M
@@ -258,6 +261,43 @@ class TestSubscriptionTypes(unittest.TestCase):
     def _post_notification(self, text=None):
         return M.Notification.post(self.pg, 'metadata', text=text)
 
+    @mock.patch('allura.model.notification.defaultdict')
+    @mock.patch('allura.model.notification.Notification')
+    def test_direct_accumulation(self, mocked_notification, mocked_defaultdict):
+        class OrderedDefaultDict(collections.OrderedDict):
+            def __init__(self, factory=list, *a, **kw):
+                self._factory = factory
+                super(OrderedDefaultDict, self).__init__(*a, **kw)
+
+            def __getitem__(self, key):
+                if key not in self:
+                    value = self[key] = self._factory()
+                else:
+                    value = super(OrderedDefaultDict, self).__getitem__(key)
+                return value
+
+        notifications = mocked_notification.query.find.return_value.all.return_value = [
+                mock.Mock(_id='n0', topic='metadata', subject='s1', from_address='f1', reply_to_address='rt1', author_id='a1'),
+                mock.Mock(_id='n1', topic='metadata', subject='s2', from_address='f2', reply_to_address='rt2', author_id='a2'),
+                mock.Mock(_id='n2', topic='metadata', subject='s2', from_address='f2', reply_to_address='rt2', author_id='a2'),
+                mock.Mock(_id='n3', topic='message', subject='s3', from_address='f3', reply_to_address='rt3', author_id='a3'),
+                mock.Mock(_id='n4', topic='message', subject='s3', from_address='f3', reply_to_address='rt3', author_id='a3'),
+            ]
+        mocked_defaultdict.side_effect = OrderedDefaultDict
+
+        u0 = bson.ObjectId()
+        mbox = M.Mailbox(type='direct', user_id=u0, queue=['n0', 'n1', 'n2', 'n3', 'n4'])
+        mbox.fire('now')
+
+        mocked_notification.query.find.assert_called_once_with({'_id': {'$in': ['n0', 'n1', 'n2', 'n3', 'n4']}})
+        # first notification should be sent direct, as its key values are unique
+        notifications[0].send_direct.assert_called_once_with(u0)
+        # next two notifications should be sent as a digest as they have matching key values
+        mocked_notification.send_digest.assert_called_once_with(u0, 'f2', 's2', [notifications[1], notifications[2]], 'rt2')
+        # final two should be sent direct even though they matching keys, as they are messages
+        notifications[3].send_direct.assert_called_once_with(u0)
+        notifications[4].send_direct.assert_called_once_with(u0)
+
 def _clear_subscriptions():
         M.Mailbox.query.remove({})