Mostly minor path tidy-ups

openlp.py

@@ -39,7 +39,7 @@ def set_up_fault_handling():
     """
     # Create the cache directory if it doesn't exist, and enable the fault handler to log to an error log file
     create_paths(AppLocation.get_directory(AppLocation.CacheDir))
-    faulthandler.enable(open(str(AppLocation.get_directory(AppLocation.CacheDir) / 'error.log'), 'wb'))
+    faulthandler.enable((AppLocation.get_directory(AppLocation.CacheDir) / 'error.log').open('wb'))
 
 
 if __name__ == '__main__':

openlp/core/lib/mediamanageritem.py

@@ -332,8 +332,7 @@ class MediaManagerItem(QtWidgets.QWidget, RegistryProperties):
         """
         new_file_paths = []
         error_shown = False
-        for file_name in data['files']:
+        for file_path in data['file_paths']:
-            file_path = str_to_path(file_name)
             if file_path.suffix[1:].lower() not in self.on_new_file_masks:
                 if not error_shown:
                     critical_error_message_box(

openlp/core/ui/firsttimeform.py

@@ -401,7 +401,7 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
             screenshot = self.config.get('theme_{theme}'.format(theme=theme), 'screenshot')
             item = self.themes_list_widget.item(index)
             if item:
-                item.setIcon(build_icon(os.path.join(gettempdir(), 'openlp', screenshot)))
+                item.setIcon(build_icon(Path(gettempdir(), 'openlp', screenshot)))
 
     def _download_progress(self, count, block_size):
         """
@@ -550,9 +550,9 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
         Download selected songs, bibles and themes. Returns False on download error
         """
         # Build directories for downloads
-        songs_destination = os.path.join(gettempdir(), 'openlp')
+        songs_destination_path = Path(gettempdir(), 'openlp')
-        bibles_destination = str(AppLocation.get_section_data_path('bibles'))
+        bibles_destination_path = AppLocation.get_section_data_path('bibles')
-        themes_destination = str(AppLocation.get_section_data_path('themes'))
+        themes_destination_path = AppLocation.get_section_data_path('themes')
         missed_files = []
         # Download songs
         for i in range(self.songs_list_widget.count()):
@@ -561,7 +561,7 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
                 filename, sha256 = item.data(QtCore.Qt.UserRole)
                 self._increment_progress_bar(self.downloading.format(name=filename), 0)
                 self.previous_size = 0
-                destination = Path(songs_destination, str(filename))
+                destination = songs_destination_path / str(filename)
                 if not url_get_file(self, '{path}{name}'.format(path=self.songs_url, name=filename),
                                     destination, sha256):
                     missed_files.append('Song: {name}'.format(name=filename))
@@ -574,8 +574,7 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
                 self._increment_progress_bar(self.downloading.format(name=bible), 0)
                 self.previous_size = 0
                 if not url_get_file(self, '{path}{name}'.format(path=self.bibles_url, name=bible),
-                                    Path(bibles_destination, bible),
+                                    bibles_destination_path / bible, sha256):
-                                    sha256):
                     missed_files.append('Bible: {name}'.format(name=bible))
             bibles_iterator += 1
         # Download themes
@@ -586,8 +585,7 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
                 self._increment_progress_bar(self.downloading.format(name=theme), 0)
                 self.previous_size = 0
                 if not url_get_file(self, '{path}{name}'.format(path=self.themes_url, name=theme),
-                                    Path(themes_destination, theme),
+                                    themes_destination_path / theme, sha256):
-                                    sha256):
                     missed_files.append('Theme: {name}'.format(name=theme))
         if missed_files:
             file_list = ''

openlp/core/ui/servicemanager.py

@@ -370,7 +370,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
         :rtype: None
         """
         self._service_path = file_path
-        self.main_window.set_service_modified(self.is_modified(), file_path.name)
+        self.set_modified(self.is_modified())
         Settings().setValue('servicemanager/last file', file_path)
         if file_path and file_path.suffix == '.oszl':
             self._save_lite = True

openlp/core/widgets/views.py

@@ -23,18 +23,36 @@
 The :mod:`listpreviewwidget` is a widget that lists the slides in the slide controller.
 It is based on a QTableWidget but represents its contents in list form.
 """
-import os
-
 from PyQt5 import QtCore, QtGui, QtWidgets
 
 from openlp.core.common import is_win
 from openlp.core.common.i18n import UiStrings
 from openlp.core.common.mixins import RegistryProperties
+from openlp.core.common.path import Path
 from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
 from openlp.core.lib import ImageSource, ItemCapabilities, ServiceItem
 
 
