Merge lp:~googol-deactivatedaccount/openlp/bug-804747 into lp:openlp

Proposed by Andreas Preikschat
Status: Superseded
Proposed branch: lp:~googol-deactivatedaccount/openlp/bug-804747
Merge into: lp:openlp
Diff against target: 975 lines (+131/-290)
8 files modified
openlp/core/ui/mainwindow.py (+2/-5)
openlp/core/utils/__init__.py (+12/-1)
openlp/plugins/bibles/forms/bibleupgradeform.py (+74/-227)
openlp/plugins/bibles/forms/languageform.py (+5/-6)
openlp/plugins/bibles/lib/db.py (+30/-43)
openlp/plugins/bibles/lib/manager.py (+5/-5)
openlp/plugins/bibles/lib/mediaitem.py (+1/-1)
openlp/plugins/songs/lib/openlyricsexport.py (+2/-2)
To merge this branch: bzr merge lp:~googol-deactivatedaccount/openlp/bug-804747
Reviewer Review Type Date Requested Status
Tim Bentley Needs Fixing
Review via email: mp+71487@code.launchpad.net

This proposal supersedes a proposal from 2011-08-13.

This proposal has been superseded by a proposal from 2011-08-15.

Commit message

- Fixed bug #804747 (Bible disappear after upgrade)
- Removed not needed or redundant variables.
- Removed not needed code.
- When upgrading bibles move the files to the temp directory.
- clean ups

Description of the change

Hello,

1) Fixed bug #804747 (Bible disappear after upgrade)
The fix is in line 303. We did not pass the file name, that means that we used the version name. If you do not pass the file name the file name is crated from the version name (and since this contained Japanese characters) they were removed (so the file name was ".sqlite")
Furthermore, I changed the way we clean the file name and move the function to utils.
2) Removed not needed or redundant variables.
3) Removed not needed code.
I removed the code which allowed you to change the version name in some *rare* (!) circumstances. I do not see why the upgrade wizard should take care of any of this.
5) When upgrading bibles move the files to the temp directory.
4) clean ups

To post a comment you must log in.
Revision history for this message
Tim Bentley (trb143) wrote :

Creation of temp dir name shouls be common to easy for one to be changed and rest missed

review: Needs Fixing
1717. By Andreas Preikschat

create temp directory name in constructor

1718. By Andreas Preikschat

removed not needed code

1719. By Andreas Preikschat

fix failure dectection

1720. By Andreas Preikschat

remove print

1721. By Andreas Preikschat

r1710

1722. By Andreas Preikschat

close connection to database when the upgrade failed, to prevent errors on windows

1723. By Andreas Preikschat

removed not needed code (this case should never occur and if it does, the manager instance should delete the file)

1724. By Andreas Preikschat

improved deletion of corrupted files on windows (bug #824129)

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'openlp/core/ui/mainwindow.py'
2--- openlp/core/ui/mainwindow.py 2011-08-11 09:01:25 +0000
3+++ openlp/core/ui/mainwindow.py 2011-08-15 09:31:39 +0000
4@@ -28,6 +28,7 @@
5 import logging
6 import os
7 import sys
8+import shutil
9 from tempfile import gettempdir
10
11 from PyQt4 import QtCore, QtGui
12@@ -726,11 +727,7 @@
13 plugin.firstTime()
14 Receiver.send_message(u'openlp_process_events')
15 temp_dir = os.path.join(unicode(gettempdir()), u'openlp')
16- if not os.path.exists(temp_dir):
17- return
18- for filename in os.listdir(temp_dir):
19- delete_file(os.path.join(temp_dir, filename))
20- os.removedirs(temp_dir)
21+ shutil.rmtree(temp_dir, True)
22
23 def onFirstTimeWizardClicked(self):
24 """
25
26=== modified file 'openlp/core/utils/__init__.py'
27--- openlp/core/utils/__init__.py 2011-07-17 15:09:30 +0000
28+++ openlp/core/utils/__init__.py 2011-08-15 09:31:39 +0000
29@@ -386,6 +386,17 @@
30 else:
31 return os.path.split(path)
32
33+def clean_filename(filename):
34+ """
35+ Removes invalid characters from the given ``filename``.
36+
37+ ``filename``
38+ The "dirty" file name to clean.
39+ """
40+ if not isinstance(filename, unicode):
41+ filename = unicode(filename, u'utf-8')
42+ return re.sub(r'[/\\?*|<>\[\]":<>+%]+', u'_', filename).strip(u'_')
43+
44 def delete_file(file_path_name):
45 """
46 Deletes a file from the system.
47@@ -492,4 +503,4 @@
48 __all__ = [u'AppLocation', u'get_application_version', u'check_latest_version',
49 u'add_actions', u'get_filesystem_encoding', u'LanguageManager',
50 u'ActionList', u'get_web_page', u'file_is_unicode', u'get_uno_command',
51- u'get_uno_instance', u'delete_file']
52+ u'get_uno_instance', u'delete_file', u'clean_filename']
53
54=== modified file 'openlp/plugins/bibles/forms/bibleupgradeform.py'
55--- openlp/plugins/bibles/forms/bibleupgradeform.py 2011-07-14 18:42:38 +0000
56+++ openlp/plugins/bibles/forms/bibleupgradeform.py 2011-08-15 09:31:39 +0000
57@@ -29,17 +29,17 @@
58 import logging
59 import os
60 import shutil
61+from tempfile import gettempdir
62
63 from PyQt4 import QtCore, QtGui
64
65 from openlp.core.lib import Receiver, SettingsManager, translate, \
66 check_directory_exists
67-from openlp.core.lib.db import delete_database
68 from openlp.core.lib.ui import UiStrings, critical_error_message_box
69 from openlp.core.ui.wizard import OpenLPWizard, WizardStrings
70 from openlp.core.utils import AppLocation, delete_file
71 from openlp.plugins.bibles.lib.db import BibleDB, BibleMeta, OldBibleDB, \
72- BiblesResourcesDB, clean_filename
73+ BiblesResourcesDB
74 from openlp.plugins.bibles.lib.http import BSExtract, BGExtract, CWExtract
75
76 log = logging.getLogger(__name__)
77@@ -70,6 +70,7 @@
78 self.suffix = u'.sqlite'
79 self.settingsSection = u'bibles'
80 self.path = AppLocation.get_section_data_path(self.settingsSection)
81+ self.temp_dir = os.path.join(gettempdir(), u'openlp')
82 self.files = self.manager.old_bible_databases
83 self.success = {}
84 self.newbibles = {}
85@@ -91,20 +92,6 @@
86 log.debug(u'Stopping import')
87 self.stop_import_flag = True
88
89- def onCheckBoxIndexChanged(self, index):
90- """
91- Show/Hide warnings if CheckBox state has changed
92- """
93- for number, filename in enumerate(self.files):
94- if not self.checkBox[number].checkState() == QtCore.Qt.Checked:
95- self.verticalWidget[number].hide()
96- self.formWidget[number].hide()
97- else:
98- version_name = unicode(self.versionNameEdit[number].text())
99- if self.manager.exists(version_name):
100- self.verticalWidget[number].show()
101- self.formWidget[number].show()
102-
103 def reject(self):
104 """
105 Stop the wizard on cancel button, close button or ESC key.
106@@ -113,8 +100,6 @@
107 self.stop_import_flag = True
108 if not self.currentPage() == self.progressPage:
109 self.done(QtGui.QDialog.Rejected)
110- else:
111- self.postWizard()
112
113 def onCurrentIdChanged(self, pageId):
114 """
115@@ -124,7 +109,7 @@
116 self.preWizard()
117 self.performWizard()
118 self.postWizard()
119- elif self.page(pageId) == self.selectPage and self.maxBibles == 0:
120+ elif self.page(pageId) == self.selectPage and not self.files:
121 self.next()
122
123 def onBackupBrowseButtonClicked(self):
124@@ -243,78 +228,13 @@
125 Add the content to the scrollArea.
126 """
127 self.checkBox = {}
128- self.versionNameEdit = {}
129- self.versionNameLabel = {}
130- self.versionInfoLabel = {}
131- self.versionInfoPixmap = {}
132- self.verticalWidget = {}
133- self.horizontalLayout = {}
134- self.formWidget = {}
135- self.formLayoutAttention = {}
136 for number, filename in enumerate(self.files):
137 bible = OldBibleDB(self.mediaItem, path=self.path, file=filename[0])
138 self.checkBox[number] = QtGui.QCheckBox(self.scrollAreaContents)
139- checkBoxName = u'checkBox[%d]' % number
140- self.checkBox[number].setObjectName(checkBoxName)
141+ self.checkBox[number].setObjectName(u'checkBox[%d]' % number)
142 self.checkBox[number].setText(bible.get_name())
143 self.checkBox[number].setCheckState(QtCore.Qt.Checked)
144 self.formLayout.addWidget(self.checkBox[number])
145- self.verticalWidget[number] = QtGui.QWidget(self.scrollAreaContents)
146- verticalWidgetName = u'verticalWidget[%d]' % number
147- self.verticalWidget[number].setObjectName(verticalWidgetName)
148- self.horizontalLayout[number] = QtGui.QHBoxLayout(
149- self.verticalWidget[number])
150- self.horizontalLayout[number].setContentsMargins(25, 0, 0, 0)
151- horizontalLayoutName = u'horizontalLayout[%d]' % number
152- self.horizontalLayout[number].setObjectName(horizontalLayoutName)
153- self.versionInfoPixmap[number] = QtGui.QLabel(
154- self.verticalWidget[number])
155- versionInfoPixmapName = u'versionInfoPixmap[%d]' % number
156- self.versionInfoPixmap[number].setObjectName(versionInfoPixmapName)
157- self.versionInfoPixmap[number].setPixmap(QtGui.QPixmap(
158- u':/bibles/bibles_upgrade_alert.png'))
159- self.versionInfoPixmap[number].setAlignment(QtCore.Qt.AlignRight)
160- self.horizontalLayout[number].addWidget(
161- self.versionInfoPixmap[number])
162- self.versionInfoLabel[number] = QtGui.QLabel(
163- self.verticalWidget[number])
164- versionInfoLabelName = u'versionInfoLabel[%d]' % number
165- self.versionInfoLabel[number].setObjectName(versionInfoLabelName)
166- sizePolicy = QtGui.QSizePolicy(QtGui.QSizePolicy.Expanding,
167- QtGui.QSizePolicy.Preferred)
168- sizePolicy.setHorizontalStretch(0)
169- sizePolicy.setVerticalStretch(0)
170- sizePolicy.setHeightForWidth(
171- self.versionInfoLabel[number].sizePolicy().hasHeightForWidth())
172- self.versionInfoLabel[number].setSizePolicy(sizePolicy)
173- self.horizontalLayout[number].addWidget(
174- self.versionInfoLabel[number])
175- self.formLayout.addWidget(self.verticalWidget[number])
176- self.formWidget[number] = QtGui.QWidget(self.scrollAreaContents)
177- formWidgetName = u'formWidget[%d]' % number
178- self.formWidget[number].setObjectName(formWidgetName)
179- self.formLayoutAttention[number] = QtGui.QFormLayout(
180- self.formWidget[number])
181- self.formLayoutAttention[number].setContentsMargins(25, 0, 0, 5)
182- formLayoutAttentionName = u'formLayoutAttention[%d]' % number
183- self.formLayoutAttention[number].setObjectName(
184- formLayoutAttentionName)
185- self.versionNameLabel[number] = QtGui.QLabel(
186- self.formWidget[number])
187- self.versionNameLabel[number].setObjectName(u'VersionNameLabel')
188- self.formLayoutAttention[number].setWidget(0,
189- QtGui.QFormLayout.LabelRole, self.versionNameLabel[number])
190- self.versionNameEdit[number] = QtGui.QLineEdit(
191- self.formWidget[number])
192- self.versionNameEdit[number].setObjectName(u'VersionNameEdit')
193- self.formLayoutAttention[number].setWidget(0,
194- QtGui.QFormLayout.FieldRole, self.versionNameEdit[number])
195- self.versionNameEdit[number].setText(bible.get_name())
196- self.formLayout.addWidget(self.formWidget[number])
197- # Set up the Signal for the checkbox.
198- QtCore.QObject.connect(self.checkBox[number],
199- QtCore.SIGNAL(u'stateChanged(int)'),
200- self.onCheckBoxIndexChanged)
201 self.spacerItem = QtGui.QSpacerItem(20, 5, QtGui.QSizePolicy.Minimum,
202 QtGui.QSizePolicy.Expanding)
203 self.formLayout.addItem(self.spacerItem)
204@@ -327,23 +247,6 @@
205 for number, filename in enumerate(self.files):
206 self.formLayout.removeWidget(self.checkBox[number])
207 self.checkBox[number].setParent(None)
208- self.horizontalLayout[number].removeWidget(
209- self.versionInfoPixmap[number])
210- self.versionInfoPixmap[number].setParent(None)
211- self.horizontalLayout[number].removeWidget(
212- self.versionInfoLabel[number])
213- self.versionInfoLabel[number].setParent(None)
214- self.formLayout.removeWidget(self.verticalWidget[number])
215- self.verticalWidget[number].setParent(None)
216- self.formLayoutAttention[number].removeWidget(
217- self.versionNameLabel[number])
218- self.versionNameLabel[number].setParent(None)
219- self.formLayoutAttention[number].removeWidget(
220- self.versionNameEdit[number])
221- self.formLayoutAttention[number].deleteLater()
222- self.versionNameEdit[number].setParent(None)
223- self.formLayout.removeWidget(self.formWidget[number])
224- self.formWidget[number].setParent(None)
225 self.formLayout.removeItem(self.spacerItem)
226
227 def retranslateUi(self):
228@@ -385,12 +288,6 @@
229 self.selectPage.setSubTitle(
230 translate('BiblesPlugin.UpgradeWizardForm',
231 'Please select the Bibles to upgrade'))
232- for number, bible in enumerate(self.files):
233- self.versionNameLabel[number].setText(
234- translate('BiblesPlugin.UpgradeWizardForm', 'Version name:'))
235- self.versionInfoLabel[number].setText(
236- translate('BiblesPlugin.UpgradeWizardForm', 'This '
237- 'Bible still exists. Please change the name or uncheck it.'))
238 self.progressPage.setTitle(translate('BiblesPlugin.UpgradeWizardForm',
239 'Upgrading'))
240 self.progressPage.setSubTitle(
241@@ -425,58 +322,16 @@
242 return False
243 return True
244 elif self.currentPage() == self.selectPage:
245+ check_directory_exists(self.temp_dir)
246 for number, filename in enumerate(self.files):
247 if not self.checkBox[number].checkState() == QtCore.Qt.Checked:
248 continue
249- version_name = unicode(self.versionNameEdit[number].text())
250- if not version_name:
251- critical_error_message_box(UiStrings().EmptyField,
252- translate('BiblesPlugin.UpgradeWizardForm',
253- 'You need to specify a version name for your Bible.'))
254- self.versionNameEdit[number].setFocus()
255- return False
256- elif self.manager.exists(version_name):
257- critical_error_message_box(
258- translate('BiblesPlugin.UpgradeWizardForm',
259- 'Bible Exists'),
260- translate('BiblesPlugin.UpgradeWizardForm',
261- 'This Bible already exists. Please upgrade '
262- 'a different Bible, delete the existing one or '
263- 'uncheck.'))
264- self.versionNameEdit[number].setFocus()
265- return False
266- elif os.path.exists(os.path.join(self.path, clean_filename(
267- version_name))) and version_name == filename[1]:
268- newfilename = u'old_database_%s' % filename[0]
269- if not os.path.exists(os.path.join(self.path,
270- newfilename)):
271- os.rename(os.path.join(self.path, filename[0]),
272- os.path.join(self.path, newfilename))
273- self.files[number] = [newfilename, filename[1]]
274- continue
275- else:
276- critical_error_message_box(
277- translate('BiblesPlugin.UpgradeWizardForm',
278- 'Bible Exists'),
279- translate('BiblesPlugin.UpgradeWizardForm',
280- 'This Bible already exists. Please upgrade '
281- 'a different Bible, delete the existing one or '
282- 'uncheck.'))
283- self.verticalWidget[number].show()
284- self.formWidget[number].show()
285- self.versionNameEdit[number].setFocus()
286- return False
287- elif os.path.exists(os.path.join(self.path,
288- clean_filename(version_name))):
289- critical_error_message_box(
290- translate('BiblesPlugin.UpgradeWizardForm',
291- 'Bible Exists'),
292- translate('BiblesPlugin.UpgradeWizardForm',
293- 'This Bible already exists. Please upgrade '
294- 'a different Bible, delete the existing one or '
295- 'uncheck.'))
296- self.versionNameEdit[number].setFocus()
297- return False
298+ # Move bibles to temp dir.
299+ if not os.path.exists(os.path.join(self.temp_dir, filename[0])):
300+ shutil.move(
301+ os.path.join(self.path, filename[0]), self.temp_dir)
302+ else:
303+ delete_file(os.path.join(self.path, filename[0]))
304 return True
305 if self.currentPage() == self.progressPage:
306 return True
307@@ -495,16 +350,8 @@
308 self.files = self.manager.old_bible_databases
309 self.addScrollArea()
310 self.retranslateUi()
311- self.maxBibles = len(self.files)
312 for number, filename in enumerate(self.files):
313 self.checkBox[number].setCheckState(QtCore.Qt.Checked)
314- oldname = filename[1]
315- if self.manager.exists(oldname):
316- self.verticalWidget[number].show()
317- self.formWidget[number].show()
318- else:
319- self.verticalWidget[number].hide()
320- self.formWidget[number].hide()
321 self.progressBar.show()
322 self.restart()
323 self.finishButton.setVisible(False)
324@@ -516,9 +363,8 @@
325 Prepare the UI for the upgrade.
326 """
327 OpenLPWizard.preWizard(self)
328- self.progressLabel.setText(translate(
329- 'BiblesPlugin.UpgradeWizardForm',
330- 'Starting upgrade...'))
331+ self.progressLabel.setText(
332+ translate('BiblesPlugin.UpgradeWizardForm', 'Starting upgrade...'))
333 Receiver.send_message(u'openlp_process_events')
334
335 def performWizard(self):
336@@ -527,48 +373,50 @@
337 """
338 self.include_webbible = False
339 proxy_server = None
340- if self.maxBibles == 0:
341+ if not self.files:
342 self.progressLabel.setText(
343 translate('BiblesPlugin.UpgradeWizardForm', 'There are no '
344 'Bibles that need to be upgraded.'))
345 self.progressBar.hide()
346 return
347- self.maxBibles = 0
348+ max_bibles = 0
349 for number, file in enumerate(self.files):
350 if self.checkBox[number].checkState() == QtCore.Qt.Checked:
351- self.maxBibles += 1
352- number = 0
353- for biblenumber, filename in enumerate(self.files):
354+ max_bibles += 1
355+ oldBible = None
356+ for number, filename in enumerate(self.files):
357+ # Close the previous bible's connection.
358+ if oldBible is not None:
359+ oldBible.close_connection()
360+ # Set to None to make obvious that we have already closed the
361+ # database.
362+ oldBible = None
363 if self.stop_import_flag:
364- bible_failed = True
365+ self.success[number] = False
366 break
367- bible_failed = False
368- self.success[biblenumber] = False
369- if not self.checkBox[biblenumber].checkState() == QtCore.Qt.Checked:
370+ if not self.checkBox[number].checkState() == QtCore.Qt.Checked:
371+ self.success[number] = False
372 continue
373 self.progressBar.reset()
374- oldbible = OldBibleDB(self.mediaItem, path=self.path,
375+ oldBible = OldBibleDB(self.mediaItem, path=self.temp_dir,
376 file=filename[0])
377 name = filename[1]
378 if name is None:
379- delete_file(os.path.join(self.path, filename[0]))
380 self.incrementProgressBar(unicode(translate(
381 'BiblesPlugin.UpgradeWizardForm',
382 'Upgrading Bible %s of %s: "%s"\nFailed')) %
383- (number + 1, self.maxBibles, name),
384+ (number + 1, max_bibles, name),
385 self.progressBar.maximum() - self.progressBar.value())
386- number += 1
387+ self.success[number] = False
388 continue
389 self.progressLabel.setText(unicode(translate(
390 'BiblesPlugin.UpgradeWizardForm',
391 'Upgrading Bible %s of %s: "%s"\nUpgrading ...')) %
392- (number + 1, self.maxBibles, name))
393- if os.path.exists(os.path.join(self.path, filename[0])):
394- name = unicode(self.versionNameEdit[biblenumber].text())
395+ (number + 1, max_bibles, name))
396 self.newbibles[number] = BibleDB(self.mediaItem, path=self.path,
397- name=name)
398+ name=name, file=filename[0])
399 self.newbibles[number].register(self.plugin.upgrade_wizard)
400- metadata = oldbible.get_metadata()
401+ metadata = oldBible.get_metadata()
402 webbible = False
403 meta_data = {}
404 for meta in metadata:
405@@ -595,7 +443,6 @@
406 u'name: "%s" failed' % (
407 meta_data[u'download source'],
408 meta_data[u'download name']))
409- delete_database(self.path, clean_filename(name))
410 del self.newbibles[number]
411 critical_error_message_box(
412 translate('BiblesPlugin.UpgradeWizardForm',
413@@ -606,9 +453,9 @@
414 self.incrementProgressBar(unicode(translate(
415 'BiblesPlugin.UpgradeWizardForm',
416 'Upgrading Bible %s of %s: "%s"\nFailed')) %
417- (number + 1, self.maxBibles, name),
418+ (number + 1, max_bibles, name),
419 self.progressBar.maximum() - self.progressBar.value())
420- number += 1
421+ self.success[number] = False
422 continue
423 bible = BiblesResourcesDB.get_webbible(
424 meta_data[u'download name'],
425@@ -621,25 +468,24 @@
426 language_id = self.newbibles[number].get_language(name)
427 if not language_id:
428 log.warn(u'Upgrading from "%s" failed' % filename[0])
429- delete_database(self.path, clean_filename(name))
430 del self.newbibles[number]
431 self.incrementProgressBar(unicode(translate(
432 'BiblesPlugin.UpgradeWizardForm',
433 'Upgrading Bible %s of %s: "%s"\nFailed')) %
434- (number + 1, self.maxBibles, name),
435+ (number + 1, max_bibles, name),
436 self.progressBar.maximum() - self.progressBar.value())
437- number += 1
438+ self.success[number] = False
439 continue
440 self.progressBar.setMaximum(len(books))
441 for book in books:
442 if self.stop_import_flag:
443- bible_failed = True
444+ self.success[number] = False
445 break
446 self.incrementProgressBar(unicode(translate(
447 'BiblesPlugin.UpgradeWizardForm',
448 'Upgrading Bible %s of %s: "%s"\n'
449 'Upgrading %s ...')) %
450- (number + 1, self.maxBibles, name, book))
451+ (number + 1, max_bibles, name, book))
452 book_ref_id = self.newbibles[number].\
453 get_book_ref_id_by_name(book, len(books), language_id)
454 if not book_ref_id:
455@@ -647,24 +493,23 @@
456 u'name: "%s" aborted by user' % (
457 meta_data[u'download source'],
458 meta_data[u'download name']))
459- delete_database(self.path, clean_filename(name))
460 del self.newbibles[number]
461- bible_failed = True
462+ self.success[number] = False
463 break
464 book_details = BiblesResourcesDB.get_book_by_id(book_ref_id)
465 db_book = self.newbibles[number].create_book(book,
466 book_ref_id, book_details[u'testament_id'])
467- # Try to import still downloaded verses
468- oldbook = oldbible.get_book(book)
469+ # Try to import already downloaded verses.
470+ oldbook = oldBible.get_book(book)
471 if oldbook:
472- verses = oldbible.get_verses(oldbook[u'id'])
473+ verses = oldBible.get_verses(oldbook[u'id'])
474 if not verses:
475 log.warn(u'No verses found to import for book '
476 u'"%s"', book)
477 continue
478 for verse in verses:
479 if self.stop_import_flag:
480- bible_failed = True
481+ self.success[number] = False
482 break
483 self.newbibles[number].create_verse(db_book.id,
484 int(verse[u'chapter']),
485@@ -678,40 +523,38 @@
486 language_id = self.newbibles[number].get_language(name)
487 if not language_id:
488 log.warn(u'Upgrading books from "%s" failed' % name)
489- delete_database(self.path, clean_filename(name))
490 del self.newbibles[number]
491 self.incrementProgressBar(unicode(translate(
492 'BiblesPlugin.UpgradeWizardForm',
493 'Upgrading Bible %s of %s: "%s"\nFailed')) %
494- (number + 1, self.maxBibles, name),
495+ (number + 1, max_bibles, name),
496 self.progressBar.maximum() - self.progressBar.value())
497- number += 1
498+ self.success[number] = False
499 continue
500- books = oldbible.get_books()
501+ books = oldBible.get_books()
502 self.progressBar.setMaximum(len(books))
503 for book in books:
504 if self.stop_import_flag:
505- bible_failed = True
506+ self.success[number] = False
507 break
508 self.incrementProgressBar(unicode(translate(
509 'BiblesPlugin.UpgradeWizardForm',
510 'Upgrading Bible %s of %s: "%s"\n'
511 'Upgrading %s ...')) %
512- (number + 1, self.maxBibles, name, book[u'name']))
513+ (number + 1, max_bibles, name, book[u'name']))
514 book_ref_id = self.newbibles[number].\
515 get_book_ref_id_by_name(book[u'name'], len(books),
516 language_id)
517 if not book_ref_id:
518 log.warn(u'Upgrading books from %s " '\
519 'failed - aborted by user' % name)
520- delete_database(self.path, clean_filename(name))
521 del self.newbibles[number]
522- bible_failed = True
523+ self.success[number] = False
524 break
525 book_details = BiblesResourcesDB.get_book_by_id(book_ref_id)
526 db_book = self.newbibles[number].create_book(book[u'name'],
527 book_ref_id, book_details[u'testament_id'])
528- verses = oldbible.get_verses(book[u'id'])
529+ verses = oldBible.get_verses(book[u'id'])
530 if not verses:
531 log.warn(u'No verses found to import for book '
532 u'"%s"', book[u'name'])
533@@ -719,31 +562,30 @@
534 continue
535 for verse in verses:
536 if self.stop_import_flag:
537- bible_failed = True
538+ self.success[number] = False
539 break
540 self.newbibles[number].create_verse(db_book.id,
541 int(verse[u'chapter']),
542 int(verse[u'verse']), unicode(verse[u'text']))
543 Receiver.send_message(u'openlp_process_events')
544 self.newbibles[number].session.commit()
545- if not bible_failed:
546+ if self.success.has_key(number) and not self.success[number]:
547+ self.incrementProgressBar(unicode(translate(
548+ 'BiblesPlugin.UpgradeWizardForm',
549+ 'Upgrading Bible %s of %s: "%s"\nFailed')) %
550+ (number + 1, max_bibles, name),
551+ self.progressBar.maximum() - self.progressBar.value())
552+ else:
553+ self.success[number] = True
554 self.newbibles[number].create_meta(u'Version', name)
555- oldbible.close_connection()
556- delete_file(os.path.join(self.path, filename[0]))
557 self.incrementProgressBar(unicode(translate(
558 'BiblesPlugin.UpgradeWizardForm',
559 'Upgrading Bible %s of %s: "%s"\n'
560 'Complete')) %
561- (number + 1, self.maxBibles, name))
562- self.success[biblenumber] = True
563- else:
564- self.incrementProgressBar(unicode(translate(
565- 'BiblesPlugin.UpgradeWizardForm',
566- 'Upgrading Bible %s of %s: "%s"\nFailed')) %
567- (number + 1, self.maxBibles, name),
568- self.progressBar.maximum() - self.progressBar.value())
569- delete_database(self.path, clean_filename(name))
570- number += 1
571+ (number + 1, max_bibles, name))
572+ # Close the last bible's connection if possible.
573+ if oldBible is not None:
574+ oldBible.close_connection()
575
576 def postWizard(self):
577 """
578@@ -752,10 +594,14 @@
579 successful_import = 0
580 failed_import = 0
581 for number, filename in enumerate(self.files):
582- if number in self.success and self.success[number] == True:
583+ if self.success.has_key(number) and self.success[number]:
584 successful_import += 1
585 elif self.checkBox[number].checkState() == QtCore.Qt.Checked:
586 failed_import += 1
587+ # Delete upgraded (but not complete, corrupted, ...) bible.
588+ delete_file(os.path.join(self.path, filename[0]))
589+ # Copy not upgraded bible back.
590+ shutil.move(os.path.join(self.temp_dir, filename[0]), self.path)
591 if failed_import > 0:
592 failed_import_text = unicode(translate(
593 'BiblesPlugin.UpgradeWizardForm',
594@@ -776,7 +622,8 @@
595 'Bible(s): %s successful%s')) % (successful_import,
596 failed_import_text))
597 else:
598- self.progressLabel.setText(
599- translate('BiblesPlugin.UpgradeWizardForm', 'Upgrade '
600- 'failed.'))
601+ self.progressLabel.setText(translate(
602+ 'BiblesPlugin.UpgradeWizardForm', 'Upgrade failed.'))
603+ # Remove temp directory.
604+ shutil.rmtree(self.temp_dir, True)
605 OpenLPWizard.postWizard(self)
606
607=== modified file 'openlp/plugins/bibles/forms/languageform.py'
608--- openlp/plugins/bibles/forms/languageform.py 2011-05-26 19:13:11 +0000
609+++ openlp/plugins/bibles/forms/languageform.py 2011-08-15 09:31:39 +0000
610@@ -44,8 +44,8 @@
611 Class to manage a dialog which ask the user for a language.
612 """
613 log.info(u'LanguageForm loaded')
614-
615- def __init__(self, parent = None):
616+
617+ def __init__(self, parent=None):
618 """
619 Constructor
620 """
621@@ -57,12 +57,11 @@
622 if bible_name:
623 self.bibleLabel.setText(unicode(bible_name))
624 items = BiblesResourcesDB.get_languages()
625- for item in items:
626- self.languageComboBox.addItem(item[u'name'])
627+ self.languageComboBox.addItems([item[u'name'] for item in items])
628 return QDialog.exec_(self)
629-
630+
631 def accept(self):
632- if self.languageComboBox.currentText() == u'':
633+ if not self.languageComboBox.currentText():
634 critical_error_message_box(
635 message=translate('BiblesPlugin.LanguageForm',
636 'You need to choose a language.'))
637
638=== modified file 'openlp/plugins/bibles/lib/db.py'
639--- openlp/plugins/bibles/lib/db.py 2011-07-07 18:03:12 +0000
640+++ openlp/plugins/bibles/lib/db.py 2011-08-15 09:31:39 +0000
641@@ -39,7 +39,7 @@
642 from openlp.core.lib import Receiver, translate
643 from openlp.core.lib.db import BaseModel, init_db, Manager
644 from openlp.core.lib.ui import critical_error_message_box
645-from openlp.core.utils import AppLocation
646+from openlp.core.utils import AppLocation, clean_filename
647
648 log = logging.getLogger(__name__)
649
650@@ -63,19 +63,6 @@
651 """
652 pass
653
654-def clean_filename(filename):
655- """
656- Clean up the version name of the Bible and convert it into a valid
657- file name.
658-
659- ``filename``
660- The "dirty" file name or version name.
661- """
662- if not isinstance(filename, unicode):
663- filename = unicode(filename, u'utf-8')
664- filename = re.sub(r'[^\w]+', u'_', filename).strip(u'_')
665- return filename + u'.sqlite'
666-
667 def init_schema(url):
668 """
669 Setup a bible database connection and initialise the database schema.
670@@ -158,7 +145,7 @@
671 self.name = kwargs[u'name']
672 if not isinstance(self.name, unicode):
673 self.name = unicode(self.name, u'utf-8')
674- self.file = clean_filename(self.name)
675+ self.file = clean_filename(self.name) + u'.sqlite'
676 if u'file' in kwargs:
677 self.file = kwargs[u'file']
678 Manager.__init__(self, u'bibles', init_schema, self.file)
679@@ -210,7 +197,7 @@
680 The book_reference_id from bibles_resources.sqlite of the book.
681
682 ``testament``
683- *Defaults to 1.* The testament_reference_id from
684+ *Defaults to 1.* The testament_reference_id from
685 bibles_resources.sqlite of the testament this book belongs to.
686 """
687 log.debug(u'BibleDB.create_book("%s", "%s")', name, bk_ref_id)
688@@ -329,7 +316,7 @@
689 return self.get_object_filtered(Book, Book.book_reference_id.like(id))
690
691 def get_book_ref_id_by_name(self, book, maxbooks, language_id=None):
692- log.debug(u'BibleDB.get_book_ref_id_by_name:("%s", "%s")', book,
693+ log.debug(u'BibleDB.get_book_ref_id_by_name:("%s", "%s")', book,
694 language_id)
695 if BiblesResourcesDB.get_book(book, True):
696 book_temp = BiblesResourcesDB.get_book(book, True)
697@@ -471,7 +458,7 @@
698
699 def get_language(self, bible_name=None):
700 """
701- If no language is given it calls a dialog window where the user could
702+ If no language is given it calls a dialog window where the user could
703 select the bible language.
704 Return the language id of a bible.
705
706@@ -521,9 +508,9 @@
707 some resources which are used in the Bibles plugin.
708 A wrapper class around a small SQLite database which contains the download
709 resources, a biblelist from the different download resources, the books,
710- chapter counts and verse counts for the web download Bibles, a language
711- reference, the testament reference and some alternative book names. This
712- class contains a singleton "cursor" so that only one connection to the
713+ chapter counts and verse counts for the web download Bibles, a language
714+ reference, the testament reference and some alternative book names. This
715+ class contains a singleton "cursor" so that only one connection to the
716 SQLite database is ever used.
717 """
718 cursor = None
719@@ -582,7 +569,7 @@
720
721 ``name``
722 The name or abbreviation of the book.
723-
724+
725 ``lower``
726 True if the comparsion should be only lowercase
727 """
728@@ -592,7 +579,7 @@
729 if lower:
730 books = BiblesResourcesDB.run_sql(u'SELECT id, testament_id, name, '
731 u'abbreviation, chapters FROM book_reference WHERE '
732- u'LOWER(name) = ? OR LOWER(abbreviation) = ?',
733+ u'LOWER(name) = ? OR LOWER(abbreviation) = ?',
734 (name.lower(), name.lower()))
735 else:
736 books = BiblesResourcesDB.run_sql(u'SELECT id, testament_id, name, '
737@@ -621,7 +608,7 @@
738 if not isinstance(id, int):
739 id = int(id)
740 books = BiblesResourcesDB.run_sql(u'SELECT id, testament_id, name, '
741- u'abbreviation, chapters FROM book_reference WHERE id = ?',
742+ u'abbreviation, chapters FROM book_reference WHERE id = ?',
743 (id, ))
744 if books:
745 return {
746@@ -645,12 +632,12 @@
747 ``chapter``
748 The chapter number.
749 """
750- log.debug(u'BiblesResourcesDB.get_chapter("%s", "%s")', book_id,
751+ log.debug(u'BiblesResourcesDB.get_chapter("%s", "%s")', book_id,
752 chapter)
753 if not isinstance(chapter, int):
754 chapter = int(chapter)
755 chapters = BiblesResourcesDB.run_sql(u'SELECT id, book_reference_id, '
756- u'chapter, verse_count FROM chapters WHERE book_reference_id = ?',
757+ u'chapter, verse_count FROM chapters WHERE book_reference_id = ?',
758 (book_id,))
759 if chapters:
760 return {
761@@ -687,7 +674,7 @@
762 ``chapter``
763 The number of the chapter.
764 """
765- log.debug(u'BiblesResourcesDB.get_verse_count("%s", "%s")', book_id,
766+ log.debug(u'BiblesResourcesDB.get_verse_count("%s", "%s")', book_id,
767 chapter)
768 details = BiblesResourcesDB.get_chapter(book_id, chapter)
769 if details:
770@@ -715,7 +702,7 @@
771 }
772 else:
773 return None
774-
775+
776 @staticmethod
777 def get_webbibles(source):
778 """
779@@ -737,7 +724,7 @@
780 u'id': bible[0],
781 u'name': bible[1],
782 u'abbreviation': bible[2],
783- u'language_id': bible[3],
784+ u'language_id': bible[3],
785 u'download_source_id': bible[4]
786 }
787 for bible in bibles
788@@ -752,11 +739,11 @@
789
790 ``abbreviation``
791 The abbreviation of the webbible.
792-
793+
794 ``source``
795 The source of the webbible.
796 """
797- log.debug(u'BiblesResourcesDB.get_webbibles("%s", "%s")', abbreviation,
798+ log.debug(u'BiblesResourcesDB.get_webbibles("%s", "%s")', abbreviation,
799 source)
800 if not isinstance(abbreviation, unicode):
801 abbreviation = unicode(abbreviation)
802@@ -765,14 +752,14 @@
803 source = BiblesResourcesDB.get_download_source(source)
804 bible = BiblesResourcesDB.run_sql(u'SELECT id, name, abbreviation, '
805 u'language_id, download_source_id FROM webbibles WHERE '
806- u'download_source_id = ? AND abbreviation = ?', (source[u'id'],
807+ u'download_source_id = ? AND abbreviation = ?', (source[u'id'],
808 abbreviation))
809 if bible:
810 return {
811 u'id': bible[0][0],
812 u'name': bible[0][1],
813 u'abbreviation': bible[0][2],
814- u'language_id': bible[0][3],
815+ u'language_id': bible[0][3],
816 u'download_source_id': bible[0][4]
817 }
818 else:
819@@ -785,11 +772,11 @@
820
821 ``name``
822 The name to search the id.
823-
824+
825 ``language_id``
826 The language_id for which language should be searched
827 """
828- log.debug(u'BiblesResourcesDB.get_alternative_book_name("%s", "%s")',
829+ log.debug(u'BiblesResourcesDB.get_alternative_book_name("%s", "%s")',
830 name, language_id)
831 if language_id:
832 books = BiblesResourcesDB.run_sql(u'SELECT book_reference_id, name '
833@@ -806,7 +793,7 @@
834 @staticmethod
835 def get_language(name):
836 """
837- Return a dict containing the language id, name and code by name or
838+ Return a dict containing the language id, name and code by name or
839 abbreviation.
840
841 ``name``
842@@ -865,7 +852,7 @@
843
844 class AlternativeBookNamesDB(QtCore.QObject, Manager):
845 """
846- This class represents a database-bound alternative book names system.
847+ This class represents a database-bound alternative book names system.
848 """
849 cursor = None
850 conn = None
851@@ -874,7 +861,7 @@
852 def get_cursor():
853 """
854 Return the cursor object. Instantiate one if it doesn't exist yet.
855- If necessary loads up the database and creates the tables if the
856+ If necessary loads up the database and creates the tables if the
857 database doesn't exist.
858 """
859 if AlternativeBookNamesDB.cursor is None:
860@@ -904,7 +891,7 @@
861
862 ``parameters``
863 Any variable parameters to add to the query
864-
865+
866 ``commit``
867 If a commit statement is necessary this should be True.
868 """
869@@ -921,11 +908,11 @@
870
871 ``name``
872 The name to search the id.
873-
874+
875 ``language_id``
876 The language_id for which language should be searched
877 """
878- log.debug(u'AlternativeBookNamesDB.get_book_reference_id("%s", "%s")',
879+ log.debug(u'AlternativeBookNamesDB.get_book_reference_id("%s", "%s")',
880 name, language_id)
881 if language_id:
882 books = AlternativeBookNamesDB.run_sql(u'SELECT book_reference_id, '
883@@ -962,11 +949,11 @@
884
885 class OldBibleDB(QtCore.QObject, Manager):
886 """
887- This class conects to the old bible databases to reimport them to the new
888+ This class conects to the old bible databases to reimport them to the new
889 database scheme.
890 """
891 cursor = None
892-
893+
894 def __init__(self, parent, **kwargs):
895 """
896 The constructor loads up the database and creates and initialises the
897
898=== modified file 'openlp/plugins/bibles/lib/manager.py'
899--- openlp/plugins/bibles/lib/manager.py 2011-07-07 18:03:12 +0000
900+++ openlp/plugins/bibles/lib/manager.py 2011-08-15 09:31:39 +0000
901@@ -153,7 +153,7 @@
902 if name is None:
903 delete_file(os.path.join(self.path, filename))
904 continue
905- # Find old database versions
906+ # Find old database versions.
907 if bible.is_old_database():
908 self.old_bible_databases.append([filename, name])
909 bible.session.close()
910@@ -220,7 +220,7 @@
911 return [
912 {
913 u'name': book.name,
914- u'book_reference_id': book.book_reference_id,
915+ u'book_reference_id': book.book_reference_id,
916 u'chapters': self.db_cache[bible].get_chapter_count(book)
917 }
918 for book in self.db_cache[bible].get_books()
919@@ -229,10 +229,10 @@
920 def get_chapter_count(self, bible, book):
921 """
922 Returns the number of Chapters for a given book.
923-
924+
925 ``bible``
926 Unicode. The Bible to get the list of books from.
927-
928+
929 ``book``
930 The book object to get the chapter count for.
931 """
932@@ -295,7 +295,7 @@
933 if db_book:
934 book_id = db_book.book_reference_id
935 log.debug(u'Book name corrected to "%s"', db_book.name)
936- new_reflist.append((book_id, item[1], item[2],
937+ new_reflist.append((book_id, item[1], item[2],
938 item[3]))
939 else:
940 log.debug(u'OpenLP failed to find book %s', item[0])
941
942=== modified file 'openlp/plugins/bibles/lib/mediaitem.py'
943--- openlp/plugins/bibles/lib/mediaitem.py 2011-08-13 10:49:53 +0000
944+++ openlp/plugins/bibles/lib/mediaitem.py 2011-08-15 09:31:39 +0000
945@@ -613,7 +613,7 @@
946 if restore:
947 old_text = unicode(combo.currentText())
948 combo.clear()
949- combo.addItems([unicode(i) for i in range(range_from, range_to + 1)])
950+ combo.addItems(map(unicode, range(range_from, range_to + 1)))
951 if restore and combo.findText(old_text) != -1:
952 combo.setCurrentIndex(combo.findText(old_text))
953
954
955=== modified file 'openlp/plugins/songs/lib/openlyricsexport.py'
956--- openlp/plugins/songs/lib/openlyricsexport.py 2011-06-19 21:11:29 +0000
957+++ openlp/plugins/songs/lib/openlyricsexport.py 2011-08-15 09:31:39 +0000
958@@ -35,6 +35,7 @@
959 from lxml import etree
960
961 from openlp.core.lib import check_directory_exists, Receiver, translate
962+from openlp.core.utils import clean_filename
963 from openlp.plugins.songs.lib import OpenLyrics
964
965 log = logging.getLogger(__name__)
966@@ -72,8 +73,7 @@
967 tree = etree.ElementTree(etree.fromstring(xml))
968 filename = u'%s (%s)' % (song.title,
969 u', '.join([author.display_name for author in song.authors]))
970- filename = re.sub(
971- r'[/\\?*|<>\[\]":<>+%]+', u'_', filename).strip(u'_')
972+ filename = clean_filename(filename)
973 # Ensure the filename isn't too long for some filesystems
974 filename = u'%s.xml' % filename[0:250 - len(self.save_path)]
975 # Pass a file object, because lxml does not cope with some special