Merge lp:~robru/friends/csv-model-schema into lp:friends

[Robert Bruce Park]

Unify model schema definition into a CSV.

The holy grail ;-)

Define the model schema in one unified location, and parse it in from both model.py and service.vala. This way schema changes are much less error-prone.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:179
http://jenkins.qa.ubuntu.com/job/friends-ci/19/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/friends-raring-amd64-ci/19

Click here to trigger a rebuild:
http://s-jenkins:8080/job/friends-ci/19/rebuild

review: Approve (continuous-integration)

[Ken VanDine (ken-vandine)]

You should try to read from the source tree before trying to read the installed file.

[Ken VanDine (ken-vandine)]

Whoops, forgot to set it to needs fixing.

review: Needs Fixing

[lp:~robru/friends/csv-model-schema]

180. By Robert Bruce Park on 2013-04-05

Stop parsing CSV at import-time.

This commit makes it a lot easier to force the reading of the schema
from the source tree before installing Friends.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:180
http://jenkins.qa.ubuntu.com/job/friends-ci/20/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/friends-raring-amd64-ci/20

Click here to trigger a rebuild:
http://s-jenkins:8080/job/friends-ci/20/rebuild

review: Approve (continuous-integration)

[lp:~robru/friends/csv-model-schema]

181. By Robert Bruce Park on 2013-04-05

Unify some import logic.

182. By Robert Bruce Park on 2013-04-05

Make a comment more explicit.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:181
http://jenkins.qa.ubuntu.com/job/friends-ci/21/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/friends-raring-amd64-ci/21

Click here to trigger a rebuild:
http://s-jenkins:8080/job/friends-ci/21/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:182
http://jenkins.qa.ubuntu.com/job/friends-ci/22/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/friends-raring-amd64-ci/22

Click here to trigger a rebuild:
http://s-jenkins:8080/job/friends-ci/22/rebuild

review: Approve (continuous-integration)

[lp:~robru/friends/csv-model-schema]

183. By Robert Bruce Park on 2013-04-05

Regex is overkill.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:183
http://jenkins.qa.ubuntu.com/job/friends-ci/23/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/friends-raring-amd64-ci/23

Click here to trigger a rebuild:
http://s-jenkins:8080/job/friends-ci/23/rebuild

review: Approve (continuous-integration)

[lp:~robru/friends/csv-model-schema]

184. By Robert Bruce Park on 2013-04-05

Rebase on trunk.

185. By Robert Bruce Park on 2013-04-05

Flush revision queue every 5 minutes.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:184
http://jenkins.qa.ubuntu.com/job/friends-ci/24/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/friends-raring-amd64-ci/24

Click here to trigger a rebuild:
http://s-jenkins:8080/job/friends-ci/24/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:185
http://jenkins.qa.ubuntu.com/job/friends-ci/25/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/friends-raring-amd64-ci/25

Click here to trigger a rebuild:
http://s-jenkins:8080/job/friends-ci/25/rebuild

review: Approve (continuous-integration)

[Ken VanDine (ken-vandine)]

Looks good

review: Approve

=== added file 'data/model-schema.csv'
--- data/model-schema.csv	1970-01-01 00:00:00 +0000
+++ data/model-schema.csv	2013-04-05 18:49:21 +0000
@@ -0,0 +1,23 @@
+protocol,s
+account_id,t
+message_id,s
+stream,s
+sender,s
+sender_id,s
+sender_nick,s
+from_me,b
+timestamp,s
+message,s
+icon_uri,s
+url,s
+likes,t
+liked,b
+link_picture,s
+link_name,s
+link_url,s
+link_desc,s
+link_caption,s
+link_icon,s
+location,s
+latitude,d
+longitude,d
 
=== modified file 'debian/friends.install'
--- debian/friends.install	2013-02-07 22:05:09 +0000
+++ debian/friends.install	2013-04-05 18:49:21 +0000
@@ -1,3 +1,4 @@
 usr/bin/friends-service
 usr/share/dbus-1/services/com.canonical.Friends.Service.service
+usr/share/friends/model-schema.csv
 usr/share/glib-2.0/schemas/com.canonical.friends.gschema.xml
 
=== modified file 'friends/tests/mocks.py'
--- friends/tests/mocks.py	2013-03-14 19:44:13 +0000
+++ friends/tests/mocks.py	2013-04-05 18:49:21 +0000
@@ -40,9 +40,16 @@
 from urllib.parse import urlsplit
 from gi.repository import Dee
 
