From 5dea23662ada76b36f71be6f88fb7148c0a71076 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 9 Oct 2020 06:26:28 +0000 Subject: [PATCH] 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 | 16 +++++--- openlp/core/ui/thememanager.py | 2 + openlp/core/ui/themeprogressform.py | 11 ++++- .../openlp_core/lib/test_serviceitem.py | 16 +++++--- .../openlp_core/ui/test_themeprogressform.py | 41 +++++++++++++++++-- 5 files changed, 71 insertions(+), 15 deletions(-) diff --git a/openlp/core/lib/serviceitem.py b/openlp/core/lib/serviceitem.py index d7cce51d6..687df9422 100644 --- a/openlp/core/lib/serviceitem.py +++ b/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 +from openlp.core.lib import create_thumb, image_to_data_uri, ItemCapabilities 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) diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index eb0a956c8..bc33b3f30 100644 --- a/openlp/core/ui/thememanager.py +++ b/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() diff --git a/openlp/core/ui/themeprogressform.py b/openlp/core/ui/themeprogressform.py index 86bd5c275..568874feb 100644 --- a/openlp/core/ui/themeprogressform.py +++ b/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""" diff --git a/tests/functional/openlp_core/lib/test_serviceitem.py b/tests/functional/openlp_core/lib/test_serviceitem.py index 14a38d392..7dae51929 100644 --- a/tests/functional/openlp_core/lib/test_serviceitem.py +++ b/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, diff --git a/tests/openlp_core/ui/test_themeprogressform.py b/tests/openlp_core/ui/test_themeprogressform.py index 60f715d65..a455c1e43 100644 --- a/tests/openlp_core/ui/test_themeprogressform.py +++ b/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']