+def handle_mime_data_urls(mime_data):
+    """
+    Process the data from a drag and drop operation.
+
+    :param PyQt5.QtCore.QMimeData mime_data: The mime data from the drag and drop opperation.
+    :return: A list of file paths that were dropped
+    :rtype: list[openlp.core.common.path.Path]
+    """
+    file_paths = []
+    for url in mime_data.urls():
+        local_path = Path(url.toLocalFile())
+        if local_path.is_file():
+            file_paths.append(local_path)
+        elif local_path.is_dir():
+            for path in local_path.iterdir():
+                file_paths.append(path)
+    return file_paths
+
+
 class ListPreviewWidget(QtWidgets.QTableWidget, RegistryProperties):
     """
     A special type of QTableWidget which lists the slides in the slide controller
@@ -326,17 +344,9 @@ class ListWidgetWithDnD(QtWidgets.QListWidget):
         if event.mimeData().hasUrls():
             event.setDropAction(QtCore.Qt.CopyAction)
             event.accept()
-            files = []
+            file_paths = handle_mime_data_urls(event.mimeData())
-            for url in event.mimeData().urls():
-                local_file = os.path.normpath(url.toLocalFile())
-                if os.path.isfile(local_file):
-                    files.append(local_file)
-                elif os.path.isdir(local_file):
-                    listing = os.listdir(local_file)
-                    for file in listing:
-                        files.append(os.path.join(local_file, file))
             Registry().execute('{mime_data}_dnd'.format(mime_data=self.mime_data_text),
-                               {'files': files})
+                               {'file_paths': file_paths})
         else:
             event.ignore()
 
@@ -454,16 +464,9 @@ class TreeWidgetWithDnD(QtWidgets.QTreeWidget):
         if event.mimeData().hasUrls():
             event.setDropAction(QtCore.Qt.CopyAction)
             event.accept()
-            files = []
+            file_paths = handle_mime_data_urls(event.mimeData())
-            for url in event.mimeData().urls():
+            Registry().execute('%s_dnd' % self.mime_data_text,
-                local_file = url.toLocalFile()
+                               {'file_paths': file_paths, 'target': self.itemAt(event.pos())})
-                if os.path.isfile(local_file):
-                    files.append(local_file)
-                elif os.path.isdir(local_file):
-                    listing = os.listdir(local_file)
-                    for file_name in listing:
-                        files.append(os.path.join(local_file, file_name))
-            Registry().execute('%s_dnd' % self.mime_data_text, {'files': files, 'target': self.itemAt(event.pos())})
         elif self.allow_internal_dnd:
             event.setDropAction(QtCore.Qt.CopyAction)
             event.accept()

openlp/plugins/media/lib/mediaitem.py

@@ -302,7 +302,7 @@ class MediaMediaItem(MediaManagerItem, RegistryProperties):
         Initialize media item.
         """
         self.list_view.clear()
-        self.service_path = os.path.join(str(AppLocation.get_section_data_path(self.settings_section)), 'thumbnails')
+        self.service_path = str(AppLocation.get_section_data_path(self.settings_section) / 'thumbnails')
         create_paths(Path(self.service_path))
         self.load_list([path_to_str(file) for file in Settings().value(self.settings_section + '/media files')])
         self.rebuild_players()

openlp/plugins/songs/lib/importers/powersong.py

@@ -24,10 +24,9 @@ The :mod:`powersong` module provides the functionality for importing
 PowerSong songs into the OpenLP database.
 """
 import logging
-import fnmatch
-import os
 
 from openlp.core.common.i18n import translate
+from openlp.core.common.path import Path
 from openlp.plugins.songs.lib.importers.songimport import SongImport
 
 log = logging.getLogger(__name__)
@@ -89,26 +88,25 @@ class PowerSongImport(SongImport):
         """
         from openlp.plugins.songs.lib.importer import SongFormat
         ps_string = SongFormat.get(SongFormat.PowerSong, 'name')
-        if isinstance(self.import_source, str):
+        if isinstance(self.import_source, Path):
-            if os.path.isdir(self.import_source):
+            if self.import_source.is_dir():
                 dir = self.import_source
                 self.import_source = []
-                for file in os.listdir(dir):
+                for path in dir.glob('*.song'):
-                    if fnmatch.fnmatch(file, '*.song'):
+                    self.import_source.append(path)
-                        self.import_source.append(os.path.join(dir, file))
             else:
-                self.import_source = ''
+                self.import_source = None
         if not self.import_source or not isinstance(self.import_source, list):
             self.log_error(translate('SongsPlugin.PowerSongImport', 'No songs to import.'),
                            translate('SongsPlugin.PowerSongImport', 'No {text} files found.').format(text=ps_string))
             return
         self.import_wizard.progress_bar.setMaximum(len(self.import_source))
