Merge lp:~j-corwin/openlp/migration into lp:openlp

[Raoul Snyman]

[Jonathan Corwin (j-corwin)]

Attempt to fix song migration so it matches current file definitions.
Remove the need to edit the dmp file as part of the migration

[Raoul Snyman (raoul-snyman)]

Muhahaha! The Style Police has arrived!

(Yes, this is mostly old code, but it needs to be said!)

All strings are single-quoted and unicode:

  self.display.output("Songs processing started")

  should be:

  self.display.output(u'Songs processing started')

Why are you adding columns one at a time?

  conn.execute("""create table authors (id integer primary key ASC AUTOINCREMENT);""")
  conn.commit()
  self.display.sub_output("authors table created")
  conn.execute("""alter table authors add column first_name varchar(128);""")
  conn.commit()
  self.display.sub_output("first_name added")
  conn.execute("""alter table authors add column last_name varchar(128);""")
  conn.commit()

  Shouldn't that rather be:

  conn.execute("""
    create table authors (
      id integer primary key ASC AUTOINCREMENT,
      first_name varchar(128),
      last_name varchar(128),
      display_name varchar(255)
    );""")
  conn.commit()
  self.display.sub_output("authors table created")

Also name variables better:

  # I'm not sure what these two variables are supposed to be
  def run_cmd(self, cmd):
      f_i, f_o = os.popen4(cmd)

review: Needs Fixing

[Jonathan Corwin (j-corwin)]

On Sat, May 2, 2009 at 9:31 PM, Raoul Snyman <
<email address hidden>> wrote:

> Review: Needs Fixing
> Muhahaha! The Style Police has arrived!
>
> (Yes, this is mostly old code, but it needs to be said!)
>
> All strings are single-quoted and unicode:
>
> self.display.output("Songs processing started")
>
> should be:
>
> self.display.output(u'Songs processing started')

Was already there, nowt to do with me

>
> Why are you adding columns one at a time?
>
> conn.execute("""create table authors (id integer primary key ASC
> AUTOINCREMENT);""")
> conn.commit()
> self.display.sub_output("authors table created")
> conn.execute("""alter table authors add column first_name
> varchar(128);""")
> conn.commit()
> self.display.sub_output("first_name added")
> conn.execute("""alter table authors add column last_name varchar(128);""")
> conn.commit()
>
> Shouldn't that rather be:

> conn.execute("""
> create table authors (
> id integer primary key ASC AUTOINCREMENT,
> first_name varchar(128),
> last_name varchar(128),
> display_name varchar(255)
> );""")
> conn.commit()
> self.display.sub_output("authors table created")

I agree, but again was just copying what was already there since I didn't
know if the original author had done it that way on purpose

>
> Also name variables better:
>
> # I'm not sure what these two variables are supposed to be
> def run_cmd(self, cmd):
> f_i, f_o = os.popen4(cmd)

I didn't touch this function and have no idea what it does.

All I was doing was updating existing code so it worked... I wasn't planning
on rewriting the whole existing
module.

[Jonathan Corwin (j-corwin)]

> Muhahaha! The Style Police has arrived!

Changes pushed.

[Raoul Snyman (raoul-snyman)]

Yes, I know it's not your code, but I'm just saying this for the record...

Use the os.path module functions, they tend to take care of little things:

  conn = sqlite3.connect(self.data_path + os.sep + database)

  should rather be:

  conn = sqlite3.connect(os.path.join(self.data_path, database))

Indices really should have functional names... "author1" is not a good name:

  conn.execute("""create index if not exists author1 on authors (display_name ASC,id ASC);""")

  better:

  conn.execute("""create index if not exists author_display_name on authors (display_name ASC,id ASC);""")

