blobfiles now always stores/returns bytes

any conversation is done in the backend layer
added a special "binary_content" property to read the data as bytes
changed history generation to read property data on demand

Author: cmeerw

roundup/anypy/strings.py

@@ -27,6 +27,16 @@ def s2b(s):
     else:
         return s
 
+def bs2b(s):
+    """Convert a string object or UTF-8 encoded bytes to UTF-8 encoded bytes."""
+    if _py3:
+        if isinstance(s, bytes):
+            return s
+        else:
+            return s.encode('utf-8')
+    else:
+        return s
+
 def s2u(s, errors='strict'):
     """Convert a string object to a Unicode string."""
     if _py3:

roundup/backends/back_anydbm.py

@@ -25,7 +25,7 @@
 import os, marshal, re, weakref, string, copy, time, shutil, logging
 
 from roundup.anypy.dbm_ import anydbm, whichdb
-from roundup.anypy.strings import b2s
+from roundup.anypy.strings import b2s, bs2b
 
 from roundup import hyperdb, date, password, roundupdb, security, support
 from roundup.backends import locking
@@ -2168,7 +2168,7 @@ def create(self, **propvalues):
         newid = self.create_inner(**propvalues)
 
         # store off the content as a file
-        self.db.storefile(self.classname, newid, None, content)
+        self.db.storefile(self.classname, newid, None, bs2b(content))
 
         # fire reactors
         self.fireReactors('create', newid, None)
@@ -2183,11 +2183,14 @@ def get(self, nodeid, propname, default=_marker, cache=1):
         poss_msg = 'Possibly an access right configuration problem.'
         if propname == 'content':
             try:
-                return self.db.getfile(self.classname, nodeid, None)
+                return b2s(self.db.getfile(self.classname, nodeid, None))
             except IOError as strerror:
                 # XXX by catching this we don't see an error in the log.
                 return 'ERROR reading file: %s%s\n%s\n%s'%(
                         self.classname, nodeid, poss_msg, strerror)
+        elif propname == 'binary_content':
+            return self.db.getfile(self.classname, nodeid, None)
+
         if default is not _marker:
             return Class.get(self, nodeid, propname, default)
         else:
@@ -2220,7 +2223,7 @@ def set(self, itemid, **propvalues):
         # do content?
         if content:
             # store and possibly index
-            self.db.storefile(self.classname, itemid, None, content)
+            self.db.storefile(self.classname, itemid, None, bs2b(content))
             if self.properties['content'].indexme:
                 mime_type = self.get(itemid, 'type', self.default_mime_type)
                 self.db.indexer.add_text((self.classname, itemid, 'content'),

roundup/backends/blobfiles.py

@@ -22,8 +22,6 @@
 
 import os
 
-from roundup.anypy.strings import s2b
-
 def files_in_dir(dir):
     if not os.path.exists(dir):
         return 0
@@ -334,8 +332,6 @@ def storefile(self, classname, nodeid, property, content):
         # in multi-tracker (i.e. multi-umask) or modpython scenarios
         # the umask may have changed since last we set it.
         os.umask(self.umask)
-        if isinstance(content, str):
-            content = s2b(content)
         open(name, 'wb').write(content)
 
     def getfile(self, classname, nodeid, property):

roundup/backends/rdbms_common.py

@@ -69,7 +69,7 @@
 from roundup.date import Range
 
 from roundup.backends.back_anydbm import compile_expression
-from roundup.anypy.strings import us2s
+from roundup.anypy.strings import b2s, bs2b, us2s
 
 
 # dummy value meaning "argument not passed"
@@ -3055,7 +3055,7 @@ def create(self, **propvalues):
                 content, mime_type)
 
         # store off the content as a file
-        self.db.storefile(self.classname, newid, None, content)
+        self.db.storefile(self.classname, newid, None, bs2b(content))
 
         # fire reactors
         self.fireReactors('create', newid, None)
@@ -3070,11 +3070,14 @@ def get(self, nodeid, propname, default=_marker, cache=1):
         poss_msg = 'Possibly a access right configuration problem.'
         if propname == 'content':
             try:
-                return self.db.getfile(self.classname, nodeid, None)
+                return b2s(self.db.getfile(self.classname, nodeid, None))
             except IOError as strerror:
                 # BUG: by catching this we donot see an error in the log.
                 return 'ERROR reading file: %s%s\n%s\n%s'%(
                         self.classname, nodeid, poss_msg, strerror)
+        elif propname == 'binary_content':
+            return self.db.getfile(self.classname, nodeid, None)
+
         if default is not _marker:
             return Class.get(self, nodeid, propname, default)
         else:
@@ -3098,7 +3101,7 @@ def set(self, itemid, **propvalues):
         # do content?
         if content:
             # store and possibly index
