Give Albums their own ID counter, own group rel table

etiquette/constants.py

@@ -67,6 +67,10 @@ SQL_SYN_COLUMNS = [
     'name',
     'master',
 ]
+SQL_ALBUMGROUP_COLUMNS = [
+    'parentid',
+    'memberid',
+]
 SQL_ALBUMPHOTO_COLUMNS = [
     'albumid',
     'photoid',
@@ -88,6 +92,7 @@ SQL_USER_COLUMNS = [
 
 _sql_dictify = lambda columns: {key:index for (index, key) in enumerate(columns)}
 SQL_ALBUM = _sql_dictify(SQL_ALBUM_COLUMNS)
+SQL_ALBUMGROUP = _sql_dictify(SQL_ALBUMGROUP_COLUMNS)
 SQL_BOOKMARK = _sql_dictify(SQL_BOOKMARK_COLUMNS)
 SQL_ALBUMPHOTO = _sql_dictify(SQL_ALBUMPHOTO_COLUMNS)
 SQL_LASTID = _sql_dictify(SQL_LASTID_COLUMNS)

etiquette/objects.py

@@ -31,6 +31,10 @@ class ObjectBase:
 
 
 class GroupableMixin:
+    group_getter = None
+    group_sql_index = None
+    group_table = None
+
     def add(self, member, *, commit=True):
         '''
         Add a child object to this group.
@@ -46,33 +50,43 @@ class GroupableMixin:
         # Groupables are only allowed to have 1 parent.
         # Unlike photos which can exist in multiple albums.
         cur = self.photodb.sql.cursor()
-        cur.execute('SELECT * FROM tag_group_rel WHERE memberid == ?', [member.id])
+        cur.execute(
+            'SELECT * FROM %s WHERE memberid == ?' % self.group_table,
+            [member.id]
+        )
         fetch = cur.fetchone()
         if fetch is not None:
-            parent_id = fetch[constants.SQL_TAGGROUP['parentid']]
+            parent_id = fetch[self.group_sql_index['parentid']]
             if parent_id == self.id:
                 that_group = self
             else:
                 that_group = self.group_getter(id=parent_id)
-            raise exceptions.GroupExists('%s already in group %s' % (member.name, that_group.name))
+            raise exceptions.GroupExists('%s already in group %s' % (member, that_group))
 
         for parent in self.walk_parents():
-            if parent.id == member.id:
+            if parent == member:
-                raise exceptions.RecursiveGrouping('%s is an ancestor of %s' % (member.name, self.name))
+                raise exceptions.RecursiveGrouping('%s is an ancestor of %s' % (member, self))
 
         self.photodb._cached_frozen_children = None
-        cur.execute('INSERT INTO tag_group_rel VALUES(?, ?)', [self.id, member.id])
+        cur.execute(
+            'INSERT INTO %s VALUES(?, ?)' % self.group_table,
+            [self.id, member.id]
+        )
         if commit:
             self.photodb.log.debug('Committing - add to group')
             self.photodb.commit()
 
     def children(self):
         cur = self.photodb.sql.cursor()
-        cur.execute('SELECT * FROM tag_group_rel WHERE parentid == ?', [self.id])
+        
+        cur.execute(
+            'SELECT * FROM %s WHERE parentid == ?' % self.group_table,
+            [self.id]
+        )
         fetch = cur.fetchall()
         results = []
         for f in fetch:
-            memberid = f[constants.SQL_TAGGROUP['memberid']]
+            memberid = f[self.group_sql_index['memberid']]
             child = self.group_getter(id=memberid)
             results.append(child)
         if isinstance(self, Tag):
@@ -105,16 +119,22 @@ class GroupableMixin:
             if parent is None:
                 # Since this group was a root, children become roots by removing
                 # the row.
-                cur.execute('DELETE FROM tag_group_rel WHERE parentid == ?', [self.id])
+                cur.execute(
+                    'DELETE FROM %s WHERE parentid == ?' % self.group_table,
+                    [self.id]
+                )
             else:
                 # Since this group was a child, its parent adopts all its children.
                 cur.execute(
-                    'UPDATE tag_group_rel SET parentid == ? WHERE parentid == ?',
+                    'UPDATE %s SET parentid == ? WHERE parentid == ?' % self.group_table,
                     [parent.id, self.id]
                 )
         # Note that this part comes after the deletion of children to prevent
         # issues of recursion.
-        cur.execute('DELETE FROM tag_group_rel WHERE memberid == ?', [self.id])
+        cur.execute(
+            'DELETE FROM %s WHERE memberid == ?' % self.group_table,
+            [self.id]
+        )
         if commit:
             self.photodb.log.debug('Committing - delete tag')
             self.photodb.commit()
@@ -125,12 +145,15 @@ class GroupableMixin:
         Returned object will be of the same type as calling object.
         '''
         cur = self.photodb.sql.cursor()
-        cur.execute('SELECT * FROM tag_group_rel WHERE memberid == ?', [self.id])
+        cur.execute(
+            'SELECT * FROM %s WHERE memberid == ?' % self.group_table,
+            [self.id]
+        )
         fetch = cur.fetchone()
         if fetch is None:
             return None
 
-        parentid = fetch[constants.SQL_TAGGROUP['parentid']]
+        parentid = fetch[self.group_sql_index['parentid']]
         return self.group_getter(id=parentid)
 
     def join_group(self, group, *, commit=True):
@@ -154,7 +177,10 @@ class GroupableMixin:
         '''
         cur = self.photodb.sql.cursor()
         self.photodb._cached_frozen_children = None
-        cur.execute('DELETE FROM tag_group_rel WHERE memberid == ?', [self.id])
+        cur.execute(
+            'DELETE FROM %s WHERE memberid == ?' % self.group_table,
+            [self.id]
+        )
         if commit:
             self.photodb.log.debug('Committing - leave group')
             self.photodb.commit()
@@ -172,6 +198,9 @@ class GroupableMixin:
 
 
 class Album(ObjectBase, GroupableMixin):
+    group_sql_index = constants.SQL_ALBUMGROUP
+    group_table = 'album_group_rel'
+
     def __init__(self, photodb, db_row):
         self.photodb = photodb
         if isinstance(db_row, (list, tuple)):
@@ -737,6 +766,9 @@ class Tag(ObjectBase, GroupableMixin):
     '''
     A Tag, which can be applied to Photos for organization.
     '''
+    group_sql_index = constants.SQL_TAGGROUP
+    group_table = 'tag_group_rel'
+
     def __init__(self, photodb, db_row):
         self.photodb = photodb
         if isinstance(db_row, (list, tuple)):

etiquette/photodb.py

@@ -29,7 +29,7 @@ logging.getLogger('PIL.PngImagePlugin').setLevel(logging.WARNING)
 # Note: Setting user_version pragma in init sequence is safe because it only
 # happens after the out-of-date check occurs, so no chance of accidentally
 # overwriting it.
-DATABASE_VERSION = 5
+DATABASE_VERSION = 6
 DB_INIT = '''
 PRAGMA count_changes = OFF;
 PRAGMA cache_size = 10000;
@@ -70,6 +70,10 @@ CREATE TABLE IF NOT EXISTS album_photo_rel(
     albumid TEXT,
     photoid TEXT
 );
+CREATE TABLE IF NOT EXISTS album_group_rel(
+    parentid TEXT,
+    memberid TEXT
+);
 CREATE TABLE IF NOT EXISTS photo_tag_rel(
     photoid TEXT,
     tagid TEXT
@@ -95,9 +99,15 @@ CREATE TABLE IF NOT EXISTS users(
 
 -- Album
 CREATE INDEX IF NOT EXISTS index_album_id on albums(id);
+
+-- Album-photo relation
 CREATE INDEX IF NOT EXISTS index_albumrel_albumid on album_photo_rel(albumid);
 CREATE INDEX IF NOT EXISTS index_albumrel_photoid on album_photo_rel(photoid);
 
+-- Album-group relation
+CREATE INDEX IF NOT EXISTS index_albumgroup_parentid on tag_group_rel(parentid);
+CREATE INDEX IF NOT EXISTS index_albumgroup_memberid on tag_group_rel(memberid);
+
 -- Bookmark
 CREATE INDEX IF NOT EXISTS index_bookmark_id on bookmarks(id);
 CREATE INDEX IF NOT EXISTS index_bookmark_author on bookmarks(author_id);
@@ -122,8 +132,8 @@ CREATE INDEX IF NOT EXISTS index_tagrel_tagid on photo_tag_rel(tagid);
 CREATE INDEX IF NOT EXISTS index_tagsyn_name on tag_synonyms(name);
 
 -- Tag-group relation
-CREATE INDEX IF NOT EXISTS index_grouprel_parentid on tag_group_rel(parentid);
+CREATE INDEX IF NOT EXISTS index_taggroup_parentid on tag_group_rel(parentid);
-CREATE INDEX IF NOT EXISTS index_grouprel_memberid on tag_group_rel(memberid);
+CREATE INDEX IF NOT EXISTS index_taggroup_memberid on tag_group_rel(memberid);
 
 -- User
 CREATE INDEX IF NOT EXISTS index_user_id on users(id);
@@ -283,8 +293,7 @@ class PDBAlbumMixin:
         '''
         Create a new album. Photos can be added now or later.
         '''
-        # Albums share the tag table's ID counter
+        albumid = self.generate_id('albums')
-        albumid = self.generate_id('tags')
         title = title or ''
         description = description or ''
         if associated_directory is not None:
@@ -1340,7 +1349,7 @@ class PhotoDB(PDBAlbumMixin, PDBBookmarkMixin, PDBPhotoMixin, PDBTagMixin, PDBUs
         ID is actually used.
         '''
         table = table.lower()
-        if table not in ['photos', 'tags', 'groups', 'bookmarks']:
+        if table not in ['photos', 'tags', 'albums', 'bookmarks']:
             raise ValueError('Invalid table requested: %s.', table)
 
         cur = self.sql.cursor()

utilities/database_upgrader.py

@@ -55,6 +55,54 @@ def upgrade_4_to_5(sql):
     cur.execute('CREATE INDEX IF NOT EXISTS index_bookmark_id ON bookmarks(id)')
     cur.execute('CREATE INDEX IF NOT EXISTS index_bookmark_author ON bookmarks(author_id)')
 
+def upgrade_5_to_6(sql):
+    '''
+    When Albums were first introduced, they shared the ID counter and
+    relationship table with tags, because they were mostly identical at the time.
+    However this is very ugly and confusing and it's time to finally change it.
+    - Renames old indices `index_grouprel_*` to `index_taggroup_*`
+    - Creates new indices `index_albumgroup_*`
+    - Creates new table `album_group_rel`
+    - Moves all album group relationships out of `tag_group_rel` and
+      into `album_group_rel`
+    - Gives albums their own last_id value, starting with the current tag value.
+    '''
+    # 1. Start the id_numbers.albums value at the tags value so that the number
+    # can continue to increment safely and separately, instead of starting at
+    # zero and bumping into existing albums.
+    cur = sql.cursor()
+    cur.execute('SELECT * FROM id_numbers WHERE tab == "tags"')
+    last_id = cur.fetchone()[1]
+    cur.execute('INSERT INTO id_numbers VALUES("albums", ?)', [last_id])
+
+    # 2. Now's a good chance to rename 'index_grouprel' to 'index_taggroup'.
+    cur.execute('DROP INDEX index_grouprel_parentid')
+    cur.execute('DROP INDEX index_grouprel_memberid')
+    cur.execute('CREATE INDEX index_taggroup_parentid ON tag_group_rel(parentid)')
+    cur.execute('CREATE INDEX index_taggroup_memberid ON tag_group_rel(memberid)')
+
+    # 3. All of the album group relationships need to be moved into their
+    # own table, out of tag_group_rel
+    cur.execute('CREATE TABLE album_group_rel(parentid TEXT, memberid TEXT)')
+    cur.execute('CREATE INDEX index_albumgroup_parentid ON tag_group_rel(parentid)')
+    cur.execute('CREATE INDEX index_albumgroup_memberid ON tag_group_rel(memberid)')
+    cur.execute('SELECT id FROM albums')
+    album_ids = [f[0] for f in cur.fetchall()]
+    for album_id in album_ids:
+        cur.execute(
+            'SELECT * FROM tag_group_rel WHERE parentid == ? OR memberid == ?',
+            [album_id, album_id]
+        )
+        f = cur.fetchall()
+        if f == []:
+            continue
+        for grouprel in f:
+            cur.execute('INSERT INTO album_group_rel VALUES(?, ?)', grouprel)
+        cur.execute(
+            'DELETE FROM tag_group_rel WHERE parentid == ? OR memberid == ?',
+            [album_id, album_id]
+        )
+
 def upgrade_all(database_filename):
     '''
     Given the filename of a phototagger database, apply all of the needed