Add account table

Also add support for dropping sqlite columns in migrations.

Replace any references to user names with references to account table
entries.

Change-Id: I5a147370f5ee648ddd808f699e80a7778c21d662

gertty/alembic/versions/4cc9c46f9d8b_add_account_table.py

@@ -0,0 +1,73 @@
+"""add account table
+
+Revision ID: 4cc9c46f9d8b
+Revises: 725816dc500
+Create Date: 2014-07-23 16:01:47.462597
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '4cc9c46f9d8b'
+down_revision = '725816dc500'
+
+import warnings
+
+from alembic import op
+import sqlalchemy as sa
+
+from gertty.dbsupport import sqlite_alter_columns, sqlite_drop_columns
+
+
+def upgrade():
+    sqlite_drop_columns('message', ['name'])
+    sqlite_drop_columns('comment', ['name'])
+    sqlite_drop_columns('approval', ['name'])
+    sqlite_drop_columns('change', ['owner'])
+
+    op.create_table('account',
+    sa.Column('key', sa.Integer(), nullable=False),
+    sa.Column('id', sa.Integer(), index=True, unique=True, nullable=False),
+    sa.Column('name', sa.String(length=255)),
+    sa.Column('username', sa.String(length=255)),
+    sa.Column('email', sa.String(length=255)),
+    sa.PrimaryKeyConstraint('key')
+    )
+
+    op.create_index(op.f('ix_account_name'), 'account', ['name'], unique=True)
+    op.create_index(op.f('ix_account_username'), 'account', ['name'], unique=True)
+    op.create_index(op.f('ix_account_email'), 'account', ['name'], unique=True)
+
+    with warnings.catch_warnings():
+        warnings.simplefilter("ignore")
+        op.add_column('message', sa.Column('account_key', sa.Integer()))
+        op.add_column('comment', sa.Column('account_key', sa.Integer()))
+        op.add_column('approval', sa.Column('account_key', sa.Integer()))
+        op.add_column('change', sa.Column('account_key', sa.Integer()))
+    sqlite_alter_columns('message', [
+            sa.Column('account_key', sa.Integer(), sa.ForeignKey('account.key'))
+            ])
+    sqlite_alter_columns('comment', [
+            sa.Column('account_key', sa.Integer(), sa.ForeignKey('account.key'))
+            ])
+    sqlite_alter_columns('approval', [
+            sa.Column('account_key', sa.Integer(), sa.ForeignKey('account.key'))
+            ])
+    sqlite_alter_columns('change', [
+            sa.Column('account_key', sa.Integer(), sa.ForeignKey('account.key'))
+            ])
+
+    op.create_index(op.f('ix_message_account_key'), 'message', ['account_key'], unique=False)
+    op.create_index(op.f('ix_comment_account_key'), 'comment', ['account_key'], unique=False)
+    op.create_index(op.f('ix_approval_account_key'), 'approval', ['account_key'], unique=False)
+    op.create_index(op.f('ix_change_account_key'), 'change', ['account_key'], unique=False)
+
+    connection = op.get_bind()
+    project = sa.sql.table('project', sa.sql.column('updated', sa.DateTime))
+    connection.execute(project.update().values({'updated':None}))
+
+    approval = sa.sql.table('approval', sa.sql.column('pending'))
+    connection.execute(approval.delete().where(approval.c.pending==False))
+
+
+def downgrade():
+    pass

gertty/db.py

@@ -44,7 +44,7 @@ change_table = Table(
     Column('branch', String(255), index=True, nullable=False),
     Column('change_id', String(255), index=True, nullable=False),
     Column('topic', String(255), index=True),
-    Column('owner', String(255), index=True),
+    Column('account_key', Integer, ForeignKey("account.key"), index=True),
     Column('subject', Text, nullable=False),
     Column('created', DateTime, index=True, nullable=False),
     Column('updated', DateTime, index=True, nullable=False),
@@ -67,9 +67,9 @@ message_table = Table(
     'message', metadata,
     Column('key', Integer, primary_key=True),
     Column('revision_key', Integer, ForeignKey("revision.key"), index=True),
+    Column('account_key', Integer, ForeignKey("account.key"), index=True),
     Column('id', String(255), index=True), #, unique=True, nullable=False),
     Column('created', DateTime, index=True, nullable=False),
-    Column('name', String(255)),
     Column('message', Text, nullable=False),
     Column('pending', Boolean, index=True, nullable=False),
     )
@@ -77,10 +77,10 @@ comment_table = Table(
     'comment', metadata,
     Column('key', Integer, primary_key=True),
     Column('revision_key', Integer, ForeignKey("revision.key"), index=True),
+    Column('account_key', Integer, ForeignKey("account.key"), index=True),
     Column('id', String(255), index=True), #, unique=True, nullable=False),
     Column('in_reply_to', String(255)),
     Column('created', DateTime, index=True, nullable=False),
-    Column('name', String(255)),
     Column('file', Text, nullable=False),
     Column('parent', Boolean, nullable=False),
     Column('line', Integer),
@@ -106,12 +106,26 @@ approval_table = Table(
     'approval', metadata,
     Column('key', Integer, primary_key=True),
     Column('change_key', Integer, ForeignKey("change.key"), index=True),
-    Column('name', String(255)),
+    Column('account_key', Integer, ForeignKey("account.key"), index=True),
     Column('category', String(255), nullable=False),
     Column('value', Integer, nullable=False),
     Column('pending', Boolean, index=True, nullable=False),
     )
+account_table = Table(
+    'account', metadata,
+    Column('key', Integer, primary_key=True),
+    Column('id', Integer, index=True, unique=True, nullable=False),
+    Column('name', String(255), index=True),
+    Column('username', String(255), index=True),
+    Column('email', String(255), index=True),
+    )
 
+class Account(object):
+    def __init__(self, id, name=None, username=None, email=None):
+        self.id = id
+        self.name = name
+        self.username = username
+        self.email = email
 
 class Project(object):
     def __init__(self, name, subscribed=False, description=''):
@@ -129,16 +143,16 @@ class Project(object):
         return c
 
 class Change(object):
-    def __init__(self, project, id, number, branch, change_id,
+    def __init__(self, project, id, owner, number, branch,
-                 owner, subject, created, updated, status,
+                 change_id, subject, created, updated, status,
                  topic=False, hidden=False, reviewed=False):
         self.project_key = project.key
+        self.account_key = owner.key
         self.id = id
         self.number = number
         self.branch = branch
         self.change_id = change_id
         self.topic = topic
-        self.owner = owner
         self.subject = subject
         self.created = created
         self.updated = updated
@@ -243,21 +257,21 @@ class Revision(object):
         return c
 
 class Message(object):
-    def __init__(self, revision, id, created, name, message, pending=False):
+    def __init__(self, revision, id, author, created, message, pending=False):
         self.revision_key = revision.key
+        self.account_key = author.key
         self.id = id
         self.created = created
-        self.name = name
         self.message = message
         self.pending = pending
 
 class Comment(object):
-    def __init__(self, revision, id, in_reply_to, created, name, file, parent, line, message, pending=False):
+    def __init__(self, revision, id, author, in_reply_to, created, file, parent, line, message, pending=False):
         self.revision_key = revision.key
+        self.account_key = author.key
         self.id = id
         self.in_reply_to = in_reply_to
         self.created = created
-        self.name = name
         self.file = file
         self.parent = parent
         self.line = line
@@ -278,13 +292,14 @@ class PermittedLabel(object):
         self.value = value
 
 class Approval(object):
-    def __init__(self, change, name, category, value, pending=False):
+    def __init__(self, change, reviewer, category, value, pending=False):
         self.change_key = change.key
-        self.name = name
+        self.account_key = reviewer.key
         self.category = category
         self.value = value
         self.pending = pending
 
+mapper(Account, account_table)
 mapper(Project, project_table, properties=dict(
         changes=relationship(Change, backref='project',
                              order_by=change_table.c.number),
@@ -304,6 +319,7 @@ mapper(Project, project_table, properties=dict(
                                   ),
         ))
 mapper(Change, change_table, properties=dict(
+        owner=relationship(Account),
         revisions=relationship(Revision, backref='change',
                                order_by=revision_table.c.number),
         messages=relationship(Message,
@@ -333,11 +349,14 @@ mapper(Revision, revision_table, properties=dict(
                                       order_by=(comment_table.c.line,
                                                 comment_table.c.created)),
         ))
-mapper(Message, message_table)
+mapper(Message, message_table, properties=dict(
-mapper(Comment, comment_table)
+        author=relationship(Account)))
+mapper(Comment, comment_table, properties=dict(
+        author=relationship(Account)))
 mapper(Label, label_table)
 mapper(PermittedLabel, permitted_label_table)
-mapper(Approval, approval_table)
+mapper(Approval, approval_table, properties=dict(
+        reviewer=relationship(Account)))
 
 class Database(object):
     def __init__(self, app):
@@ -505,8 +524,36 @@ class DatabaseSession(object):
     def getPendingMessages(self):
         return self.session().query(Message).filter_by(pending=True).all()
 
+    def getAccountByID(self, id, name=None, username=None, email=None):
+        try:
+            account = self.session().query(Account).filter_by(id=id).one()
+        except sqlalchemy.orm.exc.NoResultFound:
+            account = self.createAccount(id)
+        if name is not None and account.name != name:
+            account.name = name
+        if username is not None and account.username != username:
+            account.username = username
+        if email is not None and account.email != email:
+            account.email = email
+        return account
+
+    def getAccountByUsername(self, username):
+        try:
+            return self.session().query(Account).filter_by(username=username).one()
+        except sqlalchemy.orm.exc.NoResultFound:
+            return None
+
+    def getSystemAccount(self):
+        return self.getAccountByID(0, 'Gerrit Code Review')
+
     def createProject(self, *args, **kw):
         o = Project(*args, **kw)
         self.session().add(o)
         self.session().flush()
         return o
+
+    def createAccount(self, *args, **kw):
+        a = Account(*args, **kw)
+        self.session().add(a)
+        self.session().flush()
+        return a

gertty/dbsupport.py

@@ -103,3 +103,70 @@ def sqlite_alter_columns(table_name, column_defs):
     # (re-)create indexes
     for index in indexes:
         op.create_index(op.f(index[0]), index[1], index[2], unique=index[3])
+
+def sqlite_drop_columns(table_name, drop_columns):
+    """Implement drop columns for SQLite.
+
+    The DROP COLUMN command isn't supported by SQLite specification.
+    Instead of calling DROP COLUMN it uses the following workaround:
+
+    * create temp table '{table_name}_{rand_uuid}', without
+      dropped columns;
+    * copy all data to the temp table;
+    * drop old table;
+    * rename temp table to the old table name.
+    """
+    connection = op.get_bind()
+    meta = sqlalchemy.MetaData(bind=connection)
+    meta.reflect()
+
+    # construct lists of all columns and their names
+    old_columns = []
+    new_columns = []
+    column_names = []
+    indexes = []
+    for column in meta.tables[table_name].columns:
+        if column.name not in drop_columns:
+            old_columns.append(column)
+            column_names.append(column.name)
+            col_copy = column.copy()
+            new_columns.append(col_copy)
+
+    for key in meta.tables[table_name].foreign_keys:
+        constraint = key.constraint
+        con_copy = constraint.copy()
+        new_columns.append(con_copy)
+
+    for index in meta.tables[table_name].indexes:
+        # If this is a single column index for a dropped column, don't
+        # copy it.
+        idx_columns = [col.name for col in index.columns]
+        if len(idx_columns)==1 and idx_columns[0] in drop_columns:
+            continue
+        # Otherwise, recreate the index.
+        indexes.append((index.name,
+                        table_name,
+                        [col.name for col in index.columns],
+                        index.unique))
+
+    # create temp table
+    tmp_table_name = "%s_%s" % (table_name, six.text_type(uuid.uuid4()))
+    op.create_table(tmp_table_name, *new_columns)
+    meta.reflect()
+
+    try:
+        # copy data from the old table to the temp one
+        sql_select = sqlalchemy.sql.select(old_columns)
+        connection.execute(sqlalchemy.sql.insert(meta.tables[tmp_table_name])
+                           .from_select(column_names, sql_select))
+    except Exception:
+        op.drop_table(tmp_table_name)
+        raise
+
+    # drop the old table and rename temp table to the old table name
+    op.drop_table(table_name)
+    op.rename_table(tmp_table_name, table_name)
+
+    # (re-)create indexes
+    for index in indexes:
+        op.create_index(op.f(index[0]), index[1], index[2], unique=index[3])

gertty/sync.py

@@ -87,6 +87,19 @@ class Task(object):
         self.event.wait(timeout)
         return self.succeeded
 
+class SyncOwnAccountTask(Task):
+    def __repr__(self):
+        return '<SyncOwnAccountTask>'
+
+    def run(self, sync):
+        app = sync.app
+        remote = sync.get('accounts/self')
+        with app.db.getSession() as session:
+            session.getAccountByID(remote['_account_id'],
+                                   remote.get('name'),
+                                   remote['username'],
+                                   remote.get('email'))
+
 class SyncProjectListTask(Task):
     def __repr__(self):
         return '<SyncProjectListTask>'
@@ -224,16 +237,20 @@ class SyncChangeTask(Task):
         fetches = collections.defaultdict(list)
         with app.db.getSession() as session:
             change = session.getChangeByID(self.change_id)
+            account = session.getAccountByID(remote_change['owner']['_account_id'],
+                                             name=remote_change['owner'].get('name'),
+                                             username=remote_change['owner'].get('username'),
+                                             email=remote_change['owner'].get('email'))
             if not change:
                 project = session.getProjectByName(remote_change['project'])
                 created = dateutil.parser.parse(remote_change['created'])
                 updated = dateutil.parser.parse(remote_change['updated'])
-                change = project.createChange(remote_change['id'], remote_change['_number'],
+                change = project.createChange(remote_change['id'], account, remote_change['_number'],
                                               remote_change['branch'], remote_change['change_id'],
-                                              remote_change['owner']['name'],
                                               remote_change['subject'], created,
                                               updated, remote_change['status'],
                                               topic=remote_change.get('topic'))
+            change.owner = account
             change.status = remote_change['status']
             change.subject = remote_change['subject']
             change.updated = dateutil.parser.parse(remote_change['updated'])
@@ -272,6 +289,10 @@ class SyncChangeTask(Task):
                 remote_comments_data = remote_revision['_gertty_remote_comments_data']
                 for remote_file, remote_comments in remote_comments_data.items():
                     for remote_comment in remote_comments:
+                        account = session.getAccountByID(remote_comment['author']['_account_id'],
+                                                         name=remote_comment['author'].get('name'),
+                                                         username=remote_comment['author'].get('username'),
+                                                         email=remote_comment['author'].get('email'))
                         comment = session.getCommentByID(remote_comment['id'])
                         if not comment:
                             # Normalize updated -> created
@@ -279,27 +300,35 @@ class SyncChangeTask(Task):
                             parent = False
                             if remote_comment.get('side', '') == 'PARENT':
                                 parent = True
-                            comment = revision.createComment(remote_comment['id'],
+                            comment = revision.createComment(remote_comment['id'], account,
                                                              remote_comment.get('in_reply_to'),
-                                                             created, remote_comment['author']['name'],
+                                                             created,
                                                              remote_file, parent, remote_comment.get('line'),
                                                              remote_comment['message'])
+                        else:
+                            if comment.author != account:
+                                comment.author = account
             new_message = False
             for remote_message in remote_change.get('messages', []):
+                if 'author' in remote_message:
+                    account = session.getAccountByID(remote_message['author']['_account_id'],
+                                                     name=remote_message['author'].get('name'),
+                                                     username=remote_message['author'].get('username'),
+                                                     email=remote_message['author'].get('email'))
+                    if account.username != app.config.username:
+                        new_message = True
+                else:
+                    account = session.getSystemAccount()
                 message = session.getMessageByID(remote_message['id'])
                 if not message:
                     revision = session.getRevisionByNumber(change, remote_message['_revision_number'])
                     # Normalize date -> created
                     created = dateutil.parser.parse(remote_message['date'])
-                    if 'author' in remote_message:
+                    message = revision.createMessage(remote_message['id'], account, created,
-                        author_name = remote_message['author']['name']
-                        if remote_message['author'].get('username') != app.config.username:
-                            new_message = True
-                    else:
-                        author_name = 'Gerrit Code Review'
-                    message = revision.createMessage(remote_message['id'], created,
-                                                     author_name,
                                                      remote_message['message'])
+                else:
+                    if message.author != account:
+                        message.author = account
             remote_approval_entries = {}
             remote_label_entries = {}
             user_voted = False
@@ -324,7 +353,8 @@ class SyncChangeTask(Task):
             local_approvals = {}
             local_labels = {}
             for approval in change.approvals:
-                key = '%s~%s' % (approval.category, approval.name)
+                self.log.debug(approval.key)
+                key = '%s~%s' % (approval.category, approval.reviewer.name)
                 local_approvals[key] = approval
             local_approval_keys = set(local_approvals.keys())
             for label in change.labels:
@@ -340,7 +370,11 @@ class SyncChangeTask(Task):
 
             for key in remote_approval_keys-local_approval_keys:
                 remote_approval = remote_approval_entries[key]
-                change.createApproval(remote_approval['name'],
+                account = session.getAccountByID(remote_approval['_account_id'],
+                                                 name=remote_approval.get('name'),
+                                                 username=remote_approval.get('username'),
+                                                 email=remote_approval.get('email'))
+                change.createApproval(account,
                                       remote_approval['category'],
                                       remote_approval['value'])
 
@@ -354,6 +388,11 @@ class SyncChangeTask(Task):
                 local_approval = local_approvals[key]
                 remote_approval = remote_approval_entries[key]
                 local_approval.value = remote_approval['value']
+                # For the side effect of updating account info:
+                account = session.getAccountByID(remote_approval['_account_id'],
+                                                 name=remote_approval.get('name'),
+                                                 username=remote_approval.get('username'),
+                                                 email=remote_approval.get('email'))
 
             remote_permitted_entries = {}
             for remote_label_name, remote_label_values in remote_change.get('permitted_labels', {}).items():
@@ -508,6 +547,7 @@ class Sync(object):
         self.session = requests.Session()
         self.auth = requests.auth.HTTPDigestAuth(
             self.app.config.username, self.app.config.password)
+        self.submitTask(SyncOwnAccountTask(HIGH_PRIORITY))
         self.submitTask(UploadReviewsTask(HIGH_PRIORITY))
         self.submitTask(SyncProjectListTask(HIGH_PRIORITY))
         self.submitTask(SyncSubscribedProjectsTask(HIGH_PRIORITY))

gertty/view/change.py

@@ -258,7 +258,7 @@ class ChangeMessageBox(mywid.HyperText):
     def __init__(self, app, message):
         super(ChangeMessageBox, self).__init__(u'')
         lines = message.message.split('\n')
-        text = [('change-message-name', message.name),
+        text = [('change-message-name', message.author.name),
                 ('change-message-header', ': '+lines.pop(0)),
                 ('change-message-header',
                  message.created.strftime(' (%Y-%m-%d %H:%M:%S%z)'))]
@@ -388,7 +388,7 @@ class ChangeView(urwid.WidgetWrap):
             self.change_rest_id = change.id
 
             self.change_id_label.set_text(('change-data', change.change_id))
-            self.owner_label.set_text(('change-data', change.owner))
+            self.owner_label.set_text(('change-data', change.owner.name))
             self.project_label.set_text(('change-data', change.project.name))
             self.branch_label.set_text(('change-data', change.branch))
             self.topic_label.set_text(('change-data', change.topic or ''))
@@ -413,16 +413,16 @@ class ChangeView(urwid.WidgetWrap):
             votes = mywid.Table(approval_headers)
             approvals_for_name = {}
             for approval in change.approvals:
-                approvals = approvals_for_name.get(approval.name)
+                approvals = approvals_for_name.get(approval.reviewer.name)
                 if not approvals:
                     approvals = {}
                     row = []
-                    row.append(urwid.Text(('reviewer-name', approval.name)))
+                    row.append(urwid.Text(('reviewer-name', approval.reviewer.name)))
                     for i, category in enumerate(categories):
                         w = urwid.Text(u'', align=urwid.CENTER)
                         approvals[category] = w
                         row.append(w)
-                    approvals_for_name[approval.name] = approvals
+                    approvals_for_name[approval.reviewer.name] = approvals
                     votes.addRow(row)
                 if str(approval.value) != '0':
                     if approval.value > 0:
@@ -591,6 +591,7 @@ class ChangeView(urwid.WidgetWrap):
     def saveReview(self, revision_key, approvals, message):
         message_key = None
         with self.app.db.getSession() as session:
+            account = session.getAccountByUsername(self.app.config.username)
             revision = session.getRevision(revision_key)
             change = revision.change
             pending_approvals = {}
@@ -604,7 +605,7 @@ class ChangeView(urwid.WidgetWrap):
                 value = approvals.get(category, 0)
                 approval = pending_approvals.get(category)
                 if not approval:
-                    approval = change.createApproval(u'(draft)', category, 0, pending=True)
+                    approval = change.createApproval(account, category, 0, pending=True)
                     pending_approvals[category] = approval
                 approval.value = value
             pending_message = None
@@ -613,9 +614,9 @@ class ChangeView(urwid.WidgetWrap):
                     pending_message = m
                     break
             if not pending_message:
-                pending_message = revision.createMessage(None,
+                pending_message = revision.createMessage(None, account,
                                                          datetime.datetime.utcnow(),
-                                                         u'(draft)', '', pending=True)
+                                                         '', pending=True)
             pending_message.message = message
             message_key = pending_message.key
             change.reviewed = True

gertty/view/diff.py

@@ -268,7 +268,7 @@ class DiffView(urwid.WidgetWrap):
                 if comment.pending:
                     message = comment.message
                 else:
-                    message = [('comment-name', comment.name),
+                    message = [('comment-name', comment.author.name),
                                ('comment', u': '+comment.message)]
                 comment_list.append((comment.key, message))
                 comment_lists[key] = comment_list
@@ -284,7 +284,7 @@ class DiffView(urwid.WidgetWrap):
                 if comment.pending:
                     message = comment.message
                 else:
-                    message = [('comment-name', comment.name),
+                    message = [('comment-name', comment.author.name),
                                ('comment', u': '+comment.message)]
                 comment_list.append((comment.key, message))
                 comment_lists[key] = comment_list
@@ -537,9 +537,10 @@ class DiffView(urwid.WidgetWrap):
             filename = context.old_fn
         with self.app.db.getSession() as session:
             revision = session.getRevision(revision_key)
-            comment = revision.createComment(None, None,
+            account = session.getAccountByUsername(self.app.config.username)
+            comment = revision.createComment(None, account, None,
                                              datetime.datetime.utcnow(),
-                                             None, filename, parent,
+                                             filename, parent,
                                              line_num, text, pending=True)
             key = comment.key
         return key