Merge lp:~sam92/openlp/bug-1552563-2.4 into lp:openlp/2.4

[Samuel Mehrbrodt]

Fix performance regression with Songbook search

The problem was that in each iteration the database was accessed (the song object).
Fixed this by loading all neccessary information directly in the query.

Also this reintroduces natural sorting.

[Samuel Mehrbrodt (sam92)]

lp:~sam92/openlp/bug-1552563-2.4 (revision 2629)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1510/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1421/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1359/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1155/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/746/
[SUCCESS] https://ci.openlp.io/job/Branch-05a-Code_Analysis/813/
[SUCCESS] https://ci.openlp.io/job/Branch-05b-Test_Coverage/681/

[Raoul Snyman (raoul-snyman)]

review: Approve

[Tim Bentley (trb143)]

review: Approve

=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
--- openlp/plugins/songs/lib/mediaitem.py	2016-01-15 20:37:53 +0000
+++ openlp/plugins/songs/lib/mediaitem.py	2016-04-29 17:55:06 +0000
@@ -21,7 +21,6 @@
 ###############################################################################
 
 import logging
-import re
 import os
 import shutil
 
@@ -32,6 +31,7 @@
 from openlp.core.lib import MediaManagerItem, ItemCapabilities, PluginStatus, ServiceItemContext, \
     check_item_selected, create_separated_list
 from openlp.core.lib.ui import create_widget_action
+from openlp.core.utils import 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
@@ -251,23 +251,23 @@
         self.list_view.clear()
 
         search_keywords = search_keywords.rpartition(' ')
-        search_book = search_keywords[0]
-        search_entry = re.sub(r'[^0-9]', '', search_keywords[2])
-
-        songbook_entries = (self.plugin.manager.session.query(SongBookEntry)
-                            .join(Book)
-                            .order_by(Book.name)
-                            .order_by(SongBookEntry.entry))
-        for songbook_entry in songbook_entries:
-            if songbook_entry.song.temporary:
-                continue
-            if search_book.lower() not in songbook_entry.songbook.name.lower():
-                continue
-            if search_entry not in songbook_entry.entry:
-                continue
-            song_detail = '%s #%s: %s' % (songbook_entry.songbook.name, songbook_entry.entry, songbook_entry.song.title)
+        search_book = search_keywords[0] + '%'
+        search_entry = search_keywords[2] + '%'
+        search_results = (self.plugin.manager.session.query(SongBookEntry.entry, Book.name, Song.title, Song.id)
+                          .join(Song)
+                          .join(Book)
+                          .filter(Book.name.like(search_book), SongBookEntry.entry.like(search_entry),
+                                  Song.temporary.is_(False)).all())
+
+        def get_songbook_key(result):
+            """Get the key to sort by"""
+            return (get_natural_key(result[1]), get_natural_key(result[0]), get_natural_key(result[2]))
+
+        search_results.sort(key=get_songbook_key)
+        for result in search_results:
+            song_detail = '%s #%s: %s' % (result[1], result[0], result[2])
             song_name = QtWidgets.QListWidgetItem(song_detail)
-            song_name.setData(QtCore.Qt.UserRole, songbook_entry.song.id)
+            song_name.setData(QtCore.Qt.UserRole, result[3])
             self.list_view.addItem(song_name)
 
     def on_clear_text_button_click(self):

=== modified file 'tests/functional/openlp_plugins/songs/test_openlpimporter.py'
--- tests/functional/openlp_plugins/songs/test_openlpimporter.py	2016-04-27 18:45:39 +0000
+++ tests/functional/openlp_plugins/songs/test_openlpimporter.py	2016-04-29 17:55:06 +0000
@@ -73,4 +73,3 @@
                 self.assertIsNone(importer.do_import(), 'do_import should return None when import_source is not a list')
                 self.assertEqual(mocked_import_wizard.progress_bar.setMaximum.called, False,
                                  'setMaximum on import_wizard.progress_bar should not have been called')
-