Nuria has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/129672

Change subject: Adding test for cohort uploading for cohort with cyrilic and 
arabic usernames.
......................................................................

Adding test for cohort uploading for cohort with cyrilic and arabic usernames.

Added ability to specify an arbitrary cohort file and test that it validates 
correctly.

In order to have testing bindings in mediawiki testing db create tables that
mimic production as close as possible, changes were done to mediawiki user model
and wikimetrics database. Alembic migration is included.
Without the database and binding changes the cohort uploading tests will fail.

Much testing was done regarding database connection args and encoding in
both wikimetrics and mediwiki database on staging with real data.

Bug: 63933
Change-Id: I0771e74f3d0745737e3ea96482614382fe09d961
---
A 
database_migrations/versions/43970813b4bb_changed_mediawiki_username_to_varbinary.py
A tests/static/public/testing-cohort-arabic.txt
A tests/static/public/testing-cohort-cyrilic.txt
M tests/test_models/test_validate_cohort.py
M wikimetrics/controllers/forms/cohort_upload.py
M wikimetrics/models/mediawiki/user.py
M wikimetrics/models/validate_cohort.py
M wikimetrics/utils.py
8 files changed, 256 insertions(+), 30 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/analytics/wikimetrics 
refs/changes/72/129672/1

diff --git 
a/database_migrations/versions/43970813b4bb_changed_mediawiki_username_to_varbinary.py
 
