openlp-core team mailing list archive

[Tomas Groth <tomasgroth@xxxxxxxx>]

Tomas Groth has proposed merging lp:~tomasgroth/openlp/bugfixes22 into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1464421 in OpenLP: "Connection errors in SongSelect are not handled"
  https://bugs.launchpad.net/openlp/+bug/1464421
  Bug #1465390 in OpenLP: "Traceback when going from blanked presentation to video"
  https://bugs.launchpad.net/openlp/+bug/1465390

For more details, see:
https://code.launchpad.net/~tomasgroth/openlp/bugfixes22/+merge/262904

Check mediaplayer is loaded before trying to use it when blanking. Fixes bug 1465390.
Catch songselect connections error. Fixes bug 1464421.
-- 
Your team OpenLP Core is requested to review the proposed merge of lp:~tomasgroth/openlp/bugfixes22 into lp:openlp.

=== modified file 'openlp/core/ui/media/mediacontroller.py'
--- openlp/core/ui/media/mediacontroller.py	2015-05-28 20:38:43 +0000
+++ openlp/core/ui/media/mediacontroller.py	2015-06-24 20:31:00 +0000
@@ -721,8 +721,8 @@
         display = self._define_display(self.live_controller)
         if self.live_controller.controller_type in self.current_media_players and \
                 self.current_media_players[self.live_controller.controller_type].state == MediaState.Playing:
-                self.current_media_players[self.live_controller.controller_type].pause(display)
-                self.current_media_players[self.live_controller.controller_type].set_visible(display, False)
+            self.current_media_players[self.live_controller.controller_type].pause(display)
+            self.current_media_players[self.live_controller.controller_type].set_visible(display, False)
 
     def media_blank(self, msg):
         """
@@ -737,7 +737,8 @@
             return
         Registry().execute('live_display_hide', hide_mode)
         display = self._define_display(self.live_controller)
-        if self.current_media_players[self.live_controller.controller_type].state == MediaState.Playing:
+        if self.live_controller.controller_type in self.current_media_players and \
+                self.current_media_players[self.live_controller.controller_type].state == MediaState.Playing:
             self.current_media_players[self.live_controller.controller_type].pause(display)
             self.current_media_players[self.live_controller.controller_type].set_visible(display, False)
 

=== modified file 'openlp/plugins/presentations/lib/powerpointcontroller.py'
--- openlp/plugins/presentations/lib/powerpointcontroller.py	2015-06-05 21:22:16 +0000
+++ openlp/plugins/presentations/lib/powerpointcontroller.py	2015-06-24 20:31:00 +0000
@@ -33,7 +33,6 @@
 
 if is_win():
     from win32com.client import Dispatch
-    import win32com
     import win32con
     import winreg
     import win32ui

=== modified file 'openlp/plugins/songs/lib/songselect.py'
--- openlp/plugins/songs/lib/songselect.py	2015-01-18 13:39:21 +0000
+++ openlp/plugins/songs/lib/songselect.py	2015-06-24 20:31:00 +0000
@@ -25,8 +25,10 @@
 import logging
 from http.cookiejar import CookieJar
 from urllib.parse import urlencode
-from urllib.request import HTTPCookieProcessor, HTTPError, build_opener
+from urllib.request import HTTPCookieProcessor, URLError, build_opener
 from html.parser import HTMLParser
+from html import unescape
+
 
 from bs4 import BeautifulSoup, NavigableString
 
@@ -72,7 +74,11 @@
         """
         if callback:
             callback()
-        login_page = BeautifulSoup(self.opener.open(LOGIN_URL).read(), 'lxml')
+        try:
+            login_page = BeautifulSoup(self.opener.open(LOGIN_URL).read(), 'lxml')
+        except (TypeError, URLError) as e:
+            log.exception('Could not login to SongSelect, %s', e)
+            return False
         if callback:
             callback()
         token_input = login_page.find('input', attrs={'name': '__RequestVerificationToken'})
@@ -82,7 +88,11 @@
             'Password': password,
             'RememberMe': 'false'
         })
-        posted_page = BeautifulSoup(self.opener.open(LOGIN_URL, data.encode('utf-8')).read(), 'lxml')
+        try:
+            posted_page = BeautifulSoup(self.opener.open(LOGIN_URL, data.encode('utf-8')).read(), 'lxml')
+        except (TypeError, URLError) as e:
+            log.exception('Could not login to SongSelect, %s', e)
+            return False
         if callback:
             callback()
         return not posted_page.find('input', attrs={'name': '__RequestVerificationToken'})
