Skip to content

Commit

Permalink
Issue 460: Set cascade delete pragma on connect.
Browse files Browse the repository at this point in the history
- add test to ensure stray book stats are deleted
- data cleanup migration (one-time only)
- remove LanguageRepo.delete, deletes now cascade correctly
  • Loading branch information
jzohrab committed Dec 22, 2024
1 parent bce5b46 commit e76803f
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 29 deletions.
1 change: 1 addition & 0 deletions lute/app_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ def _create_app(app_config, extra_config):
@listens_for(Pool, "connect")
def _pragmas_on_connect(dbapi_con, con_record): # pylint: disable=unused-argument
dbapi_con.execute("pragma recursive_triggers = on;")
dbapi_con.execute("pragma foreign_keys = on;")

with app.app_context():
db.create_all()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
-- Clean up bad data, where relationships are invalid.
--
-- Per issue 460, the pragma foreign_keys was not ON, so it's possible
-- (though unlikely) that some data in the db is bad/unreachable.
--
-- This is being done as a one-time fix, rather than as a repeatable
-- migration, as it would be very annoying if the data model changed
-- and I forgot to update the script!!

DELETE FROM languagedicts WHERE LdLgID NOT IN (SELECT LgID FROM languages);
DELETE FROM wordsread WHERE WrLgID NOT IN (SELECT LgID FROM languages);

DELETE FROM books WHERE BkLgID NOT IN (SELECT LgID FROM languages);
DELETE FROM bookstats WHERE BkID NOT IN (SELECT BkID FROM books);
DELETE FROM booktags WHERE BtBkID NOT IN (SELECT BkID FROM books) OR BtT2ID NOT IN (SELECT T2ID FROM tags2);

DELETE FROM texts WHERE TxBkID NOT IN (SELECT BkID FROM books);
DELETE FROM textbookmarks WHERE TbTxID NOT IN (SELECT TxID FROM texts);
DELETE FROM sentences WHERE SeTxID NOT IN (SELECT TxID FROM texts);
DELETE FROM wordsread WHERE WrTxID IS NOT NULL AND WrTxID NOT IN (SELECT TxID FROM texts);

DELETE FROM wordtags WHERE WtWoID NOT IN (SELECT WoID FROM words) OR WtTgID NOT IN (SELECT TgID FROM tags);
DELETE FROM wordimages WHERE WiWoID NOT IN (SELECT WoID FROM words);
DELETE FROM wordflashmessages WHERE WfWoID NOT IN (SELECT WoID FROM words);
DELETE FROM wordparents WHERE WpWoID NOT IN (SELECT WoID FROM words) OR WpParentWoID NOT IN (SELECT WoID FROM words);
6 changes: 3 additions & 3 deletions lute/language/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from sqlalchemy.exc import IntegrityError
from flask import Blueprint, current_app, render_template, redirect, url_for, flash
from lute.models.language import Language
from lute.models.repositories import LanguageRepository, UserSettingRepository
from lute.models.repositories import UserSettingRepository
from lute.language.service import Service
from lute.language.forms import LanguageForm
from lute.db import db
Expand Down Expand Up @@ -156,8 +156,8 @@ def delete(langid):
language = db.session.get(Language, langid)
if not language:
flash(f"Language {langid} not found")
r = LanguageRepository(db.session)
r.delete(language)
db.session.delete(language)
db.session.commit()
return redirect(url_for("language.index"))


Expand Down
23 changes: 0 additions & 23 deletions lute/models/repositories.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,29 +141,6 @@ def find_by_name(self, name):
.first()
)

def delete(self, language):
"""
Hacky method to delete language and all terms, books, and dicts
associated with it.
There is _certainly_ a better way to do this using
Sqlalchemy relationships and cascade deletes, but I
was running into problems with it (things not cascading,
or warnings ("SAWarning: Object of type <Term> not in
session, add operation along 'Language.terms' will not
proceed") during test runs. It would be nice to have
a "correct" mapping, but this is good enough for now.
TODO zzfuture fix: fix Language-Book and -Term mappings.
"""
sqls = [
"pragma foreign_keys = ON",
f"delete from languages where LgID = {language.id}",
]
for s in sqls:
self.session.execute(sqltext(s))
self.session.commit()

def all_dictionaries(self):
"All dictionaries for all languages."
lang_dicts = {}
Expand Down
10 changes: 9 additions & 1 deletion tests/orm/test_Book.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import pytest
from lute.models.book import Book, BookTag, TextBookmark, BookStats
from lute.read.service import Service
from lute.book.stats import Service as BookStatsService
from lute.db import db
from tests.dbasserts import assert_sql_result, assert_record_count_equals

Expand Down Expand Up @@ -56,10 +57,17 @@ def test_delete_book(empty_db, simple_book):
service.mark_page_read(b.id, 1, False)
service.mark_page_read(b.id, 1, True)

bss = BookStatsService(db.session)
bss.refresh_stats()

check_tables = ["books", "bookstats", "texts", "sentences", "booktags"]
for t in check_tables:
assert_record_count_equals(t, 1, f"{t} created")

db.session.delete(b)
db.session.commit()

for t in ["books", "texts", "sentences", "booktags"]:
for t in check_tables:
assert_record_count_equals(t, 0, f"{t} deleted")

sql = "select * from tags2"
Expand Down
4 changes: 2 additions & 2 deletions tests/orm/test_Language.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ def test_delete_language_removes_book_and_terms(app_context, spanish):
assert_sql_result(sqldict, ["something?[LUTE]"], "dict")
assert_record_count_equals("select * from wordsread", 1, "saved")

repo = LanguageRepository(db.session)
repo.delete(spanish)
db.session.delete(spanish)
db.session.commit()

assert_sql_result(sqlbook, [], "book deleted")
assert_sql_result(sqlterms, [], "terms deleted")
Expand Down

0 comments on commit e76803f

Please sign in to comment.