b/database_migrations/versions/43970813b4bb_changed_mediawiki_username_to_varbinary.py
new file mode 100644
index 0000000..b342612
--- /dev/null
+++ 
b/database_migrations/versions/43970813b4bb_changed_mediawiki_username_to_varbinary.py
@@ -0,0 +1,28 @@
+"""Changed mediawiki_username to VARBINARY
+
+Revision ID: 43970813b4bb
+Revises: 1a5740750a28
+Create Date: 2014-04-25 10:27:08.597354
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '43970813b4bb'
+down_revision = '1a5740750a28'
+
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects.mysql import VARBINARY
+from sqlalchemy import Column, Integer, String, Boolean
+
+
+def upgrade():
+    op.alter_column('wiki_user', 'mediawiki_username', type_=VARBINARY(255),
+                    existing_type=String(255), existing_nullable=True)
+    ## end Alembic commands ###
+
+
+def downgrade():
+    op.alter_column('wiki_user', 'mediawiki_username', type_=String(255),
+                    existing_type=VARBINARY(255), existing_nullable=True)
+    ### end Alembic commands ###
diff --git a/tests/static/public/testing-cohort-arabic.txt 
b/tests/static/public/testing-cohort-arabic.txt
new file mode 100644
index 0000000..189ea21
--- /dev/null
+++ b/tests/static/public/testing-cohort-arabic.txt
@@ -0,0 +1,14 @@
+Manar Abd El-Reheem 
+Parvati 
+تسنيم عادل 
+Shahinaz Ashraf 
+Shaimaa Khaled 
+Amaaal Sayed 
+
+Jadiga refaie 
+Hoda Medhat Said 
+Hanan Mohammed 
+عائشة عبد العزيز 
+Fatma noor 
+صفا محمد فهمي 
+مى مصطفى بركات
\ No newline at end of file
diff --git a/tests/static/public/testing-cohort-cyrilic.txt 
b/tests/static/public/testing-cohort-cyrilic.txt
new file mode 100644
index 0000000..bedba99
--- /dev/null
+++ b/tests/static/public/testing-cohort-cyrilic.txt
@@ -0,0 +1,29 @@
+007master
+00eight
+0gust1
+0neman
+0xd1ma
+0zar0
+1.Big.Picture
+100%sierreno
+1025wil
+117Avenue
+11joop
+11kingston11
+12mkwikimk12`
+12sid12
+16 Pacita
+17 marzo 2011
+18Наталь
+Абрам
+Александр
+Алексей
+АльбертAlbert
+Анатолий
+Андрей
+Антон
+Аркадий
+Марат
+Марк
+Матвей
+Михаил
\ No newline at end of file
diff --git a/tests/test_models/test_validate_cohort.py 
b/tests/test_models/test_validate_cohort.py
index 377a26a..61fd57f 100644
--- a/tests/test_models/test_validate_cohort.py
+++ b/tests/test_models/test_validate_cohort.py
@@ -1,12 +1,158 @@
 import unittest
-from nose.tools import assert_equal, raises, assert_true, assert_false
-from wikimetrics.configurables import app
-from tests.fixtures import WebTest, QueueDatabaseTest, mediawiki_project
+import os
+from nose.tools import assert_equal, raises, assert_true, assert_false, nottest
+from wikimetrics.configurables import app, get_absolute_path
+from tests.fixtures import WebTest, QueueDatabaseTest, DatabaseTest, 
mediawiki_project
 from wikimetrics.controllers.forms import CohortUpload
 from wikimetrics.models import (
     MediawikiUser, Cohort, WikiUser, ValidateCohort, User,
     normalize_project,
 )
+from wikimetrics.utils import parse_username
+
+
+class MockCohort(object):
+    pass
+
+
+class ValidateCohortEncodingTest(DatabaseTest):
+    
+    def setUp(self):
+        DatabaseTest.setUp(self)
+        self.test_report_path = os.path.join(get_absolute_path(), os.pardir, 
'tests')
+    
+    def tearDown(self):
+        pass
+    
+    def test_validate_arabic_cohort(self):
+        '''
+        Cohort with arabic names should validate
+        
+        If cohorts uploads are failing you could substitute
+        the file on this test by your file to test uploads.
+        
+        Note this test does not test the parsing of the
+        cohort file done at the controller layer.
+        '''
+        self.validate_cohort('testing-cohort-arabic.txt')
+    
+    def test_validate_cyrilic_cohort(self):
+        '''
+        Cohort with cyrilic names should validate
+        
+        Note this test does not test the parsing of the
+        cohort file done at the controller layer.
+        '''
+        self.validate_cohort('testing-cohort-cyrilic.txt')
+    
+    @nottest
+    def validate_cohort(self, filename):
+        '''
+        Given a cohort file with usernames all users but one should validate.
+        It will mingle the name of the 1st user.
+        
+        Parameters:
+            filename : Name of a file that contains a cohort with user names
+                       test will search for file in tests/static/public folder
+        '''
+        
+        names = self.create_users_from_file(filename)
+        
+        # establish ownership for this cohort otherwise things do not work
+        owner_user = User(username='test cohort owner', email='[email protected]')
+        self.session.add(owner_user)
+        self.session.commit()
+        
+        # creating here kind of like a cohortupload mock
+        # flask forms do not lend themselves to easy mocking
+        cohort_upload = MockCohort()
+        cohort_upload.name = MockCohort()
+        cohort_upload.name.data = 'testing-cohort'
+        cohort_upload.description = MockCohort()
+        cohort_upload.description.data = 'testing-cohort'
+        cohort_upload.project = MockCohort()
+        cohort_upload.project.data = mediawiki_project
+        cohort_upload.validate_as_user_ids = MockCohort()
+        cohort_upload.validate_as_user_ids.data = False
+        cohort_upload.records = []
+        
+        # mingle the name of the first user user
+        not_valid_editor_name = 'Mr Not Valid'
+        names[0] = not_valid_editor_name
+        
+        for name in names:
+            cohort_upload.records.append({
+                'username'  : name,
+                'project'   : mediawiki_project,
+            })
+        
+        # TODO clear session situation?
+        # all operations need to happen on the scope of the same session
+        # but this session passed in is going to be closed
+        vc = ValidateCohort.from_upload(cohort_upload, owner_user.id, 
self.session)
+        
+        cohort = self.session.query(Cohort).first()
+        self.session.commit()
+        vc.validate_records(self.session, cohort)
+        
+        # now we need to assert that all users but the first one validate
+        assert_equal(len(
+            self.session.query(WikiUser)
+                .filter(WikiUser.validating_cohort == cohort.id)
+                .filter(WikiUser.valid)
+                .all()
+        ), len(names) - 1)
+        
+        # retrieve the user that should not be valid, make sure it is not 
indeed
+        wiki_user = self.session.query(WikiUser)\
+            .filter(WikiUser.validating_cohort == cohort.id)\
+            .filter(WikiUser.mediawiki_username == not_valid_editor_name).one()
+        
+        assert_false(wiki_user.valid)
+    
+    @nottest
+    def create_users_from_file(self, filename):
+        """
+        Adds a bunch of users to mediawiki user table from a file
+        with usernames.
+        
+        In order to test encoding make sure the bindings of the testing and 
production
+        databases match, we try to replicate as accurate as possible the 
structure
+        of mediawiki db in our testing db but that is ongoing work that needs 
to be
+        maintaned.
+        
+        Parameters:
+            filename : Name of a file that contains a cohort with user names
+                       test will search for file in tests/static/public folder
+        Return:
+            names: Array with the names of the users created as they appear on 
the file
+                   but capitalized to mediawiki convention
+        """
+        
+        # open the cohort file
+        test_cohort_file = os.sep.join((self.test_report_path, 'static',
+                                        'public', filename))
+        f = open(test_cohort_file, 'r')
+        names = []
+        
+        # format names according to our convention
+        for line in f:
+            name = parse_username(line.strip())
+            names.append(name)
+        
+        self.mwSession.bind.engine.execute(
+            MediawikiUser.__table__.insert(), [
+                {
+                    'user_name': '{0}'.format(n),
+                    'user_registration': 20130101000000,
+                    'user_email_token_expires': 20200101000000
+                }
+                for n in names
+            ]
+        )
+        self.mwSession.commit()
+        
+        return names
 
 
 class ValidateCohortTest(WebTest):
@@ -64,7 +210,7 @@
 
 
 class ValidateCohortQueueTest(QueueDatabaseTest):
-
+    
     def setUp(self):
         QueueDatabaseTest.setUp(self)
         
@@ -78,7 +224,7 @@
         self.owner_user_id = owner_user.id
     
     def test_small_cohort(self):
-
+        
         cohort_upload = CohortUpload()
         cohort_upload.name.data = 'small_cohort'
         cohort_upload.project.data = mediawiki_project
diff --git a/wikimetrics/controllers/forms/cohort_upload.py 
b/wikimetrics/controllers/forms/cohort_upload.py
index 224e426..7f03d38 100644
--- a/wikimetrics/controllers/forms/cohort_upload.py
+++ b/wikimetrics/controllers/forms/cohort_upload.py
@@ -2,7 +2,7 @@
 from wtforms import StringField, FileField, TextAreaField, RadioField
 from wtforms.validators import Required
 from wikimetrics.metrics.form_fields import RequiredIfNot
-
+from wikimetrics.utils import parse_username
 from secure_form import WikimetricsSecureForm
 
 
@@ -84,21 +84,6 @@
                     'project'   : project,
                 })
     return records
-
-
-def parse_username(username):
-    """
-    parses uncapitalized, whitespace-padded, and weird-charactered mediawiki
-    user names into ones that have a chance of being found in the database
-    """
-    assert(type(username) != unicode)
-    username = str(username)
-    username = username.decode('utf8', errors='ignore')
-    parsed = username.strip()
-    if len(parsed) != 0:
-        parsed = parsed[0].upper() + parsed[1:]
-    
-    return parsed.encode('utf8')
 
 
 def normalize_newlines(lines):
diff --git a/wikimetrics/models/mediawiki/user.py 
b/wikimetrics/models/mediawiki/user.py
index 0ea0c62..0305af5 100644
--- a/wikimetrics/models/mediawiki/user.py
+++ b/wikimetrics/models/mediawiki/user.py
@@ -1,7 +1,7 @@
 from sqlalchemy import Column, Integer, String
 from wikimetrics.configurables import db
 from custom_columns import MediawikiTimestamp
-from sqlalchemy.dialects.mysql import TINYBLOB
+from sqlalchemy.dialects.mysql import TINYBLOB, VARBINARY
 from wikimetrics.utils import UNICODE_NULL
 
 __all__ = ['MediawikiUser']
@@ -11,9 +11,11 @@
     __tablename__ = 'user'
     
     # defaults are for user generating data methods
+    # VARBINARY bindings are needed so the table user we create
+    # in the mediawiki testing database resembles the table in production
     user_id = Column(Integer, primary_key=True)
-    user_name = Column(String(255))
-    user_real_name = Column(String(255), nullable=False, default='')
+    user_name = Column(VARBINARY(255))
+    user_real_name = Column(VARBINARY(255), nullable=False, default='')
     user_password = Column(TINYBLOB, nullable=False, default='')
     user_newpassword = Column(TINYBLOB, nullable=False, default='')
     user_newpass_time = Column(MediawikiTimestamp)
diff --git a/wikimetrics/models/validate_cohort.py 
b/wikimetrics/models/validate_cohort.py
index ab3c97a..78e45f2 100644
--- a/wikimetrics/models/validate_cohort.py
+++ b/wikimetrics/models/validate_cohort.py
@@ -2,6 +2,7 @@
 from celery import current_task
 from celery.utils.log import get_task_logger
 from flask.ext.login import current_user
+import traceback
 from wikimetrics.configurables import app, db, queue
 from sqlalchemy.orm.exc import NoResultFound, MultipleResultsFound
 from sqlalchemy.sql.expression import label, between, and_, or_
@@ -47,7 +48,7 @@
         self.validate_as_user_ids = cohort.validate_as_user_ids
     
     @classmethod
-    def from_upload(cls, cohort_upload, owner_user_id):
+    def from_upload(cls, cohort_upload, owner_user_id, session=None):
         """
         Create a new cohort and validate a list of uploaded users for it
         
