Fixes Bug #1367141: Presentations/Images with same name gets the same thumbnail

Uses the database id for thumbnails. Uses an md5 hash of the path and file name for presentations (as there is no db for presentations)

Also added code to remove the old thumbnails.

bzr-revno: 2487

openlp/core/__init__.py

@@ -263,6 +263,29 @@ class OpenLP(OpenLPMixin, QtGui.QApplication):
             return QtGui.QApplication.event(self, event)
 
 
+def parse_options(args):
+    """
+    Parse the command line arguments
+
+    :param args: list of command line arguments
+    :return: a tuple of parsed options of type optparse.Value and a list of remaining argsZ
+    """
+    # Set up command line options.
+    usage = 'Usage: %prog [options] [qt-options]'
+    parser = OptionParser(usage=usage)
+    parser.add_option('-e', '--no-error-form', dest='no_error_form', action='store_true',
+                      help='Disable the error notification form.')
+    parser.add_option('-l', '--log-level', dest='loglevel', default='warning', metavar='LEVEL',
+                      help='Set logging to LEVEL level. Valid values are "debug", "info", "warning".')
+    parser.add_option('-p', '--portable', dest='portable', action='store_true',
+                      help='Specify if this should be run as a portable app, off a USB flash drive (not implemented).')
+    parser.add_option('-d', '--dev-version', dest='dev_version', action='store_true',
+                      help='Ignore the version file and pull the version directly from Bazaar')
+    parser.add_option('-s', '--style', dest='style', help='Set the Qt4 style (passed directly to Qt4).')
+    # Parse command line options and deal with them. Use args supplied pragmatically if possible.
+    return parser.parse_args(args) if args else parser.parse_args()
+
+
 def set_up_logging(log_path):
     """
     Setup our logging using log_path
@@ -284,21 +307,7 @@ def main(args=None):
 
     :param args: Some args
     """
-    # Set up command line options.
+    (options, args) = parse_options(args)
-    usage = 'Usage: %prog [options] [qt-options]'
-    parser = OptionParser(usage=usage)
-    parser.add_option('-e', '--no-error-form', dest='no_error_form', action='store_true',
-                      help='Disable the error notification form.')
-    parser.add_option('-l', '--log-level', dest='loglevel', default='warning', metavar='LEVEL',
-                      help='Set logging to LEVEL level. Valid values are "debug", "info", "warning".')
-    parser.add_option('-p', '--portable', dest='portable', action='store_true',
-                      help='Specify if this should be run as a portable app, off a USB flash drive (not implemented).')
-    parser.add_option('-d', '--dev-version', dest='dev_version', action='store_true',
-                      help='Ignore the version file and pull the version directly from Bazaar')
-    parser.add_option('-s', '--style', dest='style', help='Set the Qt4 style (passed directly to Qt4).')
-    # Parse command line options and deal with them.
-    # Use args supplied pragmatically if possible.
-    (options, args) = parser.parse_args(args) if args else parser.parse_args()
     qt_args = []
     if options.loglevel.lower() in ['d', 'debug']:
         log.setLevel(logging.DEBUG)

openlp/core/lib/mediamanageritem.py

@@ -366,8 +366,7 @@ s
         duplicates_found = False
         files_added = False
         for file_path in files:
-            filename = os.path.split(str(file_path))[1]
+            if file_path in full_list:
-            if filename in names:
                 duplicates_found = True
             else:
                 files_added = True

openlp/plugins/images/imageplugin.py

@@ -23,6 +23,7 @@
 from PyQt4 import QtGui
 
 import logging
+import os
 
 from openlp.core.common import Registry, Settings, translate
 from openlp.core.lib import Plugin, StringContent, ImageSource, build_icon
@@ -66,10 +67,18 @@ class ImagePlugin(Plugin):
         """
         Perform tasks on application startup.
         """
+        # TODO: Can be removed when the upgrade path from 2.0.x to 2.2.x is no longer needed
         Plugin.app_startup(self)
         # Convert old settings-based image list to the database.
         files_from_config = Settings().get_files_from_config(self)
         if files_from_config:
+            for file in files_from_config:
+                filename = os.path.split(file)[1]
+                thumb = os.path.join(self.media_item.service_path, filename)
+                try:
+                    os.remove(thumb)
+                except:
+                    pass
             log.debug('Importing images list from old config: %s' % files_from_config)
             self.media_item.save_new_images_list(files_from_config)
 
