Merge lp:~sam92/openlp/bug-1533081 into lp:openlp

Proposed by Samuel Mehrbrodt
Status: Merged
Merged at revision: 2609
Proposed branch: lp:~sam92/openlp/bug-1533081
Merge into: lp:openlp
Diff against target: 249 lines (+122/-23)
3 files modified
openlp/core/utils/__init__.py (+1/-1)
openlp/plugins/songs/lib/importers/openlp.py (+32/-22)
tests/utils/test_pep8.py (+89/-0)
To merge this branch: bzr merge lp:~sam92/openlp/bug-1533081
Reviewer Review Type Date Requested Status
Raoul Snyman Approve
Tim Bentley Approve
Review via email: mp+282817@code.launchpad.net

Description of the change

* Fix importing older song dbs by migrating them first
* Fix comparing songs without title

lp:~sam92/openlp/bug-1533081 (revision 2615)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1260/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1184/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1123/
[FAILURE] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/959/
Stopping after failure

To post a comment you must log in.
Revision history for this message
Tim Bentley (trb143) :
review: Approve
Revision history for this message
Raoul Snyman (raoul-snyman) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'openlp/core/utils/__init__.py'
--- openlp/core/utils/__init__.py 2015-12-31 22:46:06 +0000
+++ openlp/core/utils/__init__.py 2016-01-15 21:49:07 +0000
@@ -529,7 +529,7 @@
529 key = [int(part) if part.isdigit() else get_locale_key(part) for part in key]529 key = [int(part) if part.isdigit() else get_locale_key(part) for part in key]
530 # Python 3 does not support comparison of different types anymore. So make sure, that we do not compare str530 # Python 3 does not support comparison of different types anymore. So make sure, that we do not compare str
531 # and int.531 # and int.
532 if string[0].isdigit():532 if string and string[0].isdigit():
533 return [b''] + key533 return [b''] + key
534 return key534 return key
535535
536536
=== modified file 'openlp/plugins/songs/lib/importers/openlp.py'
--- openlp/plugins/songs/lib/importers/openlp.py 2015-12-31 22:46:06 +0000
+++ openlp/plugins/songs/lib/importers/openlp.py 2016-01-15 21:49:07 +0000
@@ -24,15 +24,18 @@
24song databases into the current installation database.24song databases into the current installation database.
25"""25"""
26import logging26import logging
27import os
28import shutil
29import tempfile
2730
28from sqlalchemy import create_engine, MetaData, Table31from sqlalchemy import create_engine, MetaData, Table
29from sqlalchemy.orm import class_mapper, mapper, relation, scoped_session, sessionmaker32from sqlalchemy.orm import class_mapper, mapper, relation, scoped_session, sessionmaker
30from sqlalchemy.orm.exc import UnmappedClassError33from sqlalchemy.orm.exc import UnmappedClassError
3134
32from openlp.core.common import translate35from openlp.core.common import translate
33from openlp.core.lib.db import BaseModel36from openlp.core.lib.db import BaseModel, upgrade_db
34from openlp.core.ui.wizard import WizardStrings37from openlp.core.ui.wizard import WizardStrings
35from openlp.plugins.songs.lib import clean_song38from openlp.plugins.songs.lib import clean_song, upgrade
36from openlp.plugins.songs.lib.db import Author, Book, Song, Topic, MediaFile39from openlp.plugins.songs.lib.db import Author, Book, Song, Topic, MediaFile
37from .songimport import SongImport40from .songimport import SongImport
3841
@@ -67,12 +70,6 @@
67 """70 """
68 pass71 pass
6972
70 class OldBook(BaseModel):
71 """
72 Book model
73 """
74 pass
75
76 class OldMediaFile(BaseModel):73 class OldMediaFile(BaseModel):
77 """74 """
78 MediaFile model75 MediaFile model
@@ -91,26 +88,38 @@
91 """88 """
92 pass89 pass
9390
91 class OldSongBookEntry(BaseModel):
92 """
93 SongbookEntry model
94 """
95 pass
96
94 # Check the file type97 # Check the file type
95 if not self.import_source.endswith('.sqlite'):98 if not self.import_source.endswith('.sqlite'):
96 self.log_error(self.import_source, translate('SongsPlugin.OpenLPSongImport',99 self.log_error(self.import_source, translate('SongsPlugin.OpenLPSongImport',
97 'Not a valid OpenLP 2 song database.'))100 'Not a valid OpenLP 2 song database.'))
98 return101 return
99 self.import_source = 'sqlite:///%s' % self.import_source102 # Operate on a copy - We might need to apply migrations to the db and don't want to change the existing db
103 import_db = os.path.join(tempfile.gettempdir(), os.path.basename(self.import_source))
104 shutil.copy(self.import_source, import_db)
105 import_db_url = 'sqlite:///%s' % import_db
106 # Apply migrations
107 upgrade_db(import_db_url, upgrade)
100 # Load the db file108 # Load the db file
101 engine = create_engine(self.import_source)109 engine = create_engine(import_db_url)
102 source_meta = MetaData()110 source_meta = MetaData()
103 source_meta.reflect(engine)111 source_meta.reflect(engine)
104 self.source_session = scoped_session(sessionmaker(bind=engine))112 self.source_session = scoped_session(sessionmaker(bind=engine))
113
105 if 'media_files' in list(source_meta.tables.keys()):114 if 'media_files' in list(source_meta.tables.keys()):
106 has_media_files = True115 has_media_files = True
107 else:116 else:
108 has_media_files = False117 has_media_files = False
109 source_authors_table = source_meta.tables['authors']118 source_authors_table = source_meta.tables['authors']
110 source_song_books_table = source_meta.tables['song_books']
111 source_songs_table = source_meta.tables['songs']119 source_songs_table = source_meta.tables['songs']
112 source_topics_table = source_meta.tables['topics']120 source_topics_table = source_meta.tables['topics']
113 source_authors_songs_table = source_meta.tables['authors_songs']121 source_authors_songs_table = source_meta.tables['authors_songs']
122 source_songs_songbooks_table = source_meta.tables['songs_songbooks']
114 source_songs_topics_table = source_meta.tables['songs_topics']123 source_songs_topics_table = source_meta.tables['songs_topics']
115 source_media_files_songs_table = None124 source_media_files_songs_table = None
116 if has_media_files:125 if has_media_files:
@@ -122,8 +131,8 @@
122 mapper(OldMediaFile, source_media_files_table)131 mapper(OldMediaFile, source_media_files_table)
123 song_props = {132 song_props = {
124 'authors': relation(OldAuthor, backref='songs', secondary=source_authors_songs_table),133 'authors': relation(OldAuthor, backref='songs', secondary=source_authors_songs_table),
125 'book': relation(OldBook, backref='songs'),134 'topics': relation(OldTopic, backref='songs', secondary=source_songs_topics_table),
126 'topics': relation(OldTopic, backref='songs', secondary=source_songs_topics_table)135 'songbook_entries': relation(OldSongBookEntry, backref='song'),
127 }136 }
128 if has_media_files:137 if has_media_files:
129 if isinstance(source_media_files_songs_table, Table):138 if isinstance(source_media_files_songs_table, Table):
@@ -139,10 +148,6 @@
139 except UnmappedClassError:148 except UnmappedClassError:
140 mapper(OldAuthor, source_authors_table)149 mapper(OldAuthor, source_authors_table)
141 try:150 try:
142 class_mapper(OldBook)
143 except UnmappedClassError:
144 mapper(OldBook, source_song_books_table)
145 try:
146 class_mapper(OldSong)151 class_mapper(OldSong)
147 except UnmappedClassError:152 except UnmappedClassError:
148 mapper(OldSong, source_songs_table, properties=song_props)153 mapper(OldSong, source_songs_table, properties=song_props)
@@ -150,6 +155,10 @@
150 class_mapper(OldTopic)155 class_mapper(OldTopic)
151 except UnmappedClassError:156 except UnmappedClassError:
152 mapper(OldTopic, source_topics_table)157 mapper(OldTopic, source_topics_table)
158 try:
159 class_mapper(OldSongBookEntry)
160 except UnmappedClassError:
161 mapper(OldSongBookEntry, source_songs_songbooks_table)
153162
154 source_songs = self.source_session.query(OldSong).all()163 source_songs = self.source_session.query(OldSong).all()
155 if self.import_wizard:164 if self.import_wizard:
@@ -166,7 +175,6 @@
166 # Values will be set when cleaning the song.175 # Values will be set when cleaning the song.
167 new_song.search_title = ''176 new_song.search_title = ''
168 new_song.search_lyrics = ''177 new_song.search_lyrics = ''
169 new_song.song_number = song.song_number
170 new_song.lyrics = song.lyrics178 new_song.lyrics = song.lyrics
171 new_song.verse_order = song.verse_order179 new_song.verse_order = song.verse_order
172 new_song.copyright = song.copyright180 new_song.copyright = song.copyright
@@ -181,11 +189,12 @@
181 last_name=author.last_name,189 last_name=author.last_name,
182 display_name=author.display_name)190 display_name=author.display_name)
183 new_song.add_author(existing_author)191 new_song.add_author(existing_author)
184 if song.book:192 for songbook_entry in song.songbook_entries:
185 existing_song_book = self.manager.get_object_filtered(Book, Book.name == song.book.name)193 existing_song_book = self.manager.get_object_filtered(Book, Book.name == songbook_entry.book.name)
186 if existing_song_book is None:194 if existing_song_book is None:
187 existing_song_book = Book.populate(name=song.book.name, publisher=song.book.publisher)195 existing_song_book = Book.populate(name=songbook_entry.book.name,
188 new_song.book = existing_song_book196 publisher=songbook_entry.book.publisher)
197 new_song.add_songbook_entry(existing_song_book, songbook_entry.entry)
189 if song.topics:198 if song.topics:
190 for topic in song.topics:199 for topic in song.topics:
191 existing_topic = self.manager.get_object_filtered(Topic, Topic.name == topic.name)200 existing_topic = self.manager.get_object_filtered(Topic, Topic.name == topic.name)
@@ -212,3 +221,4 @@
212 break221 break
213 self.source_session.close()222 self.source_session.close()
214 engine.dispose()223 engine.dispose()
224 os.remove(import_db)
215225
=== added file 'tests/utils/test_pep8.py'
--- tests/utils/test_pep8.py 1970-01-01 00:00:00 +0000
+++ tests/utils/test_pep8.py 2016-01-15 21:49:07 +0000
@@ -0,0 +1,89 @@
1# -*- coding: utf-8 -*-
2# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
3
4###############################################################################
5# OpenLP - Open Source Lyrics Projection #
6# --------------------------------------------------------------------------- #
7# Copyright (c) 2008-2016 OpenLP Developers #
8# --------------------------------------------------------------------------- #
9# This program is free software; you can redistribute it and/or modify it #
10# under the terms of the GNU General Public License as published by the Free #
11# Software Foundation; version 2 of the License. #
12# #
13# This program is distributed in the hope that it will be useful, but WITHOUT #
14# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or #
15# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for #
16# more details. #
17# #
18# You should have received a copy of the GNU General Public License along #
19# with this program; if not, write to the Free Software Foundation, Inc., 59 #
20# Temple Place, Suite 330, Boston, MA 02111-1307 USA #
21###############################################################################
22
23"""
24Package to test for pep8 errors
25Inspired by http://blog.jameskyle.org/2014/05/pep8-pylint-tests-with-nose-xunit/
26"""
27
28import os
29import pep8
30import re
31
32from unittest import TestCase
33
34from tests.utils.constants import OPENLP_PATH
35from tests.utils import is_running_on_ci
36
37PROJ_ROOT = 'openlp'
38
39
40class TestPep8(TestCase):
41
42 def pep8_test(self):
43 """
44 Test for pep8 conformance
45 """
46 pattern = re.compile(r'.*({0}.*\.py)'.format(PROJ_ROOT))
47 pep8style = pep8.StyleGuide(reporter=CustomReport, config_file=os.path.join(OPENLP_PATH, 'setup.cfg'))
48
49 report = pep8style.init_report()
50 pep8style.input_dir(OPENLP_PATH)
51
52 for error in report.results:
53 msg = "{path}: {code} {row}, {col} - {text}"
54
55 match = pattern.match(error['path'])
56 if match:
57 rel_path = match.group(1)
58 else:
59 rel_path = error['path']
60
61 print(msg.format(
62 path=rel_path,
63 code=error['code'],
64 row=error['row'],
65 col=error['col'],
66 text=error['text']
67 ))
68 if report.results:
69 self.fail("Please fix the PEP8 warnings.")
70
71
72class CustomReport(pep8.StandardReport):
73 """
74 Collect report into an array of results.
75 """
76 results = []
77
78 def get_file_results(self):
79 if self._deferred_print:
80 self._deferred_print.sort()
81 for line_number, offset, code, text, _ in self._deferred_print:
82 self.results.append({
83 'path': self.filename,
84 'row': self.line_offset + line_number,
85 'col': offset + 1,
86 'code': code,
87 'text': text,
88 })
89 return self.file_errors