mirror of https://gitlab.com/openlp/openlp.git
Merge branch 'transpose-api-crash-fix' into 'master'
Fixing OpenLP crash (and increasing performance) caused when using Transpose API Endpoint See merge request openlp/openlp!516
This commit is contained in:
commit
78d42e27f5
|
@ -28,7 +28,6 @@ from openlp.core.api.lib import login_required
|
|||
from openlp.core.lib.plugin import PluginStatus
|
||||
from openlp.core.common.registry import Registry
|
||||
from openlp.plugins.songs.lib import transpose_lyrics
|
||||
from openlp.core.display.render import remove_tags
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
|
@ -154,22 +153,14 @@ def transpose(transpose_value):
|
|||
# make sure an service item is currently displayed and that it is a song
|
||||
if not current_item or current_item.name != 'songs':
|
||||
abort(400)
|
||||
previous_pages = {}
|
||||
live_item = current_item.to_dict()
|
||||
live_item['id'] = str(current_item.unique_identifier)
|
||||
chord_song_text = ''
|
||||
# re-create the song lyrics with OpenLP verse-tags to be able to transpose in one go so any keys are used
|
||||
for raw_slide in current_item.slides:
|
||||
verse_tag = raw_slide['verse']
|
||||
if verse_tag in previous_pages and previous_pages[verse_tag][0] == raw_slide:
|
||||
pages = previous_pages[verse_tag][1]
|
||||
else:
|
||||
pages = current_item.renderer.format_slide(raw_slide['text'], current_item)
|
||||
previous_pages[verse_tag] = (raw_slide, pages)
|
||||
for page in pages:
|
||||
chord_song_text += '---[Verse:{verse_tag}]---\n'.format(verse_tag=verse_tag)
|
||||
chord_song_text += page
|
||||
chord_song_text += '\n'
|
||||
# remove formatting tags
|
||||
chord_song_text = remove_tags(chord_song_text)
|
||||
verse_index = 1
|
||||
for item in live_item['slides']:
|
||||
chord_song_text += ('---[Verse:%d]---' % verse_index) + '\n'
|
||||
chord_song_text += item['chords'] + '\n'
|
||||
verse_index += 1
|
||||
# transpose
|
||||
transposed_lyrics = transpose_lyrics(chord_song_text, transpose_value)
|
||||
# re-split into verses
|
||||
|
|
|
@ -18,10 +18,16 @@
|
|||
# You should have received a copy of the GNU General Public License #
|
||||
# along with this program. If not, see <https://www.gnu.org/licenses/>. #
|
||||
##########################################################################
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from openlp.core.common.registry import Registry
|
||||
from openlp.core.common.enum import PluginStatus
|
||||
from openlp.core.display.render import Renderer
|
||||
from openlp.core.lib.serviceitem import ServiceItem
|
||||
from openlp.core.state import State
|
||||
from tests.openlp_core.lib.test_serviceitem import TEST_PATH as SERVICEITEM_TEST_PATH
|
||||
from tests.utils import convert_file_service_item
|
||||
|
||||
|
||||
# Search options tests
|
||||
|
@ -94,3 +100,47 @@ def test_plugin_songs_transpose_returns_plugin_exception(flask_client, settings)
|
|||
Registry().register('plugin_manager', MagicMock())
|
||||
res = flask_client.get('/api/v2/plugins/songs/transpose-live-item/test')
|
||||
assert res.status_code == 400
|
||||
|
||||
|
||||
def test_plugin_songs_transpose_wont_call_renderer(flask_client, settings):
|
||||
"""
|
||||
Tests whether the transpose endpoint won't tries to use any Renderer method; the endpoint needs to operate using
|
||||
already-primed caches (as that's what the /live-item endpoint does); also using Renderer from outside the Qt loop
|
||||
causes it to crash.
|
||||
|
||||
See https://gitlab.com/openlp/openlp/-/merge_requests/516 for some background on this.
|
||||
"""
|
||||
# GIVEN: A mocked plugin_manager, live_controller, renderer and a real service item with the internal slide cache
|
||||
# filled
|
||||
Registry().register('plugin_manager', MagicMock())
|
||||
renderer_mock = MagicMock()
|
||||
# Mocking every Renderer attribute with a shared mock so we can know whether any property of it was called
|
||||
renderer_mock_any_attr = MagicMock(name='Renderer attribute/call')
|
||||
for name in dir(Renderer):
|
||||
# Mocking every Renderer property
|
||||
# skipping dunder/hide methods
|
||||
if not name.startswith('_'):
|
||||
setattr(renderer_mock, name, renderer_mock_any_attr)
|
||||
# Mocking format_slides to return the correct data to ServiceItem
|
||||
renderer_mock.format_slide.side_effect = lambda text, item: text
|
||||
Registry().register('renderer', renderer_mock)
|
||||
service_item = ServiceItem()
|
||||
live_controller_mock = MagicMock()
|
||||
live_controller_mock.service_item = service_item
|
||||
Registry().register('live_controller', live_controller_mock)
|
||||
# Using a real item to test this
|
||||
line = convert_file_service_item(SERVICEITEM_TEST_PATH, 'serviceitem-song-linked-audio.osj')
|
||||
State().load_settings() # needed to make the below method not fail
|
||||
service_item.set_from_service(line, Path('/test/'))
|
||||
# Trying to retrive the slides to trigger the cache prime process (as it's primed when using the service item first
|
||||
# time in OpenLP)
|
||||
service_item.rendered_slides
|
||||
# Resetting the mocks to correctly compute illegal calls
|
||||
renderer_mock_any_attr.reset_mock()
|
||||
renderer_mock.format_slides.reset_mock()
|
||||
|
||||
# WHEN: The endpoint is called
|
||||
flask_client.get('/api/v2/plugins/songs/transpose-live-item/-1')
|
||||
|
||||
# THEN: The renderer should not be called
|
||||
renderer_mock_any_attr.assert_not_called()
|
||||
|
|
Loading…
Reference in New Issue