@@ -79,6 +88,7 @@ class ImagePlugin(Plugin):
 
         :param settings: The Settings object containing the old settings.
         """
+        # TODO: Can be removed when the upgrade path from 2.0.x to 2.2.x is no longer needed
         files_from_config = settings.get_files_from_config(self)
         if files_from_config:
             log.debug('Importing images list from old config: %s' % files_from_config)

openlp/plugins/images/lib/mediaitem.py

@@ -338,7 +338,8 @@ class ImageMediaItem(MediaManagerItem):
         for imageFile in images:
             log.debug('Loading image: %s', imageFile.filename)
             filename = os.path.split(imageFile.filename)[1]
-            thumb = os.path.join(self.service_path, filename)
+            ext = os.path.splitext(imageFile.filename)[1].lower()
+            thumb = os.path.join(self.service_path, "%s%s" % (str(imageFile.id), ext))
             if not os.path.exists(imageFile.filename):
                 icon = build_icon(':/general/general_delete.png')
             else:

openlp/plugins/presentations/lib/mediaitem.py

@@ -222,10 +222,7 @@ class PresentationMediaItem(MediaManagerItem):
             self.main_window.display_progress_bar(len(row_list))
             for item in items:
                 filepath = str(item.data(QtCore.Qt.UserRole))
-                for cidx in self.controllers:
+                self.clean_up_thumbnails(filepath)
-                    doc = self.controllers[cidx].add_document(filepath)
-                    doc.presentation_deleted()
-                    doc.close_presentation()
                 self.main_window.increment_progress_bar()
             self.main_window.finished_progress_bar()
             self.application.set_busy_cursor()
@@ -233,6 +230,18 @@ class PresentationMediaItem(MediaManagerItem):
                 self.list_view.takeItem(row)
             Settings().setValue(self.settings_section + '/presentations files', self.get_file_list())
 
+    def clean_up_thumbnails(self, filepath):
+        """
+        Clean up the files created such as thumbnails
+
+        :param filepath: File path of the presention to clean up after
+        :return: None
+        """
+        for cidx in self.controllers:
+            doc = self.controllers[cidx].add_document(filepath)
+            doc.presentation_deleted()
+            doc.close_presentation()
+
     def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False,
                             context=ServiceItemContext.Service, presentation_file=None):
         """

openlp/plugins/presentations/lib/presentationcontroller.py

@@ -26,7 +26,7 @@ import shutil
 
 from PyQt4 import QtCore
 
-from openlp.core.common import Registry, AppLocation, Settings, check_directory_exists
+from openlp.core.common import Registry, AppLocation, Settings, check_directory_exists, md5_hash
 from openlp.core.lib import create_thumb, validate_thumb
 
 log = logging.getLogger(__name__)
@@ -132,13 +132,23 @@ class PresentationDocument(object):
         """
         The location where thumbnail images will be stored
         """
-        return os.path.join(self.controller.thumbnail_folder, self.get_file_name())
+        # TODO: If statment can be removed when the upgrade path from 2.0.x to 2.2.x is no longer needed
+        if Settings().value('presentations/thumbnail_scheme') == 'md5':
+            folder = md5_hash('', self.file_path)
+        else:
+            folder = self.get_file_name()
+        return os.path.join(self.controller.thumbnail_folder, folder)
 
     def get_temp_folder(self):
         """
         The location where thumbnail images will be stored
         """