-        for file in self.import_source:
+        for file_path in self.import_source:
             if self.stop_import_flag:
                 return
             self.set_defaults()
             parse_error = False
-            with open(file, 'rb') as song_data:
+            with file_path.open('rb') as song_data:
                 while True:
                     try:
                         label = self._read_string(song_data)
@@ -117,7 +115,7 @@ class PowerSongImport(SongImport):
                         field = self._read_string(song_data)
                     except ValueError:
                         parse_error = True
-                        self.log_error(os.path.basename(file),
+                        self.log_error(file_path.name,
                                        translate('SongsPlugin.PowerSongImport',
                                                  'Invalid {text} file. Unexpected byte value.').format(text=ps_string))
                         break
@@ -135,7 +133,7 @@ class PowerSongImport(SongImport):
                 continue
             # Check that file had TITLE field
             if not self.title:
-                self.log_error(os.path.basename(file),
+                self.log_error(file_path.name,
                                translate('SongsPlugin.PowerSongImport',
                                          'Invalid {text} file. Missing "TITLE" header.').format(text=ps_string))
                 continue

openlp/plugins/songs/lib/importers/sundayplus.py

@@ -63,18 +63,18 @@ class SundayPlusImport(SongImport):
             with file_path.open('rb') as song_file:
                 self.do_import_file(song_file)
 
-    def do_import_file(self, file):
+    def do_import_file(self, file_path):
         """
         Process the Sunday Plus file object.
         """
         self.set_defaults()
-        if not self.parse(file.read()):
+        if not self.parse(file_path.read()):
-            self.log_error(file.name)
+            self.log_error(file_path.name)
             return
         if self.title == '':
-            self.title = self.title_from_filename(file.name)
+            self.title = self.title_from_file_path(file_path)
         if not self.finish():
-            self.log_error(file.name)
+            self.log_error(file_path.name)
 
     def parse(self, data, cell=False):
         """
@@ -174,16 +174,15 @@ class SundayPlusImport(SongImport):
             i += 1
         return True
 
-    def title_from_filename(self, filename):
+    def title_from_file_path(self, file_path):
         """
         Extract the title from the filename
 
-        :param filename: File name
+        :param openlp.core.common.path.Path file_path: File being imported
-        :return:
+        :return: The song title
+        :rtype: str
         """
-        title = os.path.split(filename)[1]
+        title = file_path.stem
-        if title.endswith('.ptf'):
-            title = title[:-4]
         # For some strange reason all example files names ended with 1-7.
         if title.endswith('1-7'):
             title = title[:-3]

tests/functional/openlp_core/widgets/test_views.py

@@ -22,6 +22,7 @@
 """
 Package to test the openlp.core.widgets.views package.
 """
+import os
 from types import GeneratorType
 from unittest import TestCase
 from unittest.mock import MagicMock, patch, call
@@ -30,9 +31,27 @@ from PyQt5 import QtGui
 
 from openlp.core.common.i18n import UiStrings
 from openlp.core.lib import ImageSource
-from openlp.core.widgets.views import ListPreviewWidget, ListWidgetWithDnD, TreeWidgetWithDnD
+from openlp.core.widgets.views import ListPreviewWidget, ListWidgetWithDnD, TreeWidgetWithDnD, handle_mime_data_urls
 
 
+class TestHandleMimeDataUrls(TestCase):
+    """
+    Test the :func:`openlp.core.widgets.views.handle_mime_data_urls` function.
+    """
+    # TODO: Finish tests here!!!!!
+    mocked_path_instance_1 = MagicMock(**{'is_file.return_value': True})
+    mocked_path_instance_2 = MagicMock(**{'is_file.return_value': True})
+    with patch('openlp.core.widgets.views.Path',
+               side_effect=[mocked_path_instance_1, mocked_path_instance_2]) as mocked_path:
+        mocked_q_url_1 = MagicMock(**{'toLocalFile.return_value': os.path.join('file', 'test', 'path', '1.ext')})
+        mocked_q_url_2 = MagicMock(**{'toLocalFile.return_value': os.path.join('file', 'test', 'path', '2.ext')})
+        mocked_q_mime_data = MagicMock(**{'urls.return_value': [mocked_q_url_1, mocked_q_url_2]})
+
+        result = handle_mime_data_urls(mocked_q_mime_data)
+        mocked_path.assert_has_calls([call(os.path.join('file', 'test', 'path', '1.ext')),
+                                     call(os.path.join('file', 'test', 'path', '2.ext'))])
+        assert result == [mocked_path_instance_1, mocked_path_instance_2]
+
 class TestListPreviewWidget(TestCase):
 
     def setUp(self):