Mercurial > p > roundup > code
changeset 6806:bdd28b244839
- issue2551223 - fix timestamp truncation in mysql and postgresql
The data types used to represent timestamps in pg and mysql for
ephemeral tables: sessions and otks don't have enough signifcant
digits to work. As a result the timestamps are rounduped (up/down)
rsuling in the stored timestamp being 2 minutes (pg) or 2-3
hours(mysql) off from what it should be.
Modify db schema to use a numeric type that preserves more significant
figures. Implement schema upgrade. Document need for upgrade in
upgrading.txt.
Write tests for schema upgrade.
Implement test for updateTimestamp method on BasicDatabase that showed
this issue in the first place. Write overrides for test for
anydbm/memorydb which store timestamp properly or not at all.
| author | John Rouillard <rouilj@ieee.org> |
|---|---|
| date | Mon, 25 Jul 2022 17:20:20 -0400 |
| parents | 09d9c646ca89 |
| children | 30bb17dc9f82 |
| files | CHANGES.txt doc/upgrading.txt roundup/backends/back_mysql.py roundup/backends/back_postgresql.py roundup/backends/rdbms_common.py test/session_common.py test/test_anydbm.py test/test_memorydb.py test/test_mysql.py test/test_postgresql.py test/test_sqlite.py |
| diffstat | 11 files changed, 333 insertions(+), 10 deletions(-) [+] |
line wrap: on
line diff
--- a/CHANGES.txt Mon Jul 25 16:39:31 2022 -0400 +++ b/CHANGES.txt Mon Jul 25 17:20:20 2022 -0400 @@ -22,6 +22,10 @@ sqlite. New databases are created for session data (db-session) and one time key data (db-otk). The data is ephemeral so no need to migrate. +- issue2551223 - Timestamps are truncated in mysql and postgresql for + session and otk database tables. Modify db schema to use a numeric + type that preserves more significant figures. See upgrading.txt for + required steps. Features:
--- a/doc/upgrading.txt Mon Jul 25 16:39:31 2022 -0400 +++ b/doc/upgrading.txt Mon Jul 25 17:20:20 2022 -0400 @@ -35,6 +35,38 @@ Migrating from 2.2.0 to 2.3.0 ============================= +Rdbms version change from 7 to 8 (required) +------------------------------------------- + +This release includes a change that requires updates to the +database schema. + +Sessions and one time key (otks) tables in the Mysql and +PostgreSQL database use a numeric type that +truncates/rounds expiration timestamps. This results in +entries being purged early or late (depending on whether +it rounds up or down). The discrepancy is a couple of +days for Mysql or a couple of minutes for PostgreSQL. + +Session keys stay for a week or more and CSRF keys are +two weeks by default. As a result, this isn't usually a +visible issue. This migration updates the numeric types +to ones that supports more significant figures. + +You should backup your instance and run the +``roundup-admin -i <tracker_home> migrate`` +command for all your trackers once you've +installed the latest code base. + +Do this before you use the web, command-line or mail +interface and before any users access the tracker. + +If successful, this command will respond with either +"Tracker updated" (if you've not previously run it on an +RDBMS backend) or "No migration action required" (if you +have run it, or have used another interface to the tracker, +or are using anydbm). + Session/OTK data storage for SQLite backend changed ---------------------------------------------------
--- a/roundup/backends/back_mysql.py Mon Jul 25 16:39:31 2022 -0400 +++ b/roundup/backends/back_mysql.py Mon Jul 25 17:20:20 2022 -0400 @@ -219,13 +219,13 @@ def create_version_2_tables(self): # OTK store self.sql('''CREATE TABLE otks (otk_key VARCHAR(255), - otk_value TEXT, otk_time FLOAT(20)) + otk_value TEXT, otk_time DOUBLE) ENGINE=%s'''%self.mysql_backend) self.sql('CREATE INDEX otks_key_idx ON otks(otk_key)') # Sessions store self.sql('''CREATE TABLE sessions (session_key VARCHAR(255), - session_time FLOAT(20), session_value TEXT) + session_time DOUBLE, session_value TEXT) ENGINE=%s'''%self.mysql_backend) self.sql('''CREATE INDEX sessions_key_idx ON sessions(session_key)''') @@ -421,6 +421,13 @@ # column length and maxlength. c.execute(sql, (self.indexer.maxlength + 5,)) + def fix_version_7_tables(self): + # Modify type for session.session_time/otk.otk_time column. + sql = "alter table sessions modify session_time double" + self.sql(sql) + sql = "alter table otks modify otk_time double" + self.sql(sql) + def __repr__(self): return '<myroundsql 0x%x>'%id(self)
--- a/roundup/backends/back_postgresql.py Mon Jul 25 16:39:31 2022 -0400 +++ b/roundup/backends/back_postgresql.py Mon Jul 25 17:20:20 2022 -0400 @@ -234,12 +234,12 @@ def create_version_2_tables(self): # OTK store self.sql('''CREATE TABLE otks (otk_key VARCHAR(255), - otk_value TEXT, otk_time REAL)''') + otk_value TEXT, otk_time float)''') self.sql('CREATE INDEX otks_key_idx ON otks(otk_key)') # Sessions store self.sql('''CREATE TABLE sessions ( - session_key VARCHAR(255), session_time REAL, + session_key VARCHAR(255), session_time float, session_value TEXT)''') self.sql('''CREATE INDEX sessions_key_idx ON sessions(session_key)''') @@ -296,6 +296,14 @@ self._add_fts_table() + def fix_version_7_tables(self): + # Modify type for session.session_time/otk.otk_time column. + # float is double precision 15 signifcant digits + sql = 'alter table sessions alter column session_time type float' + self.sql(sql) + sql = 'alter table otks alter column otk_time type float' + self.sql(sql) + def add_actor_column(self): # update existing tables to have the new actor column tables = self.database_schema['tables']
--- a/roundup/backends/rdbms_common.py Mon Jul 25 16:39:31 2022 -0400 +++ b/roundup/backends/rdbms_common.py Mon Jul 25 17:20:20 2022 -0400 @@ -331,7 +331,7 @@ # update this number when we need to make changes to the SQL structure # of the backend database - current_db_version = 7 + current_db_version = 8 db_version_updated = False def upgrade_db(self): @@ -381,6 +381,10 @@ self.log_info('upgrade to version 7') self.fix_version_6_tables() + if version < 8: + self.log_info('upgrade to version 8') + self.fix_version_7_tables() + self.database_schema['version'] = self.current_db_version self.db_version_updated = True return 1 @@ -422,6 +426,12 @@ # You would think ALTER commands would be the same but nooo. pass + def fix_version_7_tables(self): + # Default (used by sqlite): NOOP + # Each backend mysql, postgres overrides this + # You would think ALTER commands would be the same but nooo. + pass + def _convert_journal_tables(self): """Get current journal table contents, drop the table and re-create""" c = self.cursor
--- a/test/session_common.py Mon Jul 25 16:39:31 2022 -0400 +++ b/test/session_common.py Mon Jul 25 17:20:20 2022 -0400 @@ -59,3 +59,32 @@ self.sessions.set('random_key', text='nope') self.assertEqual(self.sessions.get('random_key', 'text'), 'nope') + # overridden in dbm and memory backends + def testUpdateTimestamp(self): + def get_ts_via_sql(self): + sql = '''select %(name)s_time from %(name)ss + where %(name)s_key = '%(session)s';'''% \ + {'name': self.sessions.name, + 'session': 'random_session'} + + self.sessions.cursor.execute(sql) + db_tstamp = self.sessions.cursor.fetchone() + return db_tstamp + + # make sure timestamp is older than one minute so update will apply + timestamp = time.time() - 62 + self.sessions.set('random_session', text='hello, world!', + __timestamp=timestamp) + + self.sessions.updateTimestamp('random_session') + # this doesn't work as the rdbms backends have a + # session_time, otk_time column and the timestamp in the + # session marshalled payload isn't updated. The dbm + # backend does update the __timestamp value so it works + # for dbm. + #self.assertNotEqual (self.sessions.get('random_session', + # '__timestamp'), + # timestamp) + + # use 61 to allow a fudge factor + self.assertGreater(get_ts_via_sql(self)[0] - timestamp, 61)
--- a/test/test_anydbm.py Mon Jul 25 16:39:31 2022 -0400 +++ b/test/test_anydbm.py Mon Jul 25 17:20:20 2022 -0400 @@ -55,6 +55,19 @@ class anydbmSessionTest(anydbmOpener, SessionTest, unittest.TestCase): s2b = lambda x,y: strings.s2b(y) + # this only works for dbm. other backends don't change the __timestamp + # value, they have a separate column for the timestamp so they can + # update it with SQL. + def testUpdateTimestamp(self): + # make sure timestamp is older than one minute so update will work + timestamp = time.time() - 62 + self.sessions.set('random_session', text='hello, world!', + __timestamp=timestamp) + self.sessions.updateTimestamp('random_session') + self.assertNotEqual (self.sessions.get('random_session', + '__timestamp'), + timestamp) + class anydbmSpecialActionTestCase(anydbmOpener, SpecialActionTest, unittest.TestCase): backend = 'anydbm'
--- a/test/test_memorydb.py Mon Jul 25 16:39:31 2022 -0400 +++ b/test/test_memorydb.py Mon Jul 25 17:20:20 2022 -0400 @@ -63,5 +63,9 @@ setupSchema(self.db, 1, self.module) self.sessions = self.db.sessions + # doesn't work for memory as it uses a mock for session db. + def testUpdateTimestamp(self): + pass + # vim: set filetype=python ts=4 sw=4 et si
--- a/test/test_mysql.py Mon Jul 25 16:39:31 2022 -0400 +++ b/test/test_mysql.py Mon Jul 25 17:20:20 2022 -0400 @@ -71,7 +71,19 @@ self.db.issue.create(title="flebble frooz") self.db.commit() - if self.db.database_schema['version'] != 7: + if self.db.database_schema['version'] > 7: + # make testUpgrades run the downgrade code only. + if hasattr(self, "downgrade_only"): + # we are being called by an earlier test + self.testUpgrade_7_to_8() + self.assertEqual(self.db.database_schema['version'], 7) + else: + # we are being called directly + self.downgrade_only = True + self.testUpgrade_7_to_8() + self.assertEqual(self.db.database_schema['version'], 7) + del(self.downgrade_only) + elif self.db.database_schema['version'] != 7: self.skipTest("This test only runs for database version 7") @@ -111,6 +123,65 @@ self.assertEqual(self.db.database_schema['version'], self.db.current_db_version) + def testUpgrade_7_to_8(self): + # load the database + self.db.issue.create(title="flebble frooz") + self.db.commit() + + if self.db.database_schema['version'] != 8: + self.skipTest("This test only runs for database version 8") + + # change otk and session db's _time value to their original types + sql = "alter table sessions modify session_time float;" + self.db.sql(sql) + sql = "alter table otks modify otk_time float;" + self.db.sql(sql) + + # verify they truncate long ints. + test_double = 1658718284.7616878 + for tablename in ['otk', 'session']: + self.db.sql( + 'insert %(name)ss(%(name)s_key, %(name)s_time, %(name)s_value) ' + 'values("foo", %(double)s, "value");'%{'name': tablename, + 'double': test_double} + ) + + self.db.cursor.execute('select %(name)s_time from %(name)ss ' + 'where %(name)s_key = "foo"'%{'name': tablename}) + + self.assertNotAlmostEqual(self.db.cursor.fetchone()[0], + test_double, -1) + + # cleanup or else the inserts after the upgrade will not + # work. + self.db.sql("delete from %(name)ss where %(name)s_key='foo'"%{ + 'name': tablename} ) + + self.db.database_schema['version'] = 7 + + if hasattr(self,"downgrade_only"): + return + + # test upgrade + self.db.post_init() + + # verify they keep all signifcant digits before the decimal point + for tablename in ['otk', 'session']: + self.db.sql( + 'insert %(name)ss(%(name)s_key, %(name)s_time, %(name)s_value) ' + 'values("foo", %(double)s, "value");'%{'name': tablename, + 'double': test_double} + ) + + self.db.cursor.execute('select %(name)s_time from %(name)ss ' + 'where %(name)s_key = "foo"'%{'name': tablename}) + + # -1 compares just the integer part. No fractions. + self.assertAlmostEqual(self.db.cursor.fetchone()[0], + test_double, -1) + + self.assertEqual(self.db.database_schema['version'], 8) + @skip_mysql class mysqlROTest(mysqlOpener, ROTest, unittest.TestCase): def setUp(self):
--- a/test/test_postgresql.py Mon Jul 25 16:39:31 2022 -0400 +++ b/test/test_postgresql.py Mon Jul 25 17:20:20 2022 -0400 @@ -84,9 +84,19 @@ self.db.issue.create(title="flebble frooz") self.db.commit() - if self.db.database_schema['version'] != 7: - # consider calling next testUpgrade script to roll back - # schema to version 7. + if self.db.database_schema['version'] > 7: + # make testUpgrades run the downgrade code only. + if hasattr(self, "downgrade_only"): + # we are being called by an earlier test + self.testUpgrade_7_to_8() + self.assertEqual(self.db.database_schema['version'], 7) + else: + # we are being called directly + self.downgrade_only = True + self.testUpgrade_7_to_8() + self.assertEqual(self.db.database_schema['version'], 7) + del(self.downgrade_only) + elif self.db.database_schema['version'] != 7: self.skipTest("This test only runs for database version 7") # remove __fts table/index; shrink length of __words._words @@ -140,6 +150,65 @@ self.assertEqual(self.db.database_schema['version'], self.db.current_db_version) + def testUpgrade_7_to_8(self): + """ change _time fields in BasicDatabases to double """ + # load the database + self.db.issue.create(title="flebble frooz") + self.db.commit() + + if self.db.database_schema['version'] != 8: + self.skipTest("This test only runs for database version 8") + + # change otk and session db's _time value to their original types + sql = "alter table sessions alter column session_time type REAL;" + self.db.sql(sql) + sql = "alter table otks alter column otk_time type REAL;" + self.db.sql(sql) + + # verify they truncate long ints. + test_double = 1658718284.7616878 + for tablename in ['otk', 'session']: + self.db.sql( + 'insert into %(name)ss(%(name)s_key, %(name)s_time, %(name)s_value) ' + "values ('foo', %(double)s, 'value');"%{'name': tablename, + 'double': test_double} + ) + + self.db.cursor.execute('select %(name)s_time from %(name)ss ' + "where %(name)s_key = 'foo'"%{'name': tablename}) + + self.assertNotAlmostEqual(self.db.cursor.fetchone()[0], + test_double, -1) + + # cleanup or else the inserts after the upgrade will not + # work. + self.db.sql("delete from %(name)ss where %(name)s_key='foo'"%{ + 'name': tablename} ) + + self.db.database_schema['version'] = 7 + + if hasattr(self,"downgrade_only"): + return + + # test upgrade altering row + self.db.post_init() + + # verify they keep all signifcant digits before the decimal point + for tablename in ['otk', 'session']: + self.db.sql( + 'insert into %(name)ss(%(name)s_key, %(name)s_time, %(name)s_value) ' + "values ('foo', %(double)s, 'value');"%{'name': tablename, + 'double': test_double} + ) + + self.db.cursor.execute('select %(name)s_time from %(name)ss ' + "where %(name)s_key = 'foo'"%{'name': tablename}) + + self.assertAlmostEqual(self.db.cursor.fetchone()[0], + test_double, -1) + + self.assertEqual(self.db.database_schema['version'], 8) + @skip_postgresql class postgresqlROTest(postgresqlOpener, ROTest, unittest.TestCase): def setUp(self):
--- a/test/test_sqlite.py Mon Jul 25 16:39:31 2022 -0400 +++ b/test/test_sqlite.py Mon Jul 25 17:20:20 2022 -0400 @@ -19,6 +19,7 @@ import sqlite3 as sqlite from roundup.backends import get_backend, have_backend +from roundup.backends.sessions_sqlite import Sessions, OneTimeKeys from .db_test_base import DBTest, ROTest, SchemaTest, ClassicInitTest, config from .db_test_base import ConcurrentDBTest, FilterCacheTest @@ -41,7 +42,19 @@ self.db.issue.create(title="flebble frooz") self.db.commit() - if self.db.database_schema['version'] != 7: + if self.db.database_schema['version'] > 7: + # make testUpgrades run the downgrade code only. + if hasattr(self, "downgrade_only"): + # we are being called by an earlier test + self.testUpgrade_7_to_8() + self.assertEqual(self.db.database_schema['version'], 7) + else: + # we are being called directly + self.downgrade_only = True + self.testUpgrade_7_to_8() + self.assertEqual(self.db.database_schema['version'], 7) + del(self.downgrade_only) + elif self.db.database_schema['version'] != 7: self.skipTest("This test only runs for database version 7") self.db.database_schema['version'] = 6 @@ -72,6 +85,69 @@ self.assertEqual(self.db.database_schema['version'], self.db.current_db_version) + def testUpgrade_7_to_8(self): + # load the database + self.db.issue.create(title="flebble frooz") + self.db.commit() + + if self.db.database_schema['version'] != 8: + self.skipTest("This test only runs for database version 8") + + # set up separate session/otk db's. + self.db.Otk = OneTimeKeys(self.db) + self.db.Session = Sessions(self.db) + + handle={} + handle['otk'] = self.db.Otk + handle['session'] = self.db.Session + + # verify they don't truncate long ints. + test_double = 1658718284.7616878 + for tablename in ['otk', 'session']: + Bdb = handle[tablename] + Bdb.sql( + 'insert into %(name)ss(%(name)s_key, %(name)s_time, %(name)s_value) ' + 'values("foo", %(double)s, "value");'%{'name': tablename, + 'double': test_double} + ) + + Bdb.cursor.execute('select %(name)s_time from %(name)ss ' + 'where %(name)s_key = "foo"'%{'name': tablename}) + + self.assertAlmostEqual(Bdb.cursor.fetchone()[0], + test_double, -1) + + # cleanup or else the inserts after the upgrade will not + # work. + Bdb.sql("delete from %(name)ss where %(name)s_key='foo'"%{ + 'name': tablename} ) + + self.db.database_schema['version'] = 7 + + if hasattr(self,"downgrade_only"): + return + + # test upgrade altering row + self.db.post_init() + + # verify they keep all signifcant digits before the decimal point + for tablename in ['otk', 'session']: + Bdb = handle[tablename] + Bdb.sql( + 'insert into %(name)ss(%(name)s_key, %(name)s_time, %(name)s_value) ' + 'values("foo", %(double)s, "value");'%{'name': tablename, + 'double': test_double} + ) + + Bdb.cursor.execute('select %(name)s_time from %(name)ss ' + 'where %(name)s_key = "foo"'%{'name': tablename}) + + self.assertAlmostEqual(Bdb.cursor.fetchone()[0], + test_double, -1) + + self.assertEqual(self.db.database_schema['version'], 8) + + class sqliteROTest(sqliteOpener, ROTest, unittest.TestCase): pass