Reworked the extension_loader function

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.
+    :param glob_pattern: A glob pattern used to find the extension(s) to be imported. Should be relative to the
-        i.e. openlp_app_dir/plugins/*/*plugin.py
+        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):
+    app_dir = Path(AppLocation.get_directory(AppLocation.AppDir)).parent
-        filename = os.path.split(extension_path)[1]
+    for extension_path in app_dir.glob(glob_pattern):
-        if filename in excluded_files:
+        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)
+            importlib.import_module(module_name)
-            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)
         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):

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 = []

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__()

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__()

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
+        # GIVEN: A mocked `Path.glob` method which does not match any files
-        with patch('openlp.core.common.glob.iglob', return_value=[]), \
+        with patch('openlp.core.common.AppLocation.get_directory', return_value='/app/dir/openlp'), \
-                patch('openlp.core.common.importlib.machinery.SourceFileLoader') as mocked_source_file_loader:
+                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
+        # GIVEN: A mocked `Path.glob` method which returns a list of files
-        with patch('openlp.core.common.glob.iglob', return_value=['import_dir/file1.py', 'import_dir/file2.py',
+        with patch('openlp.core.common.AppLocation.get_directory', return_value='/app/dir/openlp'), \
-                                                                  'import_dir/file3.py', 'import_dir/file4.py']), \
+                patch.object(common.Path, 'glob', return_value=[Path('/app/dir/openlp/import_dir/file1.py'),
-                patch('openlp.core.common.importlib.machinery.SourceFileLoader') as mocked_source_file_loader:
+                                                                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(),
+            mocked_import_module.assert_has_calls([call('openlp.import_dir.file1'), call('openlp.import_dir.file4')])
-                                                        call('file4', 'import_dir/file4.py'), call().load_module()])
 
     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`
+        # GIVEN: A mocked `import_module` which raises an `ImportError`
-        with patch('openlp.core.common.glob.iglob', return_value=['import_dir/file1.py', 'import_dir/file2.py',
+        with patch('openlp.core.common.AppLocation.get_directory', return_value='/app/dir/openlp'), \
-                                                                  'import_dir/file3.py', 'import_dir/file4.py']), \
+                patch.object(common.Path, 'glob', return_value=[Path('/app/dir/openlp/import_dir/file1.py')]), \
-                patch('openlp.core.common.importlib.machinery.SourceFileLoader', side_effect=ImportError()), \
+                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']), \
+        with patch('openlp.core.common.AppLocation.get_directory', return_value='/app/dir/openlp'), \
-                patch('openlp.core.common.importlib.machinery.SourceFileLoader', side_effect=OSError()), \
+                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):
         """