From 1b6a54c55d64ee1f47d3276c15450cbba3ff838d Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Fri, 15 Feb 2013 19:57:05 +0000 Subject: [PATCH 01/25] started on tests for SongShowPlusImport --- openlp/plugins/songs/lib/__init__.py | 2 +- .../openlp_plugins_songs_lib/__init__.py | 0 .../test_songshowplusimport.py | 43 +++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 tests/functional/openlp_plugins_songs_lib/__init__.py create mode 100644 tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py diff --git a/openlp/plugins/songs/lib/__init__.py b/openlp/plugins/songs/lib/__init__.py index c7c24533b..4041bb12e 100644 --- a/openlp/plugins/songs/lib/__init__.py +++ b/openlp/plugins/songs/lib/__init__.py @@ -167,7 +167,7 @@ class VerseType(object): translate('SongsPlugin.VerseType', 'Intro'), translate('SongsPlugin.VerseType', 'Ending'), translate('SongsPlugin.VerseType', 'Other')] - TranslatedTags = [name[0].lower() for name in TranslatedNames] + TranslatedTags = [unicode(name[0]).lower() for name in TranslatedNames] @staticmethod def translated_tag(verse_tag, default=Other): diff --git a/tests/functional/openlp_plugins_songs_lib/__init__.py b/tests/functional/openlp_plugins_songs_lib/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py b/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py new file mode 100644 index 000000000..6189c6a7f --- /dev/null +++ b/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py @@ -0,0 +1,43 @@ +""" + Package to test the openlp.plugins.songs.lib package. +""" +import os + +from unittest import TestCase +from mock import MagicMock, patch +from openlp.plugins.songs.lib import songshowplusimport + +TESTPATH = os.path.abspath(os.path.join(os.path.dirname(__file__), u'..', u'..', u'resources')) + + +class TestSongShowPlusImport(TestCase): + + def default_test(self): + """ + Test the defaults of songshowplusimport + """ + # Given: The songshowplusimport module as imported + + # When: Imported the module should have defaults set + constants = {u'TITLE' : 1, u'AUTHOR' : 2, u'COPYRIGHT' : 3, u'CCLI_NO' : 5, u'VERSE' : 12, u'CHORUS' : 20, + u'BRIDGE' : 24, u'TOPIC' : 29, u'COMMENTS' : 30, u'VERSE_ORDER' : 31, u'SONG_BOOK' : 35, + u'SONG_NUMBER' : 36, u'CUSTOM_VERSE' : 37, u'SongShowPlusImport.otherList' : {}, + u'SongShowPlusImport.otherCount' : 0} + + # Then: The constants should not have changed. + for constant in constants: + value = constants[constant] + self.assertEquals(eval(u'songshowplusimport.%s' % constant), value, + u'%s should be set as %s' % (constant, value)) + + + def do_import_test(self): + mocked_manager = MagicMock() + songshowplusimport.SongImport = MagicMock() + + with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport') as mocked_song_import: + ssp_import_class = songshowplusimport.SongShowPlusImport(mocked_manager) + + songshowplusimport.SongShowPlusImport.importSource = '' + + self.assertEquals(ssp_import_class.SongShowPlusImport().doImport(), False) From ca2b2db6402fbe94e55e8698a00dc193714de7e2 Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Sun, 17 Feb 2013 19:37:59 +0000 Subject: [PATCH 02/25] fixed duplicates in verse order when adding verses with the same tag. fixed handling of part verses eg (1a, 1b, 1.5, etc) in SongShowPlus importer --- openlp/plugins/songs/lib/songimport.py | 3 ++- .../plugins/songs/lib/songshowplusimport.py | 16 +++++++++----- .../test_songshowplusimport.py | 22 +++++-------------- 3 files changed, 17 insertions(+), 24 deletions(-) diff --git a/openlp/plugins/songs/lib/songimport.py b/openlp/plugins/songs/lib/songimport.py index f6a84945c..0d563935f 100644 --- a/openlp/plugins/songs/lib/songimport.py +++ b/openlp/plugins/songs/lib/songimport.py @@ -260,7 +260,8 @@ class SongImport(QtCore.QObject): elif int(verse_def[1:]) > self.verseCounts[verse_def[0]]: self.verseCounts[verse_def[0]] = int(verse_def[1:]) self.verses.append([verse_def, verse_text.rstrip(), lang]) - self.verseOrderListGenerated.append(verse_def) + if verse_def not in self.verseOrderListGenerated: + self.verseOrderListGenerated.append(verse_def) def repeatVerse(self): """ diff --git a/openlp/plugins/songs/lib/songshowplusimport.py b/openlp/plugins/songs/lib/songshowplusimport.py index c5bb8832d..8e4957c71 100644 --- a/openlp/plugins/songs/lib/songshowplusimport.py +++ b/openlp/plugins/songs/lib/songshowplusimport.py @@ -32,6 +32,7 @@ SongShow Plus songs into the OpenLP database. """ import os import logging +import re import struct from openlp.core.ui.wizard import WizardStrings @@ -44,13 +45,13 @@ COPYRIGHT = 3 CCLI_NO = 5 VERSE = 12 CHORUS = 20 +BRIDGE = 24 TOPIC = 29 COMMENTS = 30 VERSE_ORDER = 31 SONG_BOOK = 35 SONG_NUMBER = 36 CUSTOM_VERSE = 37 -BRIDGE = 24 log = logging.getLogger(__name__) @@ -183,13 +184,16 @@ class SongShowPlusImport(SongImport): self.logError(file) def toOpenLPVerseTag(self, verse_name, ignore_unique=False): - if verse_name.find(" ") != -1: - verse_parts = verse_name.split(" ") - verse_type = verse_parts[0] - verse_number = verse_parts[1] + # Have we got any digits? If so, verse number is everything from the digits to the end (OpenLP does not have + # concept of part verses, so just ignore any non integers on the end (including floats)) + match = re.match(u'(\D*)(\d+)', verse_name) + if match is not None: + verse_type = match.group(1).strip() + verse_number = match.group(2) else: + # otherwise we assume number 1 and take the whole prefix as the verse tag verse_type = verse_name - verse_number = "1" + verse_number = u'1' verse_type = verse_type.lower() if verse_type == "verse": verse_tag = VerseType.Tags[VerseType.Verse] diff --git a/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py b/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py index 6189c6a7f..77733a1ba 100644 --- a/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py +++ b/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py @@ -12,28 +12,16 @@ TESTPATH = os.path.abspath(os.path.join(os.path.dirname(__file__), u'..', u'..', class TestSongShowPlusImport(TestCase): - def default_test(self): - """ - Test the defaults of songshowplusimport - """ - # Given: The songshowplusimport module as imported +#test do import + # set self.import source to non list type. Do import should return None or False? + # set self.import source to a list of files + # importWizard.progressBar should be set to the number of files in the list + # set self.stop_import_flag to true. Do import should return None or False? - # When: Imported the module should have defaults set - constants = {u'TITLE' : 1, u'AUTHOR' : 2, u'COPYRIGHT' : 3, u'CCLI_NO' : 5, u'VERSE' : 12, u'CHORUS' : 20, - u'BRIDGE' : 24, u'TOPIC' : 29, u'COMMENTS' : 30, u'VERSE_ORDER' : 31, u'SONG_BOOK' : 35, - u'SONG_NUMBER' : 36, u'CUSTOM_VERSE' : 37, u'SongShowPlusImport.otherList' : {}, - u'SongShowPlusImport.otherCount' : 0} - - # Then: The constants should not have changed. - for constant in constants: - value = constants[constant] - self.assertEquals(eval(u'songshowplusimport.%s' % constant), value, - u'%s should be set as %s' % (constant, value)) def do_import_test(self): mocked_manager = MagicMock() - songshowplusimport.SongImport = MagicMock() with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport') as mocked_song_import: ssp_import_class = songshowplusimport.SongShowPlusImport(mocked_manager) From ea86ce905c382cc674a439f3a5e76bf57b679524 Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Mon, 18 Feb 2013 17:15:07 +0000 Subject: [PATCH 03/25] Simplified if statment --- openlp/plugins/songs/lib/songshowplusimport.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/songs/lib/songshowplusimport.py b/openlp/plugins/songs/lib/songshowplusimport.py index 8e4957c71..c9f9617df 100644 --- a/openlp/plugins/songs/lib/songshowplusimport.py +++ b/openlp/plugins/songs/lib/songshowplusimport.py @@ -187,7 +187,7 @@ class SongShowPlusImport(SongImport): # Have we got any digits? If so, verse number is everything from the digits to the end (OpenLP does not have # concept of part verses, so just ignore any non integers on the end (including floats)) match = re.match(u'(\D*)(\d+)', verse_name) - if match is not None: + if match: verse_type = match.group(1).strip() verse_number = match.group(2) else: From 86fb3437599062d4972d6a0c7815ebdee1b7ca2f Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Sun, 24 Mar 2013 11:32:03 +0000 Subject: [PATCH 04/25] Added test for SongShowPlus --- .../plugins/songs/lib/songshowplusimport.py | 17 +- .../songs/test_songshowplusimport.py | 169 ++++++++++++++++++ .../openlp_plugins_songs_lib/__init__.py | 0 .../test_songshowplusimport.py | 31 ---- tests/resources/Amazing Grace.sbsong | Bin 0 -> 1018 bytes 5 files changed, 178 insertions(+), 39 deletions(-) create mode 100644 tests/functional/openlp_plugins/songs/test_songshowplusimport.py delete mode 100644 tests/functional/openlp_plugins_songs_lib/__init__.py delete mode 100644 tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py create mode 100644 tests/resources/Amazing Grace.sbsong diff --git a/openlp/plugins/songs/lib/songshowplusimport.py b/openlp/plugins/songs/lib/songshowplusimport.py index 84e888856..57a0f3236 100644 --- a/openlp/plugins/songs/lib/songshowplusimport.py +++ b/openlp/plugins/songs/lib/songshowplusimport.py @@ -89,8 +89,9 @@ class SongShowPlusImport(SongImport): * .sbsong """ - otherList = {} - otherCount = 0 + + other_count = 0 + other_list = {} def __init__(self, manager, **kwargs): """ @@ -109,8 +110,8 @@ class SongShowPlusImport(SongImport): if self.stop_import_flag: return self.sspVerseOrderList = [] - other_count = 0 - other_list = {} + self.other_count = 0 + self.other_list = {} file_name = os.path.split(file)[1] self.import_wizard.increment_progress_bar(WizardStrings.ImportingType % file_name, 0) song_data = open(file, 'rb') @@ -204,11 +205,11 @@ class SongShowPlusImport(SongImport): elif verse_type == "pre-chorus": verse_tag = VerseType.tags[VerseType.PreChorus] else: - if verse_name not in self.otherList: + if verse_name not in self.other_list: if ignore_unique: return None - self.otherCount += 1 - self.otherList[verse_name] = str(self.otherCount) + self.other_count += 1 + self.other_list[verse_name] = str(self.other_count) verse_tag = VerseType.tags[VerseType.Other] - verse_number = self.otherList[verse_name] + verse_number = self.other_list[verse_name] return verse_tag + verse_number diff --git a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py new file mode 100644 index 000000000..38962b79d --- /dev/null +++ b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py @@ -0,0 +1,169 @@ +""" +This module contains tests for the OpenLP song importer. +""" + +import os +from unittest import TestCase +from mock import call, patch, MagicMock + +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')) + +class TestSongShowPlusFileImport(TestCase): + """ + Test the functions in the :mod:`lib` module. + """ + def create_importer_test(self): + """ + Test creating an instance of the SongShow Plus file importer + """ + # GIVEN: A mocked out SongImport class, and a mocked out "manager" + with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): + mocked_manager = MagicMock() + + # WHEN: An importer object is created + importer = SongShowPlusImport(mocked_manager) + + # THEN: The importer object should not be None + self.assertIsNotNone(importer, u'Import should not be none') + + def toOpenLPVerseTag_test(self): + """ + Test toOpenLPVerseTag method + """ + # GIVEN: A mocked out SongImport class, and a mocked out "manager" + with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): + mocked_manager = MagicMock() + importer = SongShowPlusImport(mocked_manager) + + # WHEN: Supplied with the following arguments replicating verses being added + test_values = [(u'Verse 1', VerseType.tags[VerseType.Verse] + u'1'), + (u'Verse 2', VerseType.tags[VerseType.Verse] + u'2'), + (u'verse1', VerseType.tags[VerseType.Verse] + u'1'), + (u'Verse', VerseType.tags[VerseType.Verse] + u'1'), + (u'Verse1', VerseType.tags[VerseType.Verse] + u'1'), + (u'chorus 1', VerseType.tags[VerseType.Chorus] + u'1'), + (u'bridge 1', VerseType.tags[VerseType.Bridge] + u'1'), + (u'pre-chorus 1', VerseType.tags[VerseType.PreChorus] + u'1'), + (u'different 1', VerseType.tags[VerseType.Other] + u'1'), + (u'random 1', VerseType.tags[VerseType.Other] + u'2')] + + # THEN: The returned value should should correlate with the input arguments + for original_tag, openlp_tag in test_values: + self.assertEquals(importer.toOpenLPVerseTag(original_tag), openlp_tag, + u'SongShowPlusImport.toOpenLPVerseTag should return "%s" when called with "%s"' + % (openlp_tag, original_tag)) + + # WHEN: Supplied with the following arguments replicating a verse order being added + test_values = [(u'Verse 1', VerseType.tags[VerseType.Verse] + u'1'), + (u'Verse 2', VerseType.tags[VerseType.Verse] + u'2'), + (u'verse1', VerseType.tags[VerseType.Verse] + u'1'), + (u'Verse', VerseType.tags[VerseType.Verse] + u'1'), + (u'Verse1', VerseType.tags[VerseType.Verse] + u'1'), + (u'chorus 1', VerseType.tags[VerseType.Chorus] + u'1'), + (u'bridge 1', VerseType.tags[VerseType.Bridge] + u'1'), + (u'pre-chorus 1', VerseType.tags[VerseType.PreChorus] + u'1'), + (u'different 1', VerseType.tags[VerseType.Other] + u'1'), + (u'random 1', VerseType.tags[VerseType.Other] + u'2'), + (u'unused 2', None)] + + # THEN: The returned value should should correlate with the input arguments + for original_tag, openlp_tag in test_values: + self.assertEquals(importer.toOpenLPVerseTag(original_tag, ignore_unique=True), openlp_tag, + u'SongShowPlusImport.toOpenLPVerseTag should return "%s" when called with "%s"' + % (openlp_tag, original_tag)) + + + + + def import_source_test(self): + """ + Test creating an instance of the SongShow Plus file importer + """ + # GIVEN: A mocked out SongImport class, and a mocked out "manager" + with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): + mocked_manager = MagicMock() + mocked_import_wizard = MagicMock() + importer = SongShowPlusImport(mocked_manager) + importer.import_wizard = mocked_import_wizard + importer.stop_import_flag = True + + # WHEN: Import source is a string + importer.import_source = u'not a list' + + # THEN: doImport should return none and the progress bar maximum should not be set. + self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') + self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, + u'setMaxium on import_wizard.progress_bar should not have been called') + + # WHEN: Import source is an int + importer.import_source = 0 + + # THEN: doImport should return none and the progress bar maximum should not be set. + self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') + self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, + u'setMaxium on import_wizard.progress_bar should not have been called') + + # WHEN: Import source is a list + importer.import_source = [u'List', u'of', u'files'] + + # THEN: doImport should return none and the progress bar maximum should be set. + self.assertIsNone(importer.doImport(), + u'doImport should return None when import_source is a list and stop_import_flag is True') + mocked_import_wizard.progress_bar.setMaximum.assert_called_with( + len(importer.import_source)) + + def file_import_test(self): + """ + Test creating an instance of the SongShow Plus file importer + """ + + # GIVEN: A mocked out SongImport class, and a mocked out "manager" + with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): + 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: Import source is a string + importer.import_source = [os.path.join(TEST_PATH, u'Amazing Grace.sbsong')] + + # THEN: doImport should return none and the progress bar maximum should not be set. + self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') + self.assertEquals(importer.title, u'Amazing Grace (Demonstration)', + u'Title for Amazing Grace.sbsong should be "Amazing Grace (Demonstration)"') + calls = [call(u'John Newton'), call(u'Edwin Excell'), call(u'John P. Rees')] + mocked_parse_author.assert_has_calls(calls) + mocked_add_copyright.assert_called_with(u'Public Domain ') + self.assertEquals(importer.ccliNumber, 22025, u'ccliNumber should be set as 22025 for Amazing Grace.sbsong') + calls = [call(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'), + call(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'), + call(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'), + call(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'), + call(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')] + mocked_add_verse.assert_has_calls(calls) + self.assertEquals(importer.topics, [u'Assurance', u'Grace', u'Praise', u'Salvation']) + self.assertEquals(importer.comments, u'\n\n\n', u'comments should be "\\n\\n\\n" Amazing Grace.sbsong') + self.assertEquals(importer.songBookName, u'Demonstration Songs', u'songBookName should be ' + u'"Demonstration Songs"') + self.assertEquals(importer.songNumber, 0, u'songNumber should be 0') + self.assertEquals(importer.verseOrderList, [], u'verseOrderList should be empty') + mocked_finish.assert_called_with() \ No newline at end of file diff --git a/tests/functional/openlp_plugins_songs_lib/__init__.py b/tests/functional/openlp_plugins_songs_lib/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py b/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py deleted file mode 100644 index 77733a1ba..000000000 --- a/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py +++ /dev/null @@ -1,31 +0,0 @@ -""" - Package to test the openlp.plugins.songs.lib package. -""" -import os - -from unittest import TestCase -from mock import MagicMock, patch -from openlp.plugins.songs.lib import songshowplusimport - -TESTPATH = os.path.abspath(os.path.join(os.path.dirname(__file__), u'..', u'..', u'resources')) - - -class TestSongShowPlusImport(TestCase): - -#test do import - # set self.import source to non list type. Do import should return None or False? - # set self.import source to a list of files - # importWizard.progressBar should be set to the number of files in the list - # set self.stop_import_flag to true. Do import should return None or False? - - - - def do_import_test(self): - mocked_manager = MagicMock() - - with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport') as mocked_song_import: - ssp_import_class = songshowplusimport.SongShowPlusImport(mocked_manager) - - songshowplusimport.SongShowPlusImport.importSource = '' - - self.assertEquals(ssp_import_class.SongShowPlusImport().doImport(), False) diff --git a/tests/resources/Amazing Grace.sbsong b/tests/resources/Amazing Grace.sbsong new file mode 100644 index 0000000000000000000000000000000000000000..14b7c35971ad673c98ca9ad8e128e0ad3922444c GIT binary patch literal 1018 zcmZ8g-)j>=5KdZq+S7<2K31nFL?J{8_~cWhwT*}r8_4tKZgRJ{w;Oi%dX4xXC@3QS z&3<#&AL_#;vpcijeDlprzt`(M!k6q#EA>g+f{wh(n4TVR#*FANZ?v30Foz50$#@$t?#ZjWC-u_kj1F9- z5cULjg1FV&!S79p*qKaTOkO51`}lU{u8X*JW!;7)U$Xm#=!j^#q&ql%lYoBm<+8X! zirc4S*HCDfBgK*_xZ39XgLGc1NI{)(PKp}OF)PXFk4zQAJ0oWyOrruB7vhMPbtDTQ zRnbZiUJcR(oT$a-*WMWg=CN@3C0w?WAH%s|v`mm5DWj^3GE%jnl9k8V(F(?BkWOuW z5eTQ;1@de(gW`CQN)>C*nRa!cT<0BH2dviX4q}c1OILfE(MtOeX?Y1CoIVSu?c`jd z-Z`IB32JNaDjlFg;T%96>Iau&9cUpT!qcrG8)voWAVeUGHby+5)NG(1h_9WO*+D`S zBBEiqfNu1PiEZA#6%OBp!;R$Yy!38Jm9iVkl`Ys~R-)4;v}nO9B$GCj=nyI6S>+qb zT*Y88oP*t8k}kdLGzCqCe6fT?tN%1@IUB&BK$HX^q4Qhl>?A)IC0lBEh-6EKiAnJQ zYApyZ6>g*>kmj}5(m>R1WrI*;J65%YZ_y%HM}`Bsq&9Fm3hk!3d?;!wh>b{$9};$1 zuX5fJlwC-YlNRmz#i=r9?Fv7HTNWzWPSX_sy+1Rg B8W{ip literal 0 HcmV?d00001 From c3492d6aedd1ce06f3cb680f8eb3c72ae61b2968 Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Sun, 24 Mar 2013 13:25:31 +0000 Subject: [PATCH 05/25] Added an extra test song, and cleaned up the test some what --- .../songs/test_songshowplusimport.py | 228 +++++++++++------- .../Beautiful Garden Of Prayer.sbsong | Bin 0 -> 964 bytes 2 files changed, 140 insertions(+), 88 deletions(-) create mode 100644 tests/resources/Beautiful Garden Of Prayer.sbsong diff --git a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py index 38962b79d..4fb1e2479 100644 --- a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py +++ b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py @@ -1,5 +1,5 @@ """ -This module contains tests for the OpenLP song importer. +This module contains tests for the SongShow Plus song importer. """ import os @@ -10,10 +10,51 @@ 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')) +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': []}} -class TestSongShowPlusFileImport(TestCase): + +class TestSongShowPlusImport(TestCase): """ - Test the functions in the :mod:`lib` module. + Test the functions in the :mod:`songshowplusimport` module. """ def create_importer_test(self): """ @@ -29,6 +70,43 @@ class TestSongShowPlusFileImport(TestCase): # THEN: The importer object should not be None self.assertIsNotNone(importer, u'Import should not be none') + def import_source_test(self): + """ + Test SongShowPlusImport.doImport handles different import_source values + """ + # GIVEN: A mocked out SongImport class, and a mocked out "manager" + with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): + mocked_manager = MagicMock() + mocked_import_wizard = MagicMock() + importer = SongShowPlusImport(mocked_manager) + importer.import_wizard = mocked_import_wizard + importer.stop_import_flag = True + + # WHEN: Import source is a string + importer.import_source = u'not a list' + + # THEN: doImport should return none and the progress bar maximum should not be set. + self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') + self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, + u'setMaxium on import_wizard.progress_bar should not have been called') + + # WHEN: Import source is an int + importer.import_source = 0 + + # THEN: doImport should return none and the progress bar maximum should not be set. + self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') + self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, + u'setMaxium on import_wizard.progress_bar should not have been called') + + # WHEN: Import source is a list + importer.import_source = [u'List', u'of', u'files'] + + # THEN: doImport should return none and the progress bar setMaximum should be called with the length of + # import_source. + self.assertIsNone(importer.doImport(), + u'doImport should return None when import_source is a list and stop_import_flag is True') + mocked_import_wizard.progress_bar.setMaximum.assert_called_with(len(importer.import_source)) + def toOpenLPVerseTag_test(self): """ Test toOpenLPVerseTag method @@ -75,95 +153,69 @@ class TestSongShowPlusFileImport(TestCase): u'SongShowPlusImport.toOpenLPVerseTag should return "%s" when called with "%s"' % (openlp_tag, original_tag)) - - - - def import_source_test(self): - """ - Test creating an instance of the SongShow Plus file importer - """ - # GIVEN: A mocked out SongImport class, and a mocked out "manager" - with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): - mocked_manager = MagicMock() - mocked_import_wizard = MagicMock() - importer = SongShowPlusImport(mocked_manager) - importer.import_wizard = mocked_import_wizard - importer.stop_import_flag = True - - # WHEN: Import source is a string - importer.import_source = u'not a list' - - # THEN: doImport should return none and the progress bar maximum should not be set. - self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') - self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, - u'setMaxium on import_wizard.progress_bar should not have been called') - - # WHEN: Import source is an int - importer.import_source = 0 - - # THEN: doImport should return none and the progress bar maximum should not be set. - self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') - self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, - u'setMaxium on import_wizard.progress_bar should not have been called') - - # WHEN: Import source is a list - importer.import_source = [u'List', u'of', u'files'] - - # THEN: doImport should return none and the progress bar maximum should be set. - self.assertIsNone(importer.doImport(), - u'doImport should return None when import_source is a list and stop_import_flag is True') - mocked_import_wizard.progress_bar.setMaximum.assert_called_with( - len(importer.import_source)) - def file_import_test(self): """ - Test creating an instance of the SongShow Plus file importer + Test the actual import of real song files and check that the imported data is correct. """ - # GIVEN: A mocked out SongImport class, and a mocked out "manager" + # 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'): - 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 = [] + 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: Import source is a string - importer.import_source = [os.path.join(TEST_PATH, u'Amazing Grace.sbsong')] + # WHEN: Importing each file + importer.import_source = [os.path.join(TEST_PATH, song_file)] + title = SONG_TEST_DATA[song_file][u'title'] + parse_author_calls = [call(author) for author in 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 = \ + [call(verse_text, verse_tag) for verse_text, verse_tag in 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 and the progress bar maximum should not be set. - self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') - self.assertEquals(importer.title, u'Amazing Grace (Demonstration)', - u'Title for Amazing Grace.sbsong should be "Amazing Grace (Demonstration)"') - calls = [call(u'John Newton'), call(u'Edwin Excell'), call(u'John P. Rees')] - mocked_parse_author.assert_has_calls(calls) - mocked_add_copyright.assert_called_with(u'Public Domain ') - self.assertEquals(importer.ccliNumber, 22025, u'ccliNumber should be set as 22025 for Amazing Grace.sbsong') - calls = [call(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'), - call(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'), - call(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'), - call(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'), - call(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')] - mocked_add_verse.assert_has_calls(calls) - self.assertEquals(importer.topics, [u'Assurance', u'Grace', u'Praise', u'Salvation']) - self.assertEquals(importer.comments, u'\n\n\n', u'comments should be "\\n\\n\\n" Amazing Grace.sbsong') - self.assertEquals(importer.songBookName, u'Demonstration Songs', u'songBookName should be ' - u'"Demonstration Songs"') - self.assertEquals(importer.songNumber, 0, u'songNumber should be 0') - self.assertEquals(importer.verseOrderList, [], u'verseOrderList should be empty') - mocked_finish.assert_called_with() \ No newline at end of file + # 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)) + mocked_parse_author.assert_has_calls(parse_author_calls) + 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)) + mocked_add_verse.assert_has_calls(add_verse_calls) + 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() + \ No newline at end of file diff --git a/tests/resources/Beautiful Garden Of Prayer.sbsong b/tests/resources/Beautiful Garden Of Prayer.sbsong new file mode 100644 index 0000000000000000000000000000000000000000..c227d48097fb3f8722a874447c39476a80e35828 GIT binary patch literal 964 zcmb7@ZEMs(5XW;@8r#!~puQ9a6zW4frJ(Ptv0m>@3)e5O$xV`l%}&_eT(~czpd#X@ zbS61^DhJgU8)hf}o&C+jj-n_^c*W1(p=yIJS_2=ITcU-F0xq3eql2d@)?|HfDrmCL z9<%!kmMHe$zCL+#60n0Q7yOfjBJ53>Ok^vn4?}H_s?PnnL+P|cHK<+ zfGK|n=_vkou`{it_x{}trr0f)Qf(Tx85iK9FVsr&Kq#3cE~-FtrqrMv+Fq)Fp7pd7 c;kTHNn03M{K1gvz^7zCflOD&z+hjBP1!=b$FaQ7m literal 0 HcmV?d00001 From 401da98d7cf4d3357038a50035e36f57bf89a584 Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Sun, 24 Mar 2013 14:56:22 +0000 Subject: [PATCH 06/25] increased line lengths to 120 renamed SongShowPlusImport vairables and methods to standards --- .../plugins/songs/lib/songshowplusimport.py | 35 ++++++++----------- .../songs/test_songshowplusimport.py | 11 +++--- 2 files changed, 19 insertions(+), 27 deletions(-) diff --git a/openlp/plugins/songs/lib/songshowplusimport.py b/openlp/plugins/songs/lib/songshowplusimport.py index 57a0f3236..14631ebc2 100644 --- a/openlp/plugins/songs/lib/songshowplusimport.py +++ b/openlp/plugins/songs/lib/songshowplusimport.py @@ -57,31 +57,24 @@ log = logging.getLogger(__name__) class SongShowPlusImport(SongImport): """ - The :class:`SongShowPlusImport` class provides the ability to import song - files from SongShow Plus. + The :class:`SongShowPlusImport` class provides the ability to import song files from SongShow Plus. **SongShow Plus Song File Format:** The SongShow Plus song file format is as follows: - * Each piece of data in the song file has some information that precedes - it. + * Each piece of data in the song file has some information that precedes it. * The general format of this data is as follows: - 4 Bytes, forming a 32 bit number, a key if you will, this describes what - the data is (see blockKey below) - 4 Bytes, forming a 32 bit number, which is the number of bytes until the - next block starts + 4 Bytes, forming a 32 bit number, a key if you will, this describes what the data is (see blockKey below) + 4 Bytes, forming a 32 bit number, which is the number of bytes until the next block starts 1 Byte, which tells how many bytes follows - 1 or 4 Bytes, describes how long the string is, if its 1 byte, the string - is less than 255 + 1 or 4 Bytes, describes how long the string is, if its 1 byte, the string is less than 255 The next bytes are the actual data. The next block of data follows on. - This description does differ for verses. Which includes extra bytes - stating the verse type or number. In some cases a "custom" verse is used, - in that case, this block will in include 2 strings, with the associated - string length descriptors. The first string is the name of the verse, the - second is the verse content. + This description does differ for verses. Which includes extra bytes stating the verse type or number. In some cases + a "custom" verse is used, in that case, this block will in include 2 strings, with the associated string length + descriptors. The first string is the name of the verse, the second is the verse content. The file is ended with four null bytes. @@ -109,7 +102,7 @@ class SongShowPlusImport(SongImport): for file in self.import_source: if self.stop_import_flag: return - self.sspVerseOrderList = [] + self.ssp_verse_order_list = [] self.other_count = 0 self.other_list = {} file_name = os.path.split(file)[1] @@ -164,27 +157,27 @@ class SongShowPlusImport(SongImport): elif block_key == COMMENTS: self.comments = unicode(data, u'cp1252') elif block_key == VERSE_ORDER: - verse_tag = self.toOpenLPVerseTag(data, True) + 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') - self.sspVerseOrderList.append(verse_tag) + self.ssp_verse_order_list.append(verse_tag) elif block_key == SONG_BOOK: self.songBookName = unicode(data, u'cp1252') elif block_key == SONG_NUMBER: self.songNumber = ord(data) elif block_key == CUSTOM_VERSE: - verse_tag = self.toOpenLPVerseTag(verse_name) + verse_tag = self.to_openlp_verse_tag(verse_name) self.addVerse(unicode(data, u'cp1252'), verse_tag) else: log.debug("Unrecognised blockKey: %s, data: %s" % (block_key, data)) song_data.seek(next_block_starts) - self.verseOrderList = self.sspVerseOrderList + self.verseOrderList = self.ssp_verse_order_list song_data.close() if not self.finish(): self.logError(file) - def toOpenLPVerseTag(self, verse_name, ignore_unique=False): + def to_openlp_verse_tag(self, verse_name, ignore_unique=False): # Have we got any digits? If so, verse number is everything from the digits to the end (OpenLP does not have # concept of part verses, so just ignore any non integers on the end (including floats)) match = re.match(u'(\D*)(\d+)', verse_name) diff --git a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py index 4fb1e2479..9db0ee421 100644 --- a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py +++ b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py @@ -109,7 +109,7 @@ class TestSongShowPlusImport(TestCase): def toOpenLPVerseTag_test(self): """ - Test toOpenLPVerseTag method + Test to_openlp_verse_tag method """ # GIVEN: A mocked out SongImport class, and a mocked out "manager" with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): @@ -130,8 +130,8 @@ class TestSongShowPlusImport(TestCase): # THEN: The returned value should should correlate with the input arguments for original_tag, openlp_tag in test_values: - self.assertEquals(importer.toOpenLPVerseTag(original_tag), openlp_tag, - u'SongShowPlusImport.toOpenLPVerseTag should return "%s" when called with "%s"' + self.assertEquals(importer.to_openlp_verse_tag(original_tag), openlp_tag, + u'SongShowPlusImport.to_openlp_verse_tag should return "%s" when called with "%s"' % (openlp_tag, original_tag)) # WHEN: Supplied with the following arguments replicating a verse order being added @@ -149,8 +149,8 @@ class TestSongShowPlusImport(TestCase): # THEN: The returned value should should correlate with the input arguments for original_tag, openlp_tag in test_values: - self.assertEquals(importer.toOpenLPVerseTag(original_tag, ignore_unique=True), openlp_tag, - u'SongShowPlusImport.toOpenLPVerseTag should return "%s" when called with "%s"' + 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): @@ -218,4 +218,3 @@ class TestSongShowPlusImport(TestCase): self.assertEquals(importer.verseOrderList, [], u'verseOrderList for %s should be %s' % (song_file, verse_order_list)) mocked_finish.assert_called_with() - \ No newline at end of file From 4d0f9f3f914221852870b509323bbe792caf2954 Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Thu, 28 Mar 2013 19:50:06 +0000 Subject: [PATCH 07/25] Tided up the test somewhat --- .../songs/test_songshowplusimport.py | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py index 9db0ee421..0fae3fd56 100644 --- a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py +++ b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py @@ -88,7 +88,15 @@ class TestSongShowPlusImport(TestCase): # THEN: doImport should return none and the progress bar maximum should not be set. self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, - u'setMaxium on import_wizard.progress_bar should not have been called') + u'setMaxium on import_wizard.progress_bar should not have been called') + + # GIVEN: A mocked out SongImport class, and a mocked out "manager" + with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): + mocked_manager = MagicMock() + mocked_import_wizard = MagicMock() + importer = SongShowPlusImport(mocked_manager) + importer.import_wizard = mocked_import_wizard + importer.stop_import_flag = True # WHEN: Import source is an int importer.import_source = 0 @@ -96,7 +104,15 @@ class TestSongShowPlusImport(TestCase): # THEN: doImport should return none and the progress bar maximum should not be set. self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, - u'setMaxium on import_wizard.progress_bar should not have been called') + u'setMaxium on import_wizard.progress_bar should not have been called') + + # GIVEN: A mocked out SongImport class, and a mocked out "manager" + with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): + mocked_manager = MagicMock() + mocked_import_wizard = MagicMock() + importer = SongShowPlusImport(mocked_manager) + importer.import_wizard = mocked_import_wizard + importer.stop_import_flag = True # WHEN: Import source is a list importer.import_source = [u'List', u'of', u'files'] @@ -104,10 +120,10 @@ class TestSongShowPlusImport(TestCase): # THEN: doImport should return none and the progress bar setMaximum should be called with the length of # import_source. self.assertIsNone(importer.doImport(), - u'doImport should return None when import_source is a list and stop_import_flag is True') + u'doImport should return None when import_source is a list and stop_import_flag is True') mocked_import_wizard.progress_bar.setMaximum.assert_called_with(len(importer.import_source)) - def toOpenLPVerseTag_test(self): + def to_openlp_verse_tag_test(self): """ Test to_openlp_verse_tag method """ @@ -134,6 +150,11 @@ class TestSongShowPlusImport(TestCase): u'SongShowPlusImport.to_openlp_verse_tag should return "%s" when called with "%s"' % (openlp_tag, original_tag)) + # GIVEN: A mocked out SongImport class, and a mocked out "manager" + with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): + mocked_manager = MagicMock() + importer = SongShowPlusImport(mocked_manager) + # WHEN: Supplied with the following arguments replicating a verse order being added test_values = [(u'Verse 1', VerseType.tags[VerseType.Verse] + u'1'), (u'Verse 2', VerseType.tags[VerseType.Verse] + u'2'), From bc032eb7e247cc18ff39c4481060dbf9518a69d1 Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Thu, 28 Mar 2013 19:56:50 +0000 Subject: [PATCH 08/25] fixed indentation --- .../functional/openlp_plugins/songs/test_songshowplusimport.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py index 0fae3fd56..aa2753485 100644 --- a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py +++ b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py @@ -106,7 +106,7 @@ class TestSongShowPlusImport(TestCase): self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, u'setMaxium on import_wizard.progress_bar should not have been called') - # GIVEN: A mocked out SongImport class, and a mocked out "manager" + # GIVEN: A mocked out SongImport class, and a mocked out "manager" with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): mocked_manager = MagicMock() mocked_import_wizard = MagicMock() From 2920eecd314298c4450d52741afd455620119801 Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Thu, 28 Mar 2013 21:09:38 +0000 Subject: [PATCH 09/25] Moved sample song files to their own directory --- .../{ => songshowplussongs}/Amazing Grace.sbsong | Bin .../Beautiful Garden Of Prayer.sbsong | Bin 2 files changed, 0 insertions(+), 0 deletions(-) rename tests/resources/{ => songshowplussongs}/Amazing Grace.sbsong (100%) rename tests/resources/{ => songshowplussongs}/Beautiful Garden Of Prayer.sbsong (100%) diff --git a/tests/resources/Amazing Grace.sbsong b/tests/resources/songshowplussongs/Amazing Grace.sbsong similarity index 100% rename from tests/resources/Amazing Grace.sbsong rename to tests/resources/songshowplussongs/Amazing Grace.sbsong diff --git a/tests/resources/Beautiful Garden Of Prayer.sbsong b/tests/resources/songshowplussongs/Beautiful Garden Of Prayer.sbsong similarity index 100% rename from tests/resources/Beautiful Garden Of Prayer.sbsong rename to tests/resources/songshowplussongs/Beautiful Garden Of Prayer.sbsong From 86f294e8707c59875eb4875b4aad4b9c22b7884a Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Thu, 28 Mar 2013 21:17:07 +0000 Subject: [PATCH 10/25] Changed path in test file --- .../functional/openlp_plugins/songs/test_songshowplusimport.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py index aa2753485..24c77d0f3 100644 --- a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py +++ b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py @@ -9,7 +9,7 @@ from mock import call, patch, MagicMock 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')) +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'], From 76ea8126294d088d5ae4e002cd860b3f1faf7764 Mon Sep 17 00:00:00 2001 From: M2j Date: Sat, 30 Mar 2013 22:54:42 +0100 Subject: [PATCH 11/25] - switch to Python3 style sorting - use ICU for string sorting --- openlp/core/ui/exceptionform.py | 6 +++ openlp/core/ui/thememanager.py | 4 +- openlp/core/utils/__init__.py | 47 ++++++++++++++----- .../plugins/bibles/forms/bibleimportform.py | 4 +- openlp/plugins/bibles/lib/mediaitem.py | 6 +-- openlp/plugins/custom/lib/db.py | 7 ++- openlp/plugins/images/lib/mediaitem.py | 12 ++--- openlp/plugins/media/lib/mediaitem.py | 6 +-- openlp/plugins/presentations/lib/mediaitem.py | 5 +- openlp/plugins/songs/forms/songexportform.py | 4 +- openlp/plugins/songs/lib/__init__.py | 36 +------------- openlp/plugins/songs/lib/mediaitem.py | 6 +-- scripts/check_dependencies.py | 1 + 13 files changed, 69 insertions(+), 75 deletions(-) diff --git a/openlp/core/ui/exceptionform.py b/openlp/core/ui/exceptionform.py index 25f8201b1..e4854d0c0 100644 --- a/openlp/core/ui/exceptionform.py +++ b/openlp/core/ui/exceptionform.py @@ -69,6 +69,11 @@ try: MAKO_VERSION = mako.__version__ except ImportError: MAKO_VERSION = u'-' +try: + import icu + ICU_VERSION = u'OK' +except ImportError: + ICU_VERSION = u'-' try: import uno arg = uno.createUnoStruct(u'com.sun.star.beans.PropertyValue') @@ -143,6 +148,7 @@ class ExceptionForm(QtGui.QDialog, Ui_ExceptionDialog): u'PyEnchant: %s\n' % ENCHANT_VERSION + \ u'PySQLite: %s\n' % SQLITE_VERSION + \ u'Mako: %s\n' % MAKO_VERSION + \ + u'pyICU: %s\n' % ICU_VERSION + \ u'pyUNO bridge: %s\n' % UNO_VERSION + \ u'VLC: %s\n' % VLC_VERSION if platform.system() == u'Linux': diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 89bbe86f8..53d4a513b 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -44,7 +44,7 @@ from openlp.core.lib.theme import ThemeXML, BackgroundType, VerticalType, Backgr from openlp.core.lib.ui import critical_error_message_box, create_widget_action from openlp.core.theme import Theme from openlp.core.ui import FileRenameForm, ThemeForm -from openlp.core.utils import AppLocation, delete_file, locale_compare, get_filesystem_encoding +from openlp.core.utils import AppLocation, delete_file, get_local_key, get_filesystem_encoding log = logging.getLogger(__name__) @@ -418,7 +418,7 @@ class ThemeManager(QtGui.QWidget): self.theme_list_widget.clear() files = AppLocation.get_files(self.settings_section, u'.png') # Sort the themes by its name considering language specific - files.sort(key=lambda file_name: unicode(file_name), cmp=locale_compare) + files.sort(key=lambda file_name: get_local_key(unicode(file_name))) # now process the file list of png files for name in files: # check to see file is in theme root directory diff --git a/openlp/core/utils/__init__.py b/openlp/core/utils/__init__.py index d32729699..625bd25b5 100644 --- a/openlp/core/utils/__init__.py +++ b/openlp/core/utils/__init__.py @@ -38,6 +38,7 @@ import re from subprocess import Popen, PIPE import sys import urllib2 +import icu from PyQt4 import QtGui, QtCore @@ -56,6 +57,7 @@ from openlp.core.lib import translate log = logging.getLogger(__name__) APPLICATION_VERSION = {} IMAGES_FILTER = None +ICU_COLLATOR = None UNO_CONNECTION_TYPE = u'pipe' #UNO_CONNECTION_TYPE = u'socket' CONTROL_CHARS = re.compile(r'[\x00-\x1F\x7F-\x9F]', re.UNICODE) @@ -379,21 +381,42 @@ def format_time(text, local_time): return re.sub('\%[a-zA-Z]', match_formatting, text) -def locale_compare(string1, string2): +def get_local_key(string): """ - Compares two strings according to the current locale settings. - - As any other compare function, returns a negative, or a positive value, - or 0, depending on whether string1 collates before or after string2 or - is equal to it. Comparison is case insensitive. + Creates a key for case insensitive, locale aware string sorting. """ - # Function locale.strcoll() from standard Python library does not work properly on Windows. - return locale.strcoll(string1.lower(), string2.lower()) + string = string.lower() + # For Python 3 on platforms other than Windows ICU is not necessary. In those cases locale.strxfrm(str) can be used. + global ICU_COLLATOR + if ICU_COLLATOR is None: + from languagemanager import LanguageManager + locale = LanguageManager.get_language() + icu_locale = icu.Locale(locale) + ICU_COLLATOR = icu.Collator.createInstance(icu_locale) + return ICU_COLLATOR.getSortKey(string) -# For performance reasons provide direct reference to compare function without wrapping it in another function making -# the string lowercase. This is needed for sorting songs. -locale_direct_compare = locale.strcoll +def get_natural_key(string): + """ + Generate a key for locale aware natural string sorting. Returns a list of strings and integers. + + ``string`` + A string, list or tuple which represents the item string. + """ + if isinstance(string, basestring): + string = re.findall(r'(\d+|\D+)', string) + if len(string) == 1: + return list(get_local_key(string[0])) + elif isinstance(string, tuple): + string = list(string) + if isinstance(string, list): + for index, part in enumerate(string): + if isinstance(part, basestring): + if part.isdigit(): + string[index] = int(part) + else: + string[index] = get_local_key(part) + return string from applocation import AppLocation @@ -403,4 +426,4 @@ from actions import ActionList __all__ = [u'AppLocation', u'ActionList', u'LanguageManager', u'get_application_version', u'check_latest_version', u'add_actions', u'get_filesystem_encoding', u'get_web_page', u'get_uno_command', u'get_uno_instance', - u'delete_file', u'clean_filename', u'format_time', u'locale_compare', u'locale_direct_compare'] + u'delete_file', u'clean_filename', u'format_time', u'get_local_key', u'get_natural_key'] diff --git a/openlp/plugins/bibles/forms/bibleimportform.py b/openlp/plugins/bibles/forms/bibleimportform.py index e360cd4a1..319989433 100644 --- a/openlp/plugins/bibles/forms/bibleimportform.py +++ b/openlp/plugins/bibles/forms/bibleimportform.py @@ -38,7 +38,7 @@ from openlp.core.lib import Settings, UiStrings, translate from openlp.core.lib.db import delete_database from openlp.core.lib.ui import critical_error_message_box from openlp.core.ui.wizard import OpenLPWizard, WizardStrings -from openlp.core.utils import AppLocation, locale_compare +from openlp.core.utils import AppLocation, get_local_key from openlp.plugins.bibles.lib.manager import BibleFormat from openlp.plugins.bibles.lib.db import BiblesResourcesDB, clean_filename @@ -455,7 +455,7 @@ class BibleImportForm(OpenLPWizard): """ self.webTranslationComboBox.clear() bibles = self.web_bible_list[index].keys() - bibles.sort(cmp=locale_compare) + bibles.sort(key=get_local_key) self.webTranslationComboBox.addItems(bibles) def onOsisBrowseButtonClicked(self): diff --git a/openlp/plugins/bibles/lib/mediaitem.py b/openlp/plugins/bibles/lib/mediaitem.py index abe3cc45a..4ae7e76b5 100644 --- a/openlp/plugins/bibles/lib/mediaitem.py +++ b/openlp/plugins/bibles/lib/mediaitem.py @@ -36,7 +36,7 @@ from openlp.core.lib import Registry, MediaManagerItem, ItemCapabilities, Servic from openlp.core.lib.searchedit import SearchEdit from openlp.core.lib.ui import set_case_insensitive_completer, create_horizontal_adjusting_combo_box, \ critical_error_message_box, find_and_set_in_combo_box, build_icon -from openlp.core.utils import locale_compare +from openlp.core.utils import get_local_key from openlp.plugins.bibles.forms import BibleImportForm, EditBibleForm from openlp.plugins.bibles.lib import LayoutStyle, DisplayStyle, VerseReferenceList, get_reference_separator, \ LanguageSelection, BibleStrings @@ -325,7 +325,7 @@ class BibleMediaItem(MediaManagerItem): # Get all bibles and sort the list. bibles = self.plugin.manager.get_bibles().keys() bibles = filter(None, bibles) - bibles.sort(cmp=locale_compare) + bibles.sort(key=get_local_key) # Load the bibles into the combo boxes. self.quickVersionComboBox.addItems(bibles) self.quickSecondComboBox.addItems(bibles) @@ -461,7 +461,7 @@ class BibleMediaItem(MediaManagerItem): for book in book_data: data = BiblesResourcesDB.get_book_by_id(book.book_reference_id) books.append(data[u'name'] + u' ') - books.sort(cmp=locale_compare) + books.sort(key=get_local_key) set_case_insensitive_completer(books, self.quickSearchEdit) def on_import_click(self): diff --git a/openlp/plugins/custom/lib/db.py b/openlp/plugins/custom/lib/db.py index cc6e45742..ad876b6b6 100644 --- a/openlp/plugins/custom/lib/db.py +++ b/openlp/plugins/custom/lib/db.py @@ -35,7 +35,7 @@ from sqlalchemy import Column, Table, types from sqlalchemy.orm import mapper from openlp.core.lib.db import BaseModel, init_db -from openlp.core.utils import locale_compare +from openlp.core.utils import get_local_key class CustomSlide(BaseModel): """ @@ -44,11 +44,10 @@ class CustomSlide(BaseModel): # By default sort the customs by its title considering language specific # characters. def __lt__(self, other): - r = locale_compare(self.title, other.title) - return True if r < 0 else False + return get_local_key(self.title) < get_local_key(other.title) def __eq__(self, other): - return 0 == locale_compare(self.title, other.title) + return get_local_key(self.title) == get_local_key(other.title) def init_schema(url): diff --git a/openlp/plugins/images/lib/mediaitem.py b/openlp/plugins/images/lib/mediaitem.py index d74b1ccab..8c6bc8d9f 100644 --- a/openlp/plugins/images/lib/mediaitem.py +++ b/openlp/plugins/images/lib/mediaitem.py @@ -36,7 +36,7 @@ from openlp.core.lib import ItemCapabilities, MediaManagerItem, Registry, Servic StringContent, TreeWidgetWithDnD, UiStrings, build_icon, check_directory_exists, check_item_selected, \ create_thumb, translate, validate_thumb from openlp.core.lib.ui import create_widget_action, critical_error_message_box -from openlp.core.utils import AppLocation, delete_file, locale_compare, get_images_filter +from openlp.core.utils import AppLocation, delete_file, get_local_key, get_images_filter from openlp.plugins.images.forms import AddGroupForm, ChooseGroupForm from openlp.plugins.images.lib.db import ImageFilenames, ImageGroups @@ -255,7 +255,7 @@ class ImageMediaItem(MediaManagerItem): The ID of the group that will be added recursively """ image_groups = self.manager.get_all_objects(ImageGroups, ImageGroups.parent_id == parent_group_id) - image_groups.sort(cmp=locale_compare, key=lambda group_object: group_object.group_name) + image_groups.sort(key=lambda group_object: get_local_key(group_object.group_name)) folder_icon = build_icon(u':/images/image_group.png') for image_group in image_groups: group = QtGui.QTreeWidgetItem() @@ -286,7 +286,7 @@ class ImageMediaItem(MediaManagerItem): combobox.clear() combobox.top_level_group_added = False image_groups = self.manager.get_all_objects(ImageGroups, ImageGroups.parent_id == parent_group_id) - image_groups.sort(cmp=locale_compare, key=lambda group_object: group_object.group_name) + image_groups.sort(key=lambda group_object: get_local_key(group_object.group_name)) for image_group in image_groups: combobox.addItem(prefix + image_group.group_name, image_group.id) self.fill_groups_combobox(combobox, image_group.id, prefix + ' ') @@ -338,7 +338,7 @@ class ImageMediaItem(MediaManagerItem): self.expand_group(open_group.id) # Sort the images by its filename considering language specific # characters. - images.sort(cmp=locale_compare, key=lambda image_object: os.path.split(unicode(image_object.filename))[1]) + images.sort(key=lambda image_object: get_local_key(os.path.split(unicode(image_object.filename))[1])) for imageFile in images: log.debug(u'Loading image: %s', imageFile.filename) filename = os.path.split(imageFile.filename)[1] @@ -525,9 +525,9 @@ class ImageMediaItem(MediaManagerItem): group_items.append(item) if isinstance(item.data(0, QtCore.Qt.UserRole), ImageFilenames): image_items.append(item) - group_items.sort(cmp=locale_compare, key=lambda item: item.text(0)) + group_items.sort(key=lambda item: get_local_key(item.text(0))) target_group.addChildren(group_items) - image_items.sort(cmp=locale_compare, key=lambda item: item.text(0)) + image_items.sort(key=lambda item: get_local_key(item.text(0))) target_group.addChildren(image_items) def generate_slide_data(self, service_item, item=None, xmlVersion=False, diff --git a/openlp/plugins/media/lib/mediaitem.py b/openlp/plugins/media/lib/mediaitem.py index 57bc6947b..26cb35cd2 100644 --- a/openlp/plugins/media/lib/mediaitem.py +++ b/openlp/plugins/media/lib/mediaitem.py @@ -37,7 +37,7 @@ from openlp.core.lib import ItemCapabilities, MediaManagerItem,MediaType, Regist from openlp.core.lib.ui import critical_error_message_box, create_horizontal_adjusting_combo_box from openlp.core.ui import DisplayController, Display, DisplayControllerType from openlp.core.ui.media import get_media_players, set_media_players -from openlp.core.utils import AppLocation, locale_compare +from openlp.core.utils import AppLocation, get_local_key log = logging.getLogger(__name__) @@ -261,7 +261,7 @@ class MediaMediaItem(MediaManagerItem): def load_list(self, media, target_group=None): # Sort the media by its filename considering language specific # characters. - media.sort(cmp=locale_compare, key=lambda filename: os.path.split(unicode(filename))[1]) + media.sort(key=lambda filename: get_local_key(os.path.split(unicode(filename))[1])) for track in media: track_info = QtCore.QFileInfo(track) if not os.path.exists(track): @@ -287,7 +287,7 @@ class MediaMediaItem(MediaManagerItem): def getList(self, type=MediaType.Audio): media = Settings().value(self.settings_section + u'/media files') - media.sort(cmp=locale_compare, key=lambda filename: os.path.split(unicode(filename))[1]) + media.sort(key=lambda filename: get_local_key(os.path.split(unicode(filename))[1])) ext = [] if type == MediaType.Audio: ext = self.media_controller.audio_extensions_list diff --git a/openlp/plugins/presentations/lib/mediaitem.py b/openlp/plugins/presentations/lib/mediaitem.py index f92562541..a87821e8c 100644 --- a/openlp/plugins/presentations/lib/mediaitem.py +++ b/openlp/plugins/presentations/lib/mediaitem.py @@ -35,7 +35,7 @@ from PyQt4 import QtCore, QtGui from openlp.core.lib import MediaManagerItem, Registry, ItemCapabilities, ServiceItemContext, Settings, UiStrings, \ build_icon, check_item_selected, create_thumb, translate, validate_thumb from openlp.core.lib.ui import critical_error_message_box, create_horizontal_adjusting_combo_box -from openlp.core.utils import locale_compare +from openlp.core.utils import get_local_key from openlp.plugins.presentations.lib import MessageListener log = logging.getLogger(__name__) @@ -153,8 +153,7 @@ class PresentationMediaItem(MediaManagerItem): if not initialLoad: self.main_window.display_progress_bar(len(files)) # Sort the presentations by its filename considering language specific characters. - files.sort(cmp=locale_compare, - key=lambda filename: os.path.split(unicode(filename))[1]) + files.sort(key=lambda filename: get_local_key(os.path.split(unicode(filename))[1])) for file in files: if not initialLoad: self.main_window.increment_progress_bar() diff --git a/openlp/plugins/songs/forms/songexportform.py b/openlp/plugins/songs/forms/songexportform.py index 79f21a454..94748d3e4 100644 --- a/openlp/plugins/songs/forms/songexportform.py +++ b/openlp/plugins/songs/forms/songexportform.py @@ -37,7 +37,7 @@ from PyQt4 import QtCore, QtGui from openlp.core.lib import Registry, UiStrings, create_separated_list, build_icon, translate from openlp.core.lib.ui import critical_error_message_box from openlp.core.ui.wizard import OpenLPWizard, WizardStrings -from openlp.plugins.songs.lib import natcmp +from openlp.core.utils import get_natural_key from openlp.plugins.songs.lib.db import Song from openlp.plugins.songs.lib.openlyricsexport import OpenLyricsExport @@ -222,7 +222,7 @@ class SongExportForm(OpenLPWizard): # Load the list of songs. self.application.set_busy_cursor() songs = self.plugin.manager.get_all_objects(Song) - songs.sort(cmp=natcmp, key=lambda song: song.sort_key) + songs.sort(key=lambda song: get_natural_key(song.sort_key)) for song in songs: # No need to export temporary songs. if song.temporary: diff --git a/openlp/plugins/songs/lib/__init__.py b/openlp/plugins/songs/lib/__init__.py index 5c1485b9e..abeec1253 100644 --- a/openlp/plugins/songs/lib/__init__.py +++ b/openlp/plugins/songs/lib/__init__.py @@ -34,7 +34,7 @@ import re from PyQt4 import QtGui from openlp.core.lib import translate -from openlp.core.utils import CONTROL_CHARS, locale_direct_compare +from openlp.core.utils import CONTROL_CHARS from db import Author from ui import SongStrings @@ -592,37 +592,3 @@ def strip_rtf(text, default_encoding=None): text = u''.join(out) return text, default_encoding - -def natcmp(a, b): - """ - Natural string comparison which mimics the behaviour of Python's internal cmp function. - """ - if len(a) <= len(b): - for i, key in enumerate(a): - if isinstance(key, int) and isinstance(b[i], int): - result = cmp(key, b[i]) - elif isinstance(key, int) and not isinstance(b[i], int): - result = locale_direct_compare(str(key), b[i]) - elif not isinstance(key, int) and isinstance(b[i], int): - result = locale_direct_compare(key, str(b[i])) - else: - result = locale_direct_compare(key, b[i]) - if result != 0: - return result - if len(a) == len(b): - return 0 - else: - return -1 - else: - for i, key in enumerate(b): - if isinstance(a[i], int) and isinstance(key, int): - result = cmp(a[i], key) - elif isinstance(a[i], int) and not isinstance(key, int): - result = locale_direct_compare(str(a[i]), key) - elif not isinstance(a[i], int) and isinstance(key, int): - result = locale_direct_compare(a[i], str(key)) - else: - result = locale_direct_compare(a[i], key) - if result != 0: - return result - return 1 diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index 0c4898fd9..81aab4d0f 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -38,12 +38,12 @@ from sqlalchemy.sql import or_ from openlp.core.lib import Registry, MediaManagerItem, ItemCapabilities, PluginStatus, ServiceItemContext, Settings, \ UiStrings, translate, check_item_selected, create_separated_list, check_directory_exists from openlp.core.lib.ui import create_widget_action -from openlp.core.utils import AppLocation +from openlp.core.utils import AppLocation, get_natural_key from openlp.plugins.songs.forms.editsongform import EditSongForm from openlp.plugins.songs.forms.songmaintenanceform import SongMaintenanceForm from openlp.plugins.songs.forms.songimportform import SongImportForm from openlp.plugins.songs.forms.songexportform import SongExportForm -from openlp.plugins.songs.lib import VerseType, clean_string, natcmp +from openlp.plugins.songs.lib import VerseType, clean_string from openlp.plugins.songs.lib.db import Author, Song, Book, MediaFile from openlp.plugins.songs.lib.ui import SongStrings from openlp.plugins.songs.lib.xml import OpenLyrics, SongXML @@ -225,7 +225,7 @@ class SongMediaItem(MediaManagerItem): log.debug(u'display results Song') self.save_auto_select_id() self.list_view.clear() - searchresults.sort(cmp=natcmp, key=lambda song: song.sort_key) + searchresults.sort(key=lambda song: get_natural_key(song.sort_key)) for song in searchresults: # Do not display temporary songs if song.temporary: diff --git a/scripts/check_dependencies.py b/scripts/check_dependencies.py index 4c0f69b91..a6e075db4 100755 --- a/scripts/check_dependencies.py +++ b/scripts/check_dependencies.py @@ -83,6 +83,7 @@ MODULES = [ 'mako', 'migrate', 'uno', + 'icu', ] From 3686da6a2eea5e1803094d70064bc0d3dd6714b6 Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Sun, 31 Mar 2013 11:13:56 +0100 Subject: [PATCH 12/25] Changed regex string type to raw --- openlp/plugins/songs/lib/songshowplusimport.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/songs/lib/songshowplusimport.py b/openlp/plugins/songs/lib/songshowplusimport.py index 14631ebc2..a72f83c4f 100644 --- a/openlp/plugins/songs/lib/songshowplusimport.py +++ b/openlp/plugins/songs/lib/songshowplusimport.py @@ -180,7 +180,7 @@ class SongShowPlusImport(SongImport): def to_openlp_verse_tag(self, verse_name, ignore_unique=False): # Have we got any digits? If so, verse number is everything from the digits to the end (OpenLP does not have # concept of part verses, so just ignore any non integers on the end (including floats)) - match = re.match(u'(\D*)(\d+)', verse_name) + match = re.match(r'(\D*)(\d+)', verse_name) if match: verse_type = match.group(1).strip() verse_number = match.group(2) From 118f295204a1c8548c31919c0cce31610b382a4b Mon Sep 17 00:00:00 2001 From: M2j Date: Sun, 31 Mar 2013 12:31:54 +0200 Subject: [PATCH 13/25] Precompute the whole comparision key for song sorting. --- openlp/core/utils/__init__.py | 29 ++++++++------------ openlp/plugins/songs/forms/songexportform.py | 3 +- openlp/plugins/songs/lib/db.py | 28 +++---------------- openlp/plugins/songs/lib/mediaitem.py | 4 +-- 4 files changed, 18 insertions(+), 46 deletions(-) diff --git a/openlp/core/utils/__init__.py b/openlp/core/utils/__init__.py index 625bd25b5..ba7e4dd25 100644 --- a/openlp/core/utils/__init__.py +++ b/openlp/core/utils/__init__.py @@ -398,25 +398,18 @@ def get_local_key(string): def get_natural_key(string): """ - Generate a key for locale aware natural string sorting. Returns a list of strings and integers. - - ``string`` - A string, list or tuple which represents the item string. + Generate a key for locale aware natural string sorting. + Returns a list of string compare keys and integers. """ - if isinstance(string, basestring): - string = re.findall(r'(\d+|\D+)', string) - if len(string) == 1: - return list(get_local_key(string[0])) - elif isinstance(string, tuple): - string = list(string) - if isinstance(string, list): - for index, part in enumerate(string): - if isinstance(part, basestring): - if part.isdigit(): - string[index] = int(part) - else: - string[index] = get_local_key(part) - return string + key = re.findall(r'(\d+|\D+)', string) + if len(key) == 1: + return list(get_local_key(string)) + for index, part in enumerate(key): + if part.isdigit(): + key[index] = int(part) + else: + key[index] = get_local_key(part) + return key from applocation import AppLocation diff --git a/openlp/plugins/songs/forms/songexportform.py b/openlp/plugins/songs/forms/songexportform.py index 94748d3e4..f0554f588 100644 --- a/openlp/plugins/songs/forms/songexportform.py +++ b/openlp/plugins/songs/forms/songexportform.py @@ -37,7 +37,6 @@ from PyQt4 import QtCore, QtGui from openlp.core.lib import Registry, UiStrings, create_separated_list, build_icon, translate from openlp.core.lib.ui import critical_error_message_box from openlp.core.ui.wizard import OpenLPWizard, WizardStrings -from openlp.core.utils import get_natural_key from openlp.plugins.songs.lib.db import Song from openlp.plugins.songs.lib.openlyricsexport import OpenLyricsExport @@ -222,7 +221,7 @@ class SongExportForm(OpenLPWizard): # Load the list of songs. self.application.set_busy_cursor() songs = self.plugin.manager.get_all_objects(Song) - songs.sort(key=lambda song: get_natural_key(song.sort_key)) + songs.sort(key=lambda song: song.sort_key) for song in songs: # No need to export temporary songs. if song.temporary: diff --git a/openlp/plugins/songs/lib/db.py b/openlp/plugins/songs/lib/db.py index db5f59357..015caa87d 100644 --- a/openlp/plugins/songs/lib/db.py +++ b/openlp/plugins/songs/lib/db.py @@ -38,6 +38,7 @@ from sqlalchemy.orm import mapper, relation, reconstructor from sqlalchemy.sql.expression import func from openlp.core.lib.db import BaseModel, init_db +from openlp.core.utils import get_natural_key class Author(BaseModel): @@ -69,36 +70,15 @@ class Song(BaseModel): def __init__(self): self.sort_key = () - def _try_int(self, s): - """ - Convert to integer if possible. - """ - try: - return int(s) - except: - return s.lower() - - def _natsort_key(self, s): - """ - Used internally to get a tuple by which s is sorted. - """ - return map(self._try_int, re.findall(r'(\d+|\D+)', s)) - - # This decorator tells sqlalchemy to call this method everytime - # any data on this object is updated. - @reconstructor def init_on_load(self): """ - Precompute a tuple to be used for sorting. + Precompute a natural sorting, locale aware sorting key. Song sorting is performance sensitive operation. - To get maximum speed lets precompute the string - used for comparison. + To get maximum speed lets precompute the sorting key. """ - # Avoid the overhead of converting string to lowercase and to QString - # with every call to sort(). - self.sort_key = self._natsort_key(self.title) + self.sort_key = get_natural_key(self.title) class Topic(BaseModel): diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index 81aab4d0f..d75124d84 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -38,7 +38,7 @@ from sqlalchemy.sql import or_ from openlp.core.lib import Registry, MediaManagerItem, ItemCapabilities, PluginStatus, ServiceItemContext, Settings, \ UiStrings, translate, check_item_selected, create_separated_list, check_directory_exists from openlp.core.lib.ui import create_widget_action -from openlp.core.utils import AppLocation, get_natural_key +from openlp.core.utils import AppLocation from openlp.plugins.songs.forms.editsongform import EditSongForm from openlp.plugins.songs.forms.songmaintenanceform import SongMaintenanceForm from openlp.plugins.songs.forms.songimportform import SongImportForm @@ -225,7 +225,7 @@ class SongMediaItem(MediaManagerItem): log.debug(u'display results Song') self.save_auto_select_id() self.list_view.clear() - searchresults.sort(key=lambda song: get_natural_key(song.sort_key)) + searchresults.sort(key=lambda song: song.sort_key) for song in searchresults: # Do not display temporary songs if song.temporary: From 7bae770458e471c05acd94fd27de78d3b8ad151f Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Sun, 31 Mar 2013 11:47:31 +0100 Subject: [PATCH 14/25] Some test had more than one GIVEN, WHEN, THEN. Fixed that --- .../openlp_plugins/songs/test_songshowplusimport.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py index 24c77d0f3..236c8bcc2 100644 --- a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py +++ b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py @@ -125,7 +125,7 @@ class TestSongShowPlusImport(TestCase): def to_openlp_verse_tag_test(self): """ - Test to_openlp_verse_tag method + Test to_openlp_verse_tag method by simulating adding a verse """ # GIVEN: A mocked out SongImport class, and a mocked out "manager" with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): @@ -150,6 +150,10 @@ class TestSongShowPlusImport(TestCase): u'SongShowPlusImport.to_openlp_verse_tag should return "%s" when called with "%s"' % (openlp_tag, original_tag)) + def to_openlp_verse_tag_verse_order_test(self): + """ + Test to_openlp_verse_tag method by simulating adding a verse to the verse order + """ # GIVEN: A mocked out SongImport class, and a mocked out "manager" with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): mocked_manager = MagicMock() From 8864e1e51b911f1ec2ecafe9dcf8c3c966a2ea6e Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Sun, 31 Mar 2013 11:55:58 +0100 Subject: [PATCH 15/25] Missed a few GIVEN, WHEN, THENS --- .../songs/test_songshowplusimport.py | 37 +++++++------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py index 236c8bcc2..1ae1e16f9 100644 --- a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py +++ b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py @@ -70,9 +70,9 @@ class TestSongShowPlusImport(TestCase): # THEN: The importer object should not be None self.assertIsNotNone(importer, u'Import should not be none') - def import_source_test(self): + def invalid_import_source_test(self): """ - Test SongShowPlusImport.doImport handles different import_source values + Test SongShowPlusImport.doImport handles different invalid import_source values """ # GIVEN: A mocked out SongImport class, and a mocked out "manager" with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): @@ -82,30 +82,19 @@ class TestSongShowPlusImport(TestCase): importer.import_wizard = mocked_import_wizard importer.stop_import_flag = True - # WHEN: Import source is a string - importer.import_source = u'not a list' + # WHEN: Import source is not a list + for source in [u'not a list', 0]: + importer.import_source = source - # THEN: doImport should return none and the progress bar maximum should not be set. - self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') - self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, - u'setMaxium on import_wizard.progress_bar should not have been called') - - # GIVEN: A mocked out SongImport class, and a mocked out "manager" - with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): - mocked_manager = MagicMock() - mocked_import_wizard = MagicMock() - importer = SongShowPlusImport(mocked_manager) - importer.import_wizard = mocked_import_wizard - importer.stop_import_flag = True - - # WHEN: Import source is an int - importer.import_source = 0 - - # THEN: doImport should return none and the progress bar maximum should not be set. - self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') - self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, - u'setMaxium on import_wizard.progress_bar should not have been called') + # THEN: doImport should return none and the progress bar maximum should not be set. + self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') + self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, + u'setMaxium on import_wizard.progress_bar should not have been called') + def valid_import_source_test(self): + """ + Test SongShowPlusImport.doImport handles different invalid import_source values + """ # GIVEN: A mocked out SongImport class, and a mocked out "manager" with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): mocked_manager = MagicMock() From 8696db9f75f018c5c344a208bad6423a371018b4 Mon Sep 17 00:00:00 2001 From: M2j Date: Sun, 31 Mar 2013 18:30:22 +0200 Subject: [PATCH 16/25] - use list compression in get_natural_key - define DIGIT_OR_NONDIGIT --- openlp/core/utils/__init__.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/openlp/core/utils/__init__.py b/openlp/core/utils/__init__.py index ba7e4dd25..739d4ad2e 100644 --- a/openlp/core/utils/__init__.py +++ b/openlp/core/utils/__init__.py @@ -62,6 +62,7 @@ UNO_CONNECTION_TYPE = u'pipe' #UNO_CONNECTION_TYPE = u'socket' CONTROL_CHARS = re.compile(r'[\x00-\x1F\x7F-\x9F]', re.UNICODE) INVALID_FILE_CHARS = re.compile(r'[\\/:\*\?"<>\|\+\[\]%]', re.UNICODE) +DIGITS_OR_NONDIGITS = re.compile(r'\d+|\D+', re.UNICODE) class VersionThread(QtCore.QThread): @@ -401,14 +402,8 @@ def get_natural_key(string): Generate a key for locale aware natural string sorting. Returns a list of string compare keys and integers. """ - key = re.findall(r'(\d+|\D+)', string) - if len(key) == 1: - return list(get_local_key(string)) - for index, part in enumerate(key): - if part.isdigit(): - key[index] = int(part) - else: - key[index] = get_local_key(part) + key = DIGITS_OR_NONDIGITS.findall(string) + key = [int(part) if part.isdigit() else get_local_key(part) for part in key] return key From b3bb9bc0c0c1a5fa38b672b954188a57c6dd27f5 Mon Sep 17 00:00:00 2001 From: M2j Date: Sun, 31 Mar 2013 18:43:08 +0200 Subject: [PATCH 17/25] Python 3 transition comment --- openlp/core/utils/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/openlp/core/utils/__init__.py b/openlp/core/utils/__init__.py index 739d4ad2e..092e67913 100644 --- a/openlp/core/utils/__init__.py +++ b/openlp/core/utils/__init__.py @@ -404,6 +404,9 @@ def get_natural_key(string): """ key = DIGITS_OR_NONDIGITS.findall(string) key = [int(part) if part.isdigit() else get_local_key(part) for part in key] + # Python 3 does not support comparision of different types anymore. So make sure, that we do not compare str and int. + #if string[0].isdigit(): + # return [''] + key return key From 316902fb88c65acce89cbe143e0430419560dff1 Mon Sep 17 00:00:00 2001 From: M2j Date: Tue, 2 Apr 2013 19:51:19 +0200 Subject: [PATCH 18/25] Add unit tests for get_local_key and get_natural_key. --- .../openlp_core_utils/test_utils.py | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/tests/functional/openlp_core_utils/test_utils.py b/tests/functional/openlp_core_utils/test_utils.py index 2e826bc61..ac4da275b 100644 --- a/tests/functional/openlp_core_utils/test_utils.py +++ b/tests/functional/openlp_core_utils/test_utils.py @@ -5,7 +5,7 @@ from unittest import TestCase from mock import patch -from openlp.core.utils import get_filesystem_encoding, _get_frozen_path +from openlp.core.utils import get_filesystem_encoding, _get_frozen_path, get_local_key, get_natural_key class TestUtils(TestCase): """ @@ -56,3 +56,28 @@ class TestUtils(TestCase): # THEN: The frozen parameter is returned assert _get_frozen_path(u'frozen', u'not frozen') == u'frozen', u'Should return "frozen"' + def get_local_key_test(self): + """ + Test the get_local_key(string) function + """ + with patch(u'openlp.core.utils.languagemanager.LanguageManager.get_language') as mocked_get_language: + # GIVEN: The language is German + # 0x00C3 (A with diaresis) should be sorted as "A". 0x00DF (sharp s) should be sorted as "ss". + mocked_get_language.return_value = u'de' + unsorted_list = [u'Auszug', u'Aushang', u'\u00C4u\u00DFerung'] + # WHEN: We sort the list and use get_locale_key() to generate the sorting keys + # THEN: We get a properly sorted list + assert sorted(unsorted_list, key=get_local_key) == [u'Aushang', u'\u00C4u\u00DFerung', u'Auszug'], u'Strings should be sorted properly' + + def get_natural_key_test(self): + """ + Test the get_natural_key(string) function + """ + with patch(u'openlp.core.utils.languagemanager.LanguageManager.get_language') as mocked_get_language: + # GIVEN: The language is English (a language, which sorts digits before letters) + mocked_get_language.return_value = u'en' + unsorted_list = [u'item 10a', u'item 3b', u'1st item'] + # WHEN: We sort the list and use get_natural_key() to generate the sorting keys + # THEN: We get a properly sorted list + assert sorted(unsorted_list, key=get_natural_key) == [u'1st item', u'item 3b', u'item 10a'], u'Numbers should be sortet naturally' + From 9612a52211e32a3fda02965eb3898232d1fc2f60 Mon Sep 17 00:00:00 2001 From: M2j Date: Tue, 2 Apr 2013 19:54:23 +0200 Subject: [PATCH 19/25] typo --- tests/functional/openlp_core_utils/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/openlp_core_utils/test_utils.py b/tests/functional/openlp_core_utils/test_utils.py index ac4da275b..1546e9a7b 100644 --- a/tests/functional/openlp_core_utils/test_utils.py +++ b/tests/functional/openlp_core_utils/test_utils.py @@ -79,5 +79,5 @@ class TestUtils(TestCase): unsorted_list = [u'item 10a', u'item 3b', u'1st item'] # WHEN: We sort the list and use get_natural_key() to generate the sorting keys # THEN: We get a properly sorted list - assert sorted(unsorted_list, key=get_natural_key) == [u'1st item', u'item 3b', u'item 10a'], u'Numbers should be sortet naturally' + assert sorted(unsorted_list, key=get_natural_key) == [u'1st item', u'item 3b', u'item 10a'], u'Numbers should be sorted naturally' From 2598674d2a2b6c23a85605e888a57e1a23553005 Mon Sep 17 00:00:00 2001 From: M2j Date: Tue, 2 Apr 2013 22:52:31 +0200 Subject: [PATCH 20/25] Add ICU version string to exception form. --- openlp/core/ui/exceptionform.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/openlp/core/ui/exceptionform.py b/openlp/core/ui/exceptionform.py index e4854d0c0..6b77a8c6f 100644 --- a/openlp/core/ui/exceptionform.py +++ b/openlp/core/ui/exceptionform.py @@ -71,7 +71,10 @@ except ImportError: MAKO_VERSION = u'-' try: import icu - ICU_VERSION = u'OK' + try: + ICU_VERSION = icu.VERSION + except AttributeError: + ICU_VERSION = u'OK' except ImportError: ICU_VERSION = u'-' try: From c3583bd21dc7b684d25f69b8df2dec164ac122e3 Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Thu, 4 Apr 2013 17:42:22 +0100 Subject: [PATCH 21/25] Added comment --- openlp/plugins/songs/lib/songimport.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/openlp/plugins/songs/lib/songimport.py b/openlp/plugins/songs/lib/songimport.py index a9383c0cb..8886e2884 100644 --- a/openlp/plugins/songs/lib/songimport.py +++ b/openlp/plugins/songs/lib/songimport.py @@ -260,6 +260,8 @@ class SongImport(QtCore.QObject): elif int(verse_def[1:]) > self.verseCounts[verse_def[0]]: self.verseCounts[verse_def[0]] = int(verse_def[1:]) self.verses.append([verse_def, verse_text.rstrip(), lang]) + # A verse_def refers to all verses with that name, adding it once adds every instance, so do not add if already + # used. if verse_def not in self.verseOrderListGenerated: self.verseOrderListGenerated.append(verse_def) From 042bf4ed2d6567b3e4c64a0bd1ba3f9e12591217 Mon Sep 17 00:00:00 2001 From: M2j Date: Fri, 5 Apr 2013 19:41:01 +0200 Subject: [PATCH 22/25] rename openlp.core.utils.get_local_key to openlp.core.utils.get_locale_key --- openlp/core/ui/thememanager.py | 4 ++-- openlp/core/utils/__init__.py | 6 +++--- openlp/plugins/bibles/forms/bibleimportform.py | 4 ++-- openlp/plugins/bibles/lib/mediaitem.py | 6 +++--- openlp/plugins/custom/lib/db.py | 6 +++--- openlp/plugins/images/lib/mediaitem.py | 12 ++++++------ openlp/plugins/media/lib/mediaitem.py | 6 +++--- openlp/plugins/presentations/lib/mediaitem.py | 4 ++-- tests/interfaces/openlp_plugins/__init__.pyc | Bin 184 -> 166 bytes 9 files changed, 24 insertions(+), 24 deletions(-) diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 53d4a513b..be0e3bfa1 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -44,7 +44,7 @@ from openlp.core.lib.theme import ThemeXML, BackgroundType, VerticalType, Backgr from openlp.core.lib.ui import critical_error_message_box, create_widget_action from openlp.core.theme import Theme from openlp.core.ui import FileRenameForm, ThemeForm -from openlp.core.utils import AppLocation, delete_file, get_local_key, get_filesystem_encoding +from openlp.core.utils import AppLocation, delete_file, get_locale_key, get_filesystem_encoding log = logging.getLogger(__name__) @@ -418,7 +418,7 @@ class ThemeManager(QtGui.QWidget): self.theme_list_widget.clear() files = AppLocation.get_files(self.settings_section, u'.png') # Sort the themes by its name considering language specific - files.sort(key=lambda file_name: get_local_key(unicode(file_name))) + files.sort(key=lambda file_name: get_locale_key(unicode(file_name))) # now process the file list of png files for name in files: # check to see file is in theme root directory diff --git a/openlp/core/utils/__init__.py b/openlp/core/utils/__init__.py index 092e67913..0e48f2fa5 100644 --- a/openlp/core/utils/__init__.py +++ b/openlp/core/utils/__init__.py @@ -382,7 +382,7 @@ def format_time(text, local_time): return re.sub('\%[a-zA-Z]', match_formatting, text) -def get_local_key(string): +def get_locale_key(string): """ Creates a key for case insensitive, locale aware string sorting. """ @@ -403,7 +403,7 @@ def get_natural_key(string): Returns a list of string compare keys and integers. """ key = DIGITS_OR_NONDIGITS.findall(string) - key = [int(part) if part.isdigit() else get_local_key(part) for part in key] + key = [int(part) if part.isdigit() else get_locale_key(part) for part in key] # Python 3 does not support comparision of different types anymore. So make sure, that we do not compare str and int. #if string[0].isdigit(): # return [''] + key @@ -417,4 +417,4 @@ from actions import ActionList __all__ = [u'AppLocation', u'ActionList', u'LanguageManager', u'get_application_version', u'check_latest_version', u'add_actions', u'get_filesystem_encoding', u'get_web_page', u'get_uno_command', u'get_uno_instance', - u'delete_file', u'clean_filename', u'format_time', u'get_local_key', u'get_natural_key'] + u'delete_file', u'clean_filename', u'format_time', u'get_locale_key', u'get_natural_key'] diff --git a/openlp/plugins/bibles/forms/bibleimportform.py b/openlp/plugins/bibles/forms/bibleimportform.py index 319989433..f8d771e77 100644 --- a/openlp/plugins/bibles/forms/bibleimportform.py +++ b/openlp/plugins/bibles/forms/bibleimportform.py @@ -38,7 +38,7 @@ from openlp.core.lib import Settings, UiStrings, translate from openlp.core.lib.db import delete_database from openlp.core.lib.ui import critical_error_message_box from openlp.core.ui.wizard import OpenLPWizard, WizardStrings -from openlp.core.utils import AppLocation, get_local_key +from openlp.core.utils import AppLocation, get_locale_key from openlp.plugins.bibles.lib.manager import BibleFormat from openlp.plugins.bibles.lib.db import BiblesResourcesDB, clean_filename @@ -455,7 +455,7 @@ class BibleImportForm(OpenLPWizard): """ self.webTranslationComboBox.clear() bibles = self.web_bible_list[index].keys() - bibles.sort(key=get_local_key) + bibles.sort(key=get_locale_key) self.webTranslationComboBox.addItems(bibles) def onOsisBrowseButtonClicked(self): diff --git a/openlp/plugins/bibles/lib/mediaitem.py b/openlp/plugins/bibles/lib/mediaitem.py index 4ae7e76b5..86a507612 100644 --- a/openlp/plugins/bibles/lib/mediaitem.py +++ b/openlp/plugins/bibles/lib/mediaitem.py @@ -36,7 +36,7 @@ from openlp.core.lib import Registry, MediaManagerItem, ItemCapabilities, Servic from openlp.core.lib.searchedit import SearchEdit from openlp.core.lib.ui import set_case_insensitive_completer, create_horizontal_adjusting_combo_box, \ critical_error_message_box, find_and_set_in_combo_box, build_icon -from openlp.core.utils import get_local_key +from openlp.core.utils import get_locale_key from openlp.plugins.bibles.forms import BibleImportForm, EditBibleForm from openlp.plugins.bibles.lib import LayoutStyle, DisplayStyle, VerseReferenceList, get_reference_separator, \ LanguageSelection, BibleStrings @@ -325,7 +325,7 @@ class BibleMediaItem(MediaManagerItem): # Get all bibles and sort the list. bibles = self.plugin.manager.get_bibles().keys() bibles = filter(None, bibles) - bibles.sort(key=get_local_key) + bibles.sort(key=get_locale_key) # Load the bibles into the combo boxes. self.quickVersionComboBox.addItems(bibles) self.quickSecondComboBox.addItems(bibles) @@ -461,7 +461,7 @@ class BibleMediaItem(MediaManagerItem): for book in book_data: data = BiblesResourcesDB.get_book_by_id(book.book_reference_id) books.append(data[u'name'] + u' ') - books.sort(key=get_local_key) + books.sort(key=get_locale_key) set_case_insensitive_completer(books, self.quickSearchEdit) def on_import_click(self): diff --git a/openlp/plugins/custom/lib/db.py b/openlp/plugins/custom/lib/db.py index ad876b6b6..253ca5432 100644 --- a/openlp/plugins/custom/lib/db.py +++ b/openlp/plugins/custom/lib/db.py @@ -35,7 +35,7 @@ from sqlalchemy import Column, Table, types from sqlalchemy.orm import mapper from openlp.core.lib.db import BaseModel, init_db -from openlp.core.utils import get_local_key +from openlp.core.utils import get_locale_key class CustomSlide(BaseModel): """ @@ -44,10 +44,10 @@ class CustomSlide(BaseModel): # By default sort the customs by its title considering language specific # characters. def __lt__(self, other): - return get_local_key(self.title) < get_local_key(other.title) + return get_locale_key(self.title) < get_locale_key(other.title) def __eq__(self, other): - return get_local_key(self.title) == get_local_key(other.title) + return get_locale_key(self.title) == get_locale_key(other.title) def init_schema(url): diff --git a/openlp/plugins/images/lib/mediaitem.py b/openlp/plugins/images/lib/mediaitem.py index 8c6bc8d9f..fc575ec0a 100644 --- a/openlp/plugins/images/lib/mediaitem.py +++ b/openlp/plugins/images/lib/mediaitem.py @@ -36,7 +36,7 @@ from openlp.core.lib import ItemCapabilities, MediaManagerItem, Registry, Servic StringContent, TreeWidgetWithDnD, UiStrings, build_icon, check_directory_exists, check_item_selected, \ create_thumb, translate, validate_thumb from openlp.core.lib.ui import create_widget_action, critical_error_message_box -from openlp.core.utils import AppLocation, delete_file, get_local_key, get_images_filter +from openlp.core.utils import AppLocation, delete_file, get_locale_key, get_images_filter from openlp.plugins.images.forms import AddGroupForm, ChooseGroupForm from openlp.plugins.images.lib.db import ImageFilenames, ImageGroups @@ -255,7 +255,7 @@ class ImageMediaItem(MediaManagerItem): The ID of the group that will be added recursively """ image_groups = self.manager.get_all_objects(ImageGroups, ImageGroups.parent_id == parent_group_id) - image_groups.sort(key=lambda group_object: get_local_key(group_object.group_name)) + image_groups.sort(key=lambda group_object: get_locale_key(group_object.group_name)) folder_icon = build_icon(u':/images/image_group.png') for image_group in image_groups: group = QtGui.QTreeWidgetItem() @@ -286,7 +286,7 @@ class ImageMediaItem(MediaManagerItem): combobox.clear() combobox.top_level_group_added = False image_groups = self.manager.get_all_objects(ImageGroups, ImageGroups.parent_id == parent_group_id) - image_groups.sort(key=lambda group_object: get_local_key(group_object.group_name)) + image_groups.sort(key=lambda group_object: get_locale_key(group_object.group_name)) for image_group in image_groups: combobox.addItem(prefix + image_group.group_name, image_group.id) self.fill_groups_combobox(combobox, image_group.id, prefix + ' ') @@ -338,7 +338,7 @@ class ImageMediaItem(MediaManagerItem): self.expand_group(open_group.id) # Sort the images by its filename considering language specific # characters. - images.sort(key=lambda image_object: get_local_key(os.path.split(unicode(image_object.filename))[1])) + images.sort(key=lambda image_object: get_locale_key(os.path.split(unicode(image_object.filename))[1])) for imageFile in images: log.debug(u'Loading image: %s', imageFile.filename) filename = os.path.split(imageFile.filename)[1] @@ -525,9 +525,9 @@ class ImageMediaItem(MediaManagerItem): group_items.append(item) if isinstance(item.data(0, QtCore.Qt.UserRole), ImageFilenames): image_items.append(item) - group_items.sort(key=lambda item: get_local_key(item.text(0))) + group_items.sort(key=lambda item: get_locale_key(item.text(0))) target_group.addChildren(group_items) - image_items.sort(key=lambda item: get_local_key(item.text(0))) + image_items.sort(key=lambda item: get_locale_key(item.text(0))) target_group.addChildren(image_items) def generate_slide_data(self, service_item, item=None, xmlVersion=False, diff --git a/openlp/plugins/media/lib/mediaitem.py b/openlp/plugins/media/lib/mediaitem.py index 26cb35cd2..7d492bc69 100644 --- a/openlp/plugins/media/lib/mediaitem.py +++ b/openlp/plugins/media/lib/mediaitem.py @@ -37,7 +37,7 @@ from openlp.core.lib import ItemCapabilities, MediaManagerItem,MediaType, Regist from openlp.core.lib.ui import critical_error_message_box, create_horizontal_adjusting_combo_box from openlp.core.ui import DisplayController, Display, DisplayControllerType from openlp.core.ui.media import get_media_players, set_media_players -from openlp.core.utils import AppLocation, get_local_key +from openlp.core.utils import AppLocation, get_locale_key log = logging.getLogger(__name__) @@ -261,7 +261,7 @@ class MediaMediaItem(MediaManagerItem): def load_list(self, media, target_group=None): # Sort the media by its filename considering language specific # characters. - media.sort(key=lambda filename: get_local_key(os.path.split(unicode(filename))[1])) + media.sort(key=lambda filename: get_locale_key(os.path.split(unicode(filename))[1])) for track in media: track_info = QtCore.QFileInfo(track) if not os.path.exists(track): @@ -287,7 +287,7 @@ class MediaMediaItem(MediaManagerItem): def getList(self, type=MediaType.Audio): media = Settings().value(self.settings_section + u'/media files') - media.sort(key=lambda filename: get_local_key(os.path.split(unicode(filename))[1])) + media.sort(key=lambda filename: get_locale_key(os.path.split(unicode(filename))[1])) ext = [] if type == MediaType.Audio: ext = self.media_controller.audio_extensions_list diff --git a/openlp/plugins/presentations/lib/mediaitem.py b/openlp/plugins/presentations/lib/mediaitem.py index a87821e8c..52dcd891f 100644 --- a/openlp/plugins/presentations/lib/mediaitem.py +++ b/openlp/plugins/presentations/lib/mediaitem.py @@ -35,7 +35,7 @@ from PyQt4 import QtCore, QtGui from openlp.core.lib import MediaManagerItem, Registry, ItemCapabilities, ServiceItemContext, Settings, UiStrings, \ build_icon, check_item_selected, create_thumb, translate, validate_thumb from openlp.core.lib.ui import critical_error_message_box, create_horizontal_adjusting_combo_box -from openlp.core.utils import get_local_key +from openlp.core.utils import get_locale_key from openlp.plugins.presentations.lib import MessageListener log = logging.getLogger(__name__) @@ -153,7 +153,7 @@ class PresentationMediaItem(MediaManagerItem): if not initialLoad: self.main_window.display_progress_bar(len(files)) # Sort the presentations by its filename considering language specific characters. - files.sort(key=lambda filename: get_local_key(os.path.split(unicode(filename))[1])) + files.sort(key=lambda filename: get_locale_key(os.path.split(unicode(filename))[1])) for file in files: if not initialLoad: self.main_window.increment_progress_bar() diff --git a/tests/interfaces/openlp_plugins/__init__.pyc b/tests/interfaces/openlp_plugins/__init__.pyc index 0d24c9eff54ac7d86f14523ef46194c4bbc24cad..41b2c438107d1adf4a5d502219772f510965f920 100644 GIT binary patch delta 58 zcmdnNxQvmV`7~wiuL^qQuBNQ e^oxKJ`MJ79sp& Date: Fri, 5 Apr 2013 19:26:39 +0100 Subject: [PATCH 23/25] Removed the use of the mock's call helper object --- .../openlp_plugins/songs/test_songshowplusimport.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py index 1ae1e16f9..86d77bbdc 100644 --- a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py +++ b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py @@ -4,7 +4,7 @@ This module contains tests for the SongShow Plus song importer. import os from unittest import TestCase -from mock import call, patch, MagicMock +from mock import patch, MagicMock from openlp.plugins.songs.lib import VerseType from openlp.plugins.songs.lib.songshowplusimport import SongShowPlusImport @@ -195,11 +195,10 @@ class TestSongShowPlusImport(TestCase): # WHEN: Importing each file importer.import_source = [os.path.join(TEST_PATH, song_file)] title = SONG_TEST_DATA[song_file][u'title'] - parse_author_calls = [call(author) for author in SONG_TEST_DATA[song_file][u'authors']] + 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 = \ - [call(verse_text, verse_tag) for verse_text, verse_tag in SONG_TEST_DATA[song_file][u'verses']] + 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'] @@ -210,13 +209,15 @@ class TestSongShowPlusImport(TestCase): # 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)) - mocked_parse_author.assert_has_calls(parse_author_calls) + 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)) - mocked_add_verse.assert_has_calls(add_verse_calls) + 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: From 6d5590d7396111d96bc6359a688fbb7fc5239121 Mon Sep 17 00:00:00 2001 From: M2j Date: Sat, 6 Apr 2013 19:17:59 +0200 Subject: [PATCH 24/25] removed tests/interfaces/openlp_plugins/__init__.pyc --- tests/interfaces/openlp_plugins/__init__.pyc | Bin 166 -> 0 bytes 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 tests/interfaces/openlp_plugins/__init__.pyc diff --git a/tests/interfaces/openlp_plugins/__init__.pyc b/tests/interfaces/openlp_plugins/__init__.pyc deleted file mode 100644 index 41b2c438107d1adf4a5d502219772f510965f920..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 166 zcmZSn%*(|R7ao|*00oRd+5w1*S%5?e14FO|NW@PANHCxg#r{As{fzwFRQ=r4%)Hd1 z68*puegA^gJf8slq|$U Date: Sat, 6 Apr 2013 20:21:23 +0200 Subject: [PATCH 25/25] renameing openlp.core.utils.get_local_key in tests --- tests/functional/openlp_core_utils/test_utils.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/functional/openlp_core_utils/test_utils.py b/tests/functional/openlp_core_utils/test_utils.py index 1546e9a7b..b9decb37e 100644 --- a/tests/functional/openlp_core_utils/test_utils.py +++ b/tests/functional/openlp_core_utils/test_utils.py @@ -5,7 +5,7 @@ from unittest import TestCase from mock import patch -from openlp.core.utils import get_filesystem_encoding, _get_frozen_path, get_local_key, get_natural_key +from openlp.core.utils import get_filesystem_encoding, _get_frozen_path, get_locale_key, get_natural_key class TestUtils(TestCase): """ @@ -56,9 +56,9 @@ class TestUtils(TestCase): # THEN: The frozen parameter is returned assert _get_frozen_path(u'frozen', u'not frozen') == u'frozen', u'Should return "frozen"' - def get_local_key_test(self): + def get_locale_key_test(self): """ - Test the get_local_key(string) function + Test the get_locale_key(string) function """ with patch(u'openlp.core.utils.languagemanager.LanguageManager.get_language') as mocked_get_language: # GIVEN: The language is German @@ -67,7 +67,8 @@ class TestUtils(TestCase): unsorted_list = [u'Auszug', u'Aushang', u'\u00C4u\u00DFerung'] # WHEN: We sort the list and use get_locale_key() to generate the sorting keys # THEN: We get a properly sorted list - assert sorted(unsorted_list, key=get_local_key) == [u'Aushang', u'\u00C4u\u00DFerung', u'Auszug'], u'Strings should be sorted properly' + test_passes = sorted(unsorted_list, key=get_locale_key) == [u'Aushang', u'\u00C4u\u00DFerung', u'Auszug'] + assert test_passes, u'Strings should be sorted properly' def get_natural_key_test(self): """ @@ -79,5 +80,6 @@ class TestUtils(TestCase): unsorted_list = [u'item 10a', u'item 3b', u'1st item'] # WHEN: We sort the list and use get_natural_key() to generate the sorting keys # THEN: We get a properly sorted list - assert sorted(unsorted_list, key=get_natural_key) == [u'1st item', u'item 3b', u'item 10a'], u'Numbers should be sorted naturally' + test_passes = sorted(unsorted_list, key=get_natural_key) == [u'1st item', u'item 3b', u'item 10a'] + assert test_passes, u'Numbers should be sorted naturally'