Phill has proposed merging lp:~phill-ridout/openlp/1194610 into lp:openlp.
Requested reviews:
Tim Bentley (trb143)
Raoul Snyman (raoul-snyman)
Related bugs:
Bug #1194610 in OpenLP: "[support-system] SongShowPlus traceback"
https://bugs.launchpad.net/openlp/+bug/1194610
For more details, see:
https://code.launchpad.net/~phill-ridout/openlp/1194610/+merge/182491
Fixes #1194610 by detecting the encoding rather than assuming it
Adds a helper module to help test songs being imported.
--
https://code.launchpad.net/~phill-ridout/openlp/1194610/+merge/182491
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/plugins/songs/lib/songshowplusimport.py'
--- openlp/plugins/songs/lib/songshowplusimport.py 2013-03-31 10:13:56 +0000
+++ openlp/plugins/songs/lib/songshowplusimport.py 2013-08-27 20:31:09 +0000
@@ -30,6 +30,7 @@
The :mod:`songshowplusimport` module provides the functionality for importing
SongShow Plus songs into the OpenLP database.
"""
+import chardet
import os
import logging
import re
@@ -134,41 +135,41 @@
log.debug(length_descriptor_size)
data = song_data.read(length_descriptor)
if block_key == TITLE:
- self.title = unicode(data, u'cp1252')
+ self.title = self.decode(data)
elif block_key == AUTHOR:
authors = data.split(" / ")
for author in authors:
if author.find(",") !=-1:
authorParts = author.split(", ")
author = authorParts[1] + " " + authorParts[0]
- self.parse_author(unicode(author, u'cp1252'))
+ self.parse_author(self.decode(author))
elif block_key == COPYRIGHT:
- self.addCopyright(unicode(data, u'cp1252'))
+ self.addCopyright(self.decode(data))
elif block_key == CCLI_NO:
self.ccliNumber = int(data)
elif block_key == VERSE:
- self.addVerse(unicode(data, u'cp1252'), "%s%s" % (VerseType.tags[VerseType.Verse], verse_no))
+ self.addVerse(self.decode(data), "%s%s" % (VerseType.tags[VerseType.Verse], verse_no))
elif block_key == CHORUS:
- self.addVerse(unicode(data, u'cp1252'), "%s%s" % (VerseType.tags[VerseType.Chorus], verse_no))
+ self.addVerse(self.decode(data), "%s%s" % (VerseType.tags[VerseType.Chorus], verse_no))
elif block_key == BRIDGE:
- self.addVerse(unicode(data, u'cp1252'), "%s%s" % (VerseType.tags[VerseType.Bridge], verse_no))
+ self.addVerse(self.decode(data), "%s%s" % (VerseType.tags[VerseType.Bridge], verse_no))
elif block_key == TOPIC:
- self.topics.append(unicode(data, u'cp1252'))
+ self.topics.append(self.decode(data))
elif block_key == COMMENTS:
- self.comments = unicode(data, u'cp1252')
+ self.comments = self.decode(data)
elif block_key == VERSE_ORDER:
verse_tag = self.to_openlp_verse_tag(data, True)
if verse_tag:
if not isinstance(verse_tag, unicode):
- verse_tag = unicode(verse_tag, u'cp1252')
+ verse_tag = self.decode(verse_tag)
self.ssp_verse_order_list.append(verse_tag)
elif block_key == SONG_BOOK:
- self.songBookName = unicode(data, u'cp1252')
+ self.songBookName = self.decode(data)
elif block_key == SONG_NUMBER:
self.songNumber = ord(data)
elif block_key == CUSTOM_VERSE:
verse_tag = self.to_openlp_verse_tag(verse_name)
- self.addVerse(unicode(data, u'cp1252'), verse_tag)
+ self.addVerse(self.decode(data), verse_tag)
else:
log.debug("Unrecognised blockKey: %s, data: %s" % (block_key, data))
song_data.seek(next_block_starts)
@@ -206,3 +207,9 @@
verse_tag = VerseType.tags[VerseType.Other]
verse_number = self.other_list[verse_name]
return verse_tag + verse_number
+
+ def decode(self, data):
+ try:
+ return unicode(data, chardet.detect(data)['encoding'])
+ except:
+ return unicode(data, u'cp1252')
\ No newline at end of file
=== added file 'tests/functional/openlp_plugins/songs/songfileimporthelper.py'
--- tests/functional/openlp_plugins/songs/songfileimporthelper.py 1970-01-01 00:00:00 +0000
+++ tests/functional/openlp_plugins/songs/songfileimporthelper.py 2013-08-27 20:31:09 +0000
@@ -0,0 +1,113 @@
+"""
+The :mod:`songfileimporthelper` modules provides a helper class and methods to easily enable testing the import of
+song files from third party applications.
+"""
+import json
+from unittest import TestCase
+from mock import patch, MagicMock
+
+class SongImportTestHelper(TestCase):
+ """
+ This class is designed to be a helper class to reduce repition when testing the import of song files.
+ """
+ def __init__(self, *args, **kwargs):
+ self.importer_module = __import__(
+ u'openlp.plugins.songs.lib.%s' % self.importer_module_name, fromlist=[self.importer_class_name])
+ self.importer_class = getattr(self.importer_module, self.importer_class_name)
+ TestCase.__init__(self, *args, **kwargs)
+
+ def setUp(self):
+ """
+ Patch and set up the mocks required.
+ """
+ self.add_copyright_patcher = patch(
+ u'openlp.plugins.songs.lib.%s.%s.addCopyright' % (self.importer_module_name, self.importer_class_name))
+ self.add_verse_patcher = patch(
+ u'openlp.plugins.songs.lib.%s.%s.addVerse' % (self.importer_module_name, self.importer_class_name))
+ self.finish_patcher = patch(
+ u'openlp.plugins.songs.lib.%s.%s.finish' % (self.importer_module_name, self.importer_class_name))
+ self.parse_author_patcher = patch(
+ u'openlp.plugins.songs.lib.%s.%s.parse_author' % (self.importer_module_name, self.importer_class_name))
+ self.song_import_patcher = patch(u'openlp.plugins.songs.lib.%s.SongImport' % self.importer_module_name)
+ self.mocked_add_copyright = self.add_copyright_patcher.start()
+ self.mocked_add_verse = self.add_verse_patcher.start()
+ self.mocked_finish = self.finish_patcher.start()
+ self.mocked_parse_author = self.parse_author_patcher.start()
+ self.mocked_song_importer = self.song_import_patcher.start()
+ self.mocked_manager = MagicMock()
+ self.mocked_import_wizard = MagicMock()
+ self.mocked_finish.return_value = True
+
+ def tearDown(self):
+ """
+ Clean up
+ """
+ self.add_copyright_patcher.stop()
+ self.add_verse_patcher.stop()
+ self.finish_patcher.stop()
+ self.parse_author_patcher.stop()
+ self.song_import_patcher.stop()
+
+ def load_external_result_data(self, file_name):
+ """
+ A method to load and return an object containing the song data from an external file.
+ """
+ result_file = open(file_name, 'rb')
+ return json.loads(result_file.read())
+
+ def file_import(self, source_file_name, result_data):
+ """
+ Import the given file and check that it has imported correctly
+ """
+ importer = self.importer_class(self.mocked_manager)
+ importer.import_wizard = self.mocked_import_wizard
+ importer.stop_import_flag = False
+ importer.topics = []
+
+ # WHEN: Importing the source file
+ importer.import_source = [source_file_name]
+ add_verse_calls = self.get_data(result_data, u'verses')
+ author_calls = self.get_data(result_data, u'authors')
+ ccli_number = self.get_data(result_data, u'ccli_number')
+ comments = self.get_data(result_data, u'comments')
+ song_book_name = self.get_data(result_data, u'song_book_name')
+ song_copyright = self.get_data(result_data, u'copyright')
+ song_number = self.get_data(result_data, u'song_number')
+ title = self.get_data(result_data, u'title')
+ topics = self.get_data(result_data, u'topics')
+ verse_order_list = self.get_data(result_data, u'verse_order_list')
+
+ # THEN: doImport should return none, the song data should be as expected, and finish should have been
+ # called.
+ self.assertIsNone(importer.doImport(), u'doImport should return None when it has completed')
+ self.assertEquals(importer.title, title, u'title for %s should be "%s"' % (source_file_name, title))
+ for author in author_calls:
+ self.mocked_parse_author.assert_any_call(author)
+ if song_copyright:
+ self.mocked_add_copyright.assert_called_with(song_copyright)
+ if ccli_number:
+ self.assertEquals(importer.ccliNumber, ccli_number, u'ccliNumber for %s should be %s'
+ % (source_file_name, ccli_number))
+ for verse_text, verse_tag in add_verse_calls:
+ self.mocked_add_verse.assert_any_call(verse_text, verse_tag)
+ if topics:
+ self.assertEquals(importer.topics, topics, u'topics for %s should be %s' % (source_file_name, topics))
+ if comments:
+ self.assertEquals(importer.comments, comments, u'comments for %s should be "%s"'
+ % (source_file_name, comments))
+ if song_book_name:
+ self.assertEquals(importer.songBookName, song_book_name, u'songBookName for %s should be "%s"'
+ % (source_file_name, song_book_name))
+ if song_number:
+ self.assertEquals(importer.songNumber, song_number, u'songNumber for %s should be %s'
+ % (source_file_name, song_number))
+ if verse_order_list:
+ self.assertEquals(importer.verseOrderList, [], u'verseOrderList for %s should be %s'
+ % (source_file_name, verse_order_list))
+ self.mocked_finish.assert_called_with()
+
+ def get_data(self, data, key):
+ if key in data:
+ return data[key]
+ return u''
+
=== modified file 'tests/functional/openlp_plugins/songs/test_songshowplusimport.py'
--- tests/functional/openlp_plugins/songs/test_songshowplusimport.py 2013-04-05 18:26:39 +0000
+++ tests/functional/openlp_plugins/songs/test_songshowplusimport.py 2013-08-27 20:31:09 +0000
@@ -6,50 +6,24 @@
from unittest import TestCase
from mock import patch, MagicMock
+from tests.functional.openlp_plugins.songs.songfileimporthelper import SongImportTestHelper
from openlp.plugins.songs.lib import VerseType
from openlp.plugins.songs.lib.songshowplusimport import SongShowPlusImport
-TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), u'../../../resources/songshowplussongs'))
-SONG_TEST_DATA = {u'Amazing Grace.sbsong':
- {u'title': u'Amazing Grace (Demonstration)',
- u'authors': [u'John Newton', u'Edwin Excell', u'John P. Rees'],
- u'copyright': u'Public Domain ',
- u'ccli_number': 22025,
- u'verses':
- [(u'Amazing grace! How sweet the sound!\r\nThat saved a wretch like me!\r\n'
- u'I once was lost, but now am found;\r\nWas blind, but now I see.', u'v1'),
- (u'\'Twas grace that taught my heart to fear,\r\nAnd grace my fears relieved.\r\n'
- u'How precious did that grace appear,\r\nThe hour I first believed.', u'v2'),
- (u'The Lord has promised good to me,\r\nHis Word my hope secures.\r\n'
- u'He will my shield and portion be\r\nAs long as life endures.', u'v3'),
- (u'Thro\' many dangers, toils and snares\r\nI have already come.\r\n'
- u'\'Tis grace that brought me safe thus far,\r\nAnd grace will lead me home.', u'v4'),
- (u'When we\'ve been there ten thousand years,\r\nBright shining as the sun,\r\n'
- u'We\'ve no less days to sing God\'s praise,\r\nThan when we first begun.', u'v5')],
- u'topics': [u'Assurance', u'Grace', u'Praise', u'Salvation'],
- u'comments': u'\n\n\n',
- u'song_book_name': u'Demonstration Songs',
- u'song_number': 0,
- u'verse_order_list': []},
- u'Beautiful Garden Of Prayer.sbsong':
- {u'title': u'Beautiful Garden Of Prayer (Demonstration)',
- u'authors': [u'Eleanor Allen Schroll', u'James H. Fillmore'],
- u'copyright': u'Public Domain ',
- u'ccli_number': 60252,
- u'verses':
- [(u'There\'s a garden where Jesus is waiting,\r\nThere\'s a place that is wondrously fair.\r\n'
- u'For it glows with the light of His presence,\r\n\'Tis the beautiful garden of prayer.', u'v1'),
- (u'There\'s a garden where Jesus is waiting,\r\nAnd I go with my burden and care.\r\n'
- u'Just to learn from His lips, words of comfort,\r\nIn the beautiful garden of prayer.', u'v2'),
- (u'There\'s a garden where Jesus is waiting,\r\nAnd He bids you to come meet Him there,\r\n'
- u'Just to bow and receive a new blessing,\r\nIn the beautiful garden of prayer.', u'v3'),
- (u'O the beautiful garden, the garden of prayer,\r\nO the beautiful garden of prayer.\r\n'
- u'There my Savior awaits, and He opens the gates\r\nTo the beautiful garden of prayer.', u'c1')],
- u'topics': [u'Devotion', u'Prayer'],
- u'comments': u'',
- u'song_book_name': u'',
- u'song_number': 0,
- u'verse_order_list': []}}
+TEST_PATH = os.path.abspath(
+ os.path.join(os.path.dirname(__file__), u'..', u'..', u'..', u'resources', u'songshowplussongs'))
+
+class TestSongShowPlusFileImport(SongImportTestHelper):
+ def __init__(self, *args, **kwargs):
+ self.importer_class_name = u'SongShowPlusImport'
+ self.importer_module_name = u'songshowplusimport'
+ SongImportTestHelper.__init__(self, *args, **kwargs)
+
+ def test_song_import(self):
+ test_import = self.file_import(os.path.join(TEST_PATH, u'Amazing Grace.sbsong'),
+ self.load_external_result_data(os.path.join(TEST_PATH, u'Amazing Grace.json')))
+ test_import = self.file_import(os.path.join(TEST_PATH, u'Beautiful Garden Of Prayer.sbsong'),
+ self.load_external_result_data(os.path.join(TEST_PATH, u'Beautiful Garden Of Prayer.json')))
class TestSongShowPlusImport(TestCase):
@@ -165,71 +139,4 @@
for original_tag, openlp_tag in test_values:
self.assertEquals(importer.to_openlp_verse_tag(original_tag, ignore_unique=True), openlp_tag,
u'SongShowPlusImport.to_openlp_verse_tag should return "%s" when called with "%s"'
- % (openlp_tag, original_tag))
-
- def file_import_test(self):
- """
- Test the actual import of real song files and check that the imported data is correct.
- """
-
- # GIVEN: Test files with a mocked out SongImport class, a mocked out "manager", a mocked out "import_wizard",
- # and mocked out "author", "add_copyright", "add_verse", "finish" methods.
- with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'):
- for song_file in SONG_TEST_DATA:
- mocked_manager = MagicMock()
- mocked_import_wizard = MagicMock()
- mocked_parse_author = MagicMock()
- mocked_add_copyright = MagicMock()
- mocked_add_verse = MagicMock()
- mocked_finish = MagicMock()
- mocked_finish.return_value = True
- importer = SongShowPlusImport(mocked_manager)
- importer.import_wizard = mocked_import_wizard
- importer.stop_import_flag = False
- importer.parse_author = mocked_parse_author
- importer.addCopyright = mocked_add_copyright
- importer.addVerse = mocked_add_verse
- importer.finish = mocked_finish
- importer.topics = []
-
- # WHEN: Importing each file
- importer.import_source = [os.path.join(TEST_PATH, song_file)]
- title = SONG_TEST_DATA[song_file][u'title']
- author_calls = SONG_TEST_DATA[song_file][u'authors']
- song_copyright = SONG_TEST_DATA[song_file][u'copyright']
- ccli_number = SONG_TEST_DATA[song_file][u'ccli_number']
- add_verse_calls = SONG_TEST_DATA[song_file][u'verses']
- topics = SONG_TEST_DATA[song_file][u'topics']
- comments = SONG_TEST_DATA[song_file][u'comments']
- song_book_name = SONG_TEST_DATA[song_file][u'song_book_name']
- song_number = SONG_TEST_DATA[song_file][u'song_number']
- verse_order_list = SONG_TEST_DATA[song_file][u'verse_order_list']
-
- # THEN: doImport should return none, the song data should be as expected, and finish should have been
- # called.
- self.assertIsNone(importer.doImport(), u'doImport should return None when it has completed')
- self.assertEquals(importer.title, title, u'title for %s should be "%s"' % (song_file, title))
- for author in author_calls:
- mocked_parse_author.assert_any_call(author)
- if song_copyright:
- mocked_add_copyright.assert_called_with(song_copyright)
- if ccli_number:
- self.assertEquals(importer.ccliNumber, ccli_number, u'ccliNumber for %s should be %s'
- % (song_file, ccli_number))
- for verse_text, verse_tag in add_verse_calls:
- mocked_add_verse.assert_any_call(verse_text, verse_tag)
- if topics:
- self.assertEquals(importer.topics, topics, u'topics for %s should be %s' % (song_file, topics))
- if comments:
- self.assertEquals(importer.comments, comments, u'comments for %s should be "%s"'
- % (song_file, comments))
- if song_book_name:
- self.assertEquals(importer.songBookName, song_book_name, u'songBookName for %s should be "%s"'
- % (song_file, song_book_name))
- if song_number:
- self.assertEquals(importer.songNumber, song_number, u'songNumber for %s should be %s'
- % (song_file, song_number))
- if verse_order_list:
- self.assertEquals(importer.verseOrderList, [], u'verseOrderList for %s should be %s'
- % (song_file, verse_order_list))
- mocked_finish.assert_called_with()
+ % (openlp_tag, original_tag))
\ No newline at end of file
=== added file 'tests/resources/songshowplussongs/Amazing Grace.json'
--- tests/resources/songshowplussongs/Amazing Grace.json 1970-01-01 00:00:00 +0000
+++ tests/resources/songshowplussongs/Amazing Grace.json 2013-08-27 20:31:09 +0000
@@ -0,0 +1,42 @@
+{
+ "authors": [
+ "John Newton",
+ "Edwin Excell",
+ "John P. Rees"
+ ],
+ "ccli_number": 22025,
+ "comments": "\n\n\n",
+ "copyright": "Public Domain ",
+ "song_book_name": "Demonstration Songs",
+ "song_number": 0,
+ "title": "Amazing Grace (Demonstration)",
+ "topics": [
+ "Assurance",
+ "Grace",
+ "Praise",
+ "Salvation"
+ ],
+ "verse_order_list": [],
+ "verses": [
+ [
+ "Amazing grace! How sweet the sound!\r\nThat saved a wretch like me!\r\nI once was lost, but now am found;\r\nWas blind, but now I see.",
+ "v1"
+ ],
+ [
+ "'Twas grace that taught my heart to fear,\r\nAnd grace my fears relieved.\r\nHow precious did that grace appear,\r\nThe hour I first believed.",
+ "v2"
+ ],
+ [
+ "The Lord has promised good to me,\r\nHis Word my hope secures.\r\nHe will my shield and portion be\r\nAs long as life endures.",
+ "v3"
+ ],
+ [
+ "Thro' many dangers, toils and snares\r\nI have already come.\r\n'Tis grace that brought me safe thus far,\r\nAnd grace will lead me home.",
+ "v4"
+ ],
+ [
+ "When we've been there ten thousand years,\r\nBright shining as the sun,\r\nWe've no less days to sing God's praise,\r\nThan when we first begun.",
+ "v5"
+ ]
+ ]
+}
\ No newline at end of file
=== added file 'tests/resources/songshowplussongs/Beautiful Garden Of Prayer.json'
--- tests/resources/songshowplussongs/Beautiful Garden Of Prayer.json 1970-01-01 00:00:00 +0000
+++ tests/resources/songshowplussongs/Beautiful Garden Of Prayer.json 2013-08-27 20:31:09 +0000
@@ -0,0 +1,35 @@
+{
+ "authors": [
+ "Eleanor Allen Schroll",
+ "James H. Fillmore"
+ ],
+ "ccli_number": 60252,
+ "comments": "",
+ "copyright": "Public Domain ",
+ "song_book_name": "",
+ "song_number": 0,
+ "title": "Beautiful Garden Of Prayer (Demonstration)",
+ "topics": [
+ "Devotion",
+ "Prayer"
+ ],
+ "verse_order_list": [],
+ "verses": [
+ [
+ "There's a garden where Jesus is waiting,\r\nThere's a place that is wondrously fair.\r\nFor it glows with the light of His presence,\r\n'Tis the beautiful garden of prayer.",
+ "v1"
+ ],
+ [
+ "There's a garden where Jesus is waiting,\r\nAnd I go with my burden and care.\r\nJust to learn from His lips, words of comfort,\r\nIn the beautiful garden of prayer.",
+ "v2"
+ ],
+ [
+ "There's a garden where Jesus is waiting,\r\nAnd He bids you to come meet Him there,\r\nJust to bow and receive a new blessing,\r\nIn the beautiful garden of prayer.",
+ "v3"
+ ],
+ [
+ "O the beautiful garden, the garden of prayer,\r\nO the beautiful garden of prayer.\r\nThere my Savior awaits, and He opens the gates\r\nTo the beautiful garden of prayer.",
+ "c1"
+ ]
+ ]
+}
\ No newline at end of file
_______________________________________________
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help : https://help.launchpad.net/ListHelp
