From eb115a0ad44bbd7929322c3972caab550aec39d0 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sun, 17 Mar 2019 10:01:52 +0000 Subject: [PATCH] More pathlib clean ups --- openlp/core/lib/__init__.py | 6 +-- openlp/core/lib/db.py | 28 ++++++++------ openlp/core/lib/mediamanageritem.py | 8 ++-- openlp/core/ui/media/__init__.py | 2 +- openlp/core/widgets/wizard.py | 38 ------------------- openlp/plugins/bibles/lib/db.py | 1 - openlp/plugins/bibles/lib/mediaitem.py | 6 +-- openlp/plugins/custom/lib/mediaitem.py | 7 +--- openlp/plugins/images/lib/mediaitem.py | 6 +-- openlp/plugins/media/lib/mediaitem.py | 6 +-- openlp/plugins/presentations/lib/mediaitem.py | 6 +-- .../presentations/lib/messagelistener.py | 10 +---- openlp/plugins/songs/lib/mediaitem.py | 6 +-- .../openlp_plugins/bibles/test_manager.py | 5 ++- 14 files changed, 45 insertions(+), 90 deletions(-) diff --git a/openlp/core/lib/__init__.py b/openlp/core/lib/__init__.py index 9e084fdd3..990b45adb 100644 --- a/openlp/core/lib/__init__.py +++ b/openlp/core/lib/__init__.py @@ -263,15 +263,13 @@ def create_thumb(image_path, thumb_path, return_icon=True, size=None): """ Create a thumbnail from the given image path and depending on ``return_icon`` it returns an icon from this thumb. - :param image_path: The image file to create the icon from. - :param thumb_path: The filename to save the thumbnail to. + :param openlp.core.common.path.Path image_path: The image file to create the icon from. + :param openlp.core.common.path.Path thumb_path: The filename to save the thumbnail to. :param return_icon: States if an icon should be build and returned from the thumb. Defaults to ``True``. :param size: Allows to state a own size (QtCore.QSize) to use. Defaults to ``None``, which means that a default height of 88 is used. :return: The final icon. """ - # TODO: To path object - thumb_path = Path(thumb_path) reader = QtGui.QImageReader(str(image_path)) if size is None: # No size given; use default height of 88 diff --git a/openlp/core/lib/db.py b/openlp/core/lib/db.py index bbdfb5907..fd459f4ef 100644 --- a/openlp/core/lib/db.py +++ b/openlp/core/lib/db.py @@ -132,8 +132,9 @@ def get_db_path(plugin_name, db_file_name=None): Create a path to a database from the plugin name and database name :param plugin_name: Name of plugin - :param db_file_name: File name of database - :return: The path to the database as type str + :param openlp.core.common.path.Path | str | None db_file_name: File name of database + :return: The path to the database + :rtype: str """ if db_file_name is None: return 'sqlite:///{path}/{plugin}.sqlite'.format(path=AppLocation.get_section_data_path(plugin_name), @@ -144,27 +145,30 @@ def get_db_path(plugin_name, db_file_name=None): return 'sqlite:///{path}/{name}'.format(path=AppLocation.get_section_data_path(plugin_name), name=db_file_name) -def handle_db_error(plugin_name, db_file_name): # TODO: To pathlib +def handle_db_error(plugin_name, db_file_path): """ Log and report to the user that a database cannot be loaded :param plugin_name: Name of plugin - :param db_file_name: File name of database + :param openlp.core.common.path.Path db_file_path: File name of database :return: None """ - db_path = get_db_path(plugin_name, db_file_name) + db_path = get_db_path(plugin_name, db_file_path) log.exception('Error loading database: {db}'.format(db=db_path)) critical_error_message_box(translate('OpenLP.Manager', 'Database Error'), translate('OpenLP.Manager', 'OpenLP cannot load your database.\n\nDatabase: {db}').format(db=db_path)) -def init_url(plugin_name, db_file_name=None): # TODO: Pathlib +def init_url(plugin_name, db_file_name=None): """ - Return the database URL. + Construct the connection string for a database. :param plugin_name: The name of the plugin for the database creation. - :param db_file_name: The database file name. Defaults to None resulting in the plugin_name being used. + :param openlp.core.common.path.Path | str | None db_file_name: The database file name. Defaults to None resulting + in the plugin_name being used. + :return: The database URL + :rtype: str """ settings = Settings() settings.beginGroup(plugin_name) @@ -345,7 +349,7 @@ class Manager(object): Runs the initialisation process that includes creating the connection to the database and the tables if they do not exist. - :param plugin_name: The name to setup paths and settings section names + :param plugin_name: The name to setup paths and settings section names :param init_schema: The init_schema function for this database :param openlp.core.common.path.Path db_file_path: The file name to use for this database. Defaults to None resulting in the plugin_name being used. @@ -357,14 +361,14 @@ class Manager(object): self.db_url = None if db_file_path: log.debug('Manager: Creating new DB url') - self.db_url = init_url(plugin_name, str(db_file_path)) + self.db_url = init_url(plugin_name, str(db_file_path)) # TOdO :PATHLIB else: self.db_url = init_url(plugin_name) if upgrade_mod: try: db_ver, up_ver = upgrade_db(self.db_url, upgrade_mod) except (SQLAlchemyError, DBAPIError): - handle_db_error(plugin_name, str(db_file_path)) + handle_db_error(plugin_name, db_file_path) return if db_ver > up_ver: critical_error_message_box( @@ -379,7 +383,7 @@ class Manager(object): try: self.session = init_schema(self.db_url) except (SQLAlchemyError, DBAPIError): - handle_db_error(plugin_name, str(db_file_path)) + handle_db_error(plugin_name, db_file_path) else: self.session = session diff --git a/openlp/core/lib/mediamanageritem.py b/openlp/core/lib/mediamanageritem.py index b84b7a24f..35adc0bfc 100644 --- a/openlp/core/lib/mediamanageritem.py +++ b/openlp/core/lib/mediamanageritem.py @@ -454,15 +454,17 @@ class MediaManagerItem(QtWidgets.QWidget, RegistryProperties): """ pass - def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False, - context=ServiceItemContext.Live): + def generate_slide_data(self, service_item, *, item=None, xml_version=False, remote=False, + context=ServiceItemContext.Live, file_path=None): """ Generate the slide data. Needs to be implemented by the plugin. + :param service_item: The service Item to be processed :param item: The database item to be used to build the service item :param xml_version: :param remote: Was this remote triggered (False) :param context: The service context + :param openlp.core.common.path.Path file_path: """ raise NotImplementedError('MediaManagerItem.generate_slide_data needs to be defined by the plugin') @@ -634,7 +636,7 @@ class MediaManagerItem(QtWidgets.QWidget, RegistryProperties): """ service_item = ServiceItem(self.plugin) service_item.add_icon() - if self.generate_slide_data(service_item, item, xml_version, remote, context): + if self.generate_slide_data(service_item, item=item, xml_version=xml_version, remote=remote, context=context): return service_item else: return None diff --git a/openlp/core/ui/media/__init__.py b/openlp/core/ui/media/__init__.py index 5f3e4a072..4e38cc122 100644 --- a/openlp/core/ui/media/__init__.py +++ b/openlp/core/ui/media/__init__.py @@ -54,7 +54,7 @@ class ItemMediaInfo(object): """ This class hold the media related info """ - file_info = None # TODO: Ptahlib? + file_info = None volume = 100 is_background = False can_loop_playback = False diff --git a/openlp/core/widgets/wizard.py b/openlp/core/widgets/wizard.py index 284caba96..5619d5a3e 100644 --- a/openlp/core/widgets/wizard.py +++ b/openlp/core/widgets/wizard.py @@ -280,41 +280,3 @@ class OpenLPWizard(QtWidgets.QWizard, RegistryProperties): self.finish_button.setVisible(True) self.cancel_button.setVisible(False) self.application.process_events() - - def get_file_name(self, title, editbox, setting_name, filters=''): - """ - Opens a FileDialog and saves the filename to the given editbox. - - :param str title: The title of the dialog. - :param QtWidgets.QLineEdit editbox: An QLineEdit. - :param str setting_name: The place where to save the last opened directory. - :param str filters: The file extension filters. It should contain the file description - as well as the file extension. For example:: - - 'OpenLP 2 Databases (*.sqlite)' - :rtype: None - """ - if filters: - filters += ';;' - filters += '%s (*)' % UiStrings().AllFiles - file_path, filter_used = FileDialog.getOpenFileName( - self, title, Settings().value(self.plugin.settings_section + '/' + setting_name), filters) - if file_path: - editbox.setText(str(file_path)) # TODO: to pathdedit - Settings().setValue(self.plugin.settings_section + '/' + setting_name, file_path.parent) - - def get_folder(self, title, editbox, setting_name): - """ - Opens a FileDialog and saves the selected folder to the given editbox. - - :param str title: The title of the dialog. - :param QtWidgets.QLineEdit editbox: An QLineEditbox. - :param str setting_name: The place where to save the last opened directory. - :rtype: None - """ - folder_path = FileDialog.getExistingDirectory( - self, title, Settings().value(self.plugin.settings_section + '/' + setting_name), - FileDialog.ShowDirsOnly) - if folder_path: - editbox.setText(str(folder_path)) # TODO: to pathedit - Settings().setValue(self.plugin.settings_section + '/' + setting_name, folder_path) diff --git a/openlp/plugins/bibles/lib/db.py b/openlp/plugins/bibles/lib/db.py index bccff3559..5f127bf05 100644 --- a/openlp/plugins/bibles/lib/db.py +++ b/openlp/plugins/bibles/lib/db.py @@ -158,7 +158,6 @@ class BibleDB(Manager): self.name = kwargs['name'] if not isinstance(self.name, str): self.name = str(self.name, 'utf-8') - # TODO: To path object self.file_path = Path(clean_filename(self.name) + '.sqlite') if 'file' in kwargs: self.file_path = kwargs['file'] diff --git a/openlp/plugins/bibles/lib/mediaitem.py b/openlp/plugins/bibles/lib/mediaitem.py index f35e9dcfa..925744142 100755 --- a/openlp/plugins/bibles/lib/mediaitem.py +++ b/openlp/plugins/bibles/lib/mediaitem.py @@ -911,16 +911,16 @@ class BibleMediaItem(MediaManagerItem): list_widget_items.append(bible_verse) return list_widget_items - def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False, - context=ServiceItemContext.Service): + def generate_slide_data(self, service_item, *, item=None, remote=False, context=ServiceItemContext.Service, + **kwargs): """ Generate the slide data. Needs to be implemented by the plugin. :param service_item: The service item to be built on :param item: The Song item to be used - :param xml_version: The xml version (not used) :param remote: Triggered from remote :param context: Why is it being generated + :param kwargs: Consume other unused args specified by the base implementation, but not use by this one. """ log.debug('generating slide data') if item: diff --git a/openlp/plugins/custom/lib/mediaitem.py b/openlp/plugins/custom/lib/mediaitem.py index 0225f0cc4..fcf42f490 100644 --- a/openlp/plugins/custom/lib/mediaitem.py +++ b/openlp/plugins/custom/lib/mediaitem.py @@ -219,15 +219,12 @@ class CustomMediaItem(MediaManagerItem): self.search_text_edit.setFocus() self.search_text_edit.selectAll() - def generate_slide_data(self, service_item, item=None, xml_version=False, - remote=False, context=ServiceItemContext.Service): + def generate_slide_data(self, service_item, *, item=None, **kwargs): """ Generate the slide data. Needs to be implemented by the plugin. :param service_item: To be updated :param item: The custom database item to be used - :param xml_version: No used - :param remote: Is this triggered by the Preview Controller or Service Manager. - :param context: Why is this item required to be build (Default Service). + :param kwargs: Consume other unused args specified by the base implementation, but not use by this one. """ item_id = self._get_id_of_item_to_generate(item, self.remote_custom) service_item.add_capability(ItemCapabilities.CanEdit) diff --git a/openlp/plugins/images/lib/mediaitem.py b/openlp/plugins/images/lib/mediaitem.py index 69bd46b27..b2eb25721 100644 --- a/openlp/plugins/images/lib/mediaitem.py +++ b/openlp/plugins/images/lib/mediaitem.py @@ -542,16 +542,16 @@ class ImageMediaItem(MediaManagerItem): image_items.sort(key=lambda item: get_natural_key(item.text(0))) target_group.addChildren(image_items) - def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False, - context=ServiceItemContext.Service): + def generate_slide_data(self, service_item, *, item=None, remote=False, context=ServiceItemContext.Service, + **kwargs): """ Generate the slide data. Needs to be implemented by the plugin. :param service_item: The service item to be built on :param item: The Song item to be used - :param xml_version: The xml version (not used) :param remote: Triggered from remote :param context: Why is it being generated + :param kwargs: Consume other unused args specified by the base implementation, but not use by this one. """ background = QtGui.QColor(Settings().value(self.settings_section + '/background color')) if item: diff --git a/openlp/plugins/media/lib/mediaitem.py b/openlp/plugins/media/lib/mediaitem.py index dad964dd5..292312626 100644 --- a/openlp/plugins/media/lib/mediaitem.py +++ b/openlp/plugins/media/lib/mediaitem.py @@ -166,16 +166,16 @@ class MediaMediaItem(MediaManagerItem, RegistryProperties): # self.display_type_combo_box.currentIndexChanged.connect(self.override_player_changed) pass - def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False, - context=ServiceItemContext.Service): + def generate_slide_data(self, service_item, *, item=None, remote=False, context=ServiceItemContext.Service, + **kwargs): """ Generate the slide data. Needs to be implemented by the plugin. :param service_item: The service item to be built on :param item: The Song item to be used - :param xml_version: The xml version (not used) :param remote: Triggered from remote :param context: Why is it being generated + :param kwargs: Consume other unused args specified by the base implementation, but not use by this one. """ if item is None: item = self.list_view.currentItem() diff --git a/openlp/plugins/presentations/lib/mediaitem.py b/openlp/plugins/presentations/lib/mediaitem.py index 106720c9d..ddec6d143 100644 --- a/openlp/plugins/presentations/lib/mediaitem.py +++ b/openlp/plugins/presentations/lib/mediaitem.py @@ -260,16 +260,16 @@ class PresentationMediaItem(MediaManagerItem): doc.presentation_deleted() doc.close_presentation() - def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False, - context=ServiceItemContext.Service, file_path=None): + def generate_slide_data(self, service_item, *, item=None, remote=False, context=ServiceItemContext.Service, + file_path=None, **kwargs): """ Generate the slide data. Needs to be implemented by the plugin. :param service_item: The service item to be built on :param item: The Song item to be used - :param xml_version: The xml version (not used) :param remote: Triggered from remote :param context: Why is it being generated + :param kwargs: Consume other unused args specified by the base implementation, but not use by this one. """ if item: items = [item] diff --git a/openlp/plugins/presentations/lib/messagelistener.py b/openlp/plugins/presentations/lib/messagelistener.py index ba50e621c..ef9dd1544 100644 --- a/openlp/plugins/presentations/lib/messagelistener.py +++ b/openlp/plugins/presentations/lib/messagelistener.py @@ -337,14 +337,8 @@ class MessageListener(object): # Create a copy of the original item, and then clear the original item so it can be filled with images item_cpy = copy.copy(item) item.__init__(None) - if is_live: - # TODO: To Path object - self.media_item.generate_slide_data(item, item_cpy, False, False, ServiceItemContext.Live, - str(file_path)) - else: - # TODO: To Path object - self.media_item.generate_slide_data(item, item_cpy, False, False, ServiceItemContext.Preview, - str(file_path)) + context = ServiceItemContext.Live if is_live else ServiceItemContext.Preview + self.media_item.generate_slide_data(item, item=item_cpy, context=context, file_path=file_path) # Some of the original serviceitem attributes is needed in the new serviceitem item.footer = item_cpy.footer item.from_service = item_cpy.from_service diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index 96b94c9f7..4af0f0096 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -557,16 +557,14 @@ class SongMediaItem(MediaManagerItem): self.plugin.manager.save_object(new_song) self.on_song_list_load() - def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False, - context=ServiceItemContext.Service): + def generate_slide_data(self, service_item, *, item=None, context=ServiceItemContext.Service, **kwargs): """ Generate the slide data. Needs to be implemented by the plugin. :param service_item: The service item to be built on :param item: The Song item to be used - :param xml_version: The xml version (not used) - :param remote: Triggered from remote :param context: Why is it being generated + :param kwargs: Consume other unused args specified by the base implementation, but not use by this one. """ log.debug('generate_slide_data: {service}, {item}, {remote}'.format(service=service_item, item=item, remote=self.remote_song)) diff --git a/tests/functional/openlp_plugins/bibles/test_manager.py b/tests/functional/openlp_plugins/bibles/test_manager.py index 8e295a52b..39a693446 100644 --- a/tests/functional/openlp_plugins/bibles/test_manager.py +++ b/tests/functional/openlp_plugins/bibles/test_manager.py @@ -55,7 +55,8 @@ class TestManager(TestCase): instance = BibleManager(MagicMock()) # We need to keep a reference to the mock for close_all as it gets set to None later on! mocked_close_all = MagicMock() - mocked_bible = MagicMock(file='KJV.sqlite', path='bibles', **{'session.close_all': mocked_close_all}) + mocked_bible = MagicMock(file_path='KJV.sqlite', path=Path('bibles'), + **{'session.close_all': mocked_close_all}) instance.db_cache = {'KJV': mocked_bible} # WHEN: Calling delete_bible with 'KJV' @@ -66,4 +67,4 @@ class TestManager(TestCase): assert result is True mocked_close_all.assert_called_once_with() assert mocked_bible.session is None - mocked_delete_file.assert_called_once_with(Path('bibles', 'KJV.sqlite')) + mocked_delete_file.assert_called_once_with(Path('bibles'), 'KJV.sqlite')