-        return os.path.join(self.controller.temp_folder, self.get_file_name())
+        # TODO: If statment can be removed when the upgrade path from 2.0.x to 2.2.x is no longer needed
+        if Settings().value('presentations/thumbnail_scheme') == 'md5':
+            folder = md5_hash('', self.file_path)
+        else:
+            folder = folder = self.get_file_name()
+        return os.path.join(self.controller.temp_folder, folder)
 
     def check_thumbnails(self):
         """

openlp/plugins/presentations/presentationplugin.py

@@ -28,7 +28,7 @@ import logging
 
 from PyQt4 import QtCore
 
-from openlp.core.common import AppLocation, translate
+from openlp.core.common import AppLocation, Settings, translate
 from openlp.core.lib import Plugin, StringContent, build_icon
 from openlp.plugins.presentations.lib import PresentationController, PresentationMediaItem, PresentationTab
 
@@ -43,7 +43,8 @@ __default_settings__ = {'presentations/override app': QtCore.Qt.Unchecked,
                         'presentations/Powerpoint': QtCore.Qt.Checked,
                         'presentations/Powerpoint Viewer': QtCore.Qt.Checked,
                         'presentations/Pdf': QtCore.Qt.Checked,
-                        'presentations/presentations files': []
+                        'presentations/presentations files': [],
+                        'presentations/thumbnail_scheme': ''
                         }
 
 
@@ -134,6 +135,19 @@ class PresentationPlugin(Plugin):
             self.register_controllers(controller)
         return bool(self.controllers)
 
+    def app_startup(self):
+        """
+        Perform tasks on application startup.
+        """
+        # TODO: Can be removed when the upgrade path from 2.0.x to 2.2.x is no longer needed
+        super().app_startup()
+        files_from_config = Settings().value('presentations/presentations files')
+        for file in files_from_config:
+            self.media_item.clean_up_thumbnails(file)
+        self.media_item.list_view.clear()
+        Settings().setValue('presentations/thumbnail_scheme', 'md5')
+        self.media_item.validate_and_load(files_from_config)
+
     def about(self):
         """
         Return information about this plugin.

tests/functional/openlp_plugins/presentations/test_impresscontroller.py

@@ -31,8 +31,10 @@ from tests.functional import patch, MagicMock
 from tests.utils.constants import TEST_RESOURCES_PATH
 from tests.helpers.testmixin import TestMixin
 
+from openlp.core.common import Settings
 from openlp.plugins.presentations.lib.impresscontroller import \
     ImpressController, ImpressDocument, TextType
+from openlp.plugins.presentations.presentationplugin import __default_settings__
 
 
 class TestImpressController(TestCase, TestMixin):
@@ -79,6 +81,7 @@ class TestImpressDocument(TestCase):
     def setUp(self):
         mocked_plugin = MagicMock()
         mocked_plugin.settings_section = 'presentations'
+        Settings().extend_default_settings(__default_settings__)
         self.file_name = os.path.join(TEST_RESOURCES_PATH, 'presentations', 'test.pptx')
         self.ppc = ImpressController(mocked_plugin)
         self.doc = ImpressDocument(self.ppc, self.file_name)

tests/functional/openlp_plugins/presentations/test_pdfcontroller.py

@@ -36,7 +36,8 @@ from tests.utils.constants import TEST_RESOURCES_PATH
 from tests.helpers.testmixin import TestMixin
 
 __default_settings__ = {
-    'presentations/enable_pdf_program': False
+    'presentations/enable_pdf_program': False,
+    'presentations/thumbnail_scheme': ''
 }
 
 SCREEN = {

tests/functional/test_init.py

@@ -22,13 +22,14 @@
 """
 Package to test the openlp.core.__init__ package.
 """
+from optparse import Values
 import os
 
 from unittest import TestCase
 from unittest.mock import MagicMock, patch
 from PyQt4 import QtCore, QtGui
 
-from openlp.core import OpenLP
+from openlp.core import OpenLP, parse_options
 from openlp.core.common import Settings
 from tests.helpers.testmixin import TestMixin
 
@@ -112,3 +113,17 @@ class TestInit(TestCase, TestMixin):
             # THEN: It should ask if we want to create a backup
             self.assertEqual(Settings().value('core/application version'), '2.2.0', 'Version should be upgraded!')
             self.assertEqual(mocked_question.call_count, 1, 'A question should have been asked!')
+
+    def parse_options_short_options_test(self):
+        """
+        Test that parse_options parses short options correctly
+        """
+        # GIVEN: A list of vaild short options
+        options = ['-e', '-l', 'debug', '-pd', '-s', 'style', 'extra', 'qt', 'args']
+
+        # WHEN: Calling parse_options
+        resluts = parse_options(options)
+
+        # THEN: A tuple should be returned with the parsed options and left over args
+        self.assertEqual(resluts, (Values({'no_error_form': True, 'dev_version': True, 'portable': True,
+                                           'style': 'style', 'loglevel': 'debug'}), ['extra', 'qt', 'args']))