- Specify OpenLP as the user agent when querying SF.net

- Fix another test
- Update tests for get_web_page

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.config = ConfigParser()
-        self.web_access = get_web_page('%s%s' % (self.web, 'download.cfg'))
+        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)

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()
+    if not header or header[0].lower() != 'user-agent':
-    log.debug('Using user agent: %s', user_agent)
+        user_agent = _get_user_agent()
-    req.add_header('User-Agent', user_agent)
+        req.add_header('User-Agent', user_agent)
     if header:
         req.add_header(header[0], header[1])
     page = None

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')

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')