Merge lp:~raoul-snyman/openlp/bug-714510 into lp:openlp

[Raoul Snyman]

Fix bug #714510 by removing the unnecessary indexes and defaulting the song_book_id on the songs table to NULL instead of 0.

Fix bug #714510 by removing the indexes, which were unnecessary and caused various foreign key constraints in real relational databases like InnoDB and PostgreSQL. I also defaulted the song book id on songs to NULL instead of 0, since NULL is a valid value for foreign keys.

[Jonathan Corwin (j-corwin)]

Part way there, I was able to start OpenLP and create a new song.
Then I loaded an existing service file which contained songs that weren't in the database, and I got:

Traceback (most recent call last):
  File "C:\Users\jonathan\Documents\projects\openlp\bug-714510\openlp\plugins\songs\lib\mediaitem.py", line 480, in serviceLoad
    editId = self.openLyrics.xml_to_song(item.xml_version)
  File "C:\Users\jonathan\Documents\projects\openlp\bug-714510\openlp\plugins\songs\lib\xml.py", line 343, in xml_to_song
    self.manager.save_object(song)
  File "C:\Users\jonathan\Documents\projects\openlp\bug-714510\openlp\core\lib\db.py", line 152, in save_object
    self.session.commit()
  File "C:\Python27\lib\site-packages\sqlalchemy-0.6.6-py2.7.egg\sqlalchemy\orm\scoping.py", line 139, in do
    return getattr(self.registry(), name)(*args, **kwargs)
  File "C:\Python27\lib\site-packages\sqlalchemy-0.6.6-py2.7.egg\sqlalchemy\orm\session.py", line 614, in commit
    self.transaction.commit()
  File "C:\Python27\lib\site-packages\sqlalchemy-0.6.6-py2.7.egg\sqlalchemy\orm\session.py", line 385, in commit
    self._prepare_impl()
  File "C:\Python27\lib\site-packages\sqlalchemy-0.6.6-py2.7.egg\sqlalchemy\orm\session.py", line 369, in _prepare_impl
    self.session.flush()
  File "C:\Python27\lib\site-packages\sqlalchemy-0.6.6-py2.7.egg\sqlalchemy\orm\session.py", line 1388, in flush
    self._flush(objects)
  File "C:\Python27\lib\site-packages\sqlalchemy-0.6.6-py2.7.egg\sqlalchemy\orm\session.py", line 1469, in _flush
    flush_context.execute()
  File "C:\Python27\lib\site-packages\sqlalchemy-0.6.6-py2.7.egg\sqlalchemy\orm\unitofwork.py", line 302, in execute
    rec.execute(self)
  File "C:\Python27\lib\site-packages\sqlalchemy-0.6.6-py2.7.egg\sqlalchemy\orm\unitofwork.py", line 446, in execute
    uow
  File "C:\Python27\lib\site-packages\sqlalchemy-0.6.6-py2.7.egg\sqlalchemy\orm\mapper.py", line 1878, in _save_obj
    execute(statement, params)
  File "C:\Python27\lib\site-packages\sqlalchemy-0.6.6-py2.7.egg\sqlalchemy\engine\base.py", line 1191, in execute
    params)
  File "C:\Python27\lib\site-packages\sqlalchemy-0.6.6-py2.7.egg\sqlalchemy\engine\base.py", line 1271, in _execute_clauseelement
    return self.__execute_context(context)
  File "C:\Python27\lib\site-packages\sqlalchemy-0.6.6-py2.7.egg\sqlalchemy\engine\base.py", line 1302, in __execute_context
    context.parameters[0], context=context)
  File "C:\Python27\lib\site-packages\sqlalchemy-0.6.6-py2.7.egg\sqlalchemy\engine\base.py", line 1401, in _cursor_execute
    context)
  File "C:\Python27\lib\site-packages\sqlalchemy-0.6.6-py2.7.egg\sqlalchemy\engine\base.py", line 1394, in _cursor_execute
    context)
  File "C:\Python27\lib\site-packages\sqlalchemy-0.6.6-py2.7.egg\sqlalchemy\engine\default.py", line 299, in do_execute
    cursor.execute(statement, parameters)
  File "C:\Python27\lib\site-packages\MySQLdb\cursors.py", line 174, in execute
    self.errorhandler(self, exc, value)
  File "C:\Python27\lib\site-packages\MySQLdb\connections.py", line 36, in defaulterrorhandler
    raise errorclass, errorvalue