+# By default, Schema.FILES will look for the system-installed schema
+# file first, and then failing that will look for the one in the
+# source tree, for performance reasons. During testing though, we want
+# to look at the source tree first, so we reverse the list here.
+from friends.utils.model import Schema
+Schema.FILES = list(reversed(Schema.FILES))
+SCHEMA = Schema()
+
 from friends.utils.base import Base
 from friends.utils.logging import LOG_FORMAT
-from friends.utils.model import COLUMN_TYPES
 
 
 try:
@@ -58,7 +65,7 @@
 # Create a test model that will not interfere with the user's environment.
 # We'll use this object as a mock of the real model.
 TestModel = Dee.SharedModel.new('com.canonical.Friends.TestSharedModel')
-TestModel.set_schema_full(COLUMN_TYPES)
+TestModel.set_schema_full(SCHEMA.TYPES)
 
 
 @mock.patch('friends.utils.http._soup', mock.Mock())
 
=== modified file 'friends/tests/test_protocols.py'
--- friends/tests/test_protocols.py	2013-04-04 19:44:01 +0000
+++ friends/tests/test_protocols.py	2013-04-05 18:49:21 +0000
@@ -26,10 +26,10 @@
 
 from friends.protocols.flickr import Flickr
 from friends.protocols.twitter import Twitter
-from friends.tests.mocks import FakeAccount, LogMock, TestModel, mock
+from friends.tests.mocks import SCHEMA, FakeAccount, LogMock, TestModel, mock
 from friends.utils.base import Base, feature, linkify_string
 from friends.utils.manager import ProtocolManager
-from friends.utils.model import COLUMN_INDICES, Model
+from friends.utils.model import Model
 
 
 class TestProtocolManager(unittest.TestCase):
@@ -282,7 +282,7 @@
         self.assertEqual(1, TestModel.get_n_rows())
         # The first published message wins.
         row = TestModel.get_row(0)
-        self.assertEqual(row[COLUMN_INDICES['message']], 'hello, @jimmy')
+        self.assertEqual(row[SCHEMA.INDICES['message']], 'hello, @jimmy')
 
     @mock.patch('friends.utils.base.Model', TestModel)
     @mock.patch('friends.utils.base._seen_ids', {})
@@ -364,10 +364,10 @@
         # See?  Two rows in the table.
         self.assertEqual(2, TestModel.get_n_rows())
         # The first row is the message from fred.
