Index: trac/env.py =================================================================== --- trac/env.py (revision 3239) +++ trac/env.py (working copy) @@ -273,12 +273,12 @@ if not cnx: cnx = self.get_db_cnx() cursor = cnx.cursor() - cursor.execute("SELECT DISTINCT s.sid, n.var_value, e.var_value " + cursor.execute("SELECT DISTINCT s.sid, n.value, e.value " "FROM session AS s " - " LEFT JOIN session AS n ON (n.sid=s.sid " - " AND n.authenticated=1 AND n.var_name = 'name') " - " LEFT JOIN session AS e ON (e.sid=s.sid " - " AND e.authenticated=1 AND e.var_name = 'email') " + " LEFT JOIN session_attribute AS n ON (n.sid=s.sid " + " and n.authenticated=1 AND n.name = 'name') " + " LEFT JOIN session_attribute AS e ON (e.sid=s.sid " + " AND e.authenticated=1 AND e.name = 'email') " "WHERE s.authenticated=1 ORDER BY s.sid") for username,name,email in cursor: yield username, name, email Index: trac/db_default.py =================================================================== --- trac/db_default.py (revision 3239) +++ trac/db_default.py (working copy) @@ -17,7 +17,7 @@ from trac.db import Table, Column, Index # Database version identifier. Used for automatic upgrades. -db_version = 17 +db_version = 18 def __mkreports(reports): """Utility function used to create report data in same syntax as the @@ -46,11 +46,17 @@ Column('name'), Column('ipnr'), Column('time', type='int')], - Table('session', key=('sid', 'authenticated', 'var_name'))[ + Table('session', key=('sid', 'authenticated'))[ Column('sid'), Column('authenticated', type='int'), - Column('var_name'), - Column('var_value')], + Column('last_visit', type='int'), + Index(['last_visit']), + Index(['authenticated'])], + Table('session_attribute', key=('sid', 'authenticated', 'name'))[ + Column('sid'), + Column('authenticated', type='int'), + Column('name'), + Column('value')], # Attachments Table('attachment', key=('type', 'id', 'filename'))[ @@ -119,7 +125,8 @@ Column('field'), Column('oldvalue'), Column('newvalue'), - Index(['ticket', 'time'])], + Index(['ticket']), + Index(['time'])], Table('ticket_custom', key=('ticket', 'name'))[ Column('ticket', type='int'), Column('name'), Index: trac/tests/env.py =================================================================== --- trac/tests/env.py (revision 3239) +++ trac/tests/env.py (working copy) @@ -26,13 +26,14 @@ def test_get_known_users(self): """Testing env.get_known_users""" cursor = self.db.cursor() - cursor.execute("INSERT INTO session " - "VALUES ('123',0,'email','a@example.com')") - cursor.executemany("INSERT INTO session VALUES (%s,1,%s,%s)", - [('tom', 'name', 'Tom'), - ('tom', 'email', 'tom@example.com'), - ('joe', 'email', 'joe@example.com'), - ('jane', 'name', 'Jane')]) + cursor.executemany("INSERT INTO session VALUES (%s,%s,0)", + [('123', 0),('tom', 1), ('joe', 1), ('jane', 1)]) + cursor.executemany("INSERT INTO session_attribute VALUES (%s,%s,%s,%s)", + [('123', 0, 'email', 'a@example.com'), + ('tom', 1, 'name', 'Tom'), + ('tom', 1, 'email', 'tom@example.com'), + ('joe', 1, 'email', 'joe@example.com'), + ('jane', 1, 'name', 'Jane')]) users = {} for username,name,email in self.env.get_known_users(self.db): users[username] = (name, email) Index: trac/upgrades/db18.py =================================================================== --- trac/upgrades/db18.py (revision 0) +++ trac/upgrades/db18.py (revision 0) @@ -0,0 +1,59 @@ +from trac.db import Table, Column, Index, DatabaseManager + +def do_upgrade(env, ver, cursor): + cursor.execute("CREATE TEMP TABLE session_old AS SELECT * FROM session") + cursor.execute("DROP TABLE session") + cursor.execute("CREATE TEMP TABLE ticket_change_old AS SELECT * FROM ticket_change") + cursor.execute("DROP TABLE ticket_change") + + # A slightly more denormalized session schema where the 'last_visit' values are + # stored in a column for performance reasons + tables = [Table('session', key=('sid', 'authenticated'))[ + Column('sid'), + Column('authenticated', type='int'), + Column('last_visit', type='int'), + Index(['last_visit']), + Index(['authenticated'])], + Table('session_attribute', key=('sid', 'authenticated', 'name'))[ + Column('sid'), + Column('authenticated', type='int'), + Column('name'), + Column('value')], + Table('ticket_change', key=('ticket', 'time', 'field'))[ + Column('ticket', type='int'), + Column('time', type='int'), + Column('author'), + Column('field'), + Column('oldvalue'), + Column('newvalue'), + Index(['ticket']), + Index(['time'])]] + + db_connector, _ = DatabaseManager(env)._get_connector() + for table in tables: + for stmt in db_connector.to_sql(table): + cursor.execute(stmt) + + # Add an index to the temporary table to speed up the conversion + cursor.execute("CREATE INDEX session_old_sid_idx ON session_old(sid)") + # Insert the sessions into the new table + db = env.get_db_cnx() + cursor.execute("INSERT INTO session (sid, last_visit, authenticated) " + "SELECT distinct s.sid,COALESCE(%s,0),s.authenticated " + "FROM session_old AS s LEFT JOIN session_old AS s2 " + "ON (s.sid=s2.sid AND s2.var_name='last_visit') " + "WHERE s.sid IS NOT NULL" + % db.cast('s2.var_value', 'int')) + cursor.execute("INSERT INTO session_attribute " + "(sid, authenticated, name, value) " + "SELECT s.sid, s.authenticated, s.var_name, s.var_value " + "FROM session_old s " + "WHERE s.var_name <> 'last_visit' AND s.sid IS NOT NULL") + + # Insert ticket change data into the new table + cursor.execute("INSERT INTO ticket_change " + "(ticket, time, author, field, oldvalue, newvalue) " + "SELECT ticket, time, author, field, oldvalue, newvalue " + "FROM ticket_change_old") + + Index: trac/web/tests/session.py =================================================================== --- trac/web/tests/session.py (revision 3239) +++ trac/web/tests/session.py (working copy) @@ -65,8 +65,7 @@ authenticated session when the user logs in. """ cursor = self.db.cursor() - cursor.execute("INSERT INTO session VALUES ('123456', 0, 'foo', 'bar')") - + cursor.execute("INSERT INTO session VALUES ('123456', 0, 0)") incookie = Cookie() incookie['trac_session'] = '123456' outcookie = Cookie() @@ -93,8 +92,7 @@ session['foo'] = 'bar' session.save() cursor = self.db.cursor() - cursor.execute("SELECT var_value FROM session WHERE sid='123456' AND " - "authenticated=0 AND var_name='foo'") + cursor.execute("SELECT value FROM session_attribute WHERE sid='123456'") self.assertEqual('bar', cursor.fetchone()[0]) def test_modify_anonymous_session_var(self): @@ -103,8 +101,9 @@ accordingly for an anonymous session. """ cursor = self.db.cursor() - cursor.execute("INSERT INTO session VALUES ('123456', 0, 'foo', 'bar')") - + cursor.execute("INSERT INTO session VALUES ('123456', 0, 0)") + cursor.execute("INSERT INTO session_attribute VALUES " + "('123456', 0, 'foo', 'bar')") incookie = Cookie() incookie['trac_session'] = '123456' req = Mock(authname='anonymous', base_path='/', incookie=incookie, @@ -113,8 +112,7 @@ self.assertEqual('bar', session['foo']) session['foo'] = 'baz' session.save() - cursor.execute("SELECT var_value FROM session WHERE sid='123456' AND " - "authenticated=0 AND var_name='foo'") + cursor.execute("SELECT value FROM session_attribute WHERE sid='123456'") self.assertEqual('baz', cursor.fetchone()[0]) def test_delete_anonymous_session_var(self): @@ -123,8 +121,9 @@ for an anonymous session. """ cursor = self.db.cursor() - cursor.execute("INSERT INTO session VALUES ('123456', 0, 'foo', 'bar')") - + cursor.execute("INSERT INTO session VALUES ('123456', 0, 0)") + cursor.execute("INSERT INTO session_attribute VALUES " + "('123456', 0, 'foo', 'bar')") incookie = Cookie() incookie['trac_session'] = '123456' req = Mock(authname='anonymous', base_path='/', incookie=incookie, @@ -133,8 +132,8 @@ self.assertEqual('bar', session['foo']) del session['foo'] session.save() - cursor.execute("SELECT COUNT(*) FROM session WHERE sid='123456' AND " - "authenticated=0 AND var_name='foo'") + cursor.execute("SELECT COUNT(*) FROM session_attribute " + "WHERE sid='123456' AND name='foo'") self.assertEqual(0, cursor.fetchone()[0]) def test_purge_anonymous_session(self): @@ -143,8 +142,10 @@ """ cursor = self.db.cursor() cursor.execute("INSERT INTO session " - "VALUES ('987654', 0, 'last_visit', %s)", + "VALUES ('987654', 0, %s)", (time.time() - PURGE_AGE - 3600,)) + cursor.execute("INSERT INTO session_attribute VALUES " + "('987654', 0, 'foo', 'bar')") # We need to modify a different session to trigger the purging incookie = Cookie() @@ -169,14 +170,17 @@ # Make sure the session has data so that it doesn't get dropped cursor = self.db.cursor() cursor.execute("INSERT INTO session " - "VALUES ('123456', 0, 'last_visit', %s)", + "VALUES ('123456', 0, %s)", (int(now - UPDATE_INTERVAL - 3600),)) + cursor.execute("INSERT INTO session_attribute VALUES " + "('123456', 0, 'foo', 'bar')") incookie = Cookie() incookie['trac_session'] = '123456' req = Mock(authname='anonymous', base_path='/', incookie=incookie, outcookie=Cookie()) session = Session(self.env, req) + del session['foo'] session.save() cursor.execute("SELECT COUNT(*) FROM session WHERE sid='123456' AND " @@ -193,8 +197,8 @@ session['foo'] = 'bar' session.save() cursor = self.db.cursor() - cursor.execute("SELECT var_value FROM session WHERE sid='john' AND " - "authenticated=1 AND var_name='foo'") + cursor.execute("SELECT value FROM session_attribute WHERE sid='john'" + "AND name='foo'") self.assertEqual('bar', cursor.fetchone()[0]) def test_modify_authenticated_session_var(self): @@ -203,15 +207,17 @@ accordingly for an authenticated session. """ cursor = self.db.cursor() - cursor.execute("INSERT INTO session VALUES ('john', 1, 'foo', 'bar')") + cursor.execute("INSERT INTO session VALUES ('john', 1, 0)") + cursor.execute("INSERT INTO session_attribute VALUES " + "('john', 1, 'foo', 'bar')") req = Mock(authname='john', base_path='/', incookie=Cookie()) session = Session(self.env, req) self.assertEqual('bar', session['foo']) session['foo'] = 'baz' session.save() - cursor.execute("SELECT var_value FROM session WHERE sid='john' AND " - "authenticated=1 AND var_name='foo'") + cursor.execute("SELECT value FROM session_attribute " + "WHERE sid='john' AND name='foo'") self.assertEqual('baz', cursor.fetchone()[0]) def test_delete_authenticated_session_var(self): @@ -220,15 +226,17 @@ for an authenticated session. """ cursor = self.db.cursor() - cursor.execute("INSERT INTO session VALUES ('john', 1, 'foo', 'bar')") + cursor.execute("INSERT INTO session VALUES ('john', 1, 0)") + cursor.execute("INSERT INTO session_attribute VALUES " + "('john', 1, 'foo', 'bar')") req = Mock(authname='john', base_path='/', incookie=Cookie()) session = Session(self.env, req) self.assertEqual('bar', session['foo']) del session['foo'] session.save() - cursor.execute("SELECT COUNT(*) FROM session WHERE sid='john' AND " - "authenticated=1 AND var_name='foo'") + cursor.execute("SELECT COUNT(*) FROM session_attribute " + "WHERE sid='john' AND name='foo'") self.assertEqual(0, cursor.fetchone()[0]) def test_update_session(self): @@ -240,9 +248,9 @@ # Make sure the session has data so that it doesn't get dropped cursor = self.db.cursor() - cursor.executemany("INSERT INTO session VALUES ('123456', 0, %s, %s)", - [('last_visit', int(now - UPDATE_INTERVAL - 3600)), - ('foo', 'bar')]) + cursor.execute("INSERT INTO session VALUES ('123456', 0, 1)") + cursor.execute("INSERT INTO session_attribute VALUES " + "('123456', 0, 'foo', 'bar')") incookie = Cookie() incookie['trac_session'] = '123456' @@ -254,8 +262,8 @@ self.assertEqual(PURGE_AGE, outcookie['trac_session']['expires']) - cursor.execute("SELECT var_value FROM session WHERE sid='123456' AND " - "authenticated=0 AND var_name='last_visit'") + cursor.execute("SELECT last_visit FROM session WHERE sid='123456' AND " + "authenticated=0") self.assertAlmostEqual(now, int(cursor.fetchone()[0]), -1) Index: trac/web/session.py =================================================================== --- trac/web/session.py (revision 3239) +++ trac/web/session.py (working copy) @@ -3,6 +3,7 @@ # Copyright (C) 2004-2005 Edgewall Software # Copyright (C) 2004 Daniel Lundin # Copyright (C) 2004-2005 Christopher Lenz +# Copyright (C) 2006 Jonas Borgström # All rights reserved. # # This software is licensed as described in the file COPYING, which @@ -34,6 +35,8 @@ self.env = env self.req = req self.sid = None + self.last_visit = 0 + self._new = True self._old = {} if req.authname == 'anonymous': if not req.incookie.has_key(COOKIE_KEY): @@ -58,16 +61,24 @@ db = self.env.get_db_cnx() cursor = db.cursor() self.sid = sid - cursor.execute("SELECT var_name,var_value FROM session " + cursor.execute("SELECT last_visit FROM session " "WHERE sid=%s AND authenticated=%s", (sid, int(authenticated))) + row = cursor.fetchone() + if not row: + return + self._new = False + self.last_visit = int(row[0]) + cursor.execute("SELECT name,value FROM session_attribute " + "WHERE sid=%s and authenticated=%s", + (sid, int(authenticated))) for name, value in cursor: self[name] = value self._old.update(self) # Refresh the session cookie if this is the first visit since over a day - if not authenticated and self.has_key('last_visit'): - if time.time() - int(self['last_visit']) > UPDATE_INTERVAL: + if not authenticated and self.last_visit: + if time.time() - self.last_visit > UPDATE_INTERVAL: self.bake_cookie() def change_sid(self, new_sid): @@ -78,8 +89,7 @@ return db = self.env.get_db_cnx() cursor = db.cursor() - cursor.execute("SELECT sid FROM session WHERE sid=%s " - "AND authenticated=0", (new_sid,)) + cursor.execute("SELECT sid FROM session WHERE sid=%s", (new_sid,)) if cursor.fetchone(): raise TracError(Markup('Session "%s" already exists.
' 'Please choose a different session ID.', @@ -87,6 +97,9 @@ self.env.log.debug('Changing session ID %s to %s' % (self.sid, new_sid)) cursor.execute("UPDATE session SET sid=%s WHERE sid=%s " "AND authenticated=0", (new_sid, self.sid)) + cursor.execute("UPDATE session_attribute SET sid=%s " + "WHERE sid=%s and authenticated=0", + (new_sid, self.sid)) db.commit() self.sid = new_sid self.bake_cookie() @@ -107,6 +120,8 @@ # simply delete the anonymous session cursor.execute("DELETE FROM session WHERE sid=%s " "AND authenticated=0", (sid,)) + cursor.execute("DELETE FROM session_attribute WHERE sid=%s " + "AND authenticated=0", (sid,)) else: # Otherwise, update the session records so that the session ID is # the user name, and the authenticated flag is set @@ -116,6 +131,10 @@ cursor.execute("UPDATE session SET sid=%s,authenticated=1 " "WHERE sid=%s AND authenticated=0", (self.req.authname, sid)) + cursor.execute("UPDATE session_attribute SET sid=%s," + "authenticated=1 WHERE sid=%s", + (self.req.authname, sid)) + self._new = False db.commit() self.sid = sid @@ -127,64 +146,50 @@ # persist it return - changed = False - now = int(time.time()) - - if self.req.authname == 'anonymous': - # Update the session last visit time if it is over an hour old, - # so that session doesn't get purged - last_visit = int(self.get('last_visit', 0)) - if now - last_visit > UPDATE_INTERVAL: - self.env.log.info("Refreshing session %s" % self.sid) - self['last_visit'] = now - - # If the only data in the session is the last_visit time, it makes - # no sense to keep the session around - if len(self.items()) == 1: - del self['last_visit'] - db = self.env.get_db_cnx() cursor = db.cursor() authenticated = int(self.req.authname != 'anonymous') - # Find all new or modified session variables and persist their values to - # the database - for k,v in self.items(): - if not self._old.has_key(k): - self.env.log.debug('Adding variable %s with value "%s" to ' - 'session %s' % (k, v, - self.sid or self.req.authname)) - cursor.execute("INSERT INTO session VALUES(%s,%s,%s,%s)", - (self.sid, authenticated, k, v)) - changed = True - elif v != self._old[k]: - self.env.log.debug('Changing variable %s from "%s" to "%s" in ' - 'session %s' % (k, self._old[k], v, - self.sid)) - cursor.execute("UPDATE session SET var_value=%s " - "WHERE sid=%s AND authenticated=%s " - "AND var_name=%s", (v, self.sid, authenticated, - k)) - changed = True + if self._new: + self._new = False + cursor.execute("INSERT INTO session (sid,last_visit,authenticated) " + "VALUES(%s,%s,%s)", (self.sid, + self.last_visit, + authenticated)) + if self._old.items() != self.items(): + attrs = [(self.sid, authenticated, k, v) for k, v in self.items()] + cursor.execute("DELETE FROM session_attribute WHERE sid=%s", + (self.sid,)) + self._old = dict(self.items()) + if attrs: + cursor.executemany("INSERT INTO session_attribute " + "(sid,authenticated,name,value) " + "VALUES(%s,%s,%s,%s)", attrs) + elif not authenticated: + # No need to keep around empty unauthenticated sessions + cursor.execute("DELETE FROM session " + "WHERE sid=%s AND authenticated=0" % (self.sid,)) + return - # Find all variables that have been deleted and also remove them from - # the database - for k in [k for k in self._old.keys() if not self.has_key(k)]: - self.env.log.debug('Deleting variable %s from session %s' - % (k, self.sid or self.req.authname)) - cursor.execute("DELETE FROM session WHERE sid=%s AND " - "authenticated=%s AND var_name=%s", - (self.sid, authenticated, k)) - changed = True - - if changed: + now = int(time.time()) + # Update the session last visit time if it is over an hour old, + # so that session doesn't get purged + if now - self.last_visit > UPDATE_INTERVAL: + self.last_visit = now + self.env.log.info("Refreshing session %s" % self.sid) + cursor.execute('UPDATE session SET last_visit=%s ' + 'WHERE sid=%s AND authenticated=%s', + (self.last_visit, self.sid, authenticated)) # Purge expired sessions. We do this only when the session was # changed as to minimize the purging. mintime = now - PURGE_AGE self.env.log.debug('Purging old, expired, sessions.') - cursor.execute("DELETE FROM session WHERE authenticated=0 AND " - "sid IN (SELECT sid FROM session WHERE " - "var_name='last_visit' AND var_value < %s)", + cursor.execute("DELETE FROM session_attribute " + "WHERE authenticated=0 AND sid " + "IN (SELECT sid FROM session WHERE " + "authenticated=0 AND last_visit < %s)", (mintime,)) - - db.commit() + cursor.execute("DELETE FROM session WHERE " + "authenticated=0 AND last_visit < %s", + (mintime,)) + db.commit()