-            self.db.storefile(self.classname, itemid, None, content)
+            self.db.storefile(self.classname, itemid, None, bs2b(content))
             if self.properties['content'].indexme:
                 mime_type = self.get(itemid, 'type', self.default_mime_type)
                 self.db.indexer.add_text((self.classname, itemid, 'content'),

roundup/cgi/templating.py

@@ -938,28 +938,6 @@ def history(self, direction='descending', dre=re.compile('^\d+$'),
         if not self.is_view_ok():
             return self._('[hidden]')
 
-        # pre-load the history with the current state
-        current = {}
-        for prop_n in self._props.keys():
-            prop = self[prop_n]
-            if not isinstance(prop, HTMLProperty):
-                continue
-            current[prop_n] = prop.plain(escape=1)
-            # make link if hrefable
-            if (prop_n in self._props and
-                    isinstance(self._props[prop_n], hyperdb.Link)):
-                classname = self._props[prop_n].classname
-                try:
-                    template = self._client.selectTemplate(classname, 'item')
-                    if template.startswith('_generic.'):
-                        raise NoTemplate('not really...')
-                except NoTemplate:
-                    pass
-                else:
-                    id = self._klass.get(self._nodeid, prop_n, None)
-                    current[prop_n] = '<a rel="nofollow" href="%s%s">%s</a>'%(
-                        classname, id, current[prop_n])
-
         # get the journal, sort and reverse
         history = self._klass.history(self._nodeid, skipquiet=(not showall))
         history.sort(key=lambda a: a[:3])
@@ -971,6 +949,7 @@ def history(self, direction='descending', dre=re.compile('^\d+$'),
 
         timezone = self._db.getUserTimezone()
         l = []
+        current = {}
         comments = {}
         for id, evt_date, user, action, args in history:
             date_s = str(evt_date.local(timezone)).replace("."," ")
@@ -999,6 +978,28 @@ def history(self, direction='descending', dre=re.compile('^\d+$'),
                             % (self._(k), str(args[k])))
                         continue
 
+                    # load the current state for the property (if we
+                    # haven't already)
+                    if k not in current:
+                        val = self[k]
+                        if not isinstance(val, HTMLProperty):
+                            current[k] = None
+                        else:
+                            current[k] = val.plain(escape=1)
+                            # make link if hrefable
+                            if (isinstance(prop, hyperdb.Link)):
+                                classname = prop.classname
+                                try:
+                                    template = self._client.selectTemplate(classname, 'item')
+                                    if template.startswith('_generic.'):
+                                        raise NoTemplate('not really...')
+                                except NoTemplate:
+                                    pass
+                                else:
+                                    linkid = self._klass.get(self._nodeid, k, None)
+                                    current[k] = '<a rel="nofollow" href="%s%s">%s</a>'%(
+                                        classname, linkid, current[k])
+
                     if args[k] and (isinstance(prop, hyperdb.Multilink) or
                             isinstance(prop, hyperdb.Link)):
                         # figure what the link class is
@@ -1084,7 +1085,7 @@ def history(self, direction='descending', dre=re.compile('^\d+$'),
                             else:
                                 old = label;
                             cell.append('%s: %s' % (self._(k), old))
-                            if k in current:
+                            if k in current and current[k] is not None:
                                 cell[-1] += ' -> %s'%current[k]
                                 current[k] = old
 
@@ -1095,49 +1096,49 @@ def history(self, direction='descending', dre=re.compile('^\d+$'),
                             d = date.Date(args[k],
                                 translator=self._client).local(timezone)
                         cell.append('%s: %s'%(self._(k), str(d)))
-                        if k in current:
+                        if k in current and current[k] is not None:
                             cell[-1] += ' -> %s' % current[k]
                             current[k] = str(d)
 
                     elif isinstance(prop, hyperdb.Interval) and args[k]:
                         val = str(date.Interval(args[k],
                             translator=self._client))
                         cell.append('%s: %s'%(self._(k), val))
-                        if k in current:
+                        if k in current and current[k] is not None:
                             cell[-1] += ' -> %s'%current[k]
                             current[k] = val
 
                     elif isinstance(prop, hyperdb.String) and args[k]:
                         val = cgi.escape(args[k])
                         cell.append('%s: %s'%(self._(k), val))
-                        if k in current:
+                        if k in current and current[k] is not None:
                             cell[-1] += ' -> %s'%current[k]
                             current[k] = val
 
                     elif isinstance(prop, hyperdb.Boolean) and args[k] is not None:
                         val = args[k] and ''"Yes" or ''"No"
                         cell.append('%s: %s'%(self._(k), val))
-                        if k in current:
+                        if k in current and current[k] is not None:
                             cell[-1] += ' -> %s'%current[k]
                             current[k] = val
 
                     elif isinstance(prop, hyperdb.Password) and args[k] is not None:
                         val = args[k].dummystr()
                         cell.append('%s: %s'%(self._(k), val))
-                        if k in current:
+                        if k in current and current[k] is not None:
                             cell[-1] += ' -> %s'%current[k]
                             current[k] = val
 
                     elif not args[k]:
-                        if k in current:
+                        if k in current and current[k] is not None:
                             cell.append('%s: %s'%(self._(k), current[k]))
                             current[k] = '(no value)'
                         else:
                             cell.append(self._('%s: (no value)')%self._(k))
 
                     else:
                         cell.append('%s: %s'%(self._(k), str(args[k])))
-                        if k in current:
+                        if k in current and current[k] is not None:
                             cell[-1] += ' -> %s'%current[k]
                             current[k] = str(args[k])
 

test/db_test_base.py

@@ -298,6 +298,11 @@ def testStringUnicode(self):
         self.db.commit()
         self.assertEqual(self.db.issue.get(nid, 'title'), ustr2)
 
+        # test set & retrieve (this time for file contents)
+        nid = self.db.file.create(content=ustr)
+        self.assertEqual(self.db.file.get(nid, 'content'), ustr)
+        self.assertEqual(self.db.file.get(nid, 'binary_content'), s2b(ustr))
+
     # Link
     def testLinkChange(self):
         self.assertRaises(IndexError, self.db.issue.create, title="spam",