From be9d9c45ff23d01bd77f62de02562265f505bdf6 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Mon, 15 May 2017 11:09:59 +0100 Subject: [PATCH] Reworked the extension_loader function --- openlp/core/common/__init__.py | 38 +++++++----- openlp/core/lib/pluginmanager.py | 2 +- openlp/core/ui/media/mediacontroller.py | 2 +- .../presentations/presentationplugin.py | 2 +- .../openlp_core_common/test_common.py | 60 ++++++++++++------- 5 files changed, 66 insertions(+), 38 deletions(-) diff --git a/openlp/core/common/__init__.py b/openlp/core/common/__init__.py index e98f1a439..e6ded5495 100644 --- a/openlp/core/common/__init__.py +++ b/openlp/core/common/__init__.py @@ -23,7 +23,6 @@ The :mod:`common` module contains most of the components and libraries that make OpenLP work. """ -import glob import hashlib import importlib import logging @@ -33,6 +32,7 @@ import sys import traceback from chardet.universaldetector import UniversalDetector from ipaddress import IPv4Address, IPv6Address, AddressValueError +from pathlib import Path from shutil import which from subprocess import check_output, CalledProcessError, STDOUT @@ -85,31 +85,41 @@ def extension_loader(glob_pattern, excluded_files=[]): A utility function to find and load OpenLP extensions, such as plugins, presentation and media controllers and importers. - :param glob_pattern: A glob pattern used to find the extension(s) to be imported. - i.e. openlp_app_dir/plugins/*/*plugin.py + :param glob_pattern: A glob pattern used to find the extension(s) to be imported. Should be relative to the + application directory. i.e. openlp/plugins/*/*plugin.py :type glob_pattern: str + :param excluded_files: A list of file names to exclude that the glob pattern may find. :type excluded_files: list of strings :return: None :rtype: None """ - for extension_path in glob.iglob(glob_pattern): - filename = os.path.split(extension_path)[1] - if filename in excluded_files: + app_dir = Path(AppLocation.get_directory(AppLocation.AppDir)).parent + for extension_path in app_dir.glob(glob_pattern): + extension_path = extension_path.relative_to(app_dir) + if extension_path.name in excluded_files: continue - module_name = os.path.splitext(filename)[0] + module_name = path_to_module(extension_path) try: - loader = importlib.machinery.SourceFileLoader(module_name, extension_path) - loader.load_module() - # TODO: A better way to do this (once we drop python 3.4 support) - # spec = importlib.util.spec_from_file_location('what.ever', 'foo.py') - # module = importlib.util.module_from_spec(spec) - # spec.loader.exec_module(module) + importlib.import_module(module_name) except (ImportError, OSError): # On some platforms importing vlc.py might cause OSError exceptions. (e.g. Mac OS X) log.warning('Failed to import {module_name} on path {extension_path}' - .format(module_name=module_name, extension_path=extension_path)) + .format(module_name=module_name, extension_path=str(extension_path))) + +def path_to_module(path): + """ + Convert a path to a module name (i.e openlp.core.common) + + :param path: The path to convert to a module name. + :type path: Path + + :return: The module name. + :rtype: str + """ + module_path = path.with_suffix('') + return '.'.join(module_path.parts) def get_frozen_path(frozen_option, non_frozen_option): diff --git a/openlp/core/lib/pluginmanager.py b/openlp/core/lib/pluginmanager.py index 9f8037866..bd2b7b9e1 100644 --- a/openlp/core/lib/pluginmanager.py +++ b/openlp/core/lib/pluginmanager.py @@ -69,7 +69,7 @@ class PluginManager(RegistryMixin, OpenLPMixin, RegistryProperties): """ Scan a directory for objects inheriting from the ``Plugin`` class. """ - glob_pattern = os.path.join(self.base_path, '*', '*plugin.py') + glob_pattern = os.path.join('openlp', 'plugins', '*', '*plugin.py') extension_loader(glob_pattern) plugin_classes = Plugin.__subclasses__() plugin_objects = [] diff --git a/openlp/core/ui/media/mediacontroller.py b/openlp/core/ui/media/mediacontroller.py index 322659cd8..13bdf3bd5 100644 --- a/openlp/core/ui/media/mediacontroller.py +++ b/openlp/core/ui/media/mediacontroller.py @@ -174,7 +174,7 @@ class MediaController(RegistryMixin, OpenLPMixin, RegistryProperties): Check to see if we have any media Player's available. """ log.debug('_check_available_media_players') - controller_dir = os.path.join(AppLocation.get_directory(AppLocation.AppDir), 'core', 'ui', 'media') + controller_dir = os.path.join('openlp', 'core', 'ui', 'media') glob_pattern = os.path.join(controller_dir, '*player.py') extension_loader(glob_pattern, ['mediaplayer.py']) player_classes = MediaPlayer.__subclasses__() diff --git a/openlp/plugins/presentations/presentationplugin.py b/openlp/plugins/presentations/presentationplugin.py index 2afa62fa2..210f8a531 100644 --- a/openlp/plugins/presentations/presentationplugin.py +++ b/openlp/plugins/presentations/presentationplugin.py @@ -121,7 +121,7 @@ class PresentationPlugin(Plugin): Check to see if we have any presentation software available. If not do not install the plugin. """ log.debug('check_pre_conditions') - controller_dir = os.path.join(AppLocation.get_directory(AppLocation.PluginsDir), 'presentations', 'lib') + controller_dir = os.path.join('openlp', 'plugins', 'presentations', 'lib') glob_pattern = os.path.join(controller_dir, '*controller.py') extension_loader(glob_pattern, ['presentationcontroller.py']) controller_classes = PresentationController.__subclasses__() diff --git a/tests/functional/openlp_core_common/test_common.py b/tests/functional/openlp_core_common/test_common.py index 7b5a9462a..e70a82328 100644 --- a/tests/functional/openlp_core_common/test_common.py +++ b/tests/functional/openlp_core_common/test_common.py @@ -22,12 +22,13 @@ """ Functional tests to test the AppLocation class and related methods. """ - +from pathlib import Path from unittest import TestCase from unittest.mock import MagicMock, call, patch +from openlp.core import common from openlp.core.common import check_directory_exists, clean_button_text, de_hump, extension_loader, is_macosx, \ - is_linux, is_win, trace_error_handler, translate + is_linux, is_win, path_to_module, trace_error_handler, translate class TestCommonFunctions(TestCase): @@ -77,41 +78,44 @@ class TestCommonFunctions(TestCase): """ Test the `extension_loader` function when no files are found """ - # GIVEN: A mocked `iglob` function which does not match any files - with patch('openlp.core.common.glob.iglob', return_value=[]), \ - patch('openlp.core.common.importlib.machinery.SourceFileLoader') as mocked_source_file_loader: + # GIVEN: A mocked `Path.glob` method which does not match any files + with patch('openlp.core.common.AppLocation.get_directory', return_value='/app/dir/openlp'), \ + patch.object(common.Path, 'glob', return_value=[]), \ + patch('openlp.core.common.importlib.import_module') as mocked_import_module: # WHEN: Calling `extension_loader` extension_loader('glob', ['file2.py', 'file3.py']) # THEN: `extension_loader` should not try to import any files - self.assertFalse(mocked_source_file_loader.called) + self.assertFalse(mocked_import_module.called) def test_extension_loader_files_found(self): """ Test the `extension_loader` function when it successfully finds and loads some files """ - # GIVEN: A mocked `iglob` function which returns a list of files - with patch('openlp.core.common.glob.iglob', return_value=['import_dir/file1.py', 'import_dir/file2.py', - 'import_dir/file3.py', 'import_dir/file4.py']), \ - patch('openlp.core.common.importlib.machinery.SourceFileLoader') as mocked_source_file_loader: + # GIVEN: A mocked `Path.glob` method which returns a list of files + with patch('openlp.core.common.AppLocation.get_directory', return_value='/app/dir/openlp'), \ + patch.object(common.Path, 'glob', return_value=[Path('/app/dir/openlp/import_dir/file1.py'), + Path('/app/dir/openlp/import_dir/file2.py'), + Path('/app/dir/openlp/import_dir/file3.py'), + Path('/app/dir/openlp/import_dir/file4.py')]), \ + patch('openlp.core.common.importlib.import_module') as mocked_import_module: # WHEN: Calling `extension_loader` with a list of files to exclude extension_loader('glob', ['file2.py', 'file3.py']) # THEN: `extension_loader` should only try to import the files that are matched by the blob, excluding the # files listed in the `excluded_files` argument - mocked_source_file_loader.assert_has_calls([call('file1', 'import_dir/file1.py'), call().load_module(), - call('file4', 'import_dir/file4.py'), call().load_module()]) + mocked_import_module.assert_has_calls([call('openlp.import_dir.file1'), call('openlp.import_dir.file4')]) def test_extension_loader_import_error(self): """ Test the `extension_loader` function when `SourceFileLoader` raises a `ImportError` """ - # GIVEN: A mocked `SourceFileLoader` which raises an `ImportError` - with patch('openlp.core.common.glob.iglob', return_value=['import_dir/file1.py', 'import_dir/file2.py', - 'import_dir/file3.py', 'import_dir/file4.py']), \ - patch('openlp.core.common.importlib.machinery.SourceFileLoader', side_effect=ImportError()), \ + # GIVEN: A mocked `import_module` which raises an `ImportError` + with patch('openlp.core.common.AppLocation.get_directory', return_value='/app/dir/openlp'), \ + patch.object(common.Path, 'glob', return_value=[Path('/app/dir/openlp/import_dir/file1.py')]), \ + patch('openlp.core.common.importlib.import_module', side_effect=ImportError()), \ patch('openlp.core.common.log') as mocked_logger: # WHEN: Calling `extension_loader` @@ -122,11 +126,12 @@ class TestCommonFunctions(TestCase): def test_extension_loader_os_error(self): """ - Test the `extension_loader` function when `SourceFileLoader` raises a `ImportError` + Test the `extension_loader` function when `import_module` raises a `ImportError` """ # GIVEN: A mocked `SourceFileLoader` which raises an `OSError` - with patch('openlp.core.common.glob.iglob', return_value=['import_dir/file1.py']), \ - patch('openlp.core.common.importlib.machinery.SourceFileLoader', side_effect=OSError()), \ + with patch('openlp.core.common.AppLocation.get_directory', return_value='/app/dir/openlp'), \ + patch.object(common.Path, 'glob', return_value=[Path('/app/dir/openlp/import_dir/file1.py')]), \ + patch('openlp.core.common.importlib.import_module', side_effect=OSError()), \ patch('openlp.core.common.log') as mocked_logger: # WHEN: Calling `extension_loader` @@ -146,7 +151,7 @@ class TestCommonFunctions(TestCase): new_string = de_hump(string) # THEN: the new string should be converted to python format - self.assertTrue(new_string == "my_class", 'The class name should have been converted') + self.assertEqual(new_string, "my_class", 'The class name should have been converted') def test_de_hump_static(self): """ @@ -159,7 +164,20 @@ class TestCommonFunctions(TestCase): new_string = de_hump(string) # THEN: the new string should be converted to python format - self.assertTrue(new_string == "my_class", 'The class name should have been preserved') + self.assertEqual(new_string, "my_class", 'The class name should have been preserved') + + def test_path_to_module(self): + """ + Test `path_to_module` when supplied with a `Path` object + """ + # GIVEN: A `Path` object + path = Path('openlp/core/ui/media/webkitplayer.py') + + # WHEN: Calling path_to_module with the `Path` object + result = path_to_module(path) + + # THEN: path_to_module should return the module name + self.assertEqual(result, 'openlp.core.ui.media.webkitplayer') def test_trace_error_handler(self): """