Merge pull request #417 from scitran/bad-timestamps-2-electric-bogaloo

Fix malformed timestamps

api/dao/hierarchy.py

@@ -355,6 +355,8 @@ def _update_hierarchy(container, container_type, metadata):
         session_obj = None
         if session.keys():
             session['modified'] = now
+            if session.get('timestamp'):
+                session['timestamp'] = dateutil.parser.parse(session['timestamp'])
             session_obj = _update_container_nulls({'_id': container['session']},  session, 'sessions')
         if session_obj is None:
             session_obj = get_container('session', container['session'])

bin/database.py

@@ -11,7 +11,7 @@ from api import config
 from api.dao import containerutil
 from api.jobs.jobs import Job
 
-CURRENT_DATABASE_VERSION = 14 # An int that is bumped when a new schema change is made
+CURRENT_DATABASE_VERSION = 15 # An int that is bumped when a new schema change is made
 
 def get_db_version():
 
@@ -315,7 +315,7 @@ def upgrade_to_10():
 
 def upgrade_to_11():
     """
-    scitran/core issue #363
+    scitran/core issue #362
 
     Restructures job objects' `inputs` field from a dict with arbitrary keys
     into a list where the key becomes the field `input`
@@ -379,6 +379,50 @@ def upgrade_to_14():
         {'_id': 'config', 'persistent.schema_path': {'$exists': True}},
         {'$unset': {'persistent.schema_path': ''}})
 
+def upgrade_to_15():
+    """
+    scitran/pull issue #417
+
+    First remove all timestamps that are empty or not mongo date or string format.
+    Then attempt to convert strings to dates, removing those that cannot be converted.
+    Mongo $type maps: String = 2, Date = 9
+    """
+    query = {}
+    query['$or'] = [
+                    {'timestamp':''},
+                    {'$and': [
+                        {'timestamp': {'$exists': True}},
+                        {'timestamp': {'$not': {'$type':2}}},
+                        {'timestamp': {'$not': {'$type':9}}}
+                    ]}
+                ]
+    unset = {'$unset': {'timestamp': ''}}
+
+    config.db.sessions.update_many(query, unset)
+    config.db.acquisitions.update_many(query, unset)
+
+    query =  {'$and': [
+                {'timestamp': {'$exists': True}},
+                {'timestamp': {'$type':2}}
+            ]}
+    sessions = config.db.sessions.find(query)
+    for s in sessions:
+        try:
+            fixed_timestamp = dateutil.parser.parse(s['timestamp'])
+        except:
+            config.db.sessions.update_one({'_id': s['_id']}, {'$unset': {'timestamp': ''}})
+            continue
+        config.db.sessions.update_one({'_id': s['_id']}, {'$set': {'timestamp': fixed_timestamp}})
+
+    acquisitions = config.db.acquisitions.find(query)
+    for a in acquisitions:
+        try:
+            fixed_timestamp = dateutil.parser.parse(a['timestamp'])
+        except:
+            config.db.sessions.update_one({'_id': a['_id']}, {'$unset': {'timestamp': ''}})
+            continue
+        config.db.sessions.update_one({'_id': a['_id']}, {'$set': {'timestamp': fixed_timestamp}})
+
 def upgrade_schema():
     """
     Upgrades db to the current schema version
