Fix possible double import of cubes modules
When cubes using the new layout are imported with 'cubicweb_<cube>' and with
'cubes.<cube>', the same module is imported twice.
Handle this by adding 'cubes.<cube>' to sys.modules when importing from
'cubicweb_<cube>'.
Move load_module() to a sub class _CubesLoader to share informations computed
in find_modules().
Don't handle subpackages in _CubesImporter and rely on normal import machinery
instead.
Add a test and use unittest from cubicweb.devtools.testlib which resolve to
unittest2 on PY2 with assertLogs() method.
--- a/cubicweb/__init__.py Wed Feb 08 10:31:26 2017 +0100
+++ b/cubicweb/__init__.py Tue Feb 07 13:47:03 2017 +0100
@@ -24,7 +24,6 @@
import logging
import os
import pickle
-import pkgutil
import sys
import types
import warnings
@@ -283,6 +282,37 @@
# Import hook for "legacy" cubes ##############################################
+class _CubesLoader(object):
+
+ def __init__(self, *modinfo):
+ self.modinfo = modinfo
+
+ def load_module(self, fullname):
+ try:
+ # If there is an existing module object named 'fullname' in
+ # sys.modules , the loader must use that existing module.
+ # Otherwise, the reload() builtin will not work correctly.
+ return sys.modules[fullname]
+ except KeyError:
+ pass
+ if fullname == 'cubes':
+ mod = sys.modules[fullname] = types.ModuleType(
+ fullname, doc='CubicWeb cubes')
+ else:
+ modname, file, pathname, description = self.modinfo
+ try:
+ mod = sys.modules[fullname] = imp.load_module(
+ modname, file, pathname, description)
+ finally:
+ # https://docs.python.org/2/library/imp.html#imp.load_module
+ # Important: the caller is responsible for closing the file
+ # argument, if it was not None, even when an exception is
+ # raised. This is best done using a try ... finally statement
+ if file is not None:
+ file.close()
+ return mod
+
+
class _CubesImporter(object):
"""Module finder handling redirection of import of "cubes.<name>"
to "cubicweb_<name>".
@@ -296,25 +326,12 @@
def find_module(self, fullname, path=None):
if fullname == 'cubes':
- return self
- elif fullname.startswith('cubes.'):
+ return _CubesLoader()
+ elif fullname.startswith('cubes.') and fullname.count('.') == 1:
modname = 'cubicweb_' + fullname.split('.', 1)[1]
try:
modinfo = imp.find_module(modname)
except ImportError:
return None
else:
- return pkgutil.ImpLoader(fullname, *modinfo)
-
- def load_module(self, fullname):
- try:
- # If there is an existing module object named 'fullname' in
- # sys.modules , the loader must use that existing module.
- # Otherwise, the reload() builtin will not work correctly.
- return sys.modules[fullname]
- except KeyError:
- pass
- if fullname != 'cubes':
- raise ImportError('No module named {0}'.format(fullname))
- mod = sys.modules[fullname] = types.ModuleType(fullname, doc='CubicWeb cubes')
- return mod
+ return _CubesLoader(modname, *modinfo)
--- a/cubicweb/test/unittest_cubes.py Wed Feb 08 10:31:26 2017 +0100
+++ b/cubicweb/test/unittest_cubes.py Tue Feb 07 13:47:03 2017 +0100
@@ -21,13 +21,12 @@
import os
from os import path
import sys
-import unittest
from six import PY2
from cubicweb import _CubesImporter
from cubicweb.cwconfig import CubicWebConfiguration
-from cubicweb.devtools.testlib import TemporaryDirectory
+from cubicweb.devtools.testlib import TemporaryDirectory, TestCase
@contextmanager
@@ -37,17 +36,20 @@
libdir = path.join(tempdir, 'libpython')
cubedir = path.join(libdir, 'cubicweb_foo')
os.makedirs(cubedir)
+ check_code = ("import logging\n"
+ "logging.getLogger('cubicweb_foo')"
+ ".warn('imported %s', __name__)\n")
with open(path.join(cubedir, '__init__.py'), 'w') as f:
- f.write('"""cubicweb_foo application package"""')
+ f.write("'cubicweb_foo application package'\n" + check_code)
with open(path.join(cubedir, 'bar.py'), 'w') as f:
- f.write('baz = 1')
+ f.write(check_code + 'baz = 1\n')
sys.path.append(libdir)
yield cubedir
finally:
sys.path.remove(libdir)
-class CubesImporterTC(unittest.TestCase):
+class CubesImporterTC(TestCase):
def setUp(self):
# During discovery, CubicWebConfiguration.cls_adjust_sys_path may be
@@ -104,6 +106,27 @@
new = importlib.reload(cubes)
self.assertIs(new, cubes)
+ def test_no_double_import(self):
+ """Check new and legacy import the same module once"""
+ with temp_cube():
+ CubicWebConfiguration.cls_adjust_sys_path()
+ with self.assertLogs('cubicweb_foo', 'WARNING') as cm:
+ from cubes.foo import bar
+ from cubicweb_foo import bar as bar2
+ self.assertIs(bar, bar2)
+ self.assertIs(sys.modules['cubes.foo'],
+ sys.modules['cubicweb_foo'])
+ self.assertEqual(cm.output, [
+ 'WARNING:cubicweb_foo:imported cubicweb_foo',
+ # module __name__ for subpackage differ along python version
+ # for PY2 it's based on how the module was imported "from
+ # cubes.foo import bar" and for PY3 based on __name__ of parent
+ # module "cubicweb_foo". Not sure if it's an issue, but PY3
+ # behavior looks better.
+ 'WARNING:cubicweb_foo:imported ' + (
+ 'cubes.foo.bar' if PY2 else 'cubicweb_foo.bar')
+ ])
+
def test_import_legacy_cube(self):
"""Check that importing a legacy cube works when sys.path got adjusted.
"""