diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py index 8a1c29dba..20a18cb82 100644 --- a/openlp/core/ui/firsttimeform.py +++ b/openlp/core/ui/firsttimeform.py @@ -37,7 +37,7 @@ import urllib.request import urllib.parse import urllib.error from tempfile import gettempdir -from configparser import ConfigParser +from configparser import ConfigParser, MissingSectionHeaderError, NoSectionError, NoOptionError from PyQt4 import QtCore, QtGui @@ -131,17 +131,23 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties): """ self.screens = screens # check to see if we have web access + self.web_access = False self.web = 'http://openlp.org/files/frw/' self.config = ConfigParser() user_agent = 'OpenLP/' + Registry().get('application').applicationVersion() - self.web_access = get_web_page('%s%s' % (self.web, 'download.cfg'), header=('User-Agent', user_agent)) - if self.web_access: - files = self.web_access.read() - self.config.read_string(files.decode()) - self.web = self.config.get('general', 'base url') - self.songs_url = self.web + self.config.get('songs', 'directory') + '/' - self.bibles_url = self.web + self.config.get('bibles', 'directory') + '/' - self.themes_url = self.web + self.config.get('themes', 'directory') + '/' + web_config = get_web_page('%s%s' % (self.web, 'download.cfg'), header=('User-Agent', user_agent)) + if web_config: + files = web_config.read() + try: + self.config.read_string(files.decode()) + self.web = self.config.get('general', 'base url') + self.songs_url = self.web + self.config.get('songs', 'directory') + '/' + self.bibles_url = self.web + self.config.get('bibles', 'directory') + '/' + self.themes_url = self.web + self.config.get('themes', 'directory') + '/' + self.web_access = True + except (NoSectionError, NoOptionError, MissingSectionHeaderError): + log.debug('A problem occured while parsing the downloaded config file') + trace_error_handler(log) self.update_screen_list_combo() self.was_download_cancelled = False self.theme_screenshot_thread = None @@ -295,7 +301,7 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties): block_count += 1 self._download_progress(block_count, block_size) filename.close() - except (ConnectionError, IOError): + except ConnectionError: trace_error_handler(log) filename.close() os.remove(f_path) @@ -381,7 +387,7 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties): filename = item.data(QtCore.Qt.UserRole) size = self._get_file_size('%s%s' % (self.themes_url, filename)) self.max_progress += size - except (ConnectionError, IOError): + except ConnectionError: trace_error_handler(log) critical_error_message_box(translate('OpenLP.FirstTimeWizard', 'Download Error'), translate('OpenLP.FirstTimeWizard', 'There was a connection problem during ' diff --git a/openlp/core/utils/__init__.py b/openlp/core/utils/__init__.py index 128c09b38..8f032cd7d 100644 --- a/openlp/core/utils/__init__.py +++ b/openlp/core/utils/__init__.py @@ -390,9 +390,9 @@ def get_web_page(url, header=None, update_openlp=False): page = None log.debug('Downloading URL = %s' % url) try: - page = urllib.request.urlopen(req) + page = urllib.request.urlopen(req, timeout=30) log.debug('Downloaded URL = %s' % page.geturl()) - except urllib.error.URLError: + except (urllib.error.URLError, ConnectionError): log.exception('The web page could not be downloaded') if not page: return None diff --git a/tests/functional/openlp_core_ui/test_firsttimeform.py b/tests/functional/openlp_core_ui/test_firsttimeform.py index ed7a7a9e8..ead9ef4cf 100644 --- a/tests/functional/openlp_core_ui/test_firsttimeform.py +++ b/tests/functional/openlp_core_ui/test_firsttimeform.py @@ -49,6 +49,22 @@ directory = bibles directory = themes """ +FAKE_BROKEN_CONFIG = b""" +[general] +base url = http://example.com/frw/ +[songs] +directory = songs +[bibles] +directory = bibles +""" + +FAKE_INVALID_CONFIG = b""" + +This is not a config file +Some text + +""" + class TestFirstTimeForm(TestCase, TestMixin): @@ -104,3 +120,33 @@ class TestFirstTimeForm(TestCase, TestMixin): self.assertEqual(expected_songs_url, first_time_form.songs_url, 'The songs URL should be correct') self.assertEqual(expected_bibles_url, first_time_form.bibles_url, 'The bibles URL should be correct') self.assertEqual(expected_themes_url, first_time_form.themes_url, 'The themes URL should be correct') + + def broken_config_test(self): + """ + Test if we can handle an config file with missing data + """ + # GIVEN: A mocked get_web_page, a First Time Wizard, an expected screen object, and a mocked broken config file + with patch('openlp.core.ui.firsttimeform.get_web_page') as mocked_get_web_page: + first_time_form = FirstTimeForm(None) + mocked_get_web_page.return_value.read.return_value = FAKE_BROKEN_CONFIG + + # WHEN: The First Time Wizard is initialised + first_time_form.initialize(MagicMock()) + + # THEN: The First Time Form should not have web access + self.assertFalse(first_time_form.web_access, 'There should not be web access with a broken config file') + + def invalid_config_test(self): + """ + Test if we can handle an config file in invalid format + """ + # GIVEN: A mocked get_web_page, a First Time Wizard, an expected screen object, and a mocked invalid config file + with patch('openlp.core.ui.firsttimeform.get_web_page') as mocked_get_web_page: + first_time_form = FirstTimeForm(None) + mocked_get_web_page.return_value.read.return_value = FAKE_INVALID_CONFIG + + # WHEN: The First Time Wizard is initialised + first_time_form.initialize(MagicMock()) + + # THEN: The First Time Form should not have web access + self.assertFalse(first_time_form.web_access, 'There should not be web access with an invalid config file')