fix: report the duplicate-placement loser as 409, not 500

README.md

@@ -88,7 +88,8 @@ The MVP implements the endpoints defined in `docs/openapi/viron-api.json` and do
 - Docker + Docker Compose (deployment)
 - Swagger/OpenAPI (API documentation)
 - JaCoCo (test coverage)
-- Planned: Flyway (future migrations); schema currently comes from `db-scripts/setup/`
+- Planned: Flyway (future migrations); schema currently comes from `db-scripts/setup/`, with
+  changes to an already-created schema kept as hand-run scripts in `db-scripts/migrations/`
 
 ---
 
@@ -108,7 +109,7 @@ viron/
  │    └── services/          # Business logic  
  ├── src/main/python/        # Python client SDK  
  ├── src/test/java/...       # Unit and integration tests  
- ├── db-scripts/             # SQL schema setup scripts  
+ ├── db-scripts/             # SQL schema setup scripts, and migrations for existing databases  
  ├── docs/  
  │    ├── MVP.md             # Implementation checklist for MVP  
  │    └── openapi/  

db-scripts/migrations/2026-08-12_entity_location_one_placement_per_entity.sql

@@ -0,0 +1,47 @@
+-- Copyright (c) 2024 Preponderous Software
+-- MIT License
+
+-- Migration for issue #200: one placement per entity.
+--
+-- Databases created before this change key viron.entity_location on (entity_id, location_id),
+-- which permits the same entity to occupy two locations at once. Two concurrent placements of an
+-- unplaced entity at different locations both pass the controller's read-then-write guard and
+-- both insert, so the invariant the rest of the service assumes is not actually enforced.
+-- Keying on entity_id alone makes the database reject the second insert, which is what lets the
+-- losing request be reported as a 409 instead of a 500.
+--
+-- db-scripts/setup/create_tables.sql already creates new databases this way; this script brings
+-- an existing one into line. It is not applied automatically (the setup scripts run only when
+-- Postgres initialises an empty volume), so run it once against each existing database:
+--
+--     psql -U "$DATABASE_DB_USERNAME" -d "$DATABASE_DB_NAME" \
+--         -f db-scripts/migrations/2026-08-12_entity_location_one_placement_per_entity.sql
+--
+-- Nothing is deleted here. If any entity is already placed twice, the migration aborts and
+-- reports the count, leaving it to an operator to decide which placement is the real one.
+
+BEGIN;
+
+DO $$
+DECLARE
+    duplicate_count INT;
+BEGIN
+    SELECT count(*) INTO duplicate_count
+    FROM (
+        SELECT entity_id
+        FROM viron.entity_location
+        GROUP BY entity_id
+        HAVING count(*) > 1
+    ) duplicates;
+
+    IF duplicate_count > 0 THEN
+        RAISE EXCEPTION
+            'viron.entity_location places % entity/entities at more than one location; resolve them before applying this migration. To list them: SELECT entity_id, location_id FROM viron.entity_location WHERE entity_id IN (SELECT entity_id FROM viron.entity_location GROUP BY entity_id HAVING count(*) > 1) ORDER BY entity_id, location_id;',
+            duplicate_count;
+    END IF;
+END $$;
+
+ALTER TABLE viron.entity_location DROP CONSTRAINT entity_location_pkey;
+ALTER TABLE viron.entity_location ADD PRIMARY KEY (entity_id);
+
+COMMIT;

db-scripts/setup/create_tables.sql

@@ -34,10 +34,14 @@ CREATE TABLE IF NOT EXISTS viron.environment (
 );
 
 -- entity_location table (entity_id, location_id)
+-- An entity occupies at most one location, so entity_id alone is the key: a location may hold
+-- many entities, but a second placement of the same entity is rejected by the database rather
+-- than only by the read-then-write guard in LocationController.addEntityToLocation, which two
+-- concurrent requests can both pass.
 CREATE TABLE IF NOT EXISTS viron.entity_location (
     entity_id INT NOT NULL,
     location_id INT NOT NULL,
-    PRIMARY KEY (entity_id, location_id),
+    PRIMARY KEY (entity_id),
     FOREIGN KEY (entity_id) REFERENCES viron.entity(entity_id),
     FOREIGN KEY (location_id) REFERENCES viron.location(location_id)
 );

docs/openapi/viron-api.json

