Sync sqlitedb and database to DID-matlab#227: delete originals after commit, don't reopen a vanished database

src/did/did_matlab_python_bridge.yaml

@@ -40,7 +40,7 @@ classes:
   - name: database
     type: class
     matlab_path: "+did/database.m"
-    matlab_last_sync_hash: "8bea92c"
+    matlab_last_sync_hash: "bb39399"
     status: ported
     python_path: "did/database.py"
     python_class: "Database"
@@ -933,6 +933,13 @@ classes:
       connection is managed differently (see the sqlitedb open_db/close_db
       note). Synchronized 2026-03-15; validation 2026-08-28.
 
+      DID-matlab#227 (bb39399, 2026-10-24) changed add_docs(..., 'Validate', false)
+      to set journal_mode=MEMORY instead of OFF, since each document is now
+      one transaction and ROLLBACK is undefined with the journal OFF.
+      Nothing to port: Python's add_docs has no such fast path and never
+      changes journal_mode. The rest of that commit is in sqlitedb.m; see
+      the sqlitedb entry.
+
   # -----------------------------------------------------------------------
   # did.document
   # -----------------------------------------------------------------------

src/did/did_matlab_python_bridge_implementations.yaml

@@ -29,7 +29,7 @@ classes:
   - name: sqlitedb
     type: class
     matlab_path: "+did/+implementations/sqlitedb.m"
-    matlab_last_sync_hash: "da83fa3"
+    matlab_last_sync_hash: "bb39399"
     status: ported
     python_path: "did/implementations/sqlitedb.py"
     python_class: "SQLiteDB"
@@ -194,6 +194,27 @@ classes:
       onto the same file, since nothing has been committed yet. MATLAB has
       the same gap, narrower because each mksqlite statement commits on its
       own; this check is not in DID-matlab.
+
+      DID-matlab#227 (bb39399, 2026-10-24) brought MATLAB to the same design: each
+      write operation is one transaction (beginWrite/commitWrite, BEGIN
+      IMMEDIATE), journaled and replayed after a reopen (recoverTransaction,
+      replayJournal), with the same pre-commit replaced-file check
+      (fileWasReplaced, unix dev:inode via Java). Synced here: two things it
+      does that Python did not. (1) delete_original sources are deleted only
+      once the operation commits (_delete_after_commit, flushed by
+      _journal_committed), so a rollback no longer leaves the only copy of a
+      file unreferenced in FileDir; reclaimed cached copies already waited
+      for the commit. A source two files share is deleted once, and a
+      missing one is not a warning, as MATLAB suppresses
+      MATLAB:DELETE:FileNotFound. (2) _recover_transaction does not reopen a
+      database file that no longer exists -- that created a new, empty
+      database there -- and raises the original error with a
+      FileNotFoundError as its cause (MATLAB: DID:SQLITEDB:DatabaseFileGone).
+      Remaining differences: MATLAB begins with BEGIN IMMEDIATE, Python's
+      sqlite3 with a deferred BEGIN at the first write; MATLAB checks
+      changes() after each write where Python compares cursor.rowcount; and
+      MATLAB restores fields_cache in abandonWrite where Python restores
+      _fields_cache in each operation's except path. Same effect each time.
       do_run_sql_query, outside a write operation, keeps its own retry: only
       when no transaction is open, reopening first for READONLY; inside an
       unjournaled transaction the error is raised at once.

src/did/implementations/sqlitedb.py

@@ -125,10 +125,14 @@ def wrapper(self, *args, **kwargs):
         if self._journal is not None:
             return method(self, *args, **kwargs)
         self._journal = []
+        self._pending_deletes = []
         try:
             return method(self, *args, **kwargs)
         finally:
+            # Whatever was not committed keeps its files (see
+            # _delete_after_commit).
             self._journal = None
+            self._pending_deletes = None
 
     return wrapper
 
@@ -227,6 +231,7 @@ def __init__(self, filename):
         self.dbid = None
         self._fields_cache = {}  # (class, field_name) -> field_idx
         self._journal = None  # statements of the current write operation
+        self._pending_deletes = None  # originals to delete once it commits
         self._db_identity = None  # (st_dev, st_ino) of the file opened
         self._open_db()
 
@@ -563,6 +568,38 @@ def _journal_committed(self):
         for entry in self._journal:
             entry["proxy"]._entry = None
         self._journal.clear()
