Fixes Bug 1209515 by sub classing QFileDialog and reimplementing getOpenFileNames and attempting to urlunquote and file paths which do not exist

bzr-revno: 2314

openlp/core/common/uistrings.py

@@ -83,6 +83,8 @@ class UiStrings(object):
         self.Error = translate('OpenLP.Ui', 'Error')
         self.Export = translate('OpenLP.Ui', 'Export')
         self.File = translate('OpenLP.Ui', 'File')
+        self.FileNotFound = translate('OpenLP.Ui', 'File Not Found')
+        self.FileNotFoundMessage = translate('OpenLP.Ui', 'File %s not found.\nPlease try selecting it individually.')
         self.FontSizePtUnit = translate('OpenLP.Ui', 'pt', 'Abbreviated font pointsize unit')
         self.Help = translate('OpenLP.Ui', 'Help')
         self.Hours = translate('OpenLP.Ui', 'h', 'The abbreviated unit for hours')

openlp/core/lib/__init__.py

@@ -330,6 +330,7 @@ def create_separated_list(string_list):
 
 
 from .registry import Registry
+from .filedialog import FileDialog
 from .screen import ScreenList
 from .listwidgetwithdnd import ListWidgetWithDnD
 from .treewidgetwithdnd import TreeWidgetWithDnD
@@ -345,4 +346,3 @@ from .dockwidget import OpenLPDockWidget
 from .imagemanager import ImageManager
 from .renderer import Renderer
 from .mediamanageritem import MediaManagerItem
-

openlp/core/lib/filedialog.py

@@ -0,0 +1,66 @@
+# -*- coding: utf-8 -*-
+# vim: autoindent shiftwidth=4 expandtab textwidth=80 tabstop=4 softtabstop=4
+
+###############################################################################
+# OpenLP - Open Source Lyrics Projection                                      #
+# --------------------------------------------------------------------------- #
+# Copyright (c) 2008-2013 Raoul Snyman                                        #
+# Portions copyright (c) 2008-2013 Tim Bentley, Gerald Britton, Jonathan      #
+# Corwin, Samuel Findlay, Michael Gorven, Scott Guerrieri, Matthias Hub,      #
+# Meinert Jordan, Armin Köhler, Erik Lundin, Edwin Lunando, Brian T. Meyer.   #
+# Joshua Miller, Stevan Pettit, Andreas Preikschat, Mattias Põldaru,          #
+# Christian Richter, Philip Ridout, Simon Scudder, Jeffrey Smith,             #
+# Maikel Stuivenberg, Martin Thompson, Jon Tibble, Dave Warnock,              #
+# Frode Woldsund, Martin Zibricky                                             #
+# --------------------------------------------------------------------------- #
+# This program is free software; you can redistribute it and/or modify it     #
+# under the terms of the GNU General Public License as published by the Free  #
+# Software Foundation; version 2 of the License.                              #
+#                                                                             #
+# This program is distributed in the hope that it will be useful, but WITHOUT #
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or       #
+# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for    #
+# more details.                                                               #
+#                                                                             #
+# You should have received a copy of the GNU General Public License along     #
+# with this program; if not, write to the Free Software Foundation, Inc., 59  #
+# Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
+###############################################################################
+
+"""
+Provide a work around for a bug in QFileDialog <https://bugs.launchpad.net/openlp/+bug/1209515>
+"""
+import logging
+import os
+from urllib import parse
+
+from PyQt4 import QtGui
+
+from openlp.core.common import UiStrings
+
+log = logging.getLogger(__name__)
+
+class FileDialog(QtGui.QFileDialog):
+    """
+    Subclass QFileDialog to work round a bug
+    """
+    @staticmethod
+    def getOpenFileNames(parent, *args, **kwargs):
+        """
+        Reimplement getOpenFileNames to fix the way it returns some file names that url encoded when selecting multiple
+        files
+        """
+        files = QtGui.QFileDialog.getOpenFileNames(parent, *args, **kwargs)
+        file_list = []
+        for file in files:
+            if not os.path.exists(file):
+                log.info('File %s not found. Attempting to unquote.')
+                file = parse.unquote(file)
+                if not os.path.exists(file):
+                    log.error('File %s not found.' % file)
+                    QtGui.QMessageBox.information(parent, UiStrings().FileNotFound,
+                        UiStrings().FileNotFoundMessage % file)
+                    continue
+                log.info('File %s found.')
+            file_list.append(file)
+        return file_list

openlp/core/lib/mediamanageritem.py

