Repos / pytaku / aca263ad32
commit aca263ad324da0bfe47f8792e1bdf9a276fdd579
Author: Bùi Thành Nhân <hi@imnhan.com>
Date: Sun Aug 9 22:22:28 2020 +0700
fix `chapter` & `read` table constraints
Turns out meddling with columns and constraints in SQLite requires
some... interesting gymnastics:
- turn foreign_keys pragma off
- rename existing table
- create new table with desired columns & constraints
- copy data over
- drop old table
- turn foreign_keys pragma back on
Also, you can't flip the foreign_keys pragma from inside a transaction,
so the whole migrator needed an update.
_Also_, even with foreign_keys=off, renaming `chapter` table to
`old_chapter` would automatically update the `read` table's foreign key
constraint to point to `old_chapter` too, which is NOT what we want. So
we need to set legacy_alter_table=on as well (as an extra bonus, this
behavior differs between sqlite versions as well - see the table at
https://www.sqlite.org/lang_altertable.html).
diff --git a/src/pytaku/database/migrations/latest_schema.sql b/src/pytaku/database/migrations/latest_schema.sql
index e1dd9b5..9ba45dd 100644
--- a/src/pytaku/database/migrations/latest_schema.sql
+++ b/src/pytaku/database/migrations/latest_schema.sql
@@ -13,22 +13,6 @@ CREATE TABLE title (
unique(id, site)
);
-CREATE TABLE IF NOT EXISTS "old_chapter" (
- id text,
- title_id text,
- site text,
- num_major integer,
- num_minor integer,
- name text,
- pages text,
- groups text,
- updated_at text default (datetime('now')), is_webtoon boolean,
-
- foreign key (title_id, site) references title (id, site),
- unique(id, title_id, site),
- unique(id, site),
- unique(num_major, num_minor, title_id)
-);
CREATE TABLE user (
id integer primary key,
username text unique,
@@ -45,16 +29,6 @@ CREATE TABLE follow (
foreign key (user_id) references user (id),
unique(user_id, title_id, site)
);
-CREATE TABLE read (
- user_id integer not null,
- chapter_id text not null,
- site text not null,
- updated_at text default (datetime('now')),
-
- foreign key (user_id) references user (id),
- foreign key (chapter_id, site) references "old_chapter" (id, site),
- unique(user_id, chapter_id, site)
-);
CREATE TABLE keyval_store (
key text primary key,
value text not null,
@@ -69,9 +43,21 @@ CREATE TABLE chapter (
name text,
pages text,
groups text,
- updated_at text default (datetime('now')), is_webtoon boolean,
+ updated_at text default (datetime('now')),
+ is_webtoon boolean,
foreign key (title_id, site) references title (id, site),
unique(site, title_id, id),
unique(site, title_id, num_major, num_minor)
);
+CREATE TABLE read (
+ user_id integer not null,
+ site text not null,
+ title_id text, -- nullable to accomodate existing mangadex rows, urgh.
+ chapter_id text not null,
+ updated_at text default (datetime('now')),
+
+ foreign key (user_id) references user (id),
+ foreign key (site, title_id, chapter_id) references chapter (site, title_id, id),
+ unique(user_id, site, title_id, chapter_id)
+);
diff --git a/src/pytaku/database/migrations/m0001.sql b/src/pytaku/database/migrations/m0001.sql
index 700acf3..02f6c8f 100644
--- a/src/pytaku/database/migrations/m0001.sql
+++ b/src/pytaku/database/migrations/m0001.sql
@@ -1,3 +1,5 @@
+begin transaction;
+
create table title (
id text,
name text,
@@ -56,3 +58,5 @@ create table read (
foreign key (chapter_id, site) references chapter (id, site),
unique(user_id, chapter_id, site)
);
+
+commit;
diff --git a/src/pytaku/database/migrations/m0002.sql b/src/pytaku/database/migrations/m0002.sql
index 15f1c6e..5cb629c 100644
--- a/src/pytaku/database/migrations/m0002.sql
+++ b/src/pytaku/database/migrations/m0002.sql
@@ -1,7 +1,10 @@
-- Add key-value store table, for a poor man's inefficient cache
+begin transaction;
create table keyval_store (
key text primary key,
value text not null,
updated_at text default (datetime('now'))
);
+
+commit;
diff --git a/src/pytaku/database/migrations/m0003.sql b/src/pytaku/database/migrations/m0003.sql
index 14d89ec..d581ff4 100644
--- a/src/pytaku/database/migrations/m0003.sql
+++ b/src/pytaku/database/migrations/m0003.sql
@@ -2,8 +2,11 @@
-- SQLite doesn't let you do that directly so gotta create a new table
-- then copy existing data over.
-alter table chapter rename to old_chapter;
+pragma foreign_keys = off; -- to let us do anything at all
+pragma legacy_alter_table = on; -- prevent foreign keys from renaming to 'old_chapter' as well
+begin transaction;
+alter table chapter rename to old_chapter;
create table chapter (
id text,
title_id text,
@@ -13,11 +16,34 @@ create table chapter (
name text,
pages text,
groups text,
- updated_at text default (datetime('now')), is_webtoon boolean,
+ updated_at text default (datetime('now')),
+ is_webtoon boolean,
foreign key (title_id, site) references title (id, site),
unique(site, title_id, id),
unique(site, title_id, num_major, num_minor)
);
-
insert into chapter select * from old_chapter;
+drop table old_chapter;
+
+
+-- "read" table needs a new "title_id" column too
+alter table read rename to old_read;
+create table read (
+ user_id integer not null,
+ site text not null,
+ title_id text, -- nullable to accomodate existing mangadex rows, urgh.
+ chapter_id text not null,
+ updated_at text default (datetime('now')),
+
+ foreign key (user_id) references user (id),
+ foreign key (site, title_id, chapter_id) references chapter (site, title_id, id),
+ unique(user_id, site, title_id, chapter_id)
+);
+insert into read (user_id, site, chapter_id, updated_at)
+ select user_id, site, chapter_id, updated_at from old_read;
+drop table old_read;
+
+commit;
+pragma foreign_keys = on;
+pragma legacy_alter_table = off;
diff --git a/src/pytaku/database/migrator.py b/src/pytaku/database/migrator.py
index b275d64..1fe88b9 100644
--- a/src/pytaku/database/migrator.py
+++ b/src/pytaku/database/migrator.py
@@ -1,3 +1,4 @@
+import datetime
import subprocess
from importlib import resources
from pathlib import Path
@@ -65,12 +66,27 @@ def migrate(overwrite_latest_schema=True):
migration_contents = _read_migrations(pending_migrations)
conn = get_conn()
- with conn: # apsw provides automatic rollback for free here
- cursor = conn.cursor()
- for version, sql in migration_contents:
- print("Migrating version", version, "...")
- cursor.execute(sql)
- cursor.execute(f"PRAGMA user_version = {version};")
+ cursor = conn.cursor()
+
+ # Backup first
+ now = datetime.datetime.utcnow().isoformat("T", "milliseconds")
+ backup_filename = f"db_backup_{now}.sqlite3"
+ print(f"Backup up to {backup_filename}...", end="")
+ cursor.execute("VACUUM main INTO ?;", (backup_filename,))
+ print(" done")
+
+ # Start migrations
+ # NOTE: this is NOT done in a transaction.
+ # You'll need to do transactions inside your sql scripts.
+ # This is to allow for drastic changes that require temporarily turning off the
+ # foreign_keys pragma, which doesn't work inside transactions.
+ # If anything goes wrong here, let it abort the whole script. You can always
+ # restore from the backup file.
+ cursor = conn.cursor()
+ for version, sql in migration_contents:
+ print("Migrating version", version, "...")
+ cursor.execute(sql)
+ cursor.execute(f"PRAGMA user_version = {version};")
if overwrite_latest_schema:
_write_db_schema_script(migrations_dir)
diff --git a/src/pytaku/decorators.py b/src/pytaku/decorators.py
index 05a9a86..7bd09a9 100644
--- a/src/pytaku/decorators.py
+++ b/src/pytaku/decorators.py
@@ -44,17 +44,28 @@ def toggle_has_read(f):
@wraps(f)
def decorated_function(*args, **kwargs):
- assert "site" in kwargs # only use on site-specific views
+ assert "site" in kwargs
+ assert "title_id" in kwargs
has_read_chapter_id = request.args.get("has_read")
unread_chapter_id = request.args.get("unread")
assert not (has_read_chapter_id and unread_chapter_id) # can't do both
if session.get("user"):
if has_read_chapter_id:
- read(session["user"]["id"], kwargs["site"], has_read_chapter_id)
+ read(
+ session["user"]["id"],
+ kwargs["site"],
+ kwargs["title_id"],
+ has_read_chapter_id,
+ )
return redirect(request.url[: request.url.rfind("?")])
elif unread_chapter_id:
- unread(session["user"]["id"], kwargs["site"], unread_chapter_id)
+ unread(
+ session["user"]["id"],
+ kwargs["site"],
+ kwargs["title_id"],
+ unread_chapter_id,
+ )
return redirect(request.url[: request.url.rfind("?")])
return f(*args, **kwargs)
diff --git a/src/pytaku/persistence.py b/src/pytaku/persistence.py
index 3b2ef01..6cedcf4 100644
--- a/src/pytaku/persistence.py
+++ b/src/pytaku/persistence.py
@@ -261,20 +261,21 @@ def get_followed_titles(user_id):
return sorted(titles, key=lambda t: len(t["chapters"]), reverse=True)
-def read(user_id, site, chapter_id):
+def read(user_id, site, title_id, chapter_id):
run_sql(
"""
- INSERT INTO read (user_id, site, chapter_id) VALUES (?, ?, ?)
- ON CONFLICT (user_id, site, chapter_id) DO UPDATE SET updated_at=datetime('now')
+ INSERT INTO read (user_id, site, title_id, chapter_id) VALUES (?,?,?,?)
+ ON CONFLICT (user_id, site, title_id, chapter_id)
+ DO UPDATE SET updated_at=datetime('now');
""",
- (user_id, site, chapter_id),
+ (user_id, site, title_id, chapter_id),
)
-def unread(user_id, site, chapter_id):
+def unread(user_id, site, title_id, chapter_id):
run_sql(
- "DELETE FROM read WHERE user_id=? AND site=? AND chapter_id=?;",
- (user_id, site, chapter_id),
+ "DELETE FROM read WHERE user_id=? AND site=? AND title_id=? AND chapter_id=?;",
+ (user_id, site, title_id, chapter_id),
)