Parameterize SQL and close JDBC resources in DbInteractions (#139, #140)

[dmccoystephenson]

Summary

Hardens the hand-rolled data layer to fix the injection/breakage risk (#139) and the JDBC resource leak (#140) together, since both live in DbInteractions.

DbInteractions rework:

• query(sql, params...) — binds parameters via PreparedStatement, runs inside try-with-resources, and returns a disconnected CachedRowSet (rows copied out) so the PreparedStatement and live ResultSet are always closed. Callers keep using the ResultSet API unchanged. Fixes the leak in DbInteractions.query/update never close the Statement (JDBC resource leak) #140.
• update(sql, params...) — binds parameters, closes the statement via try-with-resources. Fixes the leak in DbInteractions.query/update never close the Statement (JDBC resource leak) #140.
• Both are varargs, so the ~40 existing parameterless call sites compile and behave unchanged.

Parameterization (#139): the user-controlled name interpolations were moved off string concatenation to bound parameters:

• EntityFactory / EnvironmentFactory inserts
• EntityRepositoryImpl.save + updateName
• EnvironmentRepositoryImpl.findByName + updateName

Integer ids are left inline (not an injection vector); the grid/location inserts in EnvironmentFactory are int-only and untouched. The affected repository test mocks were updated to the parameterized call signatures.

Tests

Adds DbInteractionsTest (H2 in-memory, test scope) — this is what actually exercises the new binding/resource behavior, since DbInteractions is mocked everywhere else:

• parameters bind and the row round-trips;
• a value containing a SQL payload (O'Brien'); DROP TABLE person; --) is stored literally and does not inject (the table survives) — the core Data layer builds SQL by string-concatenating user-supplied names (injection / breakage risk) #139 proof;
• the returned ResultSet is disconnected and fully readable after query() returns (evidence for the DbInteractions.query/update never close the Statement (JDBC resource leak) #140 resource fix);
• invalid SQL returns null/false without throwing; a no-row update returns false.

⚠️ Needs human review — not auto-merged

This PR modifies pom.xml (adds the com.h2database:h2 test-scope dependency), which is on the do-not-auto-merge list. Flagging for manual review rather than merging automatically. The query() redesign (returning a CachedRowSet instead of a live ResultSet) is also a deliberate behavioral change in the core data layer worth a careful look.

Test plan

• ./mvnw test — 226/226 pass (was 220; +6 DbInteractionsTest), JDK 21
• DbInteractionsTest runs real SQL on H2, including the injection-payload case
• Python client — n/a (no src/main/python change)

Follow-up note

The deeper fix for query() returning a ResultSet at all (a RowMapper-style API returning List<T>) would let every repository drop its manual ResultSet handling, but that is a much larger refactor; the CachedRowSet approach here fixes the leak with minimal blast radius.

Closes #139
Closes #140

🤖 Generated with Claude Code

[dmccoystephenson]

Self-review rubric (adversarial; anchored on green CI run 27468231673 + the diff):

• Scope: PASS — 10 files, all necessary: DbInteractions rework, the 4 name-bearing call sites (2 factories + 2 repos), their updated test mocks, the new H2 test, and the pom.xml test dep. No unrelated churn.
• Resource fix (#140): PASS — both methods use try-with-resources on the PreparedStatement; query() copies rows into a CachedRowSet before the live ResultSet/statement close. DbInteractionsTest.query_returnsDisconnectedResultSet_readableAfterReturn reads the result fully after query() returns, which is only possible because it is disconnected.
• Injection fix (#139): PASS — demonstrated, not asserted. parameterizedValueWithSqlPayload_isStoredLiterally_andDoesNotInject inserts O'Brien'); DROP TABLE person; -- as a bound parameter; the table survives and the value round-trips verbatim. The data layer no longer concatenates user-controlled name values.
• Backward compatibility: PASS — both signatures are varargs, so the ~40 existing parameterless call sites compile and run unchanged; full suite is 226/226 (was 220). Repositories consume the CachedRowSet via the same ResultSet API (forward next() + getInt/getString by label/index), all supported.
• Tests-fix mocks: PASS — the 12 repository-test stubs whose SQL changed (? placeholders + param arg) were updated; EntityFactoryTest was updated for the new varargs INSERT.
• CI: PASS — build green on PR head; CI compiles and runs the H2 test.
• DTO boundary / Spec / Java-Python parallelism: N/A — internal data layer; no endpoint, DTO, or Python-client surface changed (the Python client is an HTTP client with no DbInteractions mirror).
• Override correctness: PASS — only method bodies changed; no @Override added or removed.

Honest caveats for the reviewer:

1. query() now returns a CachedRowSet rather than a live ResultSet — a deliberate behavioral change in the core data layer (the point of the leak fix). Repositories iterate forward-only, so this is compatible, but it warrants a look.
2. DbInteractionsTest covers getInt/getString by label and the factory's getInt(1) by index is standard CachedRowSet but not separately asserted here.
3. Integer ids remain string-concatenated (not an injection vector); only user-controlled strings were parameterized to bound the diff.

This PR is intentionally not auto-merged — it touches pom.xml (do-not-auto-merge). Flagged for manual review.