From 0cc2ae5998e02df32e6e0859dfc00d7a76c4bf76 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Tue, 27 Jan 2015 19:57:09 +0000 Subject: [PATCH 1/5] Fix exception raised on mac Fixes: https://launchpad.net/bugs/1414360 --- .../presentations/presentationplugin.py | 5 +- .../openlp_core_utils/test_utils.py | 56 ++++++++++++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/openlp/plugins/presentations/presentationplugin.py b/openlp/plugins/presentations/presentationplugin.py index cb92c0826..44470b578 100644 --- a/openlp/plugins/presentations/presentationplugin.py +++ b/openlp/plugins/presentations/presentationplugin.py @@ -143,7 +143,10 @@ class PresentationPlugin(Plugin): super().app_startup() files_from_config = Settings().value('presentations/presentations files') for file in files_from_config: - self.media_item.clean_up_thumbnails(file) + try: + self.media_item.clean_up_thumbnails(file) + except AttributeError: + pass self.media_item.list_view.clear() Settings().setValue('presentations/thumbnail_scheme', 'md5') self.media_item.validate_and_load(files_from_config) diff --git a/tests/functional/openlp_core_utils/test_utils.py b/tests/functional/openlp_core_utils/test_utils.py index ad7cf2831..893fb41ca 100644 --- a/tests/functional/openlp_core_utils/test_utils.py +++ b/tests/functional/openlp_core_utils/test_utils.py @@ -25,7 +25,7 @@ Functional tests to test the AppLocation class and related methods. import os from unittest import TestCase -from openlp.core.utils import clean_filename, get_filesystem_encoding, get_locale_key, \ +from openlp.core.utils import clean_filename, delete_file, get_filesystem_encoding, get_locale_key, \ get_natural_key, split_filename, _get_user_agent, get_web_page, get_uno_instance, add_actions from tests.functional import MagicMock, patch @@ -184,6 +184,60 @@ class TestUtils(TestCase): # THEN: The file name should be cleaned. self.assertEqual(wanted_name, result, 'The file name should not contain any special characters.') + def delete_file_no_path_test(self): + """" + Test the delete_file function when called with out a valid path + """ + # GIVEN: A blank path + # WEHN: Calling delete_file + result = delete_file('') + + # THEN: delete_file should return False + self.assertFalse(result, "delete_file should return False when called with ''") + + def delete_file_path_success_test(self): + """" + Test the delete_file function when it successfully deletes a file + """ + # GIVEN: A mocked os which returns True when os.path.exists is called + with patch('openlp.core.utils.os', **{'path.exists.return_value': False}) as mocked_os: + + # WHEN: Calling delete_file with a file path + result = delete_file('path/file.ext') + + # THEN: delete_file should return True + self.assertTrue(result, 'delete_file should return True when it successfully deletes a file') + + def delete_file_path_no_file_exists_test(self): + """" + Test the delete_file function when the file to remove does not exist + """ + # GIVEN: A mocked os which returns False when os.path.exists is called + with patch('openlp.core.utils.os', **{'path.exists.return_value': False}) as mocked_os: + + # WHEN: Calling delete_file with a file path + result = delete_file('path/file.ext') + + # THEN: delete_file should return True + self.assertTrue(result, 'delete_file should return True when the file doesnt exist') + + def delete_file_path_exception_test(self): + """" + Test the delete_file function when os.remove raises an exception + """ + # GIVEN: A mocked os which returns True when os.path.exists is called and raises an OSError when os.remove is + # called. + with patch('openlp.core.utils.os', **{'path.exists.return_value': True, + 'path.exists.side_effect': OSError}) as mocked_os, \ + patch('openlp.core.utils.log') as mocked_log: + + # WHEN: Calling delete_file with a file path + result = delete_file('path/file.ext') + + # THEN: delete_file should log and exception and return False + self.assertEqual(mocked_log.exception.call_count, 1) + self.assertFalse(result, 'delete_file should return False when os.remove raises an OSError') + def get_locale_key_test(self): """ Test the get_locale_key(string) function From 3fcccd70e4568976c60ff2077a949e18833ba564 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Wed, 28 Jan 2015 20:41:42 +0000 Subject: [PATCH 2/5] Fixes thumbnails in stage view --- openlp/plugins/images/lib/mediaitem.py | 36 ++++++++++++++++---------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/openlp/plugins/images/lib/mediaitem.py b/openlp/plugins/images/lib/mediaitem.py index a3ec28b37..19d96b4b2 100644 --- a/openlp/plugins/images/lib/mediaitem.py +++ b/openlp/plugins/images/lib/mediaitem.py @@ -314,6 +314,16 @@ class ImageMediaItem(MediaManagerItem): return True return return_value + def generate_thumbnail_path(self, image): + """ + Generate a path to the thumbnail + + :param imageFile: An instance of fImageFileNames + :return: A path to the thumbnail of type str + """ + ext = os.path.splitext(image.filename)[1].lower() + return os.path.join(self.service_path, '{}{}'.format(str(image.id), ext)) + def load_full_list(self, images, initial_load=False, open_group=None): """ Replace the list of images and groups in the interface. @@ -338,8 +348,7 @@ class ImageMediaItem(MediaManagerItem): for imageFile in images: log.debug('Loading image: %s', imageFile.filename) filename = os.path.split(imageFile.filename)[1] - ext = os.path.splitext(imageFile.filename)[1].lower() - thumb = os.path.join(self.service_path, "%s%s" % (str(imageFile.id), ext)) + thumb = self.generate_thumbnail_path(imageFile) if not os.path.exists(imageFile.filename): icon = build_icon(':/general/general_delete.png') else: @@ -549,24 +558,24 @@ class ImageMediaItem(MediaManagerItem): # force a nonexistent theme service_item.theme = -1 missing_items_file_names = [] - images_file_names = [] + images = [] # Expand groups to images for bitem in items: if isinstance(bitem.data(0, QtCore.Qt.UserRole), ImageGroups) or bitem.data(0, QtCore.Qt.UserRole) is None: for index in range(0, bitem.childCount()): if isinstance(bitem.child(index).data(0, QtCore.Qt.UserRole), ImageFilenames): - images_file_names.append(bitem.child(index).data(0, QtCore.Qt.UserRole).filename) + images.append(bitem.child(index).data(0, QtCore.Qt.UserRole)) elif isinstance(bitem.data(0, QtCore.Qt.UserRole), ImageFilenames): - images_file_names.append(bitem.data(0, QtCore.Qt.UserRole).filename) + images.append(bitem.data(0, QtCore.Qt.UserRole)) # Don't try to display empty groups - if not images_file_names: + if not images: return False # Find missing files - for filename in images_file_names: - if not os.path.exists(filename): - missing_items_file_names.append(filename) + for image in images: + if not os.path.exists(image.filename): + missing_items_file_names.append(image.filename) # We cannot continue, as all images do not exist. - if not images_file_names: + if not images: if not remote: critical_error_message_box( translate('ImagePlugin.MediaItem', 'Missing Image(s)'), @@ -582,9 +591,10 @@ class ImageMediaItem(MediaManagerItem): QtGui.QMessageBox.No: return False # Continue with the existing images. - for filename in images_file_names: - name = os.path.split(filename)[1] - service_item.add_from_image(filename, name, background, os.path.join(self.service_path, name)) + for image in images: + name = os.path.split(image.filename)[1] + thumbnail = self.generate_thumbnail_path(image) + service_item.add_from_image(image.filename, name, background, thumbnail) return True def check_group_exists(self, new_group): From 52a1033505a5dc9228039442f03ef7f4ac4de91e Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Thu, 29 Jan 2015 18:26:00 +0000 Subject: [PATCH 3/5] test tidies --- tests/functional/openlp_core_utils/test_utils.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/functional/openlp_core_utils/test_utils.py b/tests/functional/openlp_core_utils/test_utils.py index 893fb41ca..cca5f6825 100644 --- a/tests/functional/openlp_core_utils/test_utils.py +++ b/tests/functional/openlp_core_utils/test_utils.py @@ -200,7 +200,7 @@ class TestUtils(TestCase): Test the delete_file function when it successfully deletes a file """ # GIVEN: A mocked os which returns True when os.path.exists is called - with patch('openlp.core.utils.os', **{'path.exists.return_value': False}) as mocked_os: + with patch('openlp.core.utils.os', **{'path.exists.return_value': False}): # WHEN: Calling delete_file with a file path result = delete_file('path/file.ext') @@ -213,7 +213,7 @@ class TestUtils(TestCase): Test the delete_file function when the file to remove does not exist """ # GIVEN: A mocked os which returns False when os.path.exists is called - with patch('openlp.core.utils.os', **{'path.exists.return_value': False}) as mocked_os: + with patch('openlp.core.utils.os', **{'path.exists.return_value': False}): # WHEN: Calling delete_file with a file path result = delete_file('path/file.ext') @@ -227,8 +227,7 @@ class TestUtils(TestCase): """ # GIVEN: A mocked os which returns True when os.path.exists is called and raises an OSError when os.remove is # called. - with patch('openlp.core.utils.os', **{'path.exists.return_value': True, - 'path.exists.side_effect': OSError}) as mocked_os, \ + with patch('openlp.core.utils.os', **{'path.exists.return_value': True, 'path.exists.side_effect': OSError}), \ patch('openlp.core.utils.log') as mocked_log: # WHEN: Calling delete_file with a file path From 84dfb24674098b7da7e38ce8992808e382ee37fc Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Thu, 29 Jan 2015 18:28:00 +0000 Subject: [PATCH 4/5] pep fix --- openlp/plugins/bibles/lib/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/bibles/lib/__init__.py b/openlp/plugins/bibles/lib/__init__.py index 1d7216ee5..a8becc041 100644 --- a/openlp/plugins/bibles/lib/__init__.py +++ b/openlp/plugins/bibles/lib/__init__.py @@ -178,7 +178,7 @@ def update_reference_separators(): default_separators = [ '|'.join([ translate('BiblesPlugin', ':', 'Verse identifier e.g. Genesis 1 : 1 = Genesis Chapter 1 Verse 1'), - translate('BiblesPlugin', 'v','Verse identifier e.g. Genesis 1 v 1 = Genesis Chapter 1 Verse 1'), + translate('BiblesPlugin', 'v', 'Verse identifier e.g. Genesis 1 v 1 = Genesis Chapter 1 Verse 1'), translate('BiblesPlugin', 'V', 'Verse identifier e.g. Genesis 1 V 1 = Genesis Chapter 1 Verse 1'), translate('BiblesPlugin', 'verse', 'Verse identifier e.g. Genesis 1 verse 1 = Genesis Chapter 1 Verse 1'), translate('BiblesPlugin', 'verses', From cdafe89624a50aa886bb3c0d2b5487ed20173866 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Fri, 30 Jan 2015 21:03:53 +0000 Subject: [PATCH 5/5] Change fileName to file_name --- openlp/plugins/images/lib/mediaitem.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/openlp/plugins/images/lib/mediaitem.py b/openlp/plugins/images/lib/mediaitem.py index 19d96b4b2..a52b25d8a 100644 --- a/openlp/plugins/images/lib/mediaitem.py +++ b/openlp/plugins/images/lib/mediaitem.py @@ -318,7 +318,7 @@ class ImageMediaItem(MediaManagerItem): """ Generate a path to the thumbnail - :param imageFile: An instance of fImageFileNames + :param image: An instance of ImageFileNames :return: A path to the thumbnail of type str """ ext = os.path.splitext(image.filename)[1].lower() @@ -345,26 +345,26 @@ class ImageMediaItem(MediaManagerItem): # Sort the images by its filename considering language specific. # characters. images.sort(key=lambda image_object: get_locale_key(os.path.split(str(image_object.filename))[1])) - for imageFile in images: - log.debug('Loading image: %s', imageFile.filename) - filename = os.path.split(imageFile.filename)[1] - thumb = self.generate_thumbnail_path(imageFile) - if not os.path.exists(imageFile.filename): + for image_file in images: + log.debug('Loading image: %s', image_file.filename) + filename = os.path.split(image_file.filename)[1] + thumb = self.generate_thumbnail_path(image_file) + if not os.path.exists(image_file.filename): icon = build_icon(':/general/general_delete.png') else: - if validate_thumb(imageFile.filename, thumb): + if validate_thumb(image_file.filename, thumb): icon = build_icon(thumb) else: - icon = create_thumb(imageFile.filename, thumb) + icon = create_thumb(image_file.filename, thumb) item_name = QtGui.QTreeWidgetItem([filename]) item_name.setText(0, filename) item_name.setIcon(0, icon) - item_name.setToolTip(0, imageFile.filename) - item_name.setData(0, QtCore.Qt.UserRole, imageFile) - if imageFile.group_id == 0: + item_name.setToolTip(0, image_file.filename) + item_name.setData(0, QtCore.Qt.UserRole, image_file) + if image_file.group_id == 0: self.list_view.addTopLevelItem(item_name) else: - group_items[imageFile.group_id].addChild(item_name) + group_items[image_file.group_id].addChild(item_name) if not initial_load: self.main_window.increment_progress_bar() if not initial_load: