Merge pull request #108 from zopefoundation/dataflake/issue_107

Fix two bytes/str issues under Python 3

CHANGES.rst

@@ -4,6 +4,12 @@ Changelog
 2.1.2 (unreleased)
 ------------------
 
+- Document and fix behavior of methods that open/read/write filesystem files
+  (`#107 <https://github.com/zopefoundation/Products.GenericSetup/issues/107>`_)
+
+- Fix snapshot comparisons under Python 3
+  (`#85 <https://github.com/zopefoundation/Products.GenericSetup/issues/85>`_)
+
 
 2.1.1 (2021-03-02)
 ------------------

src/Products/GenericSetup/content.py

@@ -16,6 +16,8 @@
 from csv import reader
 from csv import writer
 
+import six
+from six import BytesIO
 from six.moves import cStringIO
 from six.moves.configparser import ConfigParser
 
@@ -143,6 +145,9 @@ def import_(self, import_context, subdir, root=False):
         if not preserve:
             preserve = []
         else:
+            # Make sure ``preserve`` is a native string
+            if six.PY3 and not isinstance(preserve, str):
+                preserve = preserve.decode('UTF-8')
             preserve = _globtest(preserve, prior)
 
         preserve.extend([x[0] for x in must_preserve])
@@ -156,6 +161,8 @@ def import_(self, import_context, subdir, root=False):
             return
 
         dialect = 'excel'
+        if six.PY3 and not isinstance(objects, str):
+            objects = objects.decode('UTF-8')
         stream = cStringIO(objects)
 
         rowiter = reader(stream, dialect)
@@ -274,7 +281,7 @@ def import_(self, import_context, subdir, root=False):
             logger = import_context.getLogger('CSAFA')
             logger.info('no .csv file for %s/%s' % (subdir, cid))
         else:
-            stream = cStringIO(data)
+            stream = BytesIO(data)
             self.context.put_csv(stream)
 
 
@@ -339,6 +346,11 @@ def put_ini(self, text):
         """
         context = self.context
         parser = ConfigParser()
+
+        # read_file/readfp expect text, not bytes
+        if isinstance(text, six.binary_type):
+            text = text.decode('UTF-8')
+
         try:
             parser.read_file(cStringIO(text))
         except AttributeError:  # Python 2
@@ -412,6 +424,6 @@ def import_(self, import_context, subdir, root=False):
             logger = import_context.getLogger('SGAIFA')
             logger.info('no .ini file for %s/%s' % (subdir, cid))
         else:
-            request = FauxDAVRequest(BODY=data, BODYFILE=cStringIO(data))
+            request = FauxDAVRequest(BODY=data, BODYFILE=BytesIO(data))
             response = FauxDAVResponse()
             self.context.PUT(request, response)

src/Products/GenericSetup/interfaces.py

@@ -85,8 +85,8 @@ def readDataFile(filename, subdir=None):
         o 'subdir' is an optional subdirectory;  if not supplied, search
           only the "root" directory.
 
-        o Return the file contents as a string, or None if the
-          file cannot be found.
+        o Return the file contents as bytes (``str`` on Python 2 and ``bytes``
+          on Python 3), or None if the file cannot be found.
         """
 
     def getLastModified(path):
@@ -138,8 +138,9 @@ def openDataFile(filename, subdir=None):
           which to find the file;  if not passed, write the file to the
           "root" of the target.
 
-        o Return a readable file-like object;  the caller is responsible
-          for calling 'close' on it.
+        o Return a readable file-like object in binary mode that will return
+          bytes when read from (``str`` on Python 2 and ``bytes`` on Python 3);
+          the caller is responsible for calling 'close' on it.
         """
 
 
@@ -186,8 +187,9 @@ def openDataFile(filename, content_type, subdir=None):
           which to write the file;  if not passed, write the file to the
           "root" of the target.
 
-        o Return a writeable file-like object;  the caller is responsible
-          for calling 'close' on it.
+        o Return a writeable file-like object in binary mode that expects
+          bytes data (``str`` on Python 2 and ``bytes`` on Python 3);
+          the caller is responsible for calling 'close' on it.
         """
 
 

src/Products/GenericSetup/tests/common.py

@@ -208,7 +208,12 @@ def readDataFile(self, filename, subdir=None):
         if subdir is not None:
             filename = '/'.join((subdir, filename))
 
-        return self._files.get(filename)
+        # readDataFile must return bytes, so we make sure it does.
+        contents = self._files.get(filename)
+        if isinstance(contents, six.text_type):
+            contents = contents.encode(self._encoding or 'UTF-8')
+
+        return contents
 
     def shouldPurge(self):
 

src/Products/GenericSetup/tests/faux_objects.py

@@ -68,7 +68,7 @@ def put_ini(self, text):
         self._was_put = text
 
 
-KNOWN_DAV = """\
+KNOWN_DAV = b"""\
 Title: %s
 Description: %s
 
@@ -79,9 +79,9 @@ def put_ini(self, text):
 @implementer(IDAVAware)
 class TestDAVAware(SimpleItem):
     _was_put = None
-    title = 'DAV title'
-    description = 'DAV description'
-    body = 'DAV body'
+    title = b'DAV title'
+    description = b'DAV description'
+    body = b'DAV body'
 
     def manage_FTPget(self):
         return KNOWN_DAV % (self.title, self.description, self.body)

src/Products/GenericSetup/tests/test_content.py

@@ -15,6 +15,8 @@
 
 import unittest
 
+import six
+
 from .conformance import ConformsToIFilesystemExporter
 from .conformance import ConformsToIFilesystemImporter
 from .conformance import ConformsToIINIAware
@@ -766,7 +768,7 @@ def test_import_with_known_CSV(self):
 one,two,three
 four,five,six
 """
-        NEW_CSV = """\
+        NEW_CSV = b"""\
 four,five,six
 one,two,three
 """
@@ -857,7 +859,7 @@ def test_export_dav_file(self):
     def test_import_dav_file(self):
         from .common import DummyImportContext
         from .faux_objects import KNOWN_DAV
-        VALUES = ('Title: dav_file', 'Description: abc', 'body goes here')
+        VALUES = (b'Title: dav_file', b'Description: abc', b'body goes here')
         dav_file = _makeDAVAware('dav_file.html')
         adapter = self._makeOne(dav_file)
         context = DummyImportContext(None)
@@ -939,6 +941,11 @@ def _parseINI(text):
     from six.moves import cStringIO
     from six.moves.configparser import ConfigParser
     parser = ConfigParser()
+
+    # read_file/readfp expect text, not bytes
+    if isinstance(text, six.binary_type):
+        text = text.decode('UTF-8')
+
     try:
         parser.read_file(cStringIO(text))
     except AttributeError:  # Python 2

src/Products/GenericSetup/tool.py

@@ -473,6 +473,10 @@ def markupComparison(self, lines):
 
         for line in lines.splitlines():
 
+            # All the operations below expect native strings
+            if six.PY3 and not isinstance(line, str):
+                line = line.decode('UTF-8')
+
             if line.startswith('** '):
 
                 if line.find('File') > -1: