From 7b5db2c2d736ef2e6015ec6d5a08c55ed8232fd7 Mon Sep 17 00:00:00 2001 From: dieter Date: Wed, 7 Oct 2020 09:21:24 +0200 Subject: [PATCH 1/3] Replace (in "OFS") deprecated direct "id" access by "getId" calls --- CHANGES.rst | 3 ++ src/OFS/ObjectManager.py | 10 ++-- src/OFS/tests/testObjectManager.py | 83 ++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 4 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 96124a5120..bf1253c259 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. + - Update dependencies to the latest releases that still support Python 2. - Update to ``zope.interface > 5.1.0`` to fix a memory leak. 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..555919eaea 100644 --- a/src/OFS/tests/testObjectManager.py +++ b/src/OFS/tests/testObjectManager.py @@ -580,6 +580,89 @@ 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 exists + 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 + tdir_created = False + # 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) + if tempfile.tempdir is None: # pragma: no cover + tempfile.mktemp() # initialize `tempdir` + tdir = join(tempfile.tempdir, "import") + if not exists(tdir): # pragma: no cover + tdir_created = True + mkdir(tdir) + tf = tempfile.NamedTemporaryFile( + dir=tdir, delete=False) + tf.write(exported) + tf.close() + unused, tname = split(tf.name) + tmp._getImportPaths = _CallResult((tempfile.tempdir,)) + 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) + if tdir_created: # pragma: no cover + 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() From d5816ed79fa8e52f5df35781163dba25b713b1d5 Mon Sep 17 00:00:00 2001 From: dieter Date: Wed, 7 Oct 2020 13:30:48 +0200 Subject: [PATCH 2/3] make (test) temporary file creation more robust --- src/OFS/tests/testObjectManager.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/OFS/tests/testObjectManager.py b/src/OFS/tests/testObjectManager.py index 555919eaea..f434c6c71c 100644 --- a/src/OFS/tests/testObjectManager.py +++ b/src/OFS/tests/testObjectManager.py @@ -614,7 +614,6 @@ def test_export_import(self): from transaction import commit try: tf = None # temporary file required for export/import - tdir_created = False # export/import needs the object manager in ZODB s = DemoStorage() db = DB(s) @@ -628,18 +627,15 @@ def test_export_import(self): top._setObject(tmp.getId(), tmp) commit() exported = top.manage_exportObject("f", True) - if tempfile.tempdir is None: # pragma: no cover - tempfile.mktemp() # initialize `tempdir` - tdir = join(tempfile.tempdir, "import") - if not exists(tdir): # pragma: no cover - tdir_created = True - mkdir(tdir) + tdir = tempfile.mkdtemp() + idir = join(tdir, "import") + mkdir(idir) tf = tempfile.NamedTemporaryFile( - dir=tdir, delete=False) + dir=idir, delete=False) tf.write(exported) tf.close() unused, tname = split(tf.name) - tmp._getImportPaths = _CallResult((tempfile.tempdir,)) + tmp._getImportPaths = _CallResult((tdir,)) tmp.manage_importObject(tname, set_owner=False, suppress_events=True) imp_f = tmp["f"] # exception if import unsuccessful @@ -648,7 +644,7 @@ def test_export_import(self): finally: if tf is not None: # pragma: no cover unlink(tf.name) - if tdir_created: # pragma: no cover + rmdir(idir) rmdir(tdir) c.close() db.close() From 7bc6f20ccf646707ee63afc31c68200a401a1b3f Mon Sep 17 00:00:00 2001 From: dieter Date: Wed, 7 Oct 2020 15:18:13 +0200 Subject: [PATCH 3/3] make flake8 happy --- src/OFS/tests/testObjectManager.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/OFS/tests/testObjectManager.py b/src/OFS/tests/testObjectManager.py index f434c6c71c..288d333cac 100644 --- a/src/OFS/tests/testObjectManager.py +++ b/src/OFS/tests/testObjectManager.py @@ -603,7 +603,6 @@ def test_export_import(self): from os import mkdir from os import rmdir from os import unlink - from os.path import exists from os.path import join from os.path import split