@@ -387,36 +431,15 @@ def upgrade_schema():
     """
 
     db_version = get_db_version()
-    try:
-        if db_version < 1:
-            upgrade_to_1()
-        if db_version < 2:
-            upgrade_to_2()
-        if db_version < 3:
-            upgrade_to_3()
-        if db_version < 4:
-            upgrade_to_4()
-        if db_version < 5:
-            upgrade_to_5()
-        if db_version < 6:
-            upgrade_to_6()
-        if db_version < 7:
-            upgrade_to_7()
-        if db_version < 8:
-            upgrade_to_8()
-        if db_version < 9:
-            upgrade_to_9()
-        if db_version < 10:
-            upgrade_to_10()
-        if db_version < 11:
-            upgrade_to_11()
-        if db_version < 12:
-            upgrade_to_12()
-        if db_version < 13:
-            upgrade_to_13()
-        if db_version < 14:
-            upgrade_to_14()
 
+    try:
+        while db_version < CURRENT_DATABASE_VERSION:
+            db_version += 1
+            upgrade_script = 'upgrade_to_'+str(db_version)
+            globals()[upgrade_script]()
+    except KeyError as e:
+        logging.exception('Attempted to upgrade using script that does not exist: {}'.format(e))
+        sys.exit(1)
     except Exception as e:
         logging.exception('Incremental upgrade of db failed')
         sys.exit(1)
@@ -424,18 +447,19 @@ def upgrade_schema():
         config.db.singletons.update_one({'_id': 'version'}, {'$set': {'database': CURRENT_DATABASE_VERSION}})
         sys.exit(0)
 
-try:
-    if len(sys.argv) > 1:
-        if sys.argv[1] == 'confirm_schema_match':
-            confirm_schema_match()
-        elif sys.argv[1] == 'upgrade_schema':
-            upgrade_schema()
+if __name__ == '__main__':
+    try:
+        if len(sys.argv) > 1:
+            if sys.argv[1] == 'confirm_schema_match':
+                confirm_schema_match()
+            elif sys.argv[1] == 'upgrade_schema':
+                upgrade_schema()
+            else:
+                logging.error('Unknown method name given as argv to database.py')
+                sys.exit(1)
         else:
-            logging.error('Unknown method name given as argv to database.py')
+            logging.error('No method name given as argv to database.py')
             sys.exit(1)
-    else:
-        logging.error('No method name given as argv to database.py')
+    except Exception as e:
+        logging.exception('Unexpected error in database.py')
         sys.exit(1)
-except Exception as e:
-    logging.exception('Unexpected error in database.py')
-    sys.exit(1)

requirements_dev.txt

 # Development packages
 coverage==4.0.3
 coveralls==1.1
+mock==2.0.0
 nose==1.3.7
 PasteScript==2.0.2
 pylint==1.5.3

test/unit_tests/test_db_upgrade.py

+import pytest
+
+from mock import Mock, patch
+
+from bin import database
+from api import config
+
+CDV = database.CURRENT_DATABASE_VERSION
+
+def test_all_upgrade_scripts_exist():
+    for i in range(1, CDV):
+        script_name = 'upgrade_to_{}'.format(i)
+        assert hasattr(database, script_name)
+
+def test_CDV_was_bumped():
+    script_name = 'upgrade_to_{}'.format(CDV+1)
+    assert hasattr(database, script_name) is False
+
+
+@patch('api.config.get_version', Mock(return_value={'database': 5}))
+def test_get_db_version_from_config():
+    assert database.get_db_version() == 5
+
+
+@pytest.fixture(scope='function')
+def database_mock_setup():
+    setattr(config.db.singletons, 'update_one', Mock())
+    for i in range(1, CDV):
+        script_name = 'upgrade_to_{}'.format(i)
+        setattr(database, script_name, Mock())
+
+@patch('bin.database.get_db_version', Mock(return_value=0))
+def test_all_upgrade_scripts_ran(database_mock_setup):
+    with pytest.raises(SystemExit):
+        database.upgrade_schema()
+    for i in range(1, CDV):
+        script_name = 'upgrade_to_{}'.format(i)
+        assert getattr(database, script_name).called
+
+@patch('bin.database.get_db_version', Mock(return_value=CDV-4))
+def test_necessary_upgrade_scripts_ran(database_mock_setup):
+    with pytest.raises(SystemExit):
+        database.upgrade_schema()
+    # Assert the necessary scripts were called
+    for i in range(CDV-3, CDV):
+        script_name = 'upgrade_to_{}'.format(i)
+        assert getattr(database, script_name).called
+    # But not the scripts before it
+    for i in range(1, CDV-4):
+        script_name = 'upgrade_to_{}'.format(i)
+        assert getattr(database, script_name).called is False