I'm a little wary of all these "replace"s - there's probably a better way...

  replace(replace(replace(replace(replace(replace(replace(replace(

review: Approve (approve)

[Tim Bentley (trb143)]

Not user about replace(replace(replace as it's done is SQL then thats life

Does this handle Unicode character sets. If it does it simplifes the database migration process.

review: Approve

[Jonathan Corwin (j-corwin)]

On Sun, May 3, 2009 at 12:37 AM, Raoul Snyman <
<email address hidden>> wrote:
y.

>
> I'm a little wary of all these "replace"s - there's probably a better
> way...
>
> replace(replace(replace(replace(replace(replace(replace(replace(
>
> In the original code it was done in the python as lots of individual
replaces. However the python process of selecting 3000 records (in my case)
and updating each one individually took over 5 minutes on my machine.
Doing it in sqlite direct took 5 seconds...

I agree it doesn't look pretty though.

Jonathan.

[lp:~j-corwin/openlp/migration]

439. By Raoul Snyman on 2009-05-03

Added images

440. By Raoul Snyman on 2009-05-03

Merged Jonathan Corwin's changes to the database migration script.

=== modified file 'openlp/migration/migratesongs.py'
--- openlp/migration/migratesongs.py	2009-02-04 20:16:56 +0000
+++ openlp/migration/migratesongs.py	2009-05-02 21:40:10 +0000
@@ -31,231 +31,210 @@
 clear_mappers()
 mapper(Author, authors_table)
 mapper(Book, song_books_table)
-mapper(Song, songs_table,
-       properties={'authors': relation(Author, backref='songs',
-                                       secondary=authors_songs_table),
-                   'book': relation(Book, backref='songs'),
-                   'topics': relation(Topic, backref='songs',
-                                      secondary=songs_topics_table)})
+mapper(Song, songs_table, properties={
+        'authors': relation(Author, backref='songs',
+        secondary=authors_songs_table),
+        'book': relation(Book, backref='songs'),
+        'topics': relation(Topic, backref='songs',
+        secondary=songs_topics_table)})
 mapper(Topic, topics_table)
 
 class MigrateSongs():
     def __init__(self, display):
         self.display = display
-        self.config = PluginConfig("Songs")
+        self.config = PluginConfig('Songs')
         self.data_path = self.config.get_data_path()
-        self.database_files = self.config.get_files("sqlite")
+        self.database_files = self.config.get_files('sqlite')
         print self.database_files
 
     def process(self):
-        self.display.output("Songs processing started");
+        self.display.output(u'Songs processing started')
         for f in self.database_files:
             self.v_1_9_0(f)
-        self.display.output("Songs processing finished");
+        self.display.output(u'Songs processing finished')
 
     def v_1_9_0(self, database):
-        self.display.output("Migration 1.9.0 Started for "+database);
+        self.display.output(u'Migration 1.9.0 Started for ' + database)
         self._v1_9_0_authors(database)
         self._v1_9_0_topics(database)
         self._v1_9_0_songbook(database)
         self._v1_9_0_songauthors(database)
         self._v1_9_0_songtopics(database)
         self._v1_9_0_songs(database)
-        self._v1_9_0_songs_update(database)
-
-        self.display.output("Migration 1.9.0 Finished for " + database);
+        self.display.output(u'Migration 1.9.0 Finished for ' + database)
 
     def _v1_9_0_authors(self, database):
-        self.display.sub_output("Authors Started for "+database);
-        conn = sqlite3.connect(self.data_path+os.sep+database)
+        self.display.sub_output(u'Authors Started for ' + database)
+        conn = sqlite3.connect(self.data_path + os.sep + database)
         conn.execute("""alter table authors rename to authors_temp;""")
         conn.commit()
-
-        conn.execute("""create table authors add column display_name varchar(255);""")
-        conn.commit()
-        self.display.sub_output("first name created")
-        conn.execute("""alter table authors add column first_name varchar(128);""")
-        conn.commit()
-        self.display.sub_output("first name created")
-        conn.execute("""alter table authors add column last_name varchar(128);""")
-        conn.commit()
-        self.display.sub_output("last name created")
-        conn.execute("""create index if not exists author_display_name on authors (display_name ASC,id ASC);""")
-        conn.commit()
-        self.display.sub_output("index author1 created")
-        conn.execute("""create index if not exists author_last_name on authors (last_name ASC,id ASC);""")
-        conn.commit()
-        self.display.sub_output("index author2 created")
-        conn.execute("""create index if not exists author_first_name on authors (first_name ASC,id ASC);""")
-        conn.commit()
-        self.display.sub_output("index author3 created")
-        self.display.sub_output("Author Data Migration started")
-        c = conn.cursor()
-        text = c.execute("""select * from authors """) .fetchall()
+        self.display.sub_output(u'old author renamed to author_temp')
+        conn.execute("""create table authors (
+            id integer primary key ASC AUTOINCREMENT,
+            first_name varchar(128),
+            last_name varchar(128),
+            display_name varchar(255)
+            );""")
+        conn.commit()
+        self.display.sub_output(u'authors table created')
+        conn.execute("""create index if not exists author1 on authors 
+            (display_name ASC,id ASC);""")
+        conn.commit()
+        self.display.sub_output(u'index author1 created')
+        conn.execute("""create index if not exists author2 on authors 
+            (last_name ASC,id ASC);""")
+        conn.commit()
+        self.display.sub_output(u'index author2 created')
+        conn.execute("""create index if not exists author3 on authors 
+            (first_name ASC,id ASC);""")
+        conn.commit()
+        self.display.sub_output(u'index author3 created')
+        self.display.sub_output(u'Author Data Migration started')
+        conn.execute("""insert into authors (id, display_name) 
+            select authorid, authorname from authors_temp;""")
+        conn.commit()
+        self.display.sub_output(u'authors populated')
+        c = conn.cursor()        
+        text = c.execute("""select * from authors""") .fetchall()
         for author in text:
-            dispname = author[1]
-            if author[2] == None:
-                dispname = dispname.replace("'", "") # remove quotes.
-                pos = dispname.rfind(" ")
-                afn = dispname[:pos]
-                aln = dispname[pos + 1:len(dispname)]
-        #txt = text[2]
-                s = "update authors set display_name = '" + dispname + "', first_name = '" + afn + "', last_name = '" + aln + "' where id = " +str(author[0])
-                text1 = c.execute(s)
-        conn.commit()
-        conn.execute("""alter table authors drop column authorname;""")
-        conn.commit()
-        conn.close()
-        self.display.sub_output("Author Data Migration Completed")
-        self.display.sub_output("Authors Completed");
+            dispname = author[3]
+            dispname = dispname.replace("'", "") 
+            pos = dispname.rfind(" ")
+            authorfirstname = dispname[:pos]
+            authorlastname = dispname[pos + 1:len(dispname)]
+            s = "update authors set first_name = '" \
+                + authorfirstname + "', last_name = '" + authorlastname \
+                + "' where id = " + str(author[0])
+            c.execute(s)
+        conn.commit()
+        self.display.sub_output(u'Author Data Migration Completed')
+        conn.execute("""drop table authors_temp;""")
+        conn.commit()
+        conn.close()
+        self.display.sub_output(u'author_temp dropped')
+        self.display.sub_output(u'Authors Completed')
+
+    def _v1_9_0_songbook(self, database):
+        self.display.sub_output(u'SongBook Started for ' + database)
+        conn = sqlite3.connect(self.data_path + os.sep + database)
+        conn.execute("""create table if not exists song_books (
+            id integer Primary Key ASC AUTOINCREMENT,
+            name varchar(128),
+            publisher varchar(128)
+            );""")
+        conn.commit()
+        self.display.sub_output(u'songbook table created')
+        conn.execute("""create index if not exists songbook1 on song_books (name ASC,id ASC);""")
+        conn.commit()
+        self.display.sub_output(u'index songbook1 created')
+        conn.execute("""create index if not exists songbook2 on song_books (publisher ASC,id ASC);""")
+        conn.commit()
+        conn.close()
+        self.display.sub_output(u'index songbook2 created')
+        self.display.sub_output(u'SongBook Completed')
+
+    def _v1_9_0_songs(self, database):
+        self.display.sub_output(u'Songs Started for ' + database)
+        conn = sqlite3.connect(self.data_path + os.sep + database)
+        conn.execute("""alter table songs rename to songs_temp;""")
+        conn.commit()
+        conn.execute("""create table if not exists songs  (
+            id integer Primary Key ASC AUTOINCREMENT,
+            song_book_id integer,
+            title varchar(255),
+            lyrics text,
+            verse_order varchar(128),
+            copyright varchar(255),
+            comments text,
+            ccli_number varchar(64),
+            song_number varchar(64),
+            theme_name varchar(128),
+            search_title varchar(255),
+            search_lyrics text
+            );""")
+        conn.commit()
+        self.display.sub_output(u'songs table created')
+        conn.execute("""create index if not exists songs1 on songs 
+            (search_lyrics ASC,id ASC);""")
+        conn.commit()
+        self.display.sub_output(u'index songs1 created')
+        conn.execute("""create index if not exists songs2 on songs 
+            (search_title ASC,id ASC);""")
+        conn.commit()
+        self.display.sub_output(u'index songs2 created')
+        conn.execute("""insert into songs (id, title, lyrics, copyright,
+            search_title, search_lyrics, song_book_id) 
+            select songid,  songtitle, lyrics, copyrightinfo, 
+            replace(replace(replace(replace(replace(replace(replace(replace(
+            replace(songtitle,  '&', 'and'), ',', ''), ';', ''), ':', ''), 
+            '(', ''), ')', ''), '{', ''), '}',''),'?',''), 
+            replace(replace(replace(replace(replace(replace(replace(replace(
+            replace(lyrics,  '&', 'and'), ',', ''), ';', ''), ':', ''),
+            '(', ''), ')', ''), '{', ''), '}',''),'?',''),
+            0
+            from songs_temp;""")
+        conn.commit()
+        self.display.sub_output(u'songs populated')
+        conn.execute("""drop table songs_temp;""")
+        conn.commit()
+        conn.close()
+        self.display.sub_output(u'songs_temp dropped')
+        self.display.sub_output(u'Songs Completed')
 
     def _v1_9_0_topics(self, database):
-        self.display.sub_output("Topics Started for "+database);
+        self.display.sub_output(u'Topics Started for ' + database)
         conn = sqlite3.connect(self.data_path+os.sep+database)
         conn.text_factory = str
-        conn.execute("""create table if not exists topics (id integer Primary Key ASC AUTOINCREMENT);""")
-        conn.commit()
-        self.display.sub_output("Topic table created")
-        conn.execute("""alter table topics add column name varchar(128);""")
-        conn.commit()
-        self.display.sub_output("topicname added")
+        conn.execute("""create table if not exists topics 
+            (id integer Primary Key ASC AUTOINCREMENT,
+            name varchar(128));""")
+        conn.commit()
+        self.display.sub_output(u'Topic table created')
         conn.execute("""create index if not exists topic1 on topics (name ASC,id ASC);""")
         conn.commit()
         conn.close()
-        self.display.sub_output("index topic1 created")
-
-        self.display.sub_output("Topics Completed");
-
-    def _v1_9_0_songbook(self, database):
-        self.display.sub_output("SongBook Started for "+database);
-        conn = sqlite3.connect(self.data_path+os.sep+database)
-        conn.execute("""create table if not exists song_books (id integer Primary Key ASC AUTOINCREMENT);""")
-        conn.commit()
-        self.display.sub_output("SongBook table created")
-        conn.execute("""alter table song_books add column name varchar(128);""")
-        conn.commit()
-        self.display.sub_output("songbook_name added")
-        conn.execute("""alter table song_books add column publisher varchar(128);""")
-        conn.commit()
-        self.display.sub_output("songbook_publisher added")
-        conn.execute("""create index if not exists songbook1 on song_books (name ASC,id ASC);""")
-        conn.commit()
-        self.display.sub_output("index songbook1 created")
-        conn.execute("""create index if not exists songbook2 on song_books (publisher ASC,id ASC);""")
-        conn.commit()
+        self.display.sub_output(u'index topic1 created')
+
+        self.display.sub_output(u'Topics Completed')
+
+    def _v1_9_0_songauthors(self, database):
+        self.display.sub_output(u'SongAuthors Started for ' + database);
+        conn = sqlite3.connect(self.data_path + os.sep + database)
+        conn.execute("""create table if not exists authors_songs 
+            (author_id integer,
+            song_id integer);""")
+        conn.commit()
+        self.display.sub_output(u'authors_songs table created')
+        conn.execute("""insert into authors_songs (author_id, song_id) 
+            select authorid, songid from songauthors;""")
+        conn.commit()
+        self.display.sub_output(u'authors_songs populated')
+        conn.execute("""drop table songauthors;""")
+        conn.commit()
+        self.display.sub_output(u'songauthors dropped')
         conn.close()
-        self.display.sub_output("index songbook2 created")
-        self.display.sub_output("SongBook Completed");
+        self.display.sub_output(u'SongAuthors Completed')
 
     def _v1_9_0_songtopics(self, database):
-        self.display.sub_output("Songtopics Started for "+database);
+        self.display.sub_output(u'Songtopics Started for ' + database);
         conn = sqlite3.connect(self.data_path+os.sep+database)
-        conn.execute("""create table if not exists song_topics (song_id integer Primary Key ASC );""")
-        conn.commit()
-        self.display.sub_output("Songtopics table created")
-        conn.execute("""alter table song_topics add column topic_id interger;""")
-        conn.commit()
-        self.display.sub_output("songtopics_topic_id added")
+        conn.execute("""create table if not exists song_topics 
+            (song_id integer,
+            topic_id integer);""")
+        conn.commit()
+        self.display.sub_output(u'songtopics table created')
         conn.execute("""create index if not exists songtopic1 on song_topics (topic_id ASC,song_id ASC);""")
         conn.commit()
-        self.display.sub_output("index songbook1 created")
-        conn.execute("""create index if not exists songbook2 on song_topics (song_id ASC,topic_id ASC);""")
-        conn.commit()
-        conn.close()
-        self.display.sub_output("index songbook2 created")
-        self.display.sub_output("SongTopics Completed");
-
-    def _v1_9_0_songauthors(self, database):
-        self.display.sub_output("SongAuthors Started for "+database);
-        conn = sqlite3.connect(self.data_path+os.sep+database)
-        conn.execute("""alter table songauthors rename to authors_songs;""")
-        conn.commit()
-        conn.close()
-        self.display.sub_output("Table Renamed")
-        self.display.sub_output("SongAuthors Completed");
-
-
-    def _v1_9_0_songs(self, database):
-        self.display.sub_output("Songs Started for "+database);
-        conn = sqlite3.connect(self.data_path+os.sep+database)
-        conn.execute("""alter table songs add column song_book_id interger;""")
-        conn.commit()
-        self.display.sub_output("songs_song_book_id added")
-        conn.execute("""alter table songs add column verse_order varchar(128);""")
-        conn.commit()
-        self.display.sub_output("songs verse_order added")
-        conn.execute("""alter table songs add column comments text;""")
-        conn.commit()
-        self.display.sub_output("songs comments added")
-        conn.execute("""alter table songs add ccli_number varchar(64);""")
-        conn.commit()
-        self.display.sub_output("songs ccli_number added")
-        conn.execute("""alter table songs add song_number varchar(64);""")
-        conn.commit()
-        self.display.sub_output("songs song_number added")
-        conn.execute("""alter table songs add theme_name varchar(128);""")
-        conn.commit()
-        self.display.sub_output("songs theme_name added")
-        conn.execute("""alter table songs add search_title varchar(255);""")
-        conn.commit()
-        self.display.sub_output("songs search_title added")
-        conn.execute("""alter table songs add search_lyrics text;""")
-        conn.commit()
-        self.display.sub_output("songs search_lyrics added")
-
-        conn.execute("""create index if not exists songs1 on songs (search_lyrics ASC,id ASC);""")
-        conn.commit()
-        self.display.sub_output("index songs1 created")
-        conn.execute("""create index if not exists songs2 on songs (search_title ASC,id ASC);""")
-        conn.commit()
-        conn.close()
-        self.display.sub_output("index songs2 created")
-        self.display.sub_output("Songs Completed");
-
-
-
-    def _v1_9_0_songs_update(self, database):
-        self.display.sub_output("Songs Started for "+database);
-        self.db = create_engine("sqlite:///"+self.data_path+os.sep+database, encoding='utf-8' , convert_unicode=False, assert_unicode=False)
-
-        self.db.echo = True
-        metadata.bind = self.db
-        metadata.bind.echo = False
-        Session = scoped_session(sessionmaker(autoflush=True, autocommit=False))
-        Session.configure(bind=self.db)
-        #create missing table
-        songs_topics_table.create()
-        songs = Session.query(Song).all()
-        for song in songs:
-            t=song.title.replace("&", "and")
-            t=t.replace("'", "")
-            t=t.replace(",", "")
-            t=t.replace(";", "")
-            t=t.replace(":", "")
-            t=t.replace("(", "")
-            t=t.replace(")", "")
-            t=t.replace("{", "")
-            t=t.replace("}", "")
-            t=t.replace("?", "")
-            song.search_title = t
-            t=song.lyrics.replace("&", "and")
-            t=t.replace("'", "")
-            t=t.replace(",", "")
-            t=t.replace(";", "")
-            t=t.replace(":", "")
-            t=t.replace("(", "")
-            t=t.replace(")", "")
-            t=t.replace("{", "")
-            t=t.replace("}", "")
-            t=t.replace("?", "")
-            song.search_lyrics = t
-            song.song_book_id = 0
-            Session.save_or_update(song)
-            Session.commit()
+        self.display.sub_output(u'index songtopic1 created')
+        conn.execute("""create index if not exists songtopic2 on song_topics (song_id ASC,topic_id ASC);""")
+        conn.commit()
+        conn.close()
+        self.display.sub_output(u'index songtopic2 created')
+        self.display.sub_output(u'SongTopics Completed')
 
     def run_cmd(self, cmd):
-        f_i, f_o  = os.popen4(cmd)
-        out = f_o.readlines()
+        filein, fileout  = os.popen4(cmd)
+        out = fileout.readlines()
         if len(out) > 0:
-              for o in range (0, len(out)):
+            for o in range (0, len(out)):
                 self.display.sub_output(out[o])

=== modified file 'openlpcnv.pyw' (properties changed: -x to +x)