@@ -36,7 +36,7 @@ import re
 from PyQt4 import QtCore, QtGui
 
 from openlp.core.common import Settings, UiStrings, translate
-from openlp.core.lib import OpenLPToolbar, ServiceItem, StringContent, ListWidgetWithDnD, \
+from openlp.core.lib import FileDialog, OpenLPToolbar, ServiceItem, StringContent, ListWidgetWithDnD, \
     ServiceItemContext, Registry
 from openlp.core.lib.searchedit import SearchEdit
 from openlp.core.lib.ui import create_widget_action, critical_error_message_box
@@ -319,7 +319,7 @@ class MediaManagerItem(QtGui.QWidget):
         """
         Add a file to the list widget to make it available for showing
         """
-        files = QtGui.QFileDialog.getOpenFileNames(self, self.on_new_prompt,
+        files = FileDialog.getOpenFileNames(self, self.on_new_prompt,
             Settings().value(self.settings_section + '/last directory'), self.on_new_file_masks)
         log.info('New files(s) %s', files)
         if files:

openlp/core/ui/thememanager.py

@@ -39,7 +39,7 @@ from xml.etree.ElementTree import ElementTree, XML
 from PyQt4 import QtCore, QtGui
 
 from openlp.core.common import AppLocation, Settings, check_directory_exists, UiStrings, translate
-from openlp.core.lib import ImageSource, OpenLPToolbar, Registry, get_text_file_string, build_icon, \
+from openlp.core.lib import FileDialog, ImageSource, OpenLPToolbar, Registry, get_text_file_string, build_icon, \
     check_item_selected, create_thumb, validate_thumb
 from openlp.core.lib.theme import ThemeXML, BackgroundType
 from openlp.core.lib.ui import critical_error_message_box, create_widget_action
@@ -374,7 +374,7 @@ class ThemeManager(QtGui.QWidget, ThemeManagerHelper):
         Opens a file dialog to select the theme file(s) to import before attempting to extract OpenLP themes from
         those files. This process will load both OpenLP version 1 and version 2 themes.
         """
-        files = QtGui.QFileDialog.getOpenFileNames(self,
+        files = FileDialog.getOpenFileNames(self,
             translate('OpenLP.ThemeManager', 'Select Theme Import File'),
             Settings().value(self.settings_section + '/last directory import'),
             translate('OpenLP.ThemeManager', 'OpenLP Themes (*.theme *.otz)'))

openlp/plugins/songs/forms/editsongform.py

@@ -39,7 +39,7 @@ import shutil
 from PyQt4 import QtCore, QtGui
 
 from openlp.core.common import AppLocation, UiStrings, check_directory_exists, translate
-from openlp.core.lib import Registry, PluginStatus, MediaType, create_separated_list
+from openlp.core.lib import FileDialog, Registry, PluginStatus, MediaType, create_separated_list
 from openlp.core.lib.ui import set_case_insensitive_completer, critical_error_message_box, find_and_set_in_combo_box
 from openlp.plugins.songs.lib import VerseType, clean_song
 from openlp.plugins.songs.lib.db import Book, Song, Author, Topic, MediaFile
@@ -758,7 +758,7 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog):
         Loads file(s) from the filesystem.
         """
         filters = '%s (*)' % UiStrings().AllFiles
-        filenames = QtGui.QFileDialog.getOpenFileNames(self,
+        filenames = FileDialog.getOpenFileNames(self,
             translate('SongsPlugin.EditSongForm', 'Open File(s)'), '', filters)
         for filename in filenames:
             item = QtGui.QListWidgetItem(os.path.split(str(filename))[1])

openlp/plugins/songs/forms/songimportform.py

@@ -37,7 +37,7 @@ from PyQt4 import QtCore, QtGui
 
 from openlp.core.common import UiStrings, translate
 from openlp.core.common import Settings
-from openlp.core.lib import Registry
+from openlp.core.lib import FileDialog, Registry
 from openlp.core.lib.ui import critical_error_message_box
 from openlp.core.ui.wizard import OpenLPWizard, WizardStrings
 from openlp.plugins.songs.lib.importer import SongFormat, SongFormatSelect
@@ -246,7 +246,7 @@ class SongImportForm(OpenLPWizard):
         if filters:
             filters += ';;'
         filters += '%s (*)' % UiStrings().AllFiles
-        filenames = QtGui.QFileDialog.getOpenFileNames(self, title,
+        filenames = FileDialog.getOpenFileNames(self, title,
             Settings().value(self.plugin.settings_section + '/last directory import'), filters)
         if filenames:
             listbox.addItems(filenames)

tests/functional/openlp_core_lib/test_file_dialog.py

@@ -0,0 +1,73 @@
+"""
+Package to test the openlp.core.lib.filedialog package.
+"""
+from unittest import TestCase
+
+from openlp.core.common import UiStrings
+from openlp.core.lib.filedialog import FileDialog
+from tests.functional import MagicMock, patch
+
+class TestFileDialog(TestCase):
+    """
+    Test the functions in the :mod:`filedialog` module.
+    """
+    def setUp(self):
+        self.os_patcher = patch('openlp.core.lib.filedialog.os')
+        self.qt_gui_patcher = patch('openlp.core.lib.filedialog.QtGui')
+        self.ui_strings_patcher = patch('openlp.core.lib.filedialog.UiStrings')
+        self.mocked_os = self.os_patcher.start()
+        self.mocked_qt_gui = self.qt_gui_patcher.start()
+        self.mocked_ui_strings = self.ui_strings_patcher.start()
+        self.mocked_parent = MagicMock()
+
+    def tearDown(self):
+        self.os_patcher.stop()
+        self.qt_gui_patcher.stop()
+        self.ui_strings_patcher.stop()
+
+    def get_open_file_names_canceled_test(self):
+        """
+            Test that FileDialog.getOpenFileNames() returns and empty QStringList when QFileDialog is canceled
+            (returns an empty QStringList)
+        """
+        self.mocked_os.reset()
+
+        # GIVEN: An empty QStringList as a return value from QFileDialog.getOpenFileNames
+        self.mocked_qt_gui.QFileDialog.getOpenFileNames.return_value = []
+
+        # WHEN: FileDialog.getOpenFileNames is called
+        result = FileDialog.getOpenFileNames(self.mocked_parent)
+
+        # THEN: The returned value should be an empty QStringList and os.path.exists should not have been called
+        assert not self.mocked_os.path.exists.called
+        self.assertEqual(result, [],
+            'FileDialog.getOpenFileNames should return and empty list when QFileDialog.getOpenFileNames is canceled')
+
+    def returned_file_list_test(self):
+        """
+            Test that FileDialog.getOpenFileNames handles a list of files properly when QFileList.getOpenFileNames
+            returns a good file name, a urlencoded file name and a non-existing file
+        """
+        self.mocked_os.rest()
+        self.mocked_qt_gui.reset()
+
+        # GIVEN: A List of known values as a return value from QFileDialog.getOpenFileNames and a list of valid
+        #		file names.
+        self.mocked_qt_gui.QFileDialog.getOpenFileNames.return_value = [
+            '/Valid File', '/url%20encoded%20file%20%231', '/non-existing']
+        self.mocked_os.path.exists.side_effect = lambda file_name: file_name in [
+            '/Valid File', '/url encoded file #1']
+
+        # WHEN: FileDialog.getOpenFileNames is called
+        result = FileDialog.getOpenFileNames(self.mocked_parent)
+
+        # THEN: os.path.exists should have been called with known args. QmessageBox.information should have been
+        #       called. The returned result should corrilate with the input.
+        self.mocked_os.path.exists.assert_callde_with('/Valid File')
+        self.mocked_os.path.exists.assert_callde_with('/url%20encoded%20file%20%231')
+        self.mocked_os.path.exists.assert_callde_with('/url encoded file #1')
+        self.mocked_os.path.exists.assert_callde_with('/non-existing')
+        self.mocked_os.path.exists.assert_callde_with('/non-existing')
+        self.mocked_qt_gui.QmessageBox.information.called_with(self.mocked_parent, UiStrings().FileNotFound,
+            UiStrings().FileNotFoundMessage % '/non-existing')
+        self.assertEqual(result, ['/Valid File', '/url encoded file #1'], 'The returned file list is incorrect')

tests/functional/openlp_core_lib/test_htmlbuilder.py

@@ -3,13 +3,13 @@ Package to test the openlp.core.lib.htmlbuilder module.
 """
 
 from unittest import TestCase
-from mock import MagicMock, patch
 
 from PyQt4 import QtCore
 
 from openlp.core.lib.htmlbuilder import build_html, build_background_css, build_lyrics_css, build_lyrics_outline_css, \
     build_lyrics_format_css, build_footer_css
 from openlp.core.lib.theme import HorizontalType, VerticalType
+from tests.functional import MagicMock, patch
 
 
 HTML = """