Ensure context-manager ignores the auto_commit setting. (#43)

TODO.md

@@ -7,6 +7,10 @@
 - add an 'execute' method to the main class to allow executing arbitrary SQL
   queries which can be chained to the 'find_first' etc methods or just used
   directly.
+- add a `delete` method to the QueryBuilder class to allow deleting
+  single/multiple records from the database based on the query. This is in
+  addition to the `delete` method in the main class which deletes a single
+  record based on the primary key.
 - add a `rollback` method to the main class to allow manual rollbacks.
 - allow adding multiple indexes to each table as well as the primary key.
 - allow adding foreign keys and relationships to each table.
@@ -18,21 +22,14 @@
   need to be `pickled` first then stored as a BLOB in the database . Also
   support `date` which can be stored as a Unix timestamp in an integer field.
 
-## Bugs
-
-- The primary key is not available on the returned model if `create_pk` is set
-  to True. We need to add this to the returned model. This means that we cant
-  delete or update the record without the primary key.
-
 ## Housekeeping
 
 - Tidy up the test suite - remove any duplicates, sort them into logical files
   (many already are), try to reduce and centralize fixtures.
 
 ## Documentation
 
-- Ad examples using the Primary Key to update() and delete() - this will need
-  the above bug fixed first.
+- Nothing at the moment.
 
 ## Potential Filter Additions
 

sqliter/sqliter.py

@@ -89,6 +89,8 @@ def __init__(
         self.conn: Optional[sqlite3.Connection] = None
         self.reset = reset
 
+        self._in_transaction = False
+
         if self.debug:
             self._setup_logger()
 
@@ -308,7 +310,7 @@ def _maybe_commit(self) -> None:
         This method is called after operations that modify the database,
         committing changes only if auto_commit is set to True.
         """
-        if self.auto_commit and self.conn:
+        if not self._in_transaction and self.auto_commit and self.conn:
             self.conn.commit()
 
     def insert(self, model_instance: T) -> T:
@@ -515,6 +517,7 @@ def __enter__(self) -> Self:
 
         """
         self.connect()
+        self._in_transaction = True
         return self
 
     def __exit__(
@@ -552,3 +555,4 @@ def __exit__(
                 # Close the connection and reset the instance variable
                 self.conn.close()
                 self.conn = None
+                self._in_transaction = False

tests/test_context_manager.py

@@ -0,0 +1,123 @@
+"""Test the context-manager functionality."""
+
+import sqlite3
+
+import pytest
+
+from sqliter.sqliter import SqliterDB
+from tests.conftest import ExampleModel
+
+
+class TestContextManager:
+    """Test the context-manager functionality."""
+
+    def test_transaction_commit_success(self, db_mock, mocker) -> None:
+        """Test that the transaction commits successfully with no exceptions."""
+        # Mock the connection's commit method to track the commit
+        mock_commit = mocker.patch.object(db_mock, "conn", create=True)
+        mock_commit.commit = mocker.MagicMock()
+
+        # Run the context manager without errors
+        with db_mock:
+            """Dummy transaction."""
+
+        # Ensure commit was called
+        mock_commit.commit.assert_called_once()
+
+    def test_transaction_closes_connection(self, db_mock, mocker) -> None:
+        """Test the connection is closed after the transaction completes."""
+        # Mock the connection object itself
+        mock_conn = mocker.patch.object(db_mock, "conn", autospec=True)
+
+        # Run the context manager
+        with db_mock:
+            """Dummy transaction."""
+
+        # Ensure the connection is closed
+        mock_conn.close.assert_called_once()
+
+    def test_transaction_rollback_on_exception(self, db_mock, mocker) -> None:
+        """Test that the transaction rolls back when an exception occurs."""
+        # Mock the connection object and ensure it's set as db_mock.conn
+        mock_conn = mocker.Mock()
+        mocker.patch.object(db_mock, "conn", mock_conn)
+
+        # Simulate an exception within the context manager
+        message = "Simulated error"
+        with pytest.raises(ValueError, match=message), db_mock:
+            raise ValueError(message)
+
+        # Ensure rollback was called on the mocked connection
+        mock_conn.rollback.assert_called_once()
+        mock_conn.commit.assert_not_called()
+
+    def test_in_transaction_flag(self, db_mock) -> None:
+        """Test that _in_transaction is set/unset inside a transaction."""
+        assert not db_mock._in_transaction  # Initially, it should be False
+
+        with db_mock:
+            assert db_mock._in_transaction  # Should be True inside the context
+
+        assert (
+            not db_mock._in_transaction
+        )  # Should be False again after exiting the context
+
+    def test_rollback_resets_in_transaction_flag(self, db_mock, mocker) -> None:
+        """Test that _in_transaction is reset after a rollback on exception."""
+
+        def test_transaction() -> None:
+            assert db_mock._in_transaction  # Should be True during transaction
+            err_msg = "Simulated error"
+            raise ValueError(err_msg)
+
+        mock_conn = mocker.Mock()
+        mocker.patch.object(db_mock, "conn", mock_conn)
+
+        with pytest.raises(ValueError, match="Simulated error"), db_mock:
+            test_transaction()
+
+        assert (
+            not db_mock._in_transaction
+        )  # Should be reset to False after exception
+
+    def test_maybe_commit_skips_in_transaction(self, db_mock, mocker) -> None:
+        """Test that maybe_commit does not commit when inside a transaction."""
+        mock_conn = mocker.Mock()
+        mocker.patch.object(db_mock, "conn", mock_conn)
+
+        with db_mock:
+            db_mock._maybe_commit()
+            mock_conn.commit.assert_not_called()
+
+        db_mock._maybe_commit()
+        mock_conn.commit.assert_called_once()
+
+    def test_commit_called_once_in_transaction(self, mocker, tmp_path) -> None:
+        """Ensure data is committed at the end of a transaction."""
+        # Create a temporary database file
+        db_file = tmp_path / "test.db"
+
+        # Initialize the database with the file-based database
+        db_mock = SqliterDB(db_filename=str(db_file), auto_commit=True)
+        db_mock.create_table(ExampleModel)
+
+        # Use the context manager to simulate a transaction
+        with db_mock:
+            db_mock.insert(
+                ExampleModel(slug="test", name="Test", content="Test content")
+            )
+
+        # After the transaction, open a new connection to query the database
+        new_conn = sqlite3.connect(str(db_file))
+        result = new_conn.execute(
+            "SELECT * FROM test_table WHERE slug = 'test'"
+        ).fetchone()
+
+        # Assert that the data was committed
+        assert result is not None, "Data was not committed."
+        assert (
+            result[1] == "test"
+        ), f"Expected slug to be 'test', but got {result[0]}"
+
+        # Close the new connection
+        new_conn.close()

tests/test_sqliter.py

@@ -313,17 +313,16 @@ def test_transaction_commit(self, db_mock, mocker) -> None:
             db_mock.create_table(ExampleModel)
             db_mock.insert(license1)
 
-        # Ensure commit was twice - once during the insert (due to
-        # auto_commit=True) and once when the context manager exited
-        assert mock_conn.commit.call_count == 2
+        # Ensure commit was called only once, when the context manager exited.
+        assert mock_conn.commit.call_count == 1
 
     def test_transaction_manual_commit(self, mocker) -> None:
         """Test context-manager commit when auto_commit is set to False.
 
         Regardless of the auto_commit setting, the context manager should commit
         changes when exiting the context.
         """
-        db_manual = SqliterDB(":memory:", auto_commit=False)
+        db_manual = SqliterDB(":memory:", auto_commit=True)
 
         # Mock the connection and commit
         mock_conn = mocker.MagicMock()
@@ -415,45 +414,6 @@ def test_delete_existing_record(self, db_mock) -> None:
         result = db_mock.get(ExampleModel, result.pk)
         assert result is None
 
-    def test_transaction_commit_success(self, db_mock, mocker) -> None:
-        """Test that the transaction commits successfully with no exceptions."""
-        # Mock the connection's commit method to track the commit
-        mock_commit = mocker.patch.object(db_mock, "conn", create=True)
-        mock_commit.commit = mocker.MagicMock()
-
-        # Run the context manager without errors
-        with db_mock:
-            """Dummy transaction."""
-
-        # Ensure commit was called
-        mock_commit.commit.assert_called_once()
-
-    def test_transaction_closes_connection(self, db_mock, mocker) -> None:
-        """Test the connection is closed after the transaction completes."""
-        # Mock the connection object itself
-        mock_conn = mocker.patch.object(db_mock, "conn", autospec=True)
-
-        # Run the context manager
-        with db_mock:
-            """Dummy transaction."""
-
-        # Ensure the connection is closed
-        mock_conn.close.assert_called_once()
-
-    def test_transaction_rollback_on_exception(self, db_mock, mocker) -> None:
-        """Test that the transaction rolls back when an exception occurs."""
-        # Mock the connection object and ensure it's set as db_mock.conn
-        mock_conn = mocker.Mock()
-        mocker.patch.object(db_mock, "conn", mock_conn)
-
-        # Simulate an exception within the context manager
-        message = "Simulated error"
-        with pytest.raises(ValueError, match=message), db_mock:
-            raise ValueError(message)
-
-        # Ensure rollback was called on the mocked connection
-        mock_conn.rollback.assert_called_once()
-
     def test_select_with_exclude_single_field(
         self,
         db_mock_detailed: SqliterDB,