Fix web api issue and crash with small theme

Potentially fixes some other issues we didn't know about too...
This just stops anything interfering with creating slides,
and stops fetching the slides halfway through processing

openlp/core/lib/serviceitem.py

@@ -25,7 +25,6 @@ import datetime
 import logging
 import ntpath
 import os
-import urllib.request
 import uuid
 from copy import deepcopy
 from pathlib import Path
@@ -39,9 +38,9 @@ from openlp.core.common.enum import ServiceItemType
 from openlp.core.common.i18n import translate
 from openlp.core.common.mixins import RegistryProperties
 from openlp.core.common.registry import Registry
+from openlp.core.common.utils import wait_for
 from openlp.core.display.render import remove_tags, render_tags, render_chords_for_printing
-from openlp.core.lib import ItemCapabilities
+from openlp.core.lib import create_thumb, image_to_data_uri, ItemCapabilities
-from openlp.core.lib import create_thumb
 from openlp.core.lib.theme import BackgroundType, TransitionSpeed
 from openlp.core.state import State
 from openlp.core.ui.icons import UiIcons
@@ -70,6 +69,7 @@ class ServiceItem(RegistryProperties):
         self._rendered_slides = None
         self._display_slides = None
         self._print_slides = None
+        self._creating_slides = False
         self.title = ''
         self.slides = []
         self.processor = None
@@ -202,6 +202,8 @@ class ServiceItem(RegistryProperties):
         """
         Create frames for rendering and display
         """
+        wait_for(lambda: not self._creating_slides)
+        self._creating_slides = True
         self._rendered_slides = []
         self._display_slides = []
 
@@ -234,12 +236,14 @@ class ServiceItem(RegistryProperties):
                 }
                 self._display_slides.append(display_slide)
                 index += 1
+        self._creating_slides = False
 
     @property
     def rendered_slides(self):
         """
         Render the frames and return them
         """
+        wait_for(lambda: not self._creating_slides)
         if not self._rendered_slides:
             self._create_slides()
         return self._rendered_slides
@@ -249,6 +253,7 @@ class ServiceItem(RegistryProperties):
         """
         Render the frames and return them
         """
+        wait_for(lambda: not self._creating_slides)
         if not self._display_slides:
             self._create_slides()
         return self._display_slides
@@ -868,6 +873,7 @@ class ServiceItem(RegistryProperties):
     def to_dict(self):
         """
         Convert the service item into a dictionary
+        Images and thumbnails are put in dict as data uri strings.
         """
         data_dict = {
             'title': self.title,
@@ -902,7 +908,7 @@ class ServiceItem(RegistryProperties):
                 full_thumbnail_path = AppLocation.get_data_path() / thumbnail_path
                 if not full_thumbnail_path.exists():
                     create_thumb(Path(self.get_frame_path(index)), full_thumbnail_path, False)
-                item['img'] = urllib.request.pathname2url(os.path.sep + str(thumbnail_path))
+                item['img'] = image_to_data_uri(full_thumbnail_path)
                 item['text'] = str(frame['title'])
                 item['html'] = str(frame['title'])
             else:
@@ -921,7 +927,7 @@ class ServiceItem(RegistryProperties):
                         log.warning('Service item "{title}" is missing a thumbnail or has an invalid thumbnail path'
                                     .format(title=self.title))
                     else:
-                        item['img'] = urllib.request.pathname2url(os.path.sep + str(relative_file))
+                        item['img'] = image_to_data_uri(AppLocation.get_data_path() / relative_file)
                 item['text'] = str(frame['title'])
                 item['html'] = str(frame['title'])
             data_dict['slides'].append(item)

openlp/core/ui/thememanager.py

@@ -738,6 +738,8 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
         for theme_name in theme_name_list:
             theme_data = self._get_theme_data(theme_name)
             preview_pixmap = self.progress_form.get_preview(theme_name, theme_data)
+            if preview_pixmap is None:
+                break
             self.save_preview(theme_name, preview_pixmap)
         self.progress_form.close()
         self.load_themes()

openlp/core/ui/themeprogressform.py

@@ -49,11 +49,20 @@ class ThemeProgressForm(QtWidgets.QDialog, UiThemeProgressDialog, RegistryProper
         return super().show()
 
     def get_preview(self, theme_name, theme_data):
+        """
+        Fetch a screenshot of the webengine preview
+
+        :return: Image of the webengine, can be None
+        """
         self.label.setText(theme_name)
         self.progress_bar.setValue(self.progress_bar.value() + 1)
         wait_for(lambda: self.theme_display.is_initialised)
         self.theme_display.set_scale(float(self.theme_display.width()) / self.renderer.width())
-        return self.theme_display.generate_preview(theme_data, generate_screenshot=True)
+        screenshot = self.theme_display.generate_preview(theme_data, generate_screenshot=True)
+        if self.isVisible():
+            return screenshot
+        else:
+            return None
 
     def _get_theme_list(self):
         """Property getter"""

tests/functional/openlp_core/lib/test_serviceitem.py

@@ -24,7 +24,7 @@ Package to test the openlp.core.lib package.
 import os
 import pytest
 from pathlib import Path
-from unittest.mock import Mock, MagicMock, patch, ANY
+from unittest.mock import Mock, MagicMock, patch
 
 from openlp.core.common import ThemeLevel, is_win
 from openlp.core.common.enum import ServiceItemType
@@ -809,7 +809,9 @@ def test_to_dict_text_item(state_media, settings, service_item_env):
     assert result == expected_dict
 
 
-def test_to_dict_image_item(state_media, settings, service_item_env):
+@patch('openlp.core.lib.serviceitem.AppLocation.get_data_path')
+@patch('openlp.core.lib.serviceitem.image_to_data_uri')
+def test_to_dict_image_item(mocked_image_to_data_uri, mocked_get_data_path, state_media, settings, service_item_env):
     """
     Test that the to_dict() method returns the correct data for the service item
     """
@@ -820,6 +822,8 @@ def test_to_dict_image_item(state_media, settings, service_item_env):
     service_item.add_icon = MagicMock()
     FormattingTags.load_tags()
     line = convert_file_service_item(TEST_PATH, 'serviceitem_image_2.osj')
+    mocked_get_data_path.return_value = Path('/path/to/')
+    mocked_image_to_data_uri.side_effect = lambda x: 'your image uri at: {}'.format(x)
 
     with patch('openlp.core.lib.serviceitem.sha256_file_hash') as mocked_sha256_file_hash:
         mocked_sha256_file_hash.return_value = '3a7ccbdb0b5a3db169c4692d7aad0ec8'
@@ -841,7 +845,7 @@ def test_to_dict_image_item(state_media, settings, service_item_env):
         'slides': [
             {
                 'html': 'image_1.jpg',
-                'img': '/images/thumbnails/image_1.jpg',
+                'img': 'your image uri at: /path/to/images/thumbnails/image_1.jpg',
                 'selected': False,
                 'tag': 1,
                 'text': 'image_1.jpg',
@@ -856,7 +860,8 @@ def test_to_dict_image_item(state_media, settings, service_item_env):
 
 
 @patch('openlp.core.lib.serviceitem.AppLocation.get_data_path')
-def test_to_dict_presentation_item(mocked_get_data_path, state_media, settings, service_item_env):
+@patch('openlp.core.lib.serviceitem.image_to_data_uri')
+def test_to_dict_presentation_item(mocked_image_uri, mocked_get_data_path, state_media, settings, service_item_env):
     """
     Test that the to_dict() method returns the correct data for the service item
     """
@@ -870,6 +875,7 @@ def test_to_dict_presentation_item(mocked_get_data_path, state_media, settings,
     image = Path('thumbnails/abcd/slide1.png')
     display_title = 'DisplayTitle'
     notes = 'Note1\nNote2\n'
+    mocked_image_uri.side_effect = lambda x: 'your img uri at: {}'.format(x)
 
     # WHEN: adding presentation to service_item
     with patch('openlp.core.lib.serviceitem.sha256_file_hash') as mocked_sha256_file_hash,\
@@ -898,7 +904,7 @@ def test_to_dict_presentation_item(mocked_get_data_path, state_media, settings,
                 'tag': 1,
                 'text': 'test.pptx',
                 'title': '',
-                'img': ANY
+                'img': 'your img uri at: /path/to/presentations/thumbnails/4a067fed6834ea2bc4b8819f11636365/slide1.png'
             }
         ],
         'theme': None,

tests/openlp_core/ui/test_themeprogressform.py

@@ -36,7 +36,9 @@ def _get_theme_progress_form():
 
 
 def test_init(qapp):
-    """Test that the ThemeProgressForm is created without problems"""
+    """
+    Test that the ThemeProgressForm is created without problems
+    """
     # GIVEN: ThemeProgressForm class
     # WHEN: An object is instatiated
     # THEN: There is no problem
@@ -46,7 +48,9 @@ def test_init(qapp):
 @patch('openlp.core.ui.themeprogressform.ScreenList')
 @patch('openlp.core.ui.themeprogressform.QtWidgets.QDialog.show')
 def test_show(mocked_show, MockScreenList, settings):
-    """Test that the ThemeProgressForm is created without problems"""
+    """
+    Test that the ThemeProgressForm is created without problems
+    """
     # GIVEN: ThemeProgressForm object
     form = _get_theme_progress_form()
     mocked_screen_list = MagicMock()
@@ -70,7 +74,9 @@ def test_show(mocked_show, MockScreenList, settings):
 @patch('openlp.core.ui.themeprogressform.ScreenList')
 @patch('openlp.core.ui.themeprogressform.QtWidgets.QDialog.show')
 def test_show_divide_by_zero(mocked_show, MockScreenList, settings):
-    """Test that the ThemeProgressForm is created without problems even if there's a divide by zero exception"""
+    """
+    Test that the ThemeProgressForm is created without problems even if there's a divide by zero exception
+    """
     # GIVEN: ThemeProgressForm object
     form = _get_theme_progress_form()
     mocked_screen_list = MagicMock()
@@ -92,7 +98,9 @@ def test_show_divide_by_zero(mocked_show, MockScreenList, settings):
 
 
 def test_get_preview(settings):
-    """Test that the get_preview() method returns a preview image"""
+    """
+    Test that the get_preview() method returns a preview image
+    """
     # GIVEN: ThemeProgressForm object
     Registry.create()
     mocked_renderer = MagicMock()
@@ -100,6 +108,7 @@ def test_get_preview(settings):
     test_theme_name = 'Test Theme'
     test_theme_data = {'name': test_theme_name}
     form = _get_theme_progress_form()
+    form.isVisible = MagicMock(return_value=True)
     form.progress_bar = MagicMock(**{'value.return_value': 0})
     form.label = MagicMock()
     form.theme_display = MagicMock(**{'width.return_value': 192, 'generate_preview.return_value': 'preview'})
@@ -119,6 +128,30 @@ def test_get_preview(settings):
     assert preview == 'preview'
 
 
+def test_get_preview_not_visible(settings):
+    """
+    Test that the get_preview() method does not return a preview image when display is not visible
+    """
+    # GIVEN: ThemeProgressForm object
+    Registry.create()
+    mocked_renderer = MagicMock()
+    Registry().register('renderer', mocked_renderer)
+    test_theme_name = 'Test Theme'
+    test_theme_data = {'name': test_theme_name}
+    form = _get_theme_progress_form()
+    form.isVisible = MagicMock(return_value=False)
+    form.progress_bar = MagicMock(**{'value.return_value': 0})
+    form.label = MagicMock()
+    form.theme_display = MagicMock(**{'width.return_value': 192, 'generate_preview.return_value': 'preview'})
+    form.renderer.width = MagicMock(return_value=1920)
+
+    # WHEN: get_preview() is called
+    preview = form.get_preview(test_theme_name, test_theme_data)
+
+    # THEN: Result should be None
+    assert preview is None
+
+
 def test_theme_list(qapp):
     # GIVEN: ThemeProgressForm object and theme list
     test_theme_list = ['Theme 1', 'Theme 2']