allura-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jetm...@apache.org
Subject [1/2] git commit: [#7472] ticket:633 Don't trigger add_artifact task by view count change
Date Thu, 21 Aug 2014 13:31:43 GMT
Repository: allura
Updated Branches:
  refs/heads/je/42cc_7472 [created] c10942afc


[#7472] ticket:633 Don't trigger add_artifact task by view count change


Project: http://git-wip-us.apache.org/repos/asf/allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/allura/commit/daed1cab
Tree: http://git-wip-us.apache.org/repos/asf/allura/tree/daed1cab
Diff: http://git-wip-us.apache.org/repos/asf/allura/diff/daed1cab

Branch: refs/heads/je/42cc_7472
Commit: daed1cab93b098f3870cce8b635f588303c38b85
Parents: d54e5d4
Author: Igor Bondarenko <jetmind2@gmail.com>
Authored: Wed Aug 20 16:45:37 2014 +0300
Committer: Igor Bondarenko <jetmind2@gmail.com>
Committed: Thu Aug 21 13:41:11 2014 +0300

----------------------------------------------------------------------
 Allura/allura/model/discuss.py           | 10 +++++
 Allura/allura/model/session.py           | 15 +++++---
 Allura/allura/tests/unit/test_discuss.py | 21 ++++++++++
 Allura/allura/tests/unit/test_session.py | 55 ++++++++++++++++++++++-----
 4 files changed, 86 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/allura/blob/daed1cab/Allura/allura/model/discuss.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/discuss.py b/Allura/allura/model/discuss.py
index 22456e5..8ce86b2 100644
--- a/Allura/allura/model/discuss.py
+++ b/Allura/allura/model/discuss.py
@@ -164,6 +164,16 @@ class Thread(Artifact, ActivityObject):
     first_post = RelationProperty('Post', via='first_post_id')
     ref = RelationProperty('ArtifactReference')
 
+    def should_update_index(self, old_doc, new_doc):
+        """Skip index update if only `num_views` has changed.
+
+        Value of `num_views` is updated whenever user loads thread page.
+        This generates a lot of unnecessary `add_artifacts` tasks.
+        """
+        old_doc.pop('num_views', None)
+        new_doc.pop('num_views', None)
+        return old_doc != new_doc
+
     def __json__(self, limit=None, page=None):
         return dict(
             _id=self._id,

http://git-wip-us.apache.org/repos/asf/allura/blob/daed1cab/Allura/allura/model/session.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/session.py b/Allura/allura/model/session.py
index f364aeb..8e0f551 100644
--- a/Allura/allura/model/session.py
+++ b/Allura/allura/model/session.py
@@ -30,6 +30,12 @@ from allura.tasks import index_tasks
 log = logging.getLogger(__name__)
 
 
+def _needs_update(o):
+    old = dict(state(o).original_document)
+    new = dict(state(o).document)
+    return o.should_update_index(old, new)
+
+
 class ManagedSessionExtension(SessionExtension):
 
     def __init__(self, session):
@@ -76,10 +82,6 @@ class IndexerSessionExtension(ManagedSessionExtension):
         task = tasks.get(action)
         if task:
             if action == 'add':
-                def _needs_update(o):
-                    old = dict(state(o).original_document)
-                    new = dict(state(o).document)
-                    return o.should_update_index(old, new)
                 arg = [o._id for o in obj_list if _needs_update(o)]
             else:
                 arg = [o.index_id() for o in obj_list]
@@ -119,7 +121,8 @@ class ArtifactSessionExtension(ManagedSessionExtension):
             try:
                 arefs = [
                     ArtifactReference.from_artifact(o)
-                    for o in self.objects_added + self.objects_modified]
+                    for o in self.objects_added + self.objects_modified
+                    if _needs_update(o)]
                 for obj in self.objects_added + self.objects_modified:
                     Shortlink.from_artifact(obj)
                 # Flush shortlinks
@@ -134,7 +137,7 @@ class ArtifactSessionExtension(ManagedSessionExtension):
                 g.zarkov_event('modify', extra=obj.index_id())
             for obj in self.objects_deleted:
                 g.zarkov_event('delete', extra=obj.index_id())
-        
+
         super(ArtifactSessionExtension, self).after_flush(obj)
 
     def update_index(self, objects_deleted, arefs):

http://git-wip-us.apache.org/repos/asf/allura/blob/daed1cab/Allura/allura/tests/unit/test_discuss.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/unit/test_discuss.py b/Allura/allura/tests/unit/test_discuss.py
new file mode 100644
index 0000000..4f62c22
--- /dev/null
+++ b/Allura/allura/tests/unit/test_discuss.py
@@ -0,0 +1,21 @@
+import unittest
+from mock import patch
+
+from allura import model as M
+
+
+class TestThread(unittest.TestCase):
+
+    @patch('allura.model.artifact.c')
+    def test_should_update_index(self, c):
+        p = M.Thread()
+        self.assertFalse(p.should_update_index({}, {}))
+        old = {'num_views': 1}
+        new = {'num_views': 2}
+        self.assertFalse(p.should_update_index(old, new))
+        old = {'num_views': 1, 'a': 1}
+        new = {'num_views': 2, 'a': 1}
+        self.assertFalse(p.should_update_index(old, new))
+        old = {'num_views': 1, 'a': 1}
+        new = {'num_views': 2, 'a': 2}
+        self.assertTrue(p.should_update_index(old, new))

http://git-wip-us.apache.org/repos/asf/allura/blob/daed1cab/Allura/allura/tests/unit/test_session.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/unit/test_session.py b/Allura/allura/tests/unit/test_session.py
index 7562b73..dc9711d 100644
--- a/Allura/allura/tests/unit/test_session.py
+++ b/Allura/allura/tests/unit/test_session.py
@@ -20,9 +20,14 @@ import mock
 
 from unittest import TestCase
 
+import allura
 from allura.tests import decorators as td
-from allura.model.session import IndexerSessionExtension, BatchIndexer, \
-                                 substitute_extensions
+from allura.model.session import (
+    IndexerSessionExtension,
+    BatchIndexer,
+    ArtifactSessionExtension,
+    substitute_extensions,
+)
 
 
 def test_extensions_cm():
@@ -69,7 +74,16 @@ def test_extensions_cm_flush_raises():
     assert session._kwargs['extensions'] == []
 
 
-class TestIndexerSessionExtension(TestCase):
+class TestSessionExtension(TestCase):
+
+    def _mock_indexable(self, **kw):
+        m = mock.Mock(**kw)
+        m.__ming__ = mock.MagicMock()
+        m.index_id.return_value = id(m)
+        return m
+
+
+class TestIndexerSessionExtension(TestSessionExtension):
 
     def setUp(self):
         session = mock.Mock()
@@ -79,12 +93,6 @@ class TestIndexerSessionExtension(TestCase):
         self.extension.TASKS = mock.Mock()
         self.extension.TASKS.get.return_value = self.tasks
 
-    def _mock_indexable(self, **kw):
-        m = mock.Mock(**kw)
-        m.__ming__ = mock.MagicMock()
-        m.index_id.return_value = id(m)
-        return m
-
     def test_flush(self):
         added = [self._mock_indexable(_id=i) for i in (1, 2, 3)]
         modified = [self._mock_indexable(_id=i) for i in (4, 5)]
@@ -113,6 +121,35 @@ class TestIndexerSessionExtension(TestCase):
         assert self.tasks['add'].post.call_count == 0
 
 
+class TestArtifactSessionExtension(TestSessionExtension):
+
+    def setUp(self):
+        session = mock.Mock(disable_index=False)
+        self.ExtensionClass = ArtifactSessionExtension
+        self.extension = self.ExtensionClass(session)
+
+    @mock.patch.object(allura.model.index.Shortlink, 'from_artifact')
+    @mock.patch.object(allura.model.index.ArtifactReference, 'from_artifact')
+    @mock.patch('allura.model.session.index_tasks')
+    def test_flush_skips_update(self, index_tasks, ref_fa, shortlink_fa):
+        modified = [self._mock_indexable(_id=i) for i in range(5)]
+        modified[1].should_update_index.return_value = False
+        modified[4].should_update_index.return_value = False
+        ref_fa.side_effect = lambda obj: mock.Mock(_id=obj._id)
+        self.extension.objects_modified = modified
+        self.extension.after_flush()
+        index_tasks.add_artifacts.post.assert_called_once_with([0, 2, 3])
+
+    @mock.patch('allura.model.session.index_tasks')
+    def test_flush_skips_task_if_all_objects_filtered_out(self, index_tasks):
+        modified = [self._mock_indexable(_id=i) for i in range(5)]
+        for m in modified:
+            m.should_update_index.return_value = False
+        self.extension.objects_modified = modified
+        self.extension.after_flush()
+        assert index_tasks.add_artifacts.post.call_count == 0
+
+
 class TestBatchIndexer(TestCase):
 
     def setUp(self):


Mime
View raw message