Merge lp:~phill-ridout/openlp/bug1367141-2 into lp:openlp

[Phill]

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.

Add this to your merge proposal:
--------------------------------
lp:~phill-ridout/openlp/bug1367141-2 (revision 2486)
[SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/906/
[SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/837/
[SUCCESS] http://ci.openlp.org/job/Branch-03-Interface-Tests/783/
[SUCCESS] http://ci.openlp.org/job/Branch-04a-Windows_Functional_Tests/694/
[SUCCESS] http://ci.openlp.org/job/Branch-04b-Windows_Interface_Tests/293/
[SUCCESS] http://ci.openlp.org/job/Branch-05a-Code_Analysis/435/
[SUCCESS] http://ci.openlp.org/job/Branch-05b-Test_Coverage/306/

Process finished with exit code 0

[Raoul Snyman (raoul-snyman)]

No print statements!

review: Needs Fixing

[Raoul Snyman (raoul-snyman)]

review: Approve

[Tim Bentley (trb143)]

review: Approve

=== modified file 'openlp/core/__init__.py'
--- openlp/core/__init__.py	2015-01-18 13:39:21 +0000
+++ openlp/core/__init__.py	2015-01-24 10:31:06 +0000
@@ -263,6 +263,29 @@
             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 @@
 
     :param args: Some args
     """
-    # 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.
-    (options, args) = parser.parse_args(args) if args else parser.parse_args()
+    (options, args) = parse_options(args)
     qt_args = []
     if options.loglevel.lower() in ['d', 'debug']:
         log.setLevel(logging.DEBUG)

=== modified file 'openlp/core/lib/mediamanageritem.py'
--- openlp/core/lib/mediamanageritem.py	2015-01-18 13:39:21 +0000
+++ openlp/core/lib/mediamanageritem.py	2015-01-24 10:31:06 +0000
@@ -366,8 +366,7 @@
         duplicates_found = False
         files_added = False
         for file_path in files:
-            filename = os.path.split(str(file_path))[1]
-            if filename in names:
+            if file_path in full_list:
                 duplicates_found = True
             else:
                 files_added = True

=== modified file 'openlp/plugins/images/imageplugin.py'
--- openlp/plugins/images/imageplugin.py	2015-01-18 13:39:21 +0000
+++ openlp/plugins/images/imageplugin.py	2015-01-24 10:31:06 +0000
@@ -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 @@
         """
         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 @@
 
         :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)

=== modified file 'openlp/plugins/images/lib/mediaitem.py'
--- openlp/plugins/images/lib/mediaitem.py	2015-01-19 22:49:18 +0000
+++ openlp/plugins/images/lib/mediaitem.py	2015-01-24 10:31:06 +0000
@@ -338,7 +338,8 @@
         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:

=== modified file 'openlp/plugins/presentations/lib/mediaitem.py'
--- openlp/plugins/presentations/lib/mediaitem.py	2015-01-18 13:39:21 +0000
+++ openlp/plugins/presentations/lib/mediaitem.py	2015-01-24 10:31:06 +0000
@@ -222,10 +222,7 @@
             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:
-                    doc = self.controllers[cidx].add_document(filepath)
-                    doc.presentation_deleted()
-                    doc.close_presentation()
+                self.clean_up_thumbnails(filepath)
                 self.main_window.increment_progress_bar()
             self.main_window.finished_progress_bar()
             self.application.set_busy_cursor()
@@ -233,6 +230,18 @@
                 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):
         """

=== modified file 'openlp/plugins/presentations/lib/presentationcontroller.py'
--- openlp/plugins/presentations/lib/presentationcontroller.py	2015-01-18 13:39:21 +0000
+++ openlp/plugins/presentations/lib/presentationcontroller.py	2015-01-24 10:31:06 +0000
@@ -26,7 +26,7 @@
 
 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 @@
         """
         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):
         """

=== modified file 'openlp/plugins/presentations/presentationplugin.py'
--- openlp/plugins/presentations/presentationplugin.py	2015-01-18 13:39:21 +0000
+++ openlp/plugins/presentations/presentationplugin.py	2015-01-24 10:31:06 +0000
@@ -28,7 +28,7 @@
 
 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 @@
                         '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 @@
             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.

=== modified file 'tests/functional/openlp_plugins/presentations/test_impresscontroller.py'
--- tests/functional/openlp_plugins/presentations/test_impresscontroller.py	2015-01-18 13:39:21 +0000
+++ tests/functional/openlp_plugins/presentations/test_impresscontroller.py	2015-01-24 10:31:06 +0000
@@ -31,8 +31,10 @@
 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 @@
     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)

=== modified file 'tests/functional/openlp_plugins/presentations/test_pdfcontroller.py'
--- tests/functional/openlp_plugins/presentations/test_pdfcontroller.py	2015-01-18 13:39:21 +0000
+++ tests/functional/openlp_plugins/presentations/test_pdfcontroller.py	2015-01-24 10:31:06 +0000
@@ -36,7 +36,8 @@
 from tests.helpers.testmixin import TestMixin
 
 __default_settings__ = {
-    'presentations/enable_pdf_program': False
+    'presentations/enable_pdf_program': False,
+    'presentations/thumbnail_scheme': ''
 }
 
 SCREEN = {

=== modified file 'tests/functional/test_init.py'
--- tests/functional/test_init.py	2015-01-18 13:39:21 +0000
+++ tests/functional/test_init.py	2015-01-24 10:31:06 +0000
@@ -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 @@
             # 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']))