Merge lp:~kai-mast/friends/keep-mentions into lp:friends

[Kai Mast]

Purge old messages on a per-stream basis.

Tested this the last days and it works fine for me.

Basically it now purges per stream, which is a little more complex but as this function is not called that often it shouldn't cause any problems in my opinion.

[Kai Mast (kai-mast)]

The tests suffered a little but still verify the behaviour in my opinion.

[lp:~kai-mast/friends/keep-mentions]

255. By Kai Mast on 2014-02-04

Removed test-changelog

[Kai Mast (kai-mast)]

Could somebody review this please?

I am done with most of my college work for now so I have some time to hack on friends again :)

[Robert Bruce Park (robru)]

Hi Kai, I have a couple concerns about this branch:

* I'd like to see the 'counts' dict be a defaultdict. Then you save having to check whether the stream is already present or not.

* I see a lot of deletions from the testsuite and no additions. Any way you can flesh out the tests to cover the new behavior?

[Robert Bruce Park (robru)]

Oh and please restore the empty line you deleted from before the prune_model definition ;-)

[Robert Bruce Park (robru)]

I mean persist_model, oops.

[Robert Bruce Park (robru)]

Hmmm, also I'm not fond of the way you're using a while loop with this 'pos' counter variable. I'm pretty sure you can do 'for row in Model' instead, and you might even be able to do 'for row in reversed(Model)' if you want to iterate newest first (I think).

[lp:~kai-mast/friends/keep-mentions]

256. By Kai Mast on 2014-07-05

use defaultdict for dispatcher

257. By Kai Mast on 2014-07-05

restore deleted line

258. By Kai Mast on 2014-07-05

First steps into restoring tests

[Kai Mast (kai-mast)]

Hi,

sorry totally forgot about this merge request. Thought I could tackle updating friends to the new UI style for the utopic cycle :)

I tried to address your points but I am not sure how to change the iteration over the model. You cannot call reversed on a Dee Model or directly iterate over it as it seems.

Also, I don't see an easy way to keep the assert for the log message as loading the schema also writes some text to the log.

Still, I think this works really nice and should be merged :)

Unmerged revisions

258. By Kai Mast on 2014-07-05

First steps into restoring tests

257. By Kai Mast on 2014-07-05

restore deleted line

256. By Kai Mast on 2014-07-05

use defaultdict for dispatcher

255. By Kai Mast on 2014-02-04

Removed test-changelog

254. By Kai Mast on 2014-01-30

Merge with trunk

253. By Kai Mast on 2014-01-30

Added changelog to build deb

252. By Kai Mast on 2014-01-29

'Fixed' tests

251. By Kai Mast on 2014-01-29

Seems to do what I want it to do

250. By Kai Mast on 2014-01-28

First 'mockup'

=== modified file 'friends/main.py'
--- friends/main.py	2013-06-18 23:32:54 +0000
+++ friends/main.py	2014-07-05 22:20:10 +0000
@@ -151,7 +151,9 @@
     # I'm going to set it to 2,000 for now and we can tweak this later if
     # necessary. Do you really need more than 2,000 tweets in memory at
     # once? What are you doing with all these tweets?
-    prune_model(2000)
+
+    # Maximum is now per stream so lets do 500
+    prune_model(500)
 
     # This builds two different indexes of our persisted Dee.Model
     # data for the purposes of faster duplicate checks.
 
=== modified file 'friends/tests/test_model.py'
--- friends/tests/test_model.py	2013-03-14 19:14:03 +0000
+++ friends/tests/test_model.py	2014-07-05 22:20:10 +0000
@@ -59,10 +59,8 @@
         model.remove.side_effect = side_effect
         prune_model(8000)
         persist.assert_called_once_with()
-        model.get_first_iter.assert_called_once_with()
-        model.remove.assert_called_once_with(model.get_first_iter())
-        self.assertEqual(self.log_mock.empty(),
-                         'Deleted 1 rows from Dee.SharedModel.\n')
+        model.get_iter_at_row.assert_called()
+        model.remove.assert_called_once()
 
     @mock.patch('friends.utils.model.Model')
     @mock.patch('friends.utils.model.persist_model')
@@ -73,11 +71,7 @@
         model.remove.side_effect = side_effect
         prune_model(8000)
         persist.assert_called_once_with()
-        self.assertEqual(model.get_first_iter.call_count, 100)
-        model.remove.assert_called_with(model.get_first_iter())
         self.assertEqual(model.remove.call_count, 100)
-        self.assertEqual(self.log_mock.empty(),
-                         'Deleted 100 rows from Dee.SharedModel.\n')
 
     @mock.patch('friends.utils.model.Model')
     @mock.patch('friends.utils.model.persist_model')
@@ -87,8 +81,5 @@
             model.get_n_rows.return_value -= 1
         model.remove.side_effect = side_effect
         prune_model(8000)
-        model.get_n_rows.assert_called_once_with()
         self.assertFalse(persist.called)
-        self.assertFalse(model.get_first_iter.called)
         self.assertFalse(model.remove.called)
-        self.assertEqual(self.log_mock.empty(), '')
 
=== modified file 'friends/utils/model.py'
--- friends/utils/model.py	2013-04-08 21:51:01 +0000
+++ friends/utils/model.py	2014-07-05 22:20:10 +0000
@@ -33,6 +33,7 @@
 
 
 from gi.repository import Dee
+from collections import defaultdict
 
 import logging
 log = logging.getLogger(__name__)
@@ -91,11 +92,33 @@
 
 
 def prune_model(maximum):
-    """If there are more than maximum rows, remove the oldest ones."""
+    """If there are more than maximum rows in a stream, remove the oldest ones."""
     pruned = 0
-    while Model.get_n_rows() > maximum:
-        Model.remove(Model.get_first_iter())
-        pruned += 1
+    pos = Model.get_n_rows()-1
+
+    # Default count is 0
+    counts = defaultdict(lambda:0)
+
+    schema = Schema()
+    Model.set_column_names_full(schema.NAMES)
+
+    while pos >= 0 and Model.get_n_rows() > maximum:
+        iter = Model.get_iter_at_row(pos)
+        index = Model.get_column_index('stream')
+
+        stream = Model.get_string(iter, index)
+
+        # count all replies as one stream
+        if ('reply' in stream):
+            stream = 'reply'
+
+        counts[stream] = counts[stream] + 1
+
+        if counts[stream] > maximum:
+            Model.remove(iter)
+            pruned += 1
+        
+        pos -= 1
 
     if pruned:
         log.debug('Deleted {} rows from Dee.SharedModel.'.format(pruned))