@@ -91,7 +101,10 @@
         """
         Log the user out of SongSelect
         """
-        self.opener.open(LOGOUT_URL)
+        try:
+            self.opener.open(LOGOUT_URL)
+        except (TypeError, URLError) as e:
+            log.exception('Could not log of SongSelect, %s', e)
 
     def search(self, search_text, max_results, callback=None):
         """
@@ -108,14 +121,18 @@
         while True:
             if current_page > 1:
                 params['page'] = current_page
-            results_page = BeautifulSoup(self.opener.open(SEARCH_URL + '?' + urlencode(params)).read(), 'lxml')
-            search_results = results_page.find_all('li', 'result pane')
+            try:
+                results_page = BeautifulSoup(self.opener.open(SEARCH_URL + '?' + urlencode(params)).read(), 'lxml')
+                search_results = results_page.find_all('li', 'result pane')
+            except (TypeError, URLError) as e:
+                log.exception('Could not search SongSelect, %s', e)
+                search_results = None
             if not search_results:
                 break
             for result in search_results:
                 song = {
-                    'title': self.html_parser.unescape(result.find('h3').string),
-                    'authors': [self.html_parser.unescape(author.string) for author in result.find_all('li')],
+                    'title': unescape(result.find('h3').string),
+                    'authors': [unescape(author.string) for author in result.find_all('li')],
                     'link': BASE_URL + result.find('a')['href']
                 }
                 if callback:
@@ -138,20 +155,20 @@
             callback()
         try:
             song_page = BeautifulSoup(self.opener.open(song['link']).read(), 'lxml')
-        except (TypeError, HTTPError) as e:
+        except (TypeError, URLError) as e:
             log.exception('Could not get song from SongSelect, %s', e)
             return None
         if callback:
             callback()
         try:
             lyrics_page = BeautifulSoup(self.opener.open(song['link'] + '/lyrics').read(), 'lxml')
-        except (TypeError, HTTPError):
+        except (TypeError, URLError):
             log.exception('Could not get lyrics from SongSelect')
             return None
         if callback:
             callback()
         song['copyright'] = '/'.join([li.string for li in song_page.find('ul', 'copyright').find_all('li')])
-        song['copyright'] = self.html_parser.unescape(song['copyright'])
+        song['copyright'] = unescape(song['copyright'])
         song['ccli_number'] = song_page.find('ul', 'info').find('li').string.split(':')[1].strip()
         song['verses'] = []
         verses = lyrics_page.find('section', 'lyrics').find_all('p')
@@ -164,9 +181,9 @@
                 else:
                     verse['lyrics'] += '\n'
             verse['lyrics'] = verse['lyrics'].strip(' \n\r\t')
-            song['verses'].append(self.html_parser.unescape(verse))
+            song['verses'].append(unescape(verse))
         for counter, author in enumerate(song['authors']):
-            song['authors'][counter] = self.html_parser.unescape(author)
+            song['authors'][counter] = unescape(author)
         return song
 
     def save_song(self, song):

=== modified file 'tests/functional/openlp_plugins/songs/test_songselect.py'
--- tests/functional/openlp_plugins/songs/test_songselect.py	2015-03-31 20:58:51 +0000
+++ tests/functional/openlp_plugins/songs/test_songselect.py	2015-06-24 20:31:00 +0000
@@ -82,6 +82,23 @@
         self.assertFalse(result, 'The login method should have returned False')
 
     @patch('openlp.plugins.songs.lib.songselect.build_opener')
+    def login_except_test(self,  mocked_build_opener):
+        """
+        Test that when logging in to SongSelect fails, the login method raises URLError
+        """
+        # GIVEN: A bunch of mocked out stuff and an importer object
+        mocked_build_opener.open.side_effect = URLError('Fake URLError')
+        mock_callback = MagicMock()
+        importer = SongSelectImport(None)
+
+        # WHEN: The login method is called after being rigged to fail
+        result = importer.login('username', 'password', mock_callback)
+
+        # THEN: callback was called 1 time and False was returned
+        self.assertEqual(1, mock_callback.call_count, 'callback should have been called 1 times')
+        self.assertFalse(result, 'The login method should have returned False')
+
+    @patch('openlp.plugins.songs.lib.songselect.build_opener')
     @patch('openlp.plugins.songs.lib.songselect.BeautifulSoup')
     def login_succeeds_test(self, MockedBeautifulSoup, mocked_build_opener):
         """