fix: make the multi-statement write paths atomic

src/main/java/preponderous/viron/config/DataSourceConfig.java

@@ -8,10 +8,13 @@
 import com.zaxxer.hikari.HikariDataSource;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.jdbc.datasource.DataSourceTransactionManager;
+import org.springframework.transaction.PlatformTransactionManager;
 
 /**
  * Supplies the pooled {@link DataSource} that
- * {@link preponderous.viron.database.DbInteractions} draws connections from.
+ * {@link preponderous.viron.database.DbInteractions} draws connections from, together with the
+ * transaction manager that gives the multi-statement write paths their boundaries.
  *
  * <p>The pool is built from {@link DbConfig} rather than Spring Boot's
  * {@code spring.datasource.*} properties, so the existing {@code database.*} property
@@ -39,4 +42,22 @@ public DataSource dataSource(DbConfig dbConfig) {
         dataSource.setPassword(dbConfig.getDbPassword());
         return dataSource;
     }
+
+    /**
+     * Manages the transactions that {@code @Transactional} write paths run in.
+     *
+     * <p>This is the manager {@code DataSourceUtils} cooperates with: it binds one pooled
+     * connection to the current thread with auto-commit off for the length of the transaction,
+     * so every {@link preponderous.viron.database.DbInteractions} call made inside the boundary
+     * joins it and a thrown runtime exception discards the whole sequence rather than leaving
+     * a half-applied cascade behind.
+     *
+     * <p>Spring Boot would auto-configure an equivalent manager, but only while exactly one
+     * {@link DataSource} candidate exists and no manager is declared. Declaring it next to the
+     * pool it wraps keeps the boundaries from depending on that condition continuing to hold.
+     */
+    @Bean
+    public PlatformTransactionManager transactionManager(DataSource dataSource) {
+        return new DataSourceTransactionManager(dataSource);
+    }
 }

src/main/java/preponderous/viron/controllers/EntityController.java

@@ -3,6 +3,7 @@
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.http.HttpStatus;
+import org.springframework.transaction.annotation.Transactional;
 import org.springframework.validation.annotation.Validated;
 import org.springframework.web.bind.annotation.*;
 import preponderous.viron.dto.CreateEntityRequest;
@@ -73,8 +74,17 @@ public EntityDto createEntity(@Valid @RequestBody CreateEntityRequest request) {
         return entityMapper.toDto(newEntity);
     }
 
+    /**
+     * Deletes an entity and its placement.
+     *
+     * <p>{@link EntityRepository#deleteById(int)} clears the placement before deleting the
+     * entity, so the two statements run in one transaction: the {@link ServiceException} raised
+     * when the entity delete does not take effect rolls the placement back, rather than leaving
+     * an entity that has silently lost where it was.
+     */
     @DeleteMapping("/{id}")
     @ResponseStatus(HttpStatus.NO_CONTENT)
