forked from openlp/openlp
Made the FTW able to handle if the downloaded config file is invalid.
Fixes: https://launchpad.net/bugs/1222944
This commit is contained in:
parent
529f94c504
commit
48c561e34b
@ -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 '
|
||||
|
@ -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
|
||||
|
@ -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"""
|
||||
<html>
|
||||
<head><title>This is not a config file</title></head>
|
||||
<body>Some text</body>
|
||||
</html>
|
||||
"""
|
||||
|
||||
|
||||
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')
|
||||
|
Loading…
Reference in New Issue
Block a user