Milimetric has submitted this change and it was merged.
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, 281 insertions(+), 73 deletions(-)
Approvals:
Milimetric: Looks good to me, approved
jenkins-bot: Verified
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..9857ae2
--- /dev/null
+++ b/tests/static/public/testing-cohort-arabic.txt
@@ -0,0 +1,11 @@
+juanita
+ الموسوعة
+ الحرة
+ التي
+ يستطيع
+ الجميع
+ تحريرها.
+ توجد
+ الآن
+ 276245 مقالة
+ بالعربية
\ 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..600eec1
--- /dev/null
+++ b/tests/static/public/testing-cohort-cyrilic.txt
@@ -0,0 +1,16 @@
+juanita
+pepita
+fulanita
+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..f4c91a8 100644
--- a/tests/test_models/test_validate_cohort.py
+++ b/tests/test_models/test_validate_cohort.py
@@ -1,35 +1,181 @@
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):
-
+
def test_normalize_project_shorthand(self):
normal = normalize_project('en')
assert_equal(normal, 'enwiki')
-
+
def test_normalize_project_uppercase(self):
normal = normalize_project(mediawiki_project.upper())
assert_equal(normal, mediawiki_project)
-
+
def test_normalize_project_nonexistent(self):
normal = normalize_project('blah')
assert_equal(normal, None)
-
+
def test_validate_cohorts(self):
self.helper_reset_validation()
self.cohort.validate_as_user_ids = False
self.session.commit()
v = ValidateCohort(self.cohort)
v.validate_records(self.session, self.cohort)
-
+
assert_equal(self.cohort.validated, True)
assert_equal(len(
self.session.query(WikiUser)
@@ -37,7 +183,7 @@
.filter(WikiUser.valid)
.all()
), 4)
-
+
def test_validate_cohorts_with_invalid_wikiusers(self):
self.helper_reset_validation()
self.cohort.validate_as_user_ids = False
@@ -47,7 +193,7 @@
self.session.commit()
v = ValidateCohort(self.cohort)
v.validate_records(self.session, self.cohort)
-
+
assert_equal(self.cohort.validated, True)
assert_equal(len(
self.session.query(WikiUser)
@@ -67,18 +213,16 @@
def setUp(self):
QueueDatabaseTest.setUp(self)
-
self.mwSession.add(MediawikiUser(user_name='Editor test-specific-0'))
self.mwSession.add(MediawikiUser(user_name='Editor test-specific-1'))
self.mwSession.commit()
-
+
owner_user = User()
self.session.add(owner_user)
self.session.commit()
self.owner_user_id = owner_user.id
-
- def test_small_cohort(self):
+ def test_small_cohort(self):
cohort_upload = CohortUpload()
cohort_upload.name.data = 'small_cohort'
cohort_upload.project.data = mediawiki_project
@@ -91,11 +235,11 @@
# one user with invalid project
{'username': 'Nonexisting2', 'project': 'Nonexisting'},
]
-
+
v = ValidateCohort.from_upload(cohort_upload, self.owner_user_id)
v.task.delay(v).get()
self.session.commit()
-
+
assert_equal(self.session.query(WikiUser).filter(
WikiUser.mediawiki_username == 'Editor
test-specific-0').one().valid, True)
assert_equal(self.session.query(WikiUser).filter(
@@ -104,19 +248,19 @@
WikiUser.mediawiki_username == 'Nonexisting').one().valid, False)
assert_equal(self.session.query(WikiUser).filter(
WikiUser.mediawiki_username == 'Nonexisting2').one().valid, False)
-
+
def test_from_upload_exception(self):
cohort_upload = CohortUpload()
cohort_upload.name.data = 'small_cohort'
cohort_upload.project.data = 'wiki'
cohort_upload.records = [{'fake': 'dict'}]
-
+
v = ValidateCohort.from_upload(cohort_upload, self.owner_user_id)
assert_equal(v, None)
class BasicTests(unittest.TestCase):
-
+
def test_repr(self):
cohort = Cohort(id=1)
v = ValidateCohort(cohort)
diff --git a/wikimetrics/controllers/forms/cohort_upload.py
b/wikimetrics/controllers/forms/cohort_upload.py
index 9e5d8a7..e890b1e 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
@@ -19,7 +19,7 @@
('True', 'User Ids (Numbers found in the user_id column of the user
table)'),
('False', 'User Names (Names found in the user_name column of the user
table)')
])
-
+
@classmethod
def from_request(cls, request):
"""
@@ -28,14 +28,14 @@
values = request.form.copy()
values.update(request.files)
return cls(values)
-
+
def parse_records(self):
"""
You must call this to parse self.records out of the csv file
-
+
Parameters
request : the request with the file to parse
-
+
Returns
nothing, but sets self.records to the parsed lines of the csv
"""
@@ -54,11 +54,11 @@
def parse_records(unparsed, default_project):
"""
Parses records read from a csv file
-
+
Parameters
unparsed : records in array form, as read from a csv
default_project : the default project to attribute to records without
one
-
+
Returns
the parsed records in this form:
{'username':'parsed username', 'project':'as specified or default'}
@@ -77,27 +77,13 @@
else:
username = r[0]
project = default_project
-
+
if username is not None and len(username):
records.append({
'username' : parse_username(username),
'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
- """
- 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..24e71ec 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_
@@ -30,14 +31,14 @@
* Updating the cohort to validated == True once all users have been
validated
"""
task = async_validate
-
+
def __init__(self, cohort):
"""
Parameters:
cohort : an existing cohort
config : global config, we need to know
if we are on dev or testing to validate project name
-
+
Instantiates with these properties:
cohort_id : id of an existing cohort with validated
== False
validate_as_user_ids : if True, records will be checked against
user_id
@@ -45,16 +46,16 @@
"""
self.cohort_id = cohort.id
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
-
+
Parameters:
cohort_upload : the cohort upload form, parsed by WTForms
owner_user_id : the Wikimetrics user id that is uploading
-
+
Returns:
An instance of ValidateCohort
"""
@@ -67,11 +68,11 @@
validated=False,
validate_as_user_ids=cohort_upload.validate_as_user_ids.data ==
'True',
)
- session = db.get_session()
+ session = session or db.get_session()
try:
session.add(cohort)
session.commit()
-
+
cohort_user = CohortUser(
user_id=owner_user_id,
cohort_id=cohort.id,
@@ -79,7 +80,7 @@
)
session.add(cohort_user)
session.commit()
-
+
session.execute(
WikiUser.__table__.insert(), [
{
@@ -98,7 +99,7 @@
return None
finally:
session.close()
-
+
def run(self):
session = db.get_session()
try:
@@ -108,7 +109,7 @@
self.validate_records(session, cohort)
finally:
session.close()
-
+
def validate_records(self, session, cohort):
"""
Fetches the wiki_user(s) already added for self.cohort_id and validates
@@ -116,9 +117,9 @@
or user_name. Once done, sets the valid state and deletes any
duplicates.
Then, it finishes filling in the data model by inserting corresponding
records into the cohort_wiki_users table.
-
+
This is meant to execute asynchronously on celery
-
+
Parameters
session : an active wikimetrics db session to use
cohort : the cohort to validate; must belong to session
@@ -134,16 +135,16 @@
CohortWikiUser.cohort_id == cohort.id
))
session.commit()
-
+
wikiusers = session.query(WikiUser) \
.filter(WikiUser.validating_cohort == cohort.id) \
.all()
-
+
deduplicated = deduplicate_by_key(
wikiusers,
lambda r: (r.mediawiki_username, r.project)
)
-
+
wikiusers_by_project = {}
for wu in deduplicated:
try:
@@ -152,12 +153,12 @@
wu.reason_invalid = 'invalid project:
{0}'.format(wu.project)
wu.valid = False
continue
-
+
wu.project = normalized_project
if wu.project not in wikiusers_by_project:
wikiusers_by_project[wu.project] = []
wikiusers_by_project[wu.project].append(wu)
-
+
# validate bunches of records to update the UI but not kill
performance
if len(wikiusers_by_project[wu.project]) > 999:
validate_users(
@@ -169,18 +170,18 @@
wikiusers_by_project[wu.project] = []
except:
continue
-
+
# validate anything that wasn't big enough for a batch
for project, wikiusers in wikiusers_by_project.iteritems():
if len(wikiusers) > 0:
validate_users(wikiusers, project, self.validate_as_user_ids)
session.commit()
-
+
unique_and_validated = deduplicate_by_key(
deduplicated,
lambda r: (r.mediawiki_username, r.project)
)
-
+
session.execute(
CohortWikiUser.__table__.insert(), [
{
@@ -189,7 +190,7 @@
} for wu in unique_and_validated
]
)
-
+
# clean up any duplicate wiki_user records
session.execute(WikiUser.__table__.delete().where(and_(
WikiUser.validating_cohort == cohort.id,
@@ -197,7 +198,7 @@
)))
cohort.validated = True
session.commit()
-
+
def __repr__(self):
return '<ValidateCohort("{0}")>'.format(self.cohort_id)
@@ -232,7 +233,7 @@
"""
session = db.get_mw_session(project)
users_dict = {wu.mediawiki_username: wu for wu in wikiusers}
-
+
try:
# validate
if validate_as_user_ids:
@@ -240,7 +241,7 @@
clause = MediawikiUser.user_id.in_(keys_as_ints)
else:
clause = MediawikiUser.user_name.in_(users_dict.keys())
-
+
matches = session.query(MediawikiUser).filter(clause).all()
# update results
for match in matches:
@@ -248,22 +249,28 @@
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
users_dict[key].reason_invalid = None
# remove valid matches
users_dict.pop(key)
-
+
# mark the rest invalid
+ # key is going to be a string if bindings are correct, but careful!
+ # it might be a string with chars that cannot be represented w/ ascii
+ # the 'reason_invalid' does not need to have the user_id,
+ # it is on the record on the table
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():
diff --git a/wikimetrics/utils.py b/wikimetrics/utils.py
index 9c2a1df..eeca6f5 100644
--- a/wikimetrics/utils.py
+++ b/wikimetrics/utils.py
@@ -221,3 +221,17 @@
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
+ """
+ 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: merged
Gerrit-Change-Id: I0771e74f3d0745737e3ea96482614382fe09d961
Gerrit-PatchSet: 5
Gerrit-Project: analytics/wikimetrics
Gerrit-Branch: master
Gerrit-Owner: Nuria <[email protected]>
Gerrit-Reviewer: Milimetric <[email protected]>
Gerrit-Reviewer: Nuria <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
