fixed journal marshalling in RDBMS backends [SF#943627]

fixed handling of key values starting with numbers [SF#941363]

fixed journal "param" column size in RDBMS backends

Author: Richard Jones

CHANGES.txt

@@ -10,6 +10,9 @@ Fixed:
 - fix nested scope bug in rdbms multilink sorting
 - re-seed the random number generator for each request
 - postgresql backend altered to not use popen (thanks Georges Martin)
+- fixed journal marshalling in RDBMS backends (sf bug 943627)
+- fixed handling of key values starting with numbers (sf bug 941363)
+- fixed journal "param" column size in RDBMS backends
 
 
 2004-04-18 0.7.0b3

doc/index.txt

@@ -102,6 +102,7 @@ Fredrik Lundh,
 Georges Martin,
 Gordon McMillan,
 John F Meinel Jr,
+Truls E. N�ss,
 Patrick Ohly,
 Luke Opperman,
 Will Partain,

roundup/backends/back_mysql.py

@@ -130,6 +130,7 @@ class Database(Database):
         hyperdb.Password  : str,
         hyperdb.Boolean   : int,
         hyperdb.Number    : lambda x: x,
+        hyperdb.Multilink : lambda x: x,    # used in journal marshalling
     }
 
     def sql_open_connection(self):
@@ -302,13 +303,14 @@ def add_new_columns_v2(self):
                     print >>hyperdb.DEBUG, '... data', entry
                 execute(sql, tuple(entry))
 
-            # now load up the old journal data
+            # now load up the old journal data to migrate it
             cols = ','.join('nodeid date tag action params'.split())
             sql = 'select %s from %s__journal'%(cols, cn)
             if __debug__:
                 print >>hyperdb.DEBUG, 'migration', (self, sql)
             execute(sql)
 
+            # data conversions
             olddata = []
             for nodeid, journaldate, journaltag, action, params in \
                     self.cursor.fetchall():
@@ -394,7 +396,7 @@ def create_journal_table(self, spec):
             for x in 'nodeid date tag action params'.split()])
         sql = '''create table %s__journal (
             nodeid integer, date timestamp, tag varchar(255),
-            action varchar(255), params varchar(255)) type=%s'''%(
+            action varchar(255), params text) type=%s'''%(
             spec.classname, self.mysql_backend)
         if __debug__:
             print >>hyperdb.DEBUG, 'create_journal_table', (self, sql)

roundup/backends/back_sqlite.py

@@ -1,4 +1,4 @@
-# $Id: back_sqlite.py,v 1.26 2004-04-18 23:05:18 richard Exp $
+# $Id: back_sqlite.py,v 1.27 2004-05-02 23:16:05 richard Exp $
 '''Implements a backend for SQLite.
 
 See https://pysqlite.sourceforge.net/ for pysqlite info
@@ -36,6 +36,7 @@ class Database(rdbms_common.Database):
         hyperdb.Password  : str,
         hyperdb.Boolean   : int,
         hyperdb.Number    : lambda x: x,
+        hyperdb.Multilink : lambda x: x,    # used in journal marshalling
     }
     sql_to_hyperdb_value = {
         hyperdb.String : str,
@@ -45,6 +46,7 @@ class Database(rdbms_common.Database):
         hyperdb.Password  : lambda x: password.Password(encrypted=x),
         hyperdb.Boolean   : int,
         hyperdb.Number    : rdbms_common._num_cvt,
+        hyperdb.Multilink : lambda x: x,    # used in journal marshalling
     }
 
     def sql_open_connection(self):

roundup/backends/rdbms_common.py

@@ -1,4 +1,4 @@
-# $Id: rdbms_common.py,v 1.95 2004-04-26 20:59:25 richard Exp $
+# $Id: rdbms_common.py,v 1.96 2004-05-02 23:16:05 richard Exp $
 ''' Relational database (SQL) backend common code.
 
 Basics:
@@ -438,7 +438,7 @@ def create_journal_table(self, spec):
             for x in 'nodeid date tag action params'.split()])
         sql = '''create table %s__journal (
             nodeid integer, date timestamp, tag varchar(255),
-            action varchar(255), params varchar(25))'''%spec.classname
+            action varchar(255), params text)'''%spec.classname
         if __debug__:
             print >>hyperdb.DEBUG, 'create_journal_table', (self, sql)
         self.cursor.execute(sql)
@@ -619,12 +619,14 @@ def clear(self):
 
     hyperdb_to_sql_value = {
         hyperdb.String : str,
+        # fractional seconds by default
         hyperdb.Date   : lambda x: x.formal(sep=' ', sec='%.3f'),
         hyperdb.Link   : int,
         hyperdb.Interval  : str,
         hyperdb.Password  : str,
         hyperdb.Boolean   : lambda x: x and 'TRUE' or 'FALSE',
         hyperdb.Number    : lambda x: x,
+        hyperdb.Multilink : lambda x: x,    # used in journal marshalling
     }
     def addnode(self, classname, nodeid, node):
         ''' Add the specified node to its class's db.
@@ -820,6 +822,7 @@ def setnode(self, classname, nodeid, values, multilink_changes={}):
         hyperdb.Password  : lambda x: password.Password(encrypted=x),
         hyperdb.Boolean   : int,
         hyperdb.Number    : _num_cvt,
+        hyperdb.Multilink : lambda x: x,    # used in journal marshalling
     }
     def getnode(self, classname, nodeid):
         ''' Get a node from the database.
@@ -973,6 +976,26 @@ def addjournal(self, classname, nodeid, action, params, creator=None,
         if __debug__:
             print >>hyperdb.DEBUG, 'addjournal', (nodeid, journaldate,
                 journaltag, action, params)
+    
+        # make the journalled data marshallable
+        if isinstance(params, type({})):
+            properties = self.getclass(classname).getprops()
+            for param, value in params.items():
+                property = properties[param]
+                cvt = self.hyperdb_to_sql_value[property.__class__]
+                if isinstance(property, Password):
+                    params[param] = cvt(value)
+                elif isinstance(property, Date):
+                    params[param] = cvt(value)
+                elif isinstance(property, Interval):
+                    params[param] = cvt(value)
+                elif isinstance(property, Boolean):
+                    params[param] = cvt(value)
+
+        params = repr(params)
+
+        dc = self.hyperdb_to_sql_value[hyperdb.Date]
+        journaldate = dc(journaldate)
 
         self.save_journal(classname, cols, nodeid, journaldate,
             journaltag, action, params)
@@ -1001,16 +1024,34 @@ def getjournal(self, classname, nodeid):
             raise IndexError, '%s has no node %s'%(classname, nodeid)
 
         cols = ','.join('nodeid date tag action params'.split())
-        return self.load_journal(classname, cols, nodeid)
+        journal = self.load_journal(classname, cols, nodeid)
+
+        # now unmarshal the data
+        dc = self.sql_to_hyperdb_value[hyperdb.Date]
+        res = []
+        properties = self.getclass(classname).getprops()
+        for nodeid, date_stamp, user, action, params in journal:
+            params = eval(params)
+            for param, value in params.items():
+                property = properties[param]
+                cvt = self.sql_to_hyperdb_value[property.__class__]
+                if isinstance(property, Password):
+                    params[param] = cvt(value)
+                elif isinstance(property, Date):
+                    params[param] = cvt(value)
+                elif isinstance(property, Interval):
+                    params[param] = cvt(value)
+                elif isinstance(property, Boolean):
+                    params[param] = cvt(value)
+            # XXX numeric ids
+            res.append((str(nodeid), dc(date_stamp), user, action, params))
+        return res
 
     def save_journal(self, classname, cols, nodeid, journaldate,
             journaltag, action, params):
         ''' Save the journal entry to the database
         '''
-        # make the params db-friendly
-        params = repr(params)
-        dc = self.hyperdb_to_sql_value[hyperdb.Date]
-        entry = (nodeid, dc(journaldate), journaltag, action, params)
+        entry = (nodeid, journaldate, journaltag, action, params)
 
         # do the insert
         a = self.arg
@@ -1029,13 +1070,7 @@ def load_journal(self, classname, cols, nodeid):
         if __debug__:
             print >>hyperdb.DEBUG, 'load_journal', (self, sql, nodeid)
         self.cursor.execute(sql, (nodeid,))
-        res = []
-        dc = self.sql_to_hyperdb_value[hyperdb.Date]
-        for nodeid, date_stamp, user, action, params in self.cursor.fetchall():
-            params = eval(params)
-            # XXX numeric ids
-            res.append((str(nodeid), dc(date_stamp), user, action, params))
-        return res
+        return self.cursor.fetchall()
 
     def pack(self, pack_before):
         ''' Delete all journal entries except "create" before 'pack_before'.

roundup/backends/sessions_rdbms.py

@@ -1,4 +1,4 @@
-#$Id: sessions_rdbms.py,v 1.2 2004-03-31 23:08:39 richard Exp $
+#$Id: sessions_rdbms.py,v 1.3 2004-05-02 23:16:05 richard Exp $
 """This module defines a very basic store that's used by the CGI interface
 to store session and one-time-key information.
 
@@ -25,7 +25,7 @@ def exists(self, infoid):
         n = self.name
         self.cursor.execute('select count(*) from %ss where %s_key=%s'%(n,
             n, self.db.arg), (infoid,))
-        return self.cursor.fetchone()[0]
+        return int(self.cursor.fetchone()[0])
 
     _marker = []
     def get(self, infoid, value, default=_marker):

roundup/cgi/actions.py

@@ -1,4 +1,4 @@
-#$Id: actions.py,v 1.23 2004-04-05 06:13:42 richard Exp $
+#$Id: actions.py,v 1.24 2004-05-02 23:16:05 richard Exp $
 
 import re, cgi, StringIO, urllib, Cookie, time, random
 
@@ -494,7 +494,8 @@ def handle(self):
         try:
             message = self._editnodes(props, links)
         except (ValueError, KeyError, IndexError, exceptions.Reject), message:
-            self.client.error_message.append(_('Apply Error: ') + str(message))
+            import traceback;traceback.print_exc()
+            self.client.error_message.append(_('Edit Error: ') + str(message))
             return
 
         # commit now that all the tricky stuff is done
@@ -532,7 +533,6 @@ def handle(self):
         try:
             # when it hits the None element, it'll set self.nodeid
             messages = self._editnodes(props, links)
-
         except (ValueError, KeyError, IndexError, exceptions.Reject), message:
             # these errors might just be indicative of user dumbness
             self.client.error_message.append(_('Error: ') + str(message))

roundup/cgi/client.py

@@ -1,4 +1,4 @@
-# $Id: client.py,v 1.173 2004-04-25 22:19:15 richard Exp $
+# $Id: client.py,v 1.174 2004-05-02 23:16:05 richard Exp $
 
 """WWW request handler (also used in the stand-alone server).
 """
@@ -624,13 +624,13 @@ def set_cookie(self, user):
         """
         sessions = self.db.getSessionManager()
 
-        # generate a session key
-        s = '%s%s'%(time.time(), random.random())
-        print s
-        self.session = binascii.b2a_base64(s).strip()
-        while sessions.exists(self.session):
+        # generate a unique session key
+        while 1:
             s = '%s%s'%(time.time(), random.random())
-            self.session = binascii.b2a_base64(s).strip()
+            s = binascii.b2a_base64(s).strip()
+            if not sessions.exists(s):
+                break
+        self.session = s
 
         # clean up the base64
         if self.session[-1] == '=':

roundup/cgi/templating.py

@@ -289,10 +289,11 @@ def classes(self):
         for item in l:
             if item == 'user':
                 m.append(HTMLUserClass(self._client, item))
-            m.append(HTMLClass(self._client, item))
+            else:
+                m.append(HTMLClass(self._client, item))
         return m
 
-def lookupIds(db, prop, ids, fail_ok=0, num_re=re.compile('-?\d+')):
+def lookupIds(db, prop, ids, fail_ok=0, num_re=re.compile('^-?\d+$')):
     ''' "fail_ok" should be specified if we wish to pass through bad values
         (most likely form values that we wish to represent back to the user)
     '''
@@ -310,7 +311,7 @@ def lookupIds(db, prop, ids, fail_ok=0, num_re=re.compile('-?\d+')):
                     l.append(entry)
     return l
 
-def lookupKeys(linkcl, key, ids, num_re=re.compile('-?\d+')):
+def lookupKeys(linkcl, key, ids, num_re=re.compile('^-?\d+$')):
     ''' Look up the "key" values for "ids" list - though some may already
     be key values, not ids.
     '''
@@ -449,7 +450,7 @@ def designator(self):
         ''' Return this class' designator (classname) '''
         return self._classname
 
-    def getItem(self, itemid, num_re=re.compile('-?\d+')):
+    def getItem(self, itemid, num_re=re.compile('^-?\d+$')):
         ''' Get an item of this class by its item id.
         '''
         # make sure we're looking at an itemid
@@ -680,7 +681,7 @@ def journal(self, direction='descending'):
         # XXX do this
         return []
 
-    def history(self, direction='descending', dre=re.compile('\d+')):
+    def history(self, direction='descending', dre=re.compile('^\d+$')):
         self.view_check()
 
         l = ['<table class="history">'
@@ -1523,6 +1524,8 @@ class MultilinkHTMLProperty(HTMLProperty):
     def __init__(self, *args, **kwargs):
         HTMLProperty.__init__(self, *args, **kwargs)
         if self._value:
+            self._value = lookupIds(self._db, self._prop, self._value,
+                fail_ok=1)
             sortfun = make_sort_function(self._db, self._prop.classname)
             self._value.sort(sortfun)
 

roundup/hyperdb.py

@@ -15,7 +15,7 @@
 # BASIS, AND THERE IS NO OBLIGATION WHATSOEVER TO PROVIDE MAINTENANCE,
 # SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
 # 
-# $Id: hyperdb.py,v 1.96 2004-02-11 23:55:08 richard Exp $
+# $Id: hyperdb.py,v 1.97 2004-05-02 23:16:05 richard Exp $
 
 """Hyperdatabase implementation, especially field types.
 """
@@ -592,7 +592,7 @@ class HyperdbValueError(ValueError):
     ''' Error converting a raw value into a Hyperdb value '''
     pass
 
-def convertLinkValue(db, propname, prop, value, idre=re.compile('\d+')):
+def convertLinkValue(db, propname, prop, value, idre=re.compile('^\d+$')):
     ''' Convert the link value (may be id or key value) to an id value. '''
     linkcl = db.classes[prop.classname]
     if not idre.match(value):