@@ -370,7 +370,7 @@
     "/api/v1/locations/{locationId}/entity/{entityId}": {
       "put": {
         "summary": "Add entity to location",
-        "description": "Places an unplaced entity at the location. An entity occupies at most one location, so the request is a no-op when the entity is already at that location and a conflict when it is placed elsewhere.",
+        "description": "Places an unplaced entity at the location. An entity occupies at most one location, so the request is a no-op when the entity is already at that location and a conflict when it is placed elsewhere. The same answers are given when two placements of the same entity are made at once: exactly one of them takes effect, and the others are resolved against the placement it left behind.",
         "parameters": [
           { "$ref": "#/components/parameters/LocationIdParam" },
           { "$ref": "#/components/parameters/EntityIdParam" }
@@ -384,7 +384,7 @@
             }
           },
           "409": {
-            "description": "Entity is already placed at a different location",
+            "description": "Entity is already placed at a different location, including when a concurrent request placed it there first",
             "content": {
               "application/json": { "schema": { "$ref": "#/components/schemas/ErrorResponse" } }
             }

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

@@ -2,6 +2,7 @@
 
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
+import org.springframework.dao.DuplicateKeyException;
 import org.springframework.http.HttpStatus;
 import org.springframework.validation.annotation.Validated;
 import org.springframework.web.bind.annotation.*;
@@ -72,6 +73,13 @@ public LocationDto getLocationOfEntity(@PathVariable @Min(1) int entityId) {
      * Places an unplaced entity at {@code locationId}. An entity occupies at most one location,
      * so a request for an entity that is already placed elsewhere is a conflict; a request for an
      * entity already at the target is a no-op, keeping the {@code PUT} idempotent.
+     *
+     * <p>The placement check below cannot decide the outcome on its own: two concurrent requests
+     * for the same unplaced entity both read no placement and both go on to insert. The primary
+     * key on {@code viron.entity_location.entity_id} is what settles which of them wins, so the
+     * loser is recognised by the {@link DuplicateKeyException} its insert raises and answered
+     * from the placement the winner committed — the same answer the check above would have given
+     * had the two requests arrived in sequence (#200).
      */
     @PutMapping("/{locationId}/entity/{entityId}")
     public void addEntityToLocation(@PathVariable("entityId") @Min(1) int entityId, @PathVariable("locationId") @Min(1) int locationId) {
@@ -83,15 +91,36 @@ public void addEntityToLocation(@PathVariable("entityId") @Min(1) int entityId,
         }
         Optional<Location> currentLocation = locationRepository.findByEntityId(entityId);
         if (currentLocation.isPresent()) {
-            if (currentLocation.get().getLocationId() == locationId) {
-                return;
+            reportPlacement(entityId, locationId, currentLocation.get());
+            return;
+        }
+        try {
+            if (!locationRepository.addEntityToLocation(entityId, locationId)) {
+                throw new ServiceException("Failed to add entity " + entityId + " to location " + locationId);
+            }
+        } catch (DuplicateKeyException e) {
+            log.info("Entity {} was placed concurrently while adding it to location {}", entityId, locationId);
+            Optional<Location> winner = locationRepository.findByEntityId(entityId);
+            if (winner.isEmpty()) {
+                // The winning placement was removed again before it could be read back, so there
+                // is no location to name. The request still failed on a conflict, not a fault.
+                throw new ConflictException("Entity " + entityId
+                        + " was placed by a concurrent request and could not be added to location " + locationId);
             }
-            throw new ConflictException("Entity " + entityId + " is already placed at location "
-                    + currentLocation.get().getLocationId());
+            reportPlacement(entityId, locationId, winner.get());
         }
-        if (!locationRepository.addEntityToLocation(entityId, locationId)) {
-            throw new ServiceException("Failed to add entity " + entityId + " to location " + locationId);
+    }
+
+    /**
+     * Answers a placement request for an entity that is already placed: silence when it is
+     * already where the request wanted it, a conflict naming its actual location otherwise.
+     */
+    private static void reportPlacement(int entityId, int requestedLocationId, Location placement) {
+        if (placement.getLocationId() == requestedLocationId) {
+            return;
         }
+        throw new ConflictException("Entity " + entityId + " is already placed at location "
+                + placement.getLocationId());
     }
 
     @DeleteMapping("/{locationId}/entity/{entityId}")

src/main/java/preponderous/viron/database/DbInteractions.java

@@ -15,6 +15,7 @@
 
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.dao.DuplicateKeyException;
 import org.springframework.jdbc.datasource.DataSourceUtils;
 import org.springframework.stereotype.Component;
 
@@ -42,6 +43,10 @@
 @Component
 @Slf4j
 public class DbInteractions {
+
+    /** SQL state for a unique/primary-key violation, as reported by both Postgres and H2. */
+    private static final String SQL_STATE_UNIQUE_VIOLATION = "23505";
+
     private final DataSource dataSource;
 
     @Autowired
@@ -124,18 +129,72 @@ public <T> Optional<T> queryOne(String query, RowMapper<T> mapper, Object... par
      * @return {@code true} if at least one row was affected, {@code false} otherwise (including on error)
      */
     public boolean update(String query, Object... params) {
+        try {
+            return updateReportingDuplicateKey(query, params);
+        } catch (DuplicateKeyException e) {
+            // Flattened to false so this method keeps the contract every existing caller was
+            // written against; callers that need to tell a conflict apart from any other write
+            // failure call updateReportingDuplicateKey directly.
+            log.error("Error executing update: {}", e.getMessage());
+            return false;
+        }
+    }
+
+    /**
+     * Execute a parameterized INSERT/UPDATE/DELETE, reporting a unique or primary-key violation
+     * to the caller instead of flattening it to {@code false}.
+     *
+     * <p>Identical to {@link #update(String, Object...)} in every other respect. It exists because
+     * a request that loses a race against a concurrent write is a conflict, not a server fault,
+     * and the two are indistinguishable once the {@link SQLException} has been swallowed
+     * (#200): {@link preponderous.viron.repositories.LocationRepositoryImpl#addEntityToLocation}
+     * needs the distinction to answer 409 rather than 500.
+     *
+     * <p>Detection is by SQL state {@code 23505} (unique violation), which Postgres and the H2
+     * database the tests run against both report. Every other failure is logged and reported as
+     * {@code false}, exactly as before.
+     *
+     * @param query  SQL with {@code ?} placeholders for each parameter
+     * @param params values to bind to the placeholders, in order
+     * @return {@code true} if at least one row was affected, {@code false} otherwise (including on
+     *         any error that is not a unique/primary-key violation)
+     * @throws DuplicateKeyException if the statement violated a unique or primary-key constraint
+     */
+    public boolean updateReportingDuplicateKey(String query, Object... params) {
         Connection connection = DataSourceUtils.getConnection(dataSource);
         try (PreparedStatement statement = connection.prepareStatement(query)) {
             bindParameters(statement, params);
             return statement.executeUpdate() > 0;
         } catch (SQLException e) {
+            if (isUniqueViolation(e)) {
+                throw new DuplicateKeyException("Update violated a unique constraint: " + e.getMessage(), e);
+            }
             log.error("Error executing update: {}", e.getMessage());
         } finally {
             DataSourceUtils.releaseConnection(connection, dataSource);
         }
         return false;
     }
 
+    /**
+     * True if {@code e}, or any exception chained behind it, reports SQL state {@code 23505}.
+     *
+     * <p>The chain is walked because a driver may wrap the violation it actually hit: the
+     * Postgres driver reports it directly, but batched or nested failures surface through
+     * {@link SQLException#getNextException()}.
+     */
+    private static boolean isUniqueViolation(SQLException e) {
+        for (SQLException current = e; current != null; current = current.getNextException()) {
+            if (SQL_STATE_UNIQUE_VIOLATION.equals(current.getSQLState())) {
+                return true;
+            }
+            if (current == current.getNextException()) {
+                break;
+            }
+        }
+        return false;
+    }
+
     private void bindParameters(PreparedStatement statement, Object... params) throws SQLException {
         for (int i = 0; i < params.length; i++) {
             statement.setObject(i + 1, params[i]);

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

@@ -10,6 +10,14 @@ public interface LocationRepository {
     List<Location> findByEnvironmentId(int environmentId);
     List<Location> findByGridId(int gridId);
     Optional<Location> findByEntityId(int entityId);
+    /**
+     * Places an entity at a location.
+     *
+     * @throws org.springframework.dao.DuplicateKeyException if the entity is already placed —
+     *         the database, not the caller's prior read, is what decides this, so a request that
+     *         lost a race against a concurrent placement lands here rather than returning
+     *         {@code false}
+     */
     boolean addEntityToLocation(int entityId, int locationId);
     boolean removeEntityFromLocation(int entityId, int locationId);
     boolean removeEntityFromCurrentLocation(int entityId);

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

@@ -57,10 +57,19 @@ public Optional<Location> findByEntityId(int entityId) {
         return dbInteractions.queryOne(query, this::mapResultSetToLocation, entityId);
     }
 
+    /**
+     * {@inheritDoc}
+     *
+     * <p>The insert is issued through
+     * {@link DbInteractions#updateReportingDuplicateKey(String, Object...)} so that the primary
+     * key on {@code entity_id} — the only thing that actually serialises two concurrent
+     * placements of the same unplaced entity — reaches the caller as a conflict rather than as
+     * an indistinguishable {@code false}.
+     */
     @Override
     public boolean addEntityToLocation(int entityId, int locationId) {
         String query = "INSERT INTO viron.entity_location (entity_id, location_id) VALUES (?, ?)";
-        return dbInteractions.update(query, entityId, locationId);
+        return dbInteractions.updateReportingDuplicateKey(query, entityId, locationId);
     }
 
     @Override