Replace (in "OFS") deprecated direct "id" access by "getId" calls (#907)

* Replace (in "OFS") deprecated direct "id" access by "getId" calls

* make (test) temporary file creation more robust

* make flake8 happy

Co-authored-by: Jens Vagelpohl <jens@netz.ooo>
Co-authored-by: Michael Howitz <mh@gocept.com>

CHANGES.rst

@@ -10,6 +10,9 @@ https://zope.readthedocs.io/en/2.13/CHANGES.html
 4.5.4 (unreleased)
 ------------------
 
+- Replace (in ``OFS``) the deprecated direct ``id`` access by
+  ``getId`` calls.
+
 - Updated ZMI dependencies for Font Awesome, jQuery and bootstrap
 
 - Revised debug info GUI

src/OFS/ObjectManager.py

@@ -666,7 +666,8 @@ def manage_importObject(self, file, REQUEST=None, set_owner=1,
         imported = self._importObjectFromFile(
             filepath, verify=bool(REQUEST), set_owner=set_owner,
             suppress_events=suppress_events)
-        id = imported.id
+        getId = getattr(aq_base(imported), "getId", None)  # aq wrapped
+        id = imported.getId() if getId is not None else imported.id
         if getattr(id, '__func__', None) is not None:
             id = id()
 
@@ -691,7 +692,8 @@ def _importObjectFromFile(self, filepath, verify=1, set_owner=1,
         ob = connection.importFile(filepath)
         if verify:
             self._verifyObjectPaste(ob, validate_src=0)
-        id = ob.id
+        getId = getattr(ob, "getId", None)  # not acquisition wrapped
+        id = getId() if getId is not None else ob.id
         if getattr(id, '__func__', None) is not None:
             id = id()
         self._setObject(id, ob, set_owner=set_owner,
@@ -967,9 +969,9 @@ def findChildren(obj, dirname=''):
     for name, child in obj.objectItems():
         if hasattr(aq_base(child), 'isPrincipiaFolderish') and \
            child.isPrincipiaFolderish:
-            lst.extend(findChildren(child, dirname + obj.id + '/'))
+            lst.extend(findChildren(child, dirname + obj.getId() + '/'))
         else:
-            lst.append((dirname + obj.id + "/" + name, child))
+            lst.append((dirname + obj.getId() + "/" + name, child))
 
     return lst
 

src/OFS/tests/testObjectManager.py

@@ -580,6 +580,84 @@ def test_FTPList(self):
         self.assertEqual(data[0][0], '.')
         self.assertEqual(data[1][0], '..')
 
+    def test_findChildren(self):
+        from OFS.Folder import Folder
+        from OFS.Image import File
+        from OFS.ObjectManager import findChildren
+        top = Folder("top")
+        f1 = File("f1", "", b"")
+        top._setObject(f1.getId(), f1)
+        fo = Folder("fo")
+        top._setObject(fo.getId(), fo)
+        f2 = File("f2", "", b"")
+        fo._setObject(f2.getId(), f2)
+        self.assertEqual(
+            [ci[0] for ci in findChildren(top)],
+            ["top/f1",
+             # surprisingly, `findChildren` ignores folderish children
+             # "top/fo",
+             "top/fo/f2"])
+
+    def test_export_import(self):
+        import tempfile
+        from os import mkdir
+        from os import rmdir
+        from os import unlink
+        from os.path import join
+        from os.path import split
+
+        from OFS.Folder import Folder
+        from OFS.Image import File
+        from ZODB.DemoStorage import DemoStorage
+        from ZODB.DB import DB
+        from transaction import commit
+        try:
+            tf = None  # temporary file required for export/import
+            # export/import needs the object manager in ZODB
+            s = DemoStorage()
+            db = DB(s)
+            c = db.open()
+            root = c.root()
+            top = Folder("top")
+            f = File("f", "", b"")
+            top._setObject(f.getId(), f)
+            root["top"] = top
+            tmp = Folder("tmp")
+            top._setObject(tmp.getId(), tmp)
+            commit()
+            exported = top.manage_exportObject("f", True)
+            tdir = tempfile.mkdtemp()
+            idir = join(tdir, "import")
+            mkdir(idir)
+            tf = tempfile.NamedTemporaryFile(
+                dir=idir, delete=False)
+            tf.write(exported)
+            tf.close()
+            unused, tname = split(tf.name)
+            tmp._getImportPaths = _CallResult((tdir,))
+            tmp.manage_importObject(tname, set_owner=False,
+                                    suppress_events=True)
+            imp_f = tmp["f"]  # exception if import unsuccessful
+            self.assertIsInstance(imp_f, File)
+            commit()
+        finally:
+            if tf is not None:  # pragma: no cover
+                unlink(tf.name)
+                rmdir(idir)
+                rmdir(tdir)
+            c.close()
+            db.close()
+            s.close()
+
+
+class _CallResult(object):
+    """Auxiliary class to provide defined call results."""
+    def __init__(self, result):
+        self.result = result
+
+    def __call__(self):
+        return self.result
+
 
 _marker = object()