@@ -67,7 +68,8 @@
             validated=False,
             validate_as_user_ids=cohort_upload.validate_as_user_ids.data == 
'True',
         )
-        session = db.get_session()
+        if session is None:
+            session = db.get_session()
         try:
             session.add(cohort)
             session.commit()
@@ -248,6 +250,7 @@
                 key = str(match.user_id)
             else:
                 key = match.user_name
+            
             users_dict[key].mediawiki_username = match.user_name
             users_dict[key].mediawiki_userid = match.user_id
             users_dict[key].valid = True
@@ -256,15 +259,19 @@
             users_dict.pop(key)
         
         # mark the rest invalid
+        # key is going to be a str if bindings are correct, but careful!
+        # it might be astring with chars that cannot be represented on ascii
+        # the user id does not really need to be in the message however
         for key in users_dict.keys():
             if validate_as_user_ids:
-                users_dict[key].reason_invalid = u'invalid user_id: 
{0}'.format(key)
+                users_dict[key].reason_invalid = "invalid user_id"
             else:
-                users_dict[key].reason_invalid = u'invalid user_name: 
{0}'.format(key)
+                users_dict[key].reason_invalid = "invalid user_name"
             users_dict[key].valid = False
     except Exception, e:
-        task_logger.error(e)
-
+        msg = traceback.print_exc()
+        task_logger.error(msg)
+        
         # clear out the dictionary in case of an exception, and raise the 
exception
         for key in users_dict.keys():
             users_dict.pop(key)
diff --git a/wikimetrics/utils.py b/wikimetrics/utils.py
index 9c2a1df..0151f79 100644
--- a/wikimetrics/utils.py
+++ b/wikimetrics/utils.py
@@ -221,3 +221,18 @@
     Converts a date to a datetime
     """
     return datetime.combine(d, datetime.min.time())
+
+
+def parse_username(username):
+    """
+    parses uncapitalized, whitespace-padded, and weird-charactered mediawiki
+    user names into ones that have a chance of being found in the database
+    """
+    assert(type(username) != unicode)
+    username = str(username)
+    username = username.decode('utf8', errors='ignore')
+    parsed = username.strip()
+    if len(parsed) != 0:
+        parsed = parsed[0].upper() + parsed[1:]
+    
+    return parsed.encode('utf8')

-- 
To view, visit https://gerrit.wikimedia.org/r/129672
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0771e74f3d0745737e3ea96482614382fe09d961
Gerrit-PatchSet: 1
Gerrit-Project: analytics/wikimetrics
Gerrit-Branch: master
Gerrit-Owner: Nuria <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to