+    @Transactional
     public void deleteEntity(@PathVariable @Min(1) int id) {
         if (entityRepository.findById(id).isEmpty()) {
             throw new NotFoundException("Entity not found with id: " + id);

src/main/java/preponderous/viron/controllers/EnvironmentController.java

@@ -7,6 +7,7 @@
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.http.HttpStatus;
+import org.springframework.transaction.annotation.Transactional;
 import org.springframework.validation.annotation.Validated;
 import org.springframework.web.bind.annotation.*;
 import preponderous.viron.dto.CreateEnvironmentRequest;
@@ -70,8 +71,17 @@ public EnvironmentDto createEnvironment(@Valid @RequestBody CreateEnvironmentReq
         return environmentMapper.toDto(newEnvironment);
     }
 
+    /**
+     * Deletes an environment and everything it contains.
+     *
+     * <p>The cascade is a long sequence of dependent deletes, so it runs in one transaction:
+     * the {@link ServiceException} thrown on the first failure discards the deletes already
+     * performed instead of leaving a partially deleted environment that no request could have
+     * produced deliberately.
+     */
     @DeleteMapping("/{id}")
     @ResponseStatus(HttpStatus.NO_CONTENT)
+    @Transactional
     public void deleteEnvironment(@PathVariable @Min(1) int id) {
         if (environmentRepository.findById(id).isEmpty()) {
             throw new NotFoundException("Environment not found with id: " + id);

src/main/java/preponderous/viron/factories/EnvironmentFactory.java

@@ -7,6 +7,7 @@
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
+import org.springframework.transaction.annotation.Transactional;
 
 import preponderous.viron.database.DbInteractions;
 import preponderous.viron.exceptions.EnvironmentCreationException;
@@ -26,11 +27,19 @@ public EnvironmentFactory(DbInteractions dbInteractions) {
      * Creates an environment containing {@code numGrids} grids, each {@code numRows} by
      * {@code numColumns} locations. Rows and columns are independent — grids need not be square.
      *
+     * <p>The environment, its grids, its locations and every association between them are
+     * inserted in one transaction, so the {@link EnvironmentCreationException} thrown partway
+     * through discards the rows already written instead of leaving an environment that is
+     * missing some of its grids or a grid that is missing some of its locations. The sequences
+     * the ids come from are not transactional, so a rolled-back attempt still consumes the ids
+     * it drew — the gap is expected and harmless.
+     *
      * @param name        name of the environment
      * @param numGrids    number of grids to create in the environment
      * @param numRows     number of rows in each grid
      * @param numColumns  number of columns in each grid
      */
+    @Transactional
     public Environment createEnvironment(String name, int numGrids, int numRows, int numColumns) throws EnvironmentCreationException {
         log.info("Attempting to create environment: '{}' with {} grids of size {}x{}", name, numGrids, numRows, numColumns);
 

src/main/java/preponderous/viron/repositories/EntityRepositoryImpl.java

@@ -91,6 +91,11 @@ public Entity save(Entity entity) {
      * placement row has to be cleared first or the entity delete is rejected by the constraint.
      * An entity that is not placed has no row to clear, which is why the first statement's result
      * is not part of the outcome — only the entity delete itself is reported.
+     *
+     * <p>The two statements are not atomic on their own: a {@code false} return means the entity
+     * survived while its placement was already cleared. Callers that need the pair to be
+     * all-or-nothing put a transaction boundary around the call and fail on {@code false} — see
+     * {@link preponderous.viron.controllers.EntityController#deleteEntity(int)}.
      */
     @Override
     public boolean deleteById(int id) {

src/test/java/preponderous/viron/config/TransactionBoundaryWiringTest.java

@@ -0,0 +1,83 @@
+// Copyright (c) 2024 Preponderous Software
+// MIT License
+
+package preponderous.viron.config;
+
+import java.lang.reflect.Method;
+
+import javax.sql.DataSource;
+
+import org.junit.jupiter.api.Test;
+import org.springframework.aop.support.AopUtils;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.context.ApplicationContext;
+import org.springframework.jdbc.datasource.DataSourceTransactionManager;
+import org.springframework.transaction.PlatformTransactionManager;
+import org.springframework.transaction.interceptor.TransactionAttributeSource;
+import preponderous.viron.controllers.EntityController;
+import preponderous.viron.controllers.EnvironmentController;
+import preponderous.viron.factories.EnvironmentFactory;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Verifies that the write paths annotated for #194 are genuinely advised at runtime: an
+ * {@code @Transactional} annotation only takes effect through a proxy, and only while a
+ * transaction manager is present. Rollback behaviour itself is proved against a real database in
+ * {@code preponderous.viron.database.TransactionRollbackTest}.
+ */
+@SpringBootTest
+class TransactionBoundaryWiringTest {
+
+    @Autowired
+    private ApplicationContext applicationContext;
+
+    @Autowired
+    private TransactionAttributeSource transactionAttributeSource;
+
+    @Test
+    void contextExposesOneTransactionManagerBoundToThePooledDataSource() {
+        assertThat(applicationContext.getBeanNamesForType(PlatformTransactionManager.class)).hasSize(1);
+
+        DataSourceTransactionManager transactionManager =
+                (DataSourceTransactionManager) applicationContext.getBean(PlatformTransactionManager.class);
+        assertThat(transactionManager.getDataSource()).isSameAs(applicationContext.getBean(DataSource.class));
+    }
+
+    @Test
+    void environmentCascadeDeleteRunsInATransaction() {
+        assertTransactional(EnvironmentController.class, "deleteEnvironment", int.class);
+    }
+
+    @Test
+    void entityDeleteRunsInATransaction() {
+        assertTransactional(EntityController.class, "deleteEntity", int.class);
+    }
+
+    @Test
+    void environmentCreationRunsInATransaction() {
+        assertTransactional(EnvironmentFactory.class, "createEnvironment",
+                String.class, int.class, int.class, int.class);
+    }
+
+    private void assertTransactional(Class<?> beanType, String methodName, Class<?>... parameterTypes) {
+        Object bean = applicationContext.getBean(beanType);
+        assertThat(AopUtils.isAopProxy(bean))
+                .as("%s must be proxied for its transaction boundary to apply", beanType.getSimpleName())
+                .isTrue();
+
+        Method method = findMethod(beanType, methodName, parameterTypes);
+        assertThat(transactionAttributeSource.getTransactionAttribute(method, AopUtils.getTargetClass(bean)))
+                .as("%s.%s must carry a transaction attribute", beanType.getSimpleName(), methodName)
+                .isNotNull();
+    }
+
+    private static Method findMethod(Class<?> beanType, String methodName, Class<?>... parameterTypes) {
+        try {
+            return beanType.getMethod(methodName, parameterTypes);
+        } catch (NoSuchMethodException e) {
+            throw new AssertionError(beanType.getSimpleName() + "." + methodName + " no longer exists", e);
+        }
+    }
+}

src/test/java/preponderous/viron/controllers/EntityControllerTest.java

@@ -9,7 +9,6 @@
 import org.springframework.boot.test.mock.mockito.MockBean;
 import org.springframework.http.MediaType;
 import org.springframework.test.web.servlet.MockMvc;
-import preponderous.viron.config.DbConfig;
 import preponderous.viron.database.DbInteractions;
 import preponderous.viron.dto.EntityDto;
 import preponderous.viron.exceptions.EntityCreationException;
@@ -50,8 +49,9 @@ class EntityControllerTest {
     @MockBean
     private DbInteractions dbInteractions;
 
-    @MockBean
-    private DbConfig dbConfig;
+    // DbConfig is left real, unlike the collaborators above: the transaction boundary on
+    // deleteEntity opens a connection from the pool DbConfig configures, and a mock would
+    // supply it a null JDBC URL.
 
     // --- GET /api/v1/entities ---
 

src/test/java/preponderous/viron/controllers/EnvironmentControllerTest.java

@@ -9,7 +9,6 @@
 import org.springframework.http.MediaType;
 import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.web.servlet.MockMvc;
-import preponderous.viron.config.DbConfig;
 import preponderous.viron.database.DbInteractions;
 import preponderous.viron.dto.EnvironmentDto;
 import preponderous.viron.exceptions.EnvironmentCreationException;
@@ -50,8 +49,9 @@ class EnvironmentControllerTest {
     @MockBean
     private DbInteractions dbInteractions;
 
-    @MockBean
-    private DbConfig dbConfig;
+    // DbConfig is left real, unlike the collaborators above: the transaction boundary on
+    // deleteEnvironment opens a connection from the pool DbConfig configures, and a mock would
+    // supply it a null JDBC URL.
 
     @Test
     void getAllEnvironments_Success() throws Exception {

src/test/java/preponderous/viron/database/TransactionRollbackTest.java

@@ -0,0 +1,133 @@
+// Copyright (c) 2024 Preponderous Software
+// MIT License
+
+package preponderous.viron.database;
+
+import com.zaxxer.hikari.HikariDataSource;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.springframework.transaction.PlatformTransactionManager;
+import org.springframework.transaction.support.TransactionTemplate;
+import preponderous.viron.config.DataSourceConfig;
+import preponderous.viron.config.DbConfig;
+import preponderous.viron.exceptions.ServiceException;
+import preponderous.viron.repositories.EntityRepositoryImpl;
+
+import javax.sql.DataSource;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+/**
+ * Proves the transaction boundaries added for #194 actually roll back, by running the real
+ * {@link EntityRepositoryImpl#deleteById(int)} — two dependent statements — against an in-memory
+ * H2 database through the production {@link DataSourceConfig} wiring.
+ *
+ * <p>The schema is a minimal stand-in for the Postgres one: the entity table, the placement table
+ * whose row has to be cleared first, and a guard table that exists only so the second statement
+ * can be made to fail on demand.
+ */
+class TransactionRollbackTest {
+
+    private static final int ENTITY_ID = 1;
+    private static final int LOCATION_ID = 7;
+
+    private DataSource dataSource;
+    private DbInteractions dbInteractions;
+    private EntityRepositoryImpl entityRepository;
+    private TransactionTemplate transactionTemplate;
+
+    @BeforeEach
+    void setUp() {
+        dataSource = new DataSourceConfig().dataSource(h2Config());
+        dbInteractions = new DbInteractions(dataSource);
+        entityRepository = new EntityRepositoryImpl(dbInteractions);
+
+        PlatformTransactionManager transactionManager = new DataSourceConfig().transactionManager(dataSource);
+        transactionTemplate = new TransactionTemplate(transactionManager);
+
+        dbInteractions.update("DROP TABLE IF EXISTS viron.entity_delete_guard");
+        dbInteractions.update("DROP TABLE IF EXISTS viron.entity_location");
+        dbInteractions.update("DROP TABLE IF EXISTS viron.entity");
+        dbInteractions.update("CREATE SCHEMA IF NOT EXISTS viron");
+        dbInteractions.update(
+                "CREATE TABLE viron.entity (entity_id INT PRIMARY KEY, name VARCHAR(255), creation_date VARCHAR(255))");
+        dbInteractions.update(
+                "CREATE TABLE viron.entity_location (entity_id INT, location_id INT, PRIMARY KEY (entity_id, location_id))");
+        dbInteractions.update(
+                "CREATE TABLE viron.entity_delete_guard (id INT PRIMARY KEY, entity_id INT NOT NULL REFERENCES viron.entity(entity_id))");
+
+        dbInteractions.update("INSERT INTO viron.entity (entity_id, name, creation_date) VALUES (?, ?, ?)",
+                ENTITY_ID, "Alice", "2026-01-01");
+        dbInteractions.update("INSERT INTO viron.entity_location (entity_id, location_id) VALUES (?, ?)",
+                ENTITY_ID, LOCATION_ID);
+    }
+
+    @AfterEach
+    void tearDown() {
+        ((HikariDataSource) dataSource).close();
+    }
+
+    // #194: the placement must come back when the entity delete it was cleared for does not happen.
+    @Test
+    void failedEntityDelete_insideATransaction_restoresTheClearedPlacement() {
+        blockEntityDeletion();
+
+        assertThatThrownBy(() -> transactionTemplate.executeWithoutResult(status -> {
+            if (!entityRepository.deleteById(ENTITY_ID)) {
+                throw new ServiceException("Failed to delete entity with id: " + ENTITY_ID);
+            }
+        })).isInstanceOf(ServiceException.class);
+
+        assertThat(entityExists()).isTrue();
+        assertThat(placementExists()).isTrue();
+    }
+
+    // The behaviour the boundary replaces: without one, the first delete commits on its own and
+    // the surviving entity silently loses where it was.
+    @Test
+    void failedEntityDelete_withoutATransaction_leavesTheEntityWithoutItsPlacement() {
+        blockEntityDeletion();
+
+        assertThat(entityRepository.deleteById(ENTITY_ID)).isFalse();
+
+        assertThat(entityExists()).isTrue();
+        assertThat(placementExists()).isFalse();
+    }
+
+    @Test
+    void successfulDelete_insideATransaction_commitsBothStatements() {
+        transactionTemplate.executeWithoutResult(status -> {
+            if (!entityRepository.deleteById(ENTITY_ID)) {
+                throw new ServiceException("Failed to delete entity with id: " + ENTITY_ID);
+            }
+        });
+
+        assertThat(entityExists()).isFalse();
+        assertThat(placementExists()).isFalse();
+    }
+
+    /** Makes {@code DELETE FROM viron.entity} fail by pointing a foreign key at the row. */
+    private void blockEntityDeletion() {
+        dbInteractions.update("INSERT INTO viron.entity_delete_guard (id, entity_id) VALUES (?, ?)", 1, ENTITY_ID);
+    }
+
+    private boolean entityExists() {
+        return dbInteractions.queryOne("SELECT entity_id FROM viron.entity WHERE entity_id = ?",
+                rs -> rs.getInt(1), ENTITY_ID).isPresent();
+    }
+
+    private boolean placementExists() {
+        return dbInteractions.queryOne("SELECT location_id FROM viron.entity_location WHERE entity_id = ?",
+                rs -> rs.getInt(1), ENTITY_ID).isPresent();
+    }
+
+    private static DbConfig h2Config() {
+        DbConfig config = new DbConfig();
+        config.setDbUrl("jdbc:h2:mem:viron_transactionrollback;DB_CLOSE_DELAY=-1");
+        config.setDbUsername("sa");
+        config.setDbPassword("");
+        return config;
+    }
+}