diff --git a/reference_handler/reference_handler.py b/reference_handler/reference_handler.py index 2d32bbf..dd55b9e 100644 --- a/reference_handler/reference_handler.py +++ b/reference_handler/reference_handler.py @@ -159,7 +159,10 @@ def __init__(self, database): SQLite database and bulding the two tables within it. """ - self.conn = sqlite3.connect(database) + # Wait for a lock rather than failing at once: the iterations of a parallel + # loop cite into the job's one database at the same time, on a shared file + # system. + self.conn = sqlite3.connect(database, timeout=30.0) self.cur = self.conn.cursor() self._initialize_tables() @@ -359,31 +362,63 @@ def cite( doi = self._extract_doi(raw, fmt) - reference_id = self._get_reference_id(raw=raw, alias=alias, doi=doi) - - if reference_id is None: - self._create_citation(raw=raw, alias=alias, doi=doi) - reference_id = self.cur.lastrowid - self._create_context( - reference_id=reference_id, module=module, note=note, level=level - ) - else: - context_id = self._get_context_id( - reference_id=reference_id, module=module, note=note, level=level - ) + try: + reference_id = self._find_citation(raw=raw, alias=alias, doi=doi) - if context_id is None: + if reference_id is None: + self._create_citation(raw=raw, alias=alias, doi=doi) + reference_id = self.cur.lastrowid self._create_context( reference_id=reference_id, module=module, note=note, level=level ) else: - self._update_counter(context_id=context_id) + context_id = self._get_context_id( + reference_id=reference_id, module=module, note=note, level=level + ) - # Save the changes! - self.conn.commit() + if context_id is None: + self._create_context( + reference_id=reference_id, module=module, note=note, level=level + ) + else: + self._update_counter(context_id=context_id) + + # Save the changes! + self.conn.commit() + except Exception: + # Never leave the transaction open: it would hold the database's lock, + # and every other writer -- e.g. the iterations of a parallel loop -- + # would fail with "database is locked". + self.conn.rollback() + raise return reference_id + def _find_citation(self, raw=None, alias=None, doi=None): + """The id of the citation with this raw text, or else this alias or DOI. + + The same reference cited with a different text -- e.g. a program's own + citation, whose text holds its version, after an update -- is the same + citation; its text is brought up to date. Inserting it again would break + the uniqueness of the alias and DOI (a resumed job's + "UNIQUE constraint failed: citation.doi"). + """ + reference_id = self._get_reference_id(raw=raw) + if reference_id is not None: + return reference_id + for key, value in (("alias", alias), ("doi", doi)): + if value is None: + continue + self.cur.execute(f"SELECT id FROM citation WHERE {key}=?;", (value,)) + ret = self.cur.fetchall() + if len(ret) > 0: + reference_id = ret[0][0] + self.cur.execute( + "UPDATE citation SET raw=? WHERE id=?;", (raw, reference_id) + ) + return reference_id + return None + def _update_counter(self, context_id=None): """ Updates the counter for given context diff --git a/reference_handler/tests/test_updated_citations.py b/reference_handler/tests/test_updated_citations.py new file mode 100644 index 0000000..216185e --- /dev/null +++ b/reference_handler/tests/test_updated_citations.py @@ -0,0 +1,68 @@ +"""A reference cited again with a different text -- a program's citation after an +update, whose text holds its version -- is the same citation, and a failed citation +never leaves the database locked (SEAMM job 5366, 2026-10-23: a resumed job's +'UNIQUE constraint failed: citation.doi' kept the lock, and the parallel loop's +iterations then failed with 'database is locked').""" + +import sqlite3 + +import pytest + +import reference_handler + + +def raw(version): + return ( + "@misc{seamm_exec,\n" + " author = {Saxe, Paul},\n" + " title = {seamm_exec},\n" + f" version = {{{version}}},\n" + " doi = {10.5281/zenodo.0000000},\n" + "}\n" + ) + + +def cite(rh, version): + return rh.cite( + raw=raw(version), + alias="seamm_exec", + module="seamm_exec", + level=1, + note="The executor", + ) + + +def test_a_new_version_is_the_same_citation(tmp_path): + db = str(tmp_path / "references.db") + rh = reference_handler.Reference_Handler(db) + first = cite(rh, "2026.10.20") + again = cite(rh, "2026.10.23") + assert again == first + rh.cur.execute("SELECT raw FROM citation WHERE id=?;", (first,)) + assert "2026.10.23" in rh.cur.fetchone()[0] + rh.cur.execute("SELECT count FROM context WHERE reference_id=?;", (first,)) + assert rh.cur.fetchone()[0] == 2 + + +def test_a_failed_citation_does_not_keep_the_lock(tmp_path, monkeypatch): + db = str(tmp_path / "references.db") + rh = reference_handler.Reference_Handler(db) + cite(rh, "2026.10.20") + + def broken(**kwargs): + rh.cur.execute( + "INSERT INTO citation (raw, alias, doi) VALUES (?, ?, ?);", + ("@misc{x, title={x}}", "seamm_exec", None), + ) + + monkeypatch.setattr(rh, "_find_citation", lambda **kwargs: None) + monkeypatch.setattr(rh, "_create_citation", broken) + with pytest.raises(sqlite3.IntegrityError): + cite(rh, "2026.10.23") + assert not rh.conn.in_transaction + + # Another connection -- another process of the job -- can still write + other = sqlite3.connect(db, timeout=0.5) + other.execute("UPDATE context SET count = count + 1;") + other.commit() + other.close()