+        pending, self._pending_deletes = self._pending_deletes or [], []
+        for path, what in pending:
+            self._delete_original(path, what)
+
+    def _delete_after_commit(self, path, what):
+        """Delete an ingested original once the write operation commits.
+
+        Inside a write operation the delete is queued and done by
+        _journal_committed, so a rollback leaves the only copy of the file
+        where it was. Outside one it happens at once, as before. Mirrors
+        DID-matlab's sqlitedb.deleteAfterCommit.
+        """
+        if self._pending_deletes is None:
+            self._delete_original(path, what)
+        else:
+            self._pending_deletes.append((path, what))
+
+    @staticmethod
+    def _delete_original(path, what):
+        try:
+            os.remove(path)
+        except FileNotFoundError:
+            # Already gone (two files can share one original): the point is
+            # that it must not be there afterwards. MATLAB's commitWrite
+            # suppresses MATLAB:DELETE:FileNotFound for the same case.
+            pass
+        except OSError as error:
+            warnings.warn(
+                f'Ingested {what} but could not delete the original "{path}": '
+                f"{error}",
+                stacklevel=3,
+            )
 
     def _record(self, proxy, method, sql, params):
         entry = {
@@ -691,6 +728,18 @@ def _recover_transaction(self, err, pending=None, commit=False):
                 with contextlib.suppress(sqlite3.Error):
                     self.dbid.rollback()
             time.sleep(backoff)
+            if not os.path.exists(self.connection):
+                # Reopening would create a new, empty database at the path
+                # and replay into it. The database is gone; say so. Mirrors
+                # DID-matlab's DID:SQLITEDB:DatabaseFileGone.
+                print(
+                    f"DID:SQLITEDB the database file {self.connection} no "
+                    "longer exists; not reopening it, which would create a "
+                    "new, empty database there. Nothing from this operation "
+                    "was written.",
+                    file=sys.stderr,
+                )
+                raise err from FileNotFoundError(self.connection)
             # Suspended while reopening: _open_db commits on its own.
             self._journal = None
             try:
@@ -1310,14 +1359,7 @@ def _ingest_location(self, filename, entry, custom_file_handler, document_id="")
         # Only now, with a copy safely in place. MATLAB deletes the original
         # solely on the success branch, for the same reason.
         if entry.get("delete_original") and not is_remote:
-            try:
-                os.remove(source_path)
-            except OSError as error:
-                warnings.warn(
-                    f'Ingested "{filename}" but could not delete the original '
-                    f'"{source_path}": {error}',
-                    stacklevel=2,
-                )
+            self._delete_after_commit(source_path, f'"{filename}"')
         return dest_path
 
     def _populate_files(self, cursor, doc_idx, document_obj, custom_file_handler=None):
@@ -2253,14 +2295,7 @@ def _ingest_series_members(self, document_obj, custom_file_handler):
                     continue
 
                 if member.get("delete_original") and not is_remote:
-                    try:
-                        os.remove(source_path)
-                    except OSError as error:
-                        warnings.warn(
-                            f"Ingested series member but could not delete "
-                            f'the original "{source_path}": {error}',
-                            stacklevel=2,
-                        )
+                    self._delete_after_commit(source_path, "series member")
 
     def _open_series_member(self, series_locations, custom_file_handler):
         """Open a file series member: local first, then fetch through handler.

tests/test_open_doc_locations.py

@@ -17,7 +17,6 @@
 import os
 import tempfile
 import unittest
-import warnings
 from unittest import mock
 
 from did.database import FileAccessError
@@ -349,20 +348,15 @@ def _matlab_shaped_doc(self, content=b"ingested payload"):
         This used to be hand-built. Since DID-python#45 add_docs produces the
         shape itself: a local location marked `ingest` is copied to
         <db dir>/files/<uid> and the original is deleted, as MATLAB's
-        do_add_doc does. Both files here point at the SAME original, so the
-        first is ingested and the second -- reached after the original is
-        gone -- ends with no ingested copy and no original, which is the case
-        test_a_file_with_no_ingested_copy_and_no_original_is_an_error needs.
+        do_add_doc does. Both files here point at the SAME original. The
+        original is deleted only once the document commits, so both are
+        ingested, each under its own uid.
         """
         original = self._local_file("original.bin", content)
         doc = Document("demoFile", **{"demoFile.value": 1})
         doc.add_file("filename1.ext", original)
         doc.add_file("filename2.ext", original)
-        with warnings.catch_warnings():
-            # filename2's ingest cannot succeed -- filename1's delete_original
-            # already removed the source. MATLAB warns there too.
-            warnings.simplefilter("ignore")
-            self.db.add_docs([doc])
+        self.db.add_docs([doc])
 
         # The files table should carry a row per file, and filename1's copy
         # should be sitting under its uid.
@@ -400,7 +394,18 @@ def test_ingested_copy_is_found_after_the_original_is_deleted(self):
 
     def test_a_file_with_no_ingested_copy_and_no_original_is_an_error(self):
         doc, _ = self._matlab_shaped_doc()
-        # filename2.ext shares the deleted original but was never ingested.
+        # filename2.ext shares the deleted original; take its ingested copy
+        # away too, so there is neither.
+        row = (
+            self.db.dbid.cursor()
+            .execute(
+                "SELECT f.cached_location FROM docs d, files f "
+                "WHERE d.doc_idx = f.doc_idx AND d.doc_id = ? AND f.filename = ?",
+                (doc.id(), "filename2.ext"),
+            )
+            .fetchone()
+        )
+        os.remove(row["cached_location"])
         with self.assertRaises(FileAccessError) as caught:
             self.db.open_doc(doc.id(), "filename2.ext")
         self.assertEqual(caught.exception.identifier, "DID:SQLITEDB:open")

tests/test_sqlitedb_transient_errors.py

@@ -573,11 +573,12 @@ def test_a_deleted_file_is_an_error_not_a_silent_loss(
         db.add_branch("main")
         self._swap_during_add(monkeypatch, os.remove)
 
-        with pytest.raises(sqlite3.OperationalError, match="was replaced"):
+        with pytest.raises(sqlite3.OperationalError, match="was replaced") as caught:
             db.add_docs([_doc("6" * 32)], branch_id="main", validate=False)
-        # Reopening made a new, empty database at the path; the replay read
-        # no branch there and was refused rather than written.
-        assert "6" * 32 not in _doc_ids_on_disk(db.connection)
+        assert isinstance(caught.value.__cause__, FileNotFoundError)
+        # Not reopened: that would have created a new, empty database at the
+        # path and replayed into it.
+        assert not os.path.exists(db.connection)
 
     def test_a_false_alarm_costs_one_replay_onto_the_same_file(
         self, db, sleeps, monkeypatch
@@ -619,3 +620,51 @@ def test_the_identity_follows_a_reopen(self, db, sleeps):
         db.add_branch("b2", parent_branch_id="main")  # recovered by reopen
         assert db._db_identity == db._file_identity()
         assert not db._file_was_replaced()
+
+
+class TestOriginalsAreDeletedAfterCommit:
+    """delete_original waits for the commit, so a rollback keeps the file."""
+
+    def _file_doc(self, db):
+        source = os.path.join(os.path.dirname(db.connection), "source.bin")
+        with open(source, "wb") as handle:
+            handle.write(b"payload")
+        doc = Document("demoFile", **{"demoFile.value": 1})
+        doc.add_file("filename1.ext", "placeholder", ingest=0, delete_original=0)
+        doc.add_file("filename2.ext", "placeholder", ingest=0, delete_original=0)
+        _, info, _ = doc.is_in_file_list("filename1.ext")
+        info["locations"] = [
+            {
+                "location": source,
+                "location_type": "file",
+                "uid": "u-local",
+                "ingest": 1,
+                "delete_original": 1,
+                "parameters": "",
+            }
+        ]
+        return doc, source
+
+    def test_a_rolled_back_add_keeps_the_original(self, db, monkeypatch):
+        db.add_branch("main")
+        doc, source = self._file_doc(db)
+
+        def refuse(self):
+            raise sqlite3.OperationalError("disk I/O error")
+
+        monkeypatch.setattr(SQLiteDB, "_commit", refuse)
+        with pytest.raises(sqlite3.OperationalError, match="disk I/O"):
+            db.add_docs([doc], branch_id="main", validate=False)
+        assert os.path.exists(source), "a rollback must not delete the original"
+        assert doc.id() not in _doc_ids_on_disk(db.connection)
+
+        monkeypatch.undo()
+        db.add_docs([doc], branch_id="main", validate=False)
+        assert not os.path.exists(source), "deleted once the add commits"
+        assert doc.id() in _doc_ids_on_disk(db.connection)
+
+    def test_nothing_is_left_queued_after_an_operation(self, db):
+        db.add_branch("main")
+        doc, _ = self._file_doc(db)
+        db.add_docs([doc], branch_id="main", validate=False)
+        assert db._pending_deletes is None