-        self.assertEqual(TestModel.get_row(0)[COLUMN_INDICES['sender']],
+        self.assertEqual(TestModel.get_row(0)[SCHEMA.INDICES['sender']],
                          'fred')
         # The second row is the message from tedtholomew.
-        self.assertEqual(TestModel.get_row(1)[COLUMN_INDICES['sender']],
+        self.assertEqual(TestModel.get_row(1)[SCHEMA.INDICES['sender']],
                          'tedtholomew')
 
     @mock.patch('friends.utils.base.Model', TestModel)
 
=== modified file 'friends/utils/base.py'
--- friends/utils/base.py	2013-04-04 22:33:23 +0000
+++ friends/utils/base.py	2013-04-05 18:49:21 +0000
@@ -35,22 +35,22 @@
 
 from friends.errors import FriendsError, ContactsError
 from friends.utils.authentication import Authentication
-from friends.utils.model import COLUMN_INDICES, SCHEMA, DEFAULTS
-from friends.utils.model import Model, persist_model
+from friends.utils.model import Schema, Model, persist_model
 from friends.utils.notify import notify
 from friends.utils.time import ISO8601_FORMAT
 
 
 STUB = lambda *ignore, **kwignore: None
 COMMA_SPACE = ', '
-AVATAR_IDX = COLUMN_INDICES['icon_uri']
-FROM_ME_IDX = COLUMN_INDICES['from_me']
-STREAM_IDX = COLUMN_INDICES['stream']
-SENDER_IDX = COLUMN_INDICES['sender']
-MESSAGE_IDX = COLUMN_INDICES['message']
-ID_IDX = COLUMN_INDICES['message_id']
-ACCT_IDX = COLUMN_INDICES['account_id']
-TIME_IDX = COLUMN_INDICES['timestamp']
+SCHEMA = Schema()
+AVATAR_IDX = SCHEMA.INDICES['icon_uri']
+FROM_ME_IDX = SCHEMA.INDICES['from_me']
+STREAM_IDX = SCHEMA.INDICES['stream']
+SENDER_IDX = SCHEMA.INDICES['sender']
+MESSAGE_IDX = SCHEMA.INDICES['message']
+ID_IDX = SCHEMA.INDICES['message_id']
+ACCT_IDX = SCHEMA.INDICES['account_id']
+TIME_IDX = SCHEMA.INDICES['timestamp']
 
 # See friends/tests/test_protocols.py for further documentation
 LINKIFY_REGEX = re.compile(
@@ -355,8 +355,8 @@
         # the order which they appear in the SCHEMA. If any are left
         # over at the end of this, raise a TypeError indicating the
         # unexpected column names.
-        for column_name, column_type in SCHEMA:
-            args.append(kwargs.pop(column_name, DEFAULTS[column_type]))
+        for column_name, column_type in SCHEMA.COLUMNS:
+            args.append(kwargs.pop(column_name, SCHEMA.DEFAULTS[column_type]))
         if len(kwargs) > 0:
             raise TypeError('Unexpected keyword arguments: {}'.format(
                 COMMA_SPACE.join(sorted(kwargs))))
@@ -506,7 +506,7 @@
     def _calculate_row_cell(self, message_id, column_name):
         """Find x,y coords in the model based on message_id and column_name."""
         row_id = _seen_ids.get(message_id)
-        col_idx = COLUMN_INDICES.get(column_name)
+        col_idx = SCHEMA.INDICES.get(column_name)
         if None in (row_id, col_idx):
             raise FriendsError('Cell could not be found.')
         return row_id, col_idx
 
=== modified file 'friends/utils/model.py'
--- friends/utils/model.py	2013-03-13 04:36:33 +0000
+++ friends/utils/model.py	2013-04-05 18:49:21 +0000
@@ -24,11 +24,8 @@
 
 
 __all__ = [
+    'Schema',
     'Model',
-    'COLUMN_NAMES',
-    'COLUMN_TYPES',
-    'COLUMN_INDICES',
-    'DEFAULTS',
     'MODEL_DBUS_NAME',
     'persist_model',
     'prune_model',
@@ -41,46 +38,44 @@
 log = logging.getLogger(__name__)
 
 
-# DO NOT EDIT THIS WITHOUT ADJUSTING service.vala IN LOCKSTEP
-SCHEMA = (
-    ('protocol',       's'), # Same as UOA 'provider_name'
-    ('account_id',     't'), # Same as UOA account id
-    ('message_id',     's'),
-    ('stream',         's'),
-    ('sender',         's'),
-    ('sender_id',      's'),
-    ('sender_nick',    's'),
-    ('from_me',        'b'),
-    ('timestamp',      's'),
-    ('message',        's'),
-    ('icon_uri',       's'),
-    ('url',            's'),
-    ('likes',          't'),
-    ('liked',          'b'),
-    ('link_picture',   's'),
-    ('link_name',      's'),
-    ('link_url',       's'),
-    ('link_desc',      's'),
-    ('link_caption',   's'),
-    ('link_icon',      's'),
-    ('location',       's'),
-    ('latitude',       'd'),
-    ('longitude',      'd'),
-    )
-
-
-# It's useful to have separate lists of the column names and types.
-COLUMN_NAMES, COLUMN_TYPES = zip(*SCHEMA)
-# A reverse mapping from column name to the column index.  This is useful for
-# pulling column values out of a row of data.
-COLUMN_INDICES = {name: i for i, name in enumerate(COLUMN_NAMES)}
-# This defines default values for the different data types
-DEFAULTS = {
-    'b': False,
-    's': '',
-    'd': 0,
-    't': 0,
-    }
+class Schema:
+    """Represents the DeeModel schema data that we defined in CSV."""
+    DEFAULTS = {
+        'b': False,
+        's': '',
+        'd': 0,
+        't': 0,
+        }
+
+    FILES = [
+        'data/model-schema.csv',
+        '/usr/share/friends/model-schema.csv',
+        ]
+
+    def __init__(self):
+        """Parse CSV from disk."""
+        self.COLUMNS = []
+        self.NAMES = []
+        self.TYPES = []
+        self.INDICES = {}
+
+        files = self.FILES.copy()
+        while files:
+            filename = files.pop()
+            log.debug('Looking for SCHEMA in {}'.format(filename))
+            try:
+                with open(filename) as schema:
+                    for col in schema:
+                        name, variant = col.rstrip().split(',')
+                        self.COLUMNS.append((name, variant))
+                        self.NAMES.append(name)
+                        self.TYPES.append(variant)
+                log.debug(
+                    'Found {} columns for SCHEMA'.format(len(self.COLUMNS)))
+                break
+            except IOError:
+                pass
+        self.INDICES = {name: i for i, name in enumerate(self.NAMES)}
 
 
 MODEL_DBUS_NAME = 'com.canonical.Friends.Streams'
 
=== modified file 'service/src/service.vala'
--- service/src/service.vala	2013-03-13 04:36:33 +0000
+++ service/src/service.vala	2013-04-05 18:49:21 +0000
@@ -74,29 +74,17 @@
             debug ("Failed to load model from resource manager: %s", e.message);
         }
 
-        string[] SCHEMA = {"s",
-                           "t",
-                           "s",
-                           "s",
-                           "s",
-                           "s",
-                           "s",
-                           "b",
-                           "s",
-                           "s",
-                           "s",
-                           "s",
-                           "t",
-                           "b",
-                           "s",
-                           "s",
-                           "s",
-                           "s",
-                           "s",
-                           "s",
-                           "s",
-                           "d",
-                           "d"};
+        string[] SCHEMA = {};
+
+        var file = FileStream.open("/usr/share/friends/model-schema.csv", "r");
+        string line = null;
+        while (true)
+        {
+            line = file.read_line();
+            if (line == null) break;
+            SCHEMA += line.split(",")[1];
+        }
+        debug ("Found %u schema columns.", SCHEMA.length);
 
         bool schemaReset = false;
 
@@ -109,7 +97,7 @@
                 schemaReset = true;
             else
             {
-                for (int i=0; i < _SCHEMA.length;i++ )
+                for (int i=0; i < _SCHEMA.length; i++)
                 {
                     if (_SCHEMA[i] != SCHEMA[i])
                     {
@@ -165,11 +153,11 @@
                     debug ("NOT LEADER");
             });
 
-            Timeout.add_seconds (30, () => {
+            Timeout.add_seconds (300, () => {
                 shared_model.flush_revision_queue();
                 debug ("Storing model with %u rows", model.get_n_rows());
                 resources.store ((Dee.SequenceModel)model, "com.canonical.Friends.Streams");
-                return false;
+                return true;
             });
         }
 
 
=== modified file 'setup.py'
--- setup.py	2013-03-14 20:25:42 +0000
+++ setup.py	2013-04-05 18:49:21 +0000
@@ -33,7 +33,9 @@
         },
     data_files = [
         ('/usr/share/glib-2.0/schemas',
-         ['data/com.canonical.friends.gschema.xml'])
+         ['data/com.canonical.friends.gschema.xml']),
+        ('/usr/share/friends',
+         ['data/model-schema.csv']),
         ],
     entry_points = {
         'console_scripts': ['friends-dispatcher = friends.main:main'],
 
=== modified file 'tools/debug_live.py'
--- tools/debug_live.py	2013-03-13 18:05:13 +0000
+++ tools/debug_live.py	2013-04-05 18:49:21 +0000
@@ -24,6 +24,9 @@
 
 sys.path.insert(0, '.')
 
+# Ignore system-installed schema.
+from friends.tests.mocks import SCHEMA
+
 from gi.repository import GLib
 from friends.utils.logging import initialize
 
 
=== modified file 'tools/debug_slave.py'
--- tools/debug_slave.py	2013-04-04 22:33:06 +0000
+++ tools/debug_slave.py	2013-04-05 18:49:21 +0000
@@ -11,7 +11,8 @@
 from gi.repository import Dee
 from gi.repository import GLib
 
-from friends.utils.model import COLUMN_NAMES
+from friends.tests.mocks import SCHEMA
+
 
 class Slave:
     def __init__(self):
@@ -25,7 +26,7 @@
         row = self.model.get_row(itr)
         print('\n' * 5)
         for i, col in enumerate(row):
-            print('{:12}: {}'.format(COLUMN_NAMES[i], col))
+            print('{:12}: {}'.format(SCHEMA.NAMES[i], col))
         print('ROWS: ', len(self.model))