openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #23464
[Merge] lp:~tomasgroth/openlp/bug1299837 into lp:openlp
Tomas Groth has proposed merging lp:~tomasgroth/openlp/bug1299837 into lp:openlp.
Requested reviews:
OpenLP Core (openlp-core)
Related bugs:
Bug #1299837 in OpenLP: "OpenLP crashes when importing songs from EasyWorship"
https://bugs.launchpad.net/openlp/+bug/1299837
For more details, see:
https://code.launchpad.net/~tomasgroth/openlp/bug1299837/+merge/218315
Fix for crash when importing from a EasyWorship DB that contains unexpected dataformatting.
[SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/432/
[SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/388/
[SUCCESS] http://ci.openlp.org/job/Branch-03-Interface-Tests/333/
[SUCCESS] http://ci.openlp.org/job/Branch-04-Windows_Tests/294/
[SUCCESS] http://ci.openlp.org/job/Branch-05a-Code_Analysis/198/
[SUCCESS] http://ci.openlp.org/job/Branch-05b-Test_Coverage/73/
--
https://code.launchpad.net/~tomasgroth/openlp/bug1299837/+merge/218315
Your team OpenLP Core is requested to review the proposed merge of lp:~tomasgroth/openlp/bug1299837 into lp:openlp.
=== modified file 'MANIFEST.in'
--- MANIFEST.in 2011-03-19 07:20:20 +0000
+++ MANIFEST.in 2014-05-05 17:39:57 +0000
@@ -5,6 +5,8 @@
recursive-include openlp *.js
recursive-include openlp *.css
recursive-include openlp *.png
+recursive-include openlp *.ps
+recursive-include openlp *.json
recursive-include documentation *
recursive-include resources *
recursive-include scripts *
=== modified file 'openlp/plugins/songs/lib/ewimport.py'
--- openlp/plugins/songs/lib/ewimport.py 2014-04-20 19:03:35 +0000
+++ openlp/plugins/songs/lib/ewimport.py 2014-05-05 17:39:57 +0000
@@ -74,6 +74,7 @@
"""
def __init__(self, manager, **kwargs):
super(EasyWorshipSongImport, self).__init__(manager, **kwargs)
+ self.entry_error_log = ''
def do_import(self):
"""
@@ -183,7 +184,12 @@
self.set_song_import_object(authors, inflated_content)
if self.stop_import_flag:
break
- if not self.finish():
+ if self.entry_error_log:
+ self.log_error(self.import_source,
+ translate('SongsPlugin.EasyWorshipSongImport', '"%s" could not be imported. %s')
+ % (self.title, self.entry_error_log))
+ self.entry_error_log = ''
+ elif not self.finish():
self.log_error(self.import_source)
# Set file_pos for next entry
file_pos += entry_length
@@ -281,7 +287,7 @@
raw_record = db_file.read(record_size)
self.fields = self.record_structure.unpack(raw_record)
self.set_defaults()
- self.title = self.get_field(fi_title).decode()
+ self.title = self.get_field(fi_title).decode('unicode-escape')
# Get remaining fields.
copy = self.get_field(fi_copy)
admin = self.get_field(fi_admin)
@@ -289,23 +295,28 @@
authors = self.get_field(fi_author)
words = self.get_field(fi_words)
if copy:
- self.copyright = copy.decode()
+ self.copyright = copy.decode('unicode-escape')
if admin:
if copy:
self.copyright += ', '
self.copyright += translate('SongsPlugin.EasyWorshipSongImport',
- 'Administered by %s') % admin.decode()
+ 'Administered by %s') % admin.decode('unicode-escape')
if ccli:
- self.ccli_number = ccli.decode()
+ self.ccli_number = ccli.decode('unicode-escape')
if authors:
- authors = authors.decode()
+ authors = authors.decode('unicode-escape')
else:
authors = ''
# Set the SongImport object members.
self.set_song_import_object(authors, words)
if self.stop_import_flag:
break
- if not self.finish():
+ if self.entry_error_log:
+ self.log_error(self.import_source,
+ translate('SongsPlugin.EasyWorshipSongImport', '"%s" could not be imported. %s')
+ % (self.title, self.entry_error_log))
+ self.entry_error_log = ''
+ elif not self.finish():
self.log_error(self.import_source)
db_file.close()
self.memo_file.close()
@@ -328,8 +339,19 @@
self.add_author(author_name.strip())
if words:
# Format the lyrics
- result = strip_rtf(words.decode(), self.encoding)
+ result = None
+ decoded_words = None
+ try:
+ decoded_words = words.decode()
+ except UnicodeDecodeError:
+ # The unicode chars in the rtf was not escaped in the expected manor
+ self.entry_error_log = translate('SongsPlugin.EasyWorshipSongImport',
+ 'Unexpected data formatting.')
+ return
+ result = strip_rtf(decoded_words, self.encoding)
if result is None:
+ self.entry_error_log = translate('SongsPlugin.EasyWorshipSongImport',
+ 'No song text found.')
return
words, self.encoding = result
verse_type = VerseType.tags[VerseType.Verse]
=== modified file 'tests/functional/openlp_plugins/songs/test_ewimport.py'
--- tests/functional/openlp_plugins/songs/test_ewimport.py 2014-04-29 16:54:14 +0000
+++ tests/functional/openlp_plugins/songs/test_ewimport.py 2014-05-05 17:39:57 +0000
@@ -67,7 +67,21 @@
'Just to learn from His lips, words of comfort,\nIn the beautiful garden of prayer.', 'v2'),
('There\'s a garden where Jesus is waiting,\nAnd He bids you to come meet Him there,\n'
'Just to bow and receive a new blessing,\nIn the beautiful garden of prayer.', 'v3')],
- 'verse_order_list': []}]
+ 'verse_order_list': []},
+ {'title': 'Vi pløjed og vi så\'de',
+ 'authors': ['Matthias Claudius'],
+ 'copyright': 'Public Domain',
+ 'ccli_number': 0,
+ 'verses':
+ [('Vi pløjed og vi så\'de\nvor sæd i sorten jord,\nså bad vi ham os hjælpe,\nsom højt i Himlen bor,\n'
+ 'og han lod snefald hegne\nmod frosten barsk og hård,\nhan lod det tø og regne\nog varme mildt i vår.',
+ 'v1'),
+ ('Alle gode gaver\nde kommer ovenned,\nså tak da Gud, ja, pris dog Gud\nfor al hans kærlighed!', 'c1'),
+ ('Han er jo den, hvis vilje\nopholder alle ting,\nhan klæder markens lilje\nog runder himlens ring,\n'
+ 'ham lyder vind og vove,\nham rører ravnes nød,\nhvi skulle ej hans småbørn\nda og få dagligt brød?', 'v2'),
+ ('Ja, tak, du kære Fader,\nså mild, så rig, så rund,\nfor korn i hæs og lader,\nfor godt i allen stund!\n'
+ 'Vi kan jo intet give,\nsom nogen ting er værd,\nmen tag vort stakkels hjerte,\nså ringe som det er!', 'v3')],
+ 'verse_order_list': []}]
EWS_SONG_TEST_DATA =\
{'title': 'Vi pløjed og vi så\'de',
@@ -139,6 +153,7 @@
"""
Test the functions in the :mod:`ewimport` module.
"""
+
def create_field_desc_entry_test(self):
"""
Test creating an instance of the :class`FieldDescEntry` class.
@@ -467,3 +482,22 @@
for verse_text, verse_tag in EWS_SONG_TEST_DATA['verses']:
mocked_add_verse.assert_any_call(verse_text, verse_tag)
mocked_finish.assert_called_with()
+
+ def import_rtf_unescaped_unicode_test(self):
+ """
+ Test import of rtf without the expected escaping of unicode
+ """
+
+ # GIVEN: A mocked out SongImport class, a mocked out "manager" and mocked out "author" method.
+ with patch('openlp.plugins.songs.lib.ewimport.SongImport'):
+ mocked_manager = MagicMock()
+ mocked_add_author = MagicMock()
+ importer = EasyWorshipSongImportLogger(mocked_manager)
+ importer.add_author = mocked_add_author
+ importer.encoding = 'cp1252'
+
+ # WHEN: running set_song_import_object on a verse string without the needed escaping
+ importer.set_song_import_object('Test Author', b'Det som var fr\x86n begynnelsen')
+
+ # THEN: The import should fail
+ self.assertEquals(importer.entry_error_log, 'Unexpected data formatting.', 'Import should fail')
=== modified file 'tests/resources/easyworshipsongs/Songs.DB'
Binary files tests/resources/easyworshipsongs/Songs.DB 2013-04-10 16:24:53 +0000 and tests/resources/easyworshipsongs/Songs.DB 2014-05-05 17:39:57 +0000 differ
=== modified file 'tests/resources/easyworshipsongs/Songs.MB'
Binary files tests/resources/easyworshipsongs/Songs.MB 2013-04-10 16:24:53 +0000 and tests/resources/easyworshipsongs/Songs.MB 2014-05-05 17:39:57 +0000 differ
Follow ups