IntegrityError: (IntegrityError) (1452, 'Cannot add or update a child row: a ...

[Andreas Preikschat (googol-deactivatedaccount)]

Sure we need the import re?

;-)

[Raoul Snyman (raoul-snyman)]

Yes, when I tried to import some songs, it failed because the import was not there.

[Raoul Snyman (raoul-snyman)]

See line 452, or 453 with the "import re" line.

[Jonathan Corwin (j-corwin)]

review: Approve

[Tim Bentley (trb143)]

review: Approve

=== modified file 'openlp/plugins/songs/lib/db.py'
--- openlp/plugins/songs/lib/db.py	2011-05-27 08:21:23 +0000
+++ openlp/plugins/songs/lib/db.py	2011-05-30 11:23:23 +0000
@@ -165,7 +165,7 @@
         Column(u'id', types.Integer, primary_key=True),
         Column(u'first_name', types.Unicode(128)),
         Column(u'last_name', types.Unicode(128)),
-        Column(u'display_name', types.Unicode(255), nullable=False)
+        Column(u'display_name', types.Unicode(255), index=True, nullable=False)
     )
 
     # Definition of the "media_files" table
@@ -186,7 +186,7 @@
     songs_table = Table(u'songs', metadata,
         Column(u'id', types.Integer, primary_key=True),
         Column(u'song_book_id', types.Integer,
-            ForeignKey(u'song_books.id'), default=0),
+            ForeignKey(u'song_books.id'), default=None),
         Column(u'title', types.Unicode(255), nullable=False),
         Column(u'alternate_title', types.Unicode(255)),
         Column(u'lyrics', types.UnicodeText, nullable=False),
@@ -203,7 +203,7 @@
     # Definition of the "topics" table
     topics_table = Table(u'topics', metadata,
         Column(u'id', types.Integer, primary_key=True),
-        Column(u'name', types.Unicode(128), nullable=False)
+        Column(u'name', types.Unicode(128), index=True, nullable=False)
     )
 
     # Definition of the "authors_songs" table
@@ -230,27 +230,6 @@
             ForeignKey(u'topics.id'), primary_key=True)
     )
 
-    # Define table indexes
-    Index(u'authors_id', authors_table.c.id)
-    Index(u'authors_display_name_id', authors_table.c.display_name,
-        authors_table.c.id)
-    Index(u'media_files_id', media_files_table.c.id)
-    Index(u'song_books_id', song_books_table.c.id)
-    Index(u'songs_id', songs_table.c.id)
-    Index(u'topics_id', topics_table.c.id)
-    Index(u'authors_songs_author', authors_songs_table.c.author_id,
-        authors_songs_table.c.song_id)
-    Index(u'authors_songs_song', authors_songs_table.c.song_id,
-        authors_songs_table.c.author_id)
-    Index(u'media_files_songs_file', media_files_songs_table.c.media_file_id,
-        media_files_songs_table.c.song_id)
-    Index(u'media_files_songs_song', media_files_songs_table.c.song_id,
-        media_files_songs_table.c.media_file_id)
-    Index(u'topics_song_topic', songs_topics_table.c.topic_id,
-        songs_topics_table.c.song_id)
-    Index(u'topics_song_song', songs_topics_table.c.song_id,
-        songs_topics_table.c.topic_id)
-
     mapper(Author, authors_table)
     mapper(Book, song_books_table)
     mapper(MediaFile, media_files_table)

=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
--- openlp/plugins/songs/lib/mediaitem.py	2011-05-28 19:38:19 +0000
+++ openlp/plugins/songs/lib/mediaitem.py	2011-05-30 11:23:23 +0000
@@ -27,6 +27,7 @@
 
 import logging
 import locale
+import re
 
 from PyQt4 import QtCore, QtGui
 from sqlalchemy.sql import or_

=== modified file 'openlp/plugins/songs/lib/xml.py'
--- openlp/plugins/songs/lib/xml.py	2011-05-26 17:11:22 +0000
+++ openlp/plugins/songs/lib/xml.py	2011-05-30 11:23:23 +0000
@@ -514,7 +514,7 @@
         ``song``
             The song object.
         """
-        song.song_book_id = 0
+        song.song_book_id = None
         song.song_number = u''
         if hasattr(properties, u'songbooks'):
             for songbook in properties.songbooks.songbook: