diff --git a/CHANGES.rst b/CHANGES.rst index befdb874be..c2516bb42f 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -10,6 +10,9 @@ https://zope.readthedocs.io/en/2.13/CHANGES.html 4.5.2 (unreleased) ------------------ +- Replace (in ``OFS``) the deprecated direct ``id`` access by + ``getId`` calls. + - Provide a more senseful ``OFS.SimpleItem.Item_w__name__.id`` to avoid bugs by use of deprecated direct ``id`` access (as e.g. (`#903 `_). diff --git a/src/OFS/ObjectManager.py b/src/OFS/ObjectManager.py index 3e4b83b763..6ed699dc13 100644 --- a/src/OFS/ObjectManager.py +++ b/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 diff --git a/src/OFS/tests/testObjectManager.py b/src/OFS/tests/testObjectManager.py index e7f392ba71..288d333cac 100644 --- a/src/OFS/tests/testObjectManager.py +++ b/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()