GNOME.org
 

[odrs-web] Add a user_id foreign key to all tables

commit 2baf9227267d42e578b79c75833792cd5b95d6ee
Author: Richard Hughes <richard hughsie com>
Date:   Tue Jul 2 10:56:46 2019 +0100

    Add a user_id foreign key to all tables
    
    The now-duplicate user_hash will be removed in the next commit to allow pods to migrate without downtime.

 app_data/migrations/versions/19526c284b29_.py | 64 +++++++++++++++++++++++++++
 app_data/odrs/dbutils.py                      |  8 +++-
 app_data/odrs/models.py                       | 27 +++++++++--
 app_data/odrs/tests/odrs_test.py              |  6 +--
 app_data/odrs/util.py                         |  3 +-
 app_data/odrs/views_admin.py                  |  2 +-
 app_data/odrs/views_api.py                    | 47 ++++++++++++--------
 7 files changed, 130 insertions(+), 27 deletions(-)
---
diff --git a/app_data/migrations/versions/19526c284b29_.py b/app_data/migrations/versions/19526c284b29_.py
new file mode 100644
index 0000000..cd1ec34
--- /dev/null
+++ b/app_data/migrations/versions/19526c284b29_.py
@@ -0,0 +1,64 @@
+"""
+
+Revision ID: 19526c284b29
+Revises: 84deb10331db
+Create Date: 2019-07-02 10:51:23.952062
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '19526c284b29'
+down_revision = '84deb10331db'
+
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import mysql
+
+from odrs import db
+from odrs.models import User, Moderator, Event, Review, Vote
+
+def _hash_to_id(user_hash):
+    user = db.session.query(User).filter(User.user_hash == user_hash).first()
+    if not user:
+        return None
+    return user.user_id
+
+def upgrade():
+    op.add_column('eventlog', sa.Column('user_id', sa.Integer(), nullable=True))
+    op.add_column('moderators', sa.Column('user_id', sa.Integer(), nullable=True))
+    op.add_column('reviews', sa.Column('user_id', sa.Integer(), nullable=True))
+    op.add_column('votes', sa.Column('user_id', sa.Integer(), nullable=True))
+    op.create_foreign_key(None, 'eventlog', 'users', ['user_id'], ['user_id'])
+    op.create_foreign_key(None, 'moderators', 'users', ['user_id'], ['user_id'])
+    op.create_foreign_key(None, 'reviews', 'users', ['user_id'], ['user_id'])
+    op.create_foreign_key(None, 'votes', 'users', ['user_id'], ['user_id'])
+
+    print('CONVERTING Event')
+    for val in db.session.query(Event).all():
+        val.user_id = _hash_to_id(val.user_hash)
+    db.session.commit()
+
+    print('CONVERTING Moderator')
+    for val in db.session.query(Moderator).all():
+        val.user_id = _hash_to_id(val.user_hash)
+    db.session.commit()
+
+    print('CONVERTING Review')
+    for val in db.session.query(Review).all():
+        val.user_id = _hash_to_id(val.user_hash)
+    db.session.commit()
+
+    print('CONVERTING Vote')
+    for val in db.session.query(Vote).all():
+        val.user_id = _hash_to_id(val.user_hash)
+    db.session.commit()
+
+def downgrade():
+    op.drop_constraint(None, 'votes', type_='foreignkey')
+    op.drop_constraint(None, 'reviews', type_='foreignkey')
+    op.drop_constraint(None, 'moderators', type_='foreignkey')
+    op.drop_constraint(None, 'eventlog', type_='foreignkey')
+    op.drop_column('votes', 'user_id')
+    op.drop_column('reviews', 'user_id')
+    op.drop_column('moderators', 'user_id')
+    op.drop_column('eventlog', 'user_id')
diff --git a/app_data/odrs/dbutils.py b/app_data/odrs/dbutils.py
index ef25f03..8be1831 100644
--- a/app_data/odrs/dbutils.py
+++ b/app_data/odrs/dbutils.py
@@ -11,11 +11,17 @@ def init_db(db):
     db.metadata.create_all(bind=db.engine)
 
     # ensure admin user exists
-    from .models import Moderator
+    from .models import Moderator, User
+    user = db.session.query(User).filter(User.user_hash == 
'deadbeef348c0f88529f3bfd937ec1a5d90aefc7').first()
+    if not user:
+        user = User('deadbeef348c0f88529f3bfd937ec1a5d90aefc7')
+        db.session.add(user)
+        db.session.commit()
     if not db.session.query(Moderator).filter(Moderator.username == 'admin test com').first():
         mod = Moderator(username='admin test com')
         mod.password = 'Pa$$w0rd'
         mod.is_admin = True
+        mod.user_id = user.user_id
         db.session.add(mod)
         db.session.commit()
 
diff --git a/app_data/odrs/models.py b/app_data/odrs/models.py
index 134c0e3..ea100d3 100644
--- a/app_data/odrs/models.py
+++ b/app_data/odrs/models.py
@@ -11,7 +11,8 @@ import datetime
 
 from werkzeug.security import generate_password_hash, check_password_hash
 
-from sqlalchemy import Column, Integer, String, Text, DateTime, Boolean, Index
+from sqlalchemy import Column, Integer, String, Text, DateTime, Boolean, Index, ForeignKey
+from sqlalchemy.orm import relationship
 
 from odrs import db
 
@@ -51,11 +52,15 @@ class Vote(db.Model):
     vote_id = Column(Integer, primary_key=True, nullable=False, unique=True)
     date_created = Column(DateTime, nullable=False, default=datetime.datetime.utcnow)
     user_hash = Column(Text)
+    user_id = Column(Integer, ForeignKey('users.user_id'), nullable=True)
     val = Column(Integer, default=0)
     review_id = Column(Integer, default=0)
 
-    def __init__(self, user_hash, val, review_id=0):
+    user = relationship('User')
+
+    def __init__(self, user_id, user_hash, val, review_id=0):
         self.review_id = review_id
+        self.user_id = user_id
         self.user_hash = user_hash
         self.val = val
 
@@ -77,6 +82,10 @@ class User(db.Model):
     karma = Column(Integer, default=0)
     is_banned = Column(Boolean, default=False)
 
+    reviews = relationship('Review',
+                           back_populates='user',
+                           cascade='all,delete-orphan')
+
     def __init__(self, user_hash=None):
         self.user_hash = user_hash
         self.karma = 0
@@ -99,6 +108,7 @@ class Review(db.Model):
     summary = Column(Text)
     description = Column(Text)
     user_hash = Column(Text)
+    user_id = Column(Integer, ForeignKey('users.user_id'), nullable=True)
     user_addr = Column(Text)
     user_display = Column(Text)
     version = Column(Text)
@@ -108,6 +118,8 @@ class Review(db.Model):
     karma_down = Column(Integer, default=0)
     reported = Column(Integer, default=0)
 
+    user = relationship('User', back_populates='reviews')
+
     def __init__(self):
         self.app_id = None
         self.locale = None
@@ -117,6 +129,7 @@ class Review(db.Model):
         self.distro = None
         self.karma_up = 0
         self.karma_down = 0
+        self.user_id = 0
         self.user_hash = None
         self.user_display = None
         self.rating = 0
@@ -159,12 +172,16 @@ class Event(db.Model):
     date_created = Column(DateTime, nullable=False, default=datetime.datetime.utcnow)
     user_addr = Column(Text)
     user_hash = Column(Text)
+    user_id = Column(Integer, ForeignKey('users.user_id'), nullable=True)
     message = Column(Text)
     app_id = Column(Text)
     important = Column(Boolean, default=False)
 
-    def __init__(self, user_addr, user_hash=None, app_id=None, message=None, important=False):
+    user = relationship('User')
+
+    def __init__(self, user_addr, user_id=None, user_hash=None, app_id=None, message=None, important=False):
         self.user_addr = user_addr
+        self.user_id = user_id
         self.user_hash = user_hash
         self.message = message
         self.app_id = app_id
@@ -186,13 +203,17 @@ class Moderator(db.Model):
     is_enabled = Column(Boolean, default=False)
     is_admin = Column(Boolean, default=False)
     user_hash = Column(Text)
+    user_id = Column(Integer, ForeignKey('users.user_id'), nullable=True)
     locales = Column(Text)
 
+    user = relationship('User')
+
     def __init__(self, username=None, password=None, display_name=None):
         self.username = username
         self.display_name = display_name
         self.is_enabled = False
         self.is_admin = False
+        self.user_id = 0
         self.user_hash = None
         self.locales = None
         self.locales = password
diff --git a/app_data/odrs/tests/odrs_test.py b/app_data/odrs/tests/odrs_test.py
index b3d5241..ffb8c1e 100644
--- a/app_data/odrs/tests/odrs_test.py
+++ b/app_data/odrs/tests/odrs_test.py
@@ -319,12 +319,12 @@ class OdrsTest(unittest.TestCase):
 
     def test_api_moderate_locale(self):
 
-        rv = self.app.get('/1.0/reviews/api/moderate/FIXMEuserhash/en_GB')
+        rv = self.app.get('/1.0/reviews/api/moderate/{}/en_GB'.format(self.user_hash))
         assert rv.data == b'[]', rv.data
         self.review_submit()
-        rv = self.app.get('/1.0/reviews/api/moderate/FIXMEuserhash/en_GB')
+        rv = self.app.get('/1.0/reviews/api/moderate/{}/en_GB'.format(self.user_hash))
         assert b'Somebody Important' in rv.data, rv.data
-        rv = self.app.get('/1.0/reviews/api/moderate/FIXMEuserhash/fr_FR')
+        rv = self.app.get('/1.0/reviews/api/moderate/{}/fr_FR'.format(self.user_hash))
         assert rv.data == b'[]', rv.data
 
     def test_api_fetch_no_results(self):
diff --git a/app_data/odrs/util.py b/app_data/odrs/util.py
index c7e22c3..e22352d 100644
--- a/app_data/odrs/util.py
+++ b/app_data/odrs/util.py
@@ -49,6 +49,7 @@ def _get_user_key(user_hash, app_id):
     return key
 
 def _eventlog_add(user_addr=None,
+                  user_id=None,
                   user_hash=None,
                   app_id=None,
                   message=None,
@@ -56,7 +57,7 @@ def _eventlog_add(user_addr=None,
     """ Adds a warning to the event log """
     from .models import Event
     from odrs import db
-    db.session.add(Event(user_addr, user_hash, app_id, message, important))
+    db.session.add(Event(user_addr, user_id, user_hash, app_id, message, important))
     db.session.commit()
 
 def _get_rating_for_app_id(app_id, min_total=1):
diff --git a/app_data/odrs/views_admin.py b/app_data/odrs/views_admin.py
index d39f04d..16cfe65 100644
--- a/app_data/odrs/views_admin.py
+++ b/app_data/odrs/views_admin.py
@@ -625,7 +625,7 @@ def admin_vote(review_id, val_str):
         user = User(user_hash)
         db.session.add(user)
     user.karma += val
-    db.session.add(Vote(user_hash, val, review_id=review_id))
+    db.session.add(Vote(user.user_id, user_hash, val, review_id=review_id))
     db.session.commit()
     flash('Recorded vote')
     return redirect(url_for('.admin_show_review', review_id=review_id))
diff --git a/app_data/odrs/views_api.py b/app_data/odrs/views_api.py
index 1447d96..8232e11 100644
--- a/app_data/odrs/views_api.py
+++ b/app_data/odrs/views_api.py
@@ -102,6 +102,7 @@ def api_submit():
             filter(Review.app_id == item['app_id']).\
             filter(Review.user_hash == item['user_hash']).first():
         _eventlog_add(_get_client_address(),
+                      user.user_id,
                       item['user_hash'],
                       item['app_id'],
                       'already reviewed')
@@ -113,6 +114,7 @@ def api_submit():
     review.locale = item['locale']
     review.summary = _sanitised_summary(item['summary'])
     review.description = _sanitised_description(item['description'])
+    review.user_id = user.user_id
     review.user_hash = item['user_hash']
     review.version = _sanitised_version(item['version'])
     review.distro = item['distro']
@@ -130,6 +132,7 @@ def api_submit():
 
     # log and add
     _eventlog_add(_get_client_address(),
+                  review.user_id,
                   review.user_hash,
                   review.app_id,
                   'reviewed')
@@ -295,19 +298,7 @@ def _vote(val):
     if not len(item['user_skey']) == 40:
         return json_error('the user_skey is invalid')
 
-    if item['user_skey'] != _get_user_key(item['user_hash'], item['app_id']):
-        _eventlog_add(_get_client_address(), item['user_hash'], None,
-                      'invalid user_skey of %s' % item['user_skey'], important=True)
-        #print('expected user_skey of %s' % _get_user_key(item['user_hash'], item['app_id']))
-        return json_error('invalid user_skey')
-
-    # the user already has a review
-    if _vote_exists(item['review_id'], item['user_hash']):
-        _eventlog_add(_get_client_address(), item['user_hash'], item['app_id'],
-                      'duplicate vote')
-        return json_error('already voted on this app')
-
-    # update the per-user karma
+    # get user
     user = db.session.query(User).filter(User.user_hash == item['user_hash']).first()
     if not user:
         user = User(item['user_hash'])
@@ -320,12 +311,26 @@ def _vote(val):
 
         # the user is too harsh
         if val < 0 and user.karma < -50:
-            return json_error('odrs_all negative karma used up')
+            return json_error('all negative karma used up')
+
+    if item['user_skey'] != _get_user_key(item['user_hash'], item['app_id']):
+        _eventlog_add(_get_client_address(), user.user_id, item['user_hash'], None,
+                      'invalid user_skey of %s' % item['user_skey'], important=True)
+        #print('expected user_skey of %s' % _get_user_key(item['user_hash'], item['app_id']))
+        return json_error('invalid user_skey')
+
+    # the user already has a review
+    if _vote_exists(item['review_id'], item['user_hash']):
+        _eventlog_add(_get_client_address(), user.user_id, item['user_hash'], item['app_id'],
+                      'duplicate vote')
+        return json_error('already voted on this app')
+
+    # update the per-user karma
     user.karma += val
 
     review = db.session.query(Review).filter(Review.app_id == item['app_id']).first()
     if not review:
-        _eventlog_add(_get_client_address(), item['user_hash'], None,
+        _eventlog_add(_get_client_address(), user.user_id, item['user_hash'], None,
                       'invalid review ID of %s' % item['app_id'], important=True)
         return json_error('invalid review ID')
 
@@ -340,9 +345,9 @@ def _vote(val):
     db.session.commit()
 
     # add the vote to the database
-    db.session.add(Vote(item['user_hash'], val, review_id=item['review_id']))
+    db.session.add(Vote(user.user_id, item['user_hash'], val, review_id=item['review_id']))
     db.session.commit()
-    _eventlog_add(_get_client_address(), item['user_hash'], item['app_id'],
+    _eventlog_add(_get_client_address(), user.user_id, item['user_hash'], item['app_id'],
                   'voted %i on review' % val)
 
     return json_success('voted #%i %i' % (item['review_id'], val))
@@ -396,6 +401,11 @@ def api_remove():
     if not len(item['user_skey']) == 40:
         return json_error('the user_skey is invalid')
 
+    # get user
+    user = db.session.query(User).filter(User.user_hash == item['user_hash']).first()
+    if not user:
+        return json_error('no review')
+
     # the user already has a review
     review = db.session.query(Review).\
                 filter(Review.review_id == item['review_id']).\
@@ -406,13 +416,14 @@ def api_remove():
         return json_error('the app_id is invalid')
 
     if item['user_skey'] != _get_user_key(item['user_hash'], item['app_id']):
-        _eventlog_add(_get_client_address(), item['user_hash'], None,
+        _eventlog_add(_get_client_address(), user.user_id, None,
                       'invalid user_skey of %s' % item['user_skey'], important=True)
         return json_error('invalid user_skey')
 
     db.session.delete(review)
     db.session.commit()
     _eventlog_add(_get_client_address(),
+                  user.user_id,
                   item['user_hash'],
                   item['app_id'],
                   'removed review')
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]