diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py index e48395000..cd8c26f7b 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 SafeConfigParser +from configparser import ConfigParser from PyQt4 import QtCore, QtGui @@ -68,7 +68,7 @@ class ThemeScreenshotThread(QtCore.QThread): filename = config.get('theme_%s' % theme, 'filename') screenshot = config.get('theme_%s' % theme, 'screenshot') urllib.request.urlretrieve('%s%s' % (self.parent().web, screenshot), - os.path.join(gettempdir(), 'openlp', screenshot)) + os.path.join(gettempdir(), 'openlp', screenshot)) item = QtGui.QListWidgetItem(title, self.parent().themes_list_widget) item.setData(QtCore.Qt.UserRole, filename) item.setCheckState(QtCore.Qt.Unchecked) @@ -90,14 +90,16 @@ class FirstTimeForm(QtGui.QWizard, Ui_FirstTimeWizard): self.screens = screens # check to see if we have web access self.web = 'http://openlp.org/files/frw/' - self.config = SafeConfigParser() - self.web_access = get_web_page('%s%s' % (self.web, 'download.cfg')) + 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.update_screen_list_combo() self.was_download_cancelled = False self.theme_screenshot_thread = None + self.has_run_wizard = False self.downloading = translate('OpenLP.FirstTimeWizard', 'Downloading %s...') self.cancel_button.clicked.connect(self.on_cancel_button_clicked) self.no_internet_finish_button.clicked.connect(self.on_no_internet_finish_button_clicked) diff --git a/openlp/core/utils/__init__.py b/openlp/core/utils/__init__.py index 7a11fe3b2..f19d88ba9 100644 --- a/openlp/core/utils/__init__.py +++ b/openlp/core/utils/__init__.py @@ -349,9 +349,9 @@ def get_web_page(url, header=None, update_openlp=False): if not url: return None req = urllib.request.Request(url) - user_agent = _get_user_agent() - log.debug('Using user agent: %s', user_agent) - req.add_header('User-Agent', user_agent) + if not header or header[0].lower() != 'user-agent': + user_agent = _get_user_agent() + req.add_header('User-Agent', user_agent) if header: req.add_header(header[0], header[1]) page = None diff --git a/tests/functional/openlp_core_utils/test_utils.py b/tests/functional/openlp_core_utils/test_utils.py index d5760ccb0..d9cdd9582 100644 --- a/tests/functional/openlp_core_utils/test_utils.py +++ b/tests/functional/openlp_core_utils/test_utils.py @@ -325,6 +325,7 @@ class TestUtils(TestCase): mocked_request_object.add_header.assert_called_with('User-Agent', 'user_agent') self.assertEqual(1, mocked_request_object.add_header.call_count, 'There should only be 1 call to add_header') + mock_get_user_agent.assert_called_with() mock_urlopen.assert_called_with(mocked_request_object) mocked_page_object.geturl.assert_called_with() self.assertEqual(0, MockRegistry.call_count, 'The Registry() object should have never been called') @@ -351,9 +352,38 @@ class TestUtils(TestCase): # THEN: The correct methods are called with the correct arguments and a web page is returned MockRequest.assert_called_with(fake_url) - mocked_request_object.add_header.assert_called_with('Fake-Header', 'fake value') + mocked_request_object.add_header.assert_called_with(fake_header[0], fake_header[1]) self.assertEqual(2, mocked_request_object.add_header.call_count, 'There should only be 2 calls to add_header') + mock_get_user_agent.assert_called_with() + mock_urlopen.assert_called_with(mocked_request_object) + mocked_page_object.geturl.assert_called_with() + self.assertEqual(mocked_page_object, returned_page, 'The returned page should be the mock object') + + def get_web_page_with_user_agent_in_headers_test(self): + """ + Test that adding a user agent in the header when calling get_web_page() adds that user agent to the request + """ + with patch('openlp.core.utils.urllib.request.Request') as MockRequest, \ + patch('openlp.core.utils.urllib.request.urlopen') as mock_urlopen, \ + patch('openlp.core.utils._get_user_agent') as mock_get_user_agent: + # GIVEN: Mocked out objects, a fake URL and a fake header + mocked_request_object = MagicMock() + MockRequest.return_value = mocked_request_object + mocked_page_object = MagicMock() + mock_urlopen.return_value = mocked_page_object + fake_url = 'this://is.a.fake/url' + user_agent_header = ('User-Agent', 'OpenLP/2.1.0') + + # WHEN: The get_web_page() method is called + returned_page = get_web_page(fake_url, header=user_agent_header) + + # THEN: The correct methods are called with the correct arguments and a web page is returned + MockRequest.assert_called_with(fake_url) + mocked_request_object.add_header.assert_called_with(user_agent_header[0], user_agent_header[1]) + self.assertEqual(1, mocked_request_object.add_header.call_count, + 'There should only be 1 call to add_header') + self.assertEqual(0, mock_get_user_agent.call_count, '_get_user_agent should not have been called') mock_urlopen.assert_called_with(mocked_request_object) mocked_page_object.geturl.assert_called_with() self.assertEqual(mocked_page_object, returned_page, 'The returned page should be the mock object') diff --git a/tests/functional/openlp_plugins/remotes/test_router.py b/tests/functional/openlp_plugins/remotes/test_router.py index f0af48afd..b65499c64 100644 --- a/tests/functional/openlp_plugins/remotes/test_router.py +++ b/tests/functional/openlp_plugins/remotes/test_router.py @@ -168,7 +168,7 @@ class TestRouter(TestCase): # WHEN: call serve_file with an existing html file self.router.serve_file(os.path.normpath('test/dir/test.html')) - + # THEN: it should return a 200 and the file self.router.send_response.assert_called_once_with(200) self.router.send_header.assert_called_once_with('Content-type', 'text/html')