chore: dead template surface — instantiate what we ship, drop what nothing uses

docs/CONVENTIONS.md

@@ -57,6 +57,22 @@ hand-written variant because it joins roles):
 - Every write wraps `Database::get().execute_write([&](auto& txn){…})`. The
   lambda MUST take `auto& txn` (it receives `detail::TracingTxn&`, not a raw
   `pqxx::work&`). Use `execute_read_primary` for read-after-write.
+- Two more `execute_*` variants exist for forks (nothing in the template
+  itself calls them — don't let that fool you into deleting them):
+  - `execute_write_idempotent(fn)` — same as `execute_write` but with the
+    liberal read-style retry classifier, which may REPLAY the whole
+    transaction after a connection-class error whose commit already landed.
+    Safe only for writes keyed by a natural key (UPSERT by name, DELETE by
+    PK) where a replay converges instead of double-applying.
+  - `execute_transaction(IsolationLevel, fn)` — multi-statement transaction
+    at `ReadCommitted` / `RepeatableRead` / `Serializable` (one-arg overload
+    defaults to ReadCommitted). Retry classification matches
+    `execute_write` (only PG-confirmed 40001/40P01 rollbacks — exactly what
+    Serializable produces under contention).
+
+  Both are pinned by real-Postgres tests in
+  `tests/integration/test_database.cpp` and compile-checked against the DI
+  seam in `tests/unit/test_database_seam.cpp`.
 - Wrap UNIQUE/FK-tripping writes in `detail::translate_sql(...)`
   (`repositories/SqlErrors.hpp`) to turn a SQLSTATE into your typed exception —
   otherwise a constraint violation surfaces as a raw 500. For the ubiquitous

src/api/Validation.hpp

@@ -32,6 +32,7 @@
 
 #include <nlohmann/json.hpp>
 
+#include "api/RequestUtils.hpp"
 #include "utils/ErrorResponse.hpp"
 
 namespace Api::Validation {
@@ -224,8 +225,16 @@ inline void email(Errors& errs, const json& body, const std::string& field) {
 }
 
 inline void uuid(Errors& errs, const json& body, const std::string& field) {
-    static const std::regex re("^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$");
-    regex_match(errs, body, field, re, "uuid");
+    // Single source of truth for the 8-4-4-4-12 hex shape: the char-walk in
+    // RequestUtils (Api::detail::is_uuid_segment) — this used to be a second,
+    // regex-based UUID implementation that could drift from it. Same error
+    // contract as the regex validators: "bad_format" on mismatch, "not_string"
+    // on a wrong-typed field, no-op when absent (presence is require()'s job).
+    const auto* s = detail::as_string(errs, body, field);
+    if (!s)
+        return;
+    if (!Api::detail::is_uuid_segment(*s))
+        errs.add(field, "bad_format", "expected format: uuid");
 }
 
 // ---------------------------------------------------------------------------

src/messaging/Messaging.hpp

@@ -138,8 +138,6 @@ class KafkaProducer {
         return true;
     }
 
-    bool produce(const std::string& topic, const std::string& payload) { return produce(topic, "", payload); }
-
     int flush(int timeout_ms = 10000) {
         check_initialized();
         return producer->flush(timeout_ms);
@@ -307,16 +305,6 @@ class KafkaConsumer {
 
     void stop_consuming() { consuming = false; }
 
-    bool commit() {
-        check_initialized();
-        RdKafka::ErrorCode err = consumer->commitSync();
-        if (err != RdKafka::ERR_NO_ERROR) {
-            spdlog::error("Failed to commit offsets: {}", RdKafka::err2str(err));
-            return false;
-        }
-        return true;
-    }
-
     void shutdown() {
         if (initialized) {
             spdlog::info("Shutting down Kafka consumer");

src/security/Auth.cpp

@@ -281,17 +281,6 @@ bool has_role(const drogon::HttpRequestPtr& req, const std::string& role) {
     return std::find(p->roles.begin(), p->roles.end(), role) != p->roles.end();
 }
 
-bool has_any_role(const drogon::HttpRequestPtr& req, const std::vector<std::string>& roles) {
-    auto p = principal_of(req);
-    if (!p)
-        return false;
-    for (const auto& want : roles) {
-        if (std::find(p->roles.begin(), p->roles.end(), want) != p->roles.end())
-            return true;
-    }
-    return false;
-}
-
 bool auth_enforced() {
     return is_initialized() && get().config().mode != AuthMode::None;
 }
@@ -304,14 +293,6 @@ drogon::HttpResponsePtr require_role(const drogon::HttpRequestPtr& req, const st
     return ErrorResponse::make({drogon::k403Forbidden, "forbidden", "", nlohmann::json{{"required_role", role}}});
 }
 
-drogon::HttpResponsePtr require_any_role(const drogon::HttpRequestPtr& req, const std::vector<std::string>& roles) {
-    if (!auth_enforced())
-        return {};
-    if (has_any_role(req, roles))
-        return {};
-    return ErrorResponse::make({drogon::k403Forbidden, "forbidden", "", nlohmann::json{{"required_roles", roles}}});
-}
-
 drogon::HttpResponsePtr require_confirmed(const drogon::HttpRequestPtr& req) {
     if (!auth_enforced())
         return {};
@@ -351,10 +332,6 @@ bool current_user_can(const drogon::HttpRequestPtr& req, std::uint32_t perm) {
     return (have & perm) == perm;
 }
 
-bool current_user_is_admin(const drogon::HttpRequestPtr& req) {
-    return current_user_can(req, kAdminPermissionBits);
-}
-
 drogon::HttpResponsePtr require_permission(const drogon::HttpRequestPtr& req, std::uint32_t perm) {
     if (!auth_enforced())
         return {};

src/security/Auth.hpp

@@ -134,8 +134,6 @@ std::optional<AuthPrincipal> principal_of(const drogon::HttpRequestPtr& req);
 
 bool has_role(const drogon::HttpRequestPtr& req, const std::string& role);
 
-bool has_any_role(const drogon::HttpRequestPtr& req, const std::vector<std::string>& roles);
-
 /**
  * @brief True when auth checks must be enforced — the module is initialized
  *        and not running in AuthMode::None. Shared guard for the require_*
@@ -149,8 +147,6 @@ bool auth_enforced();
  */
 drogon::HttpResponsePtr require_role(const drogon::HttpRequestPtr& req, const std::string& role);
 
-drogon::HttpResponsePtr require_any_role(const drogon::HttpRequestPtr& req, const std::vector<std::string>& roles);
-
 /**
  * @brief nullptr if the caller's email is confirmed (or auth is disabled);
  *        401 if anonymous; 403 if authenticated but unconfirmed. The "confirmed"
@@ -178,8 +174,6 @@ std::uint32_t current_permissions(const drogon::HttpRequestPtr& req);
 
 bool current_user_can(const drogon::HttpRequestPtr& req, std::uint32_t perm);
 
-bool current_user_is_admin(const drogon::HttpRequestPtr& req);
-
 /**
  * @brief Returns a 403 response if the request's principal lacks the
  *        permission, or nullptr if it has it (or auth is disabled).

tests/integration/test_database.cpp

@@ -161,6 +161,88 @@ TEST_F(DatabaseManagerTest, ExecuteWrite) {
     EXPECT_EQ(result[0][0].template as<std::string>(), "test_value");
 }
 
+// --- execute_transaction / execute_write_idempotent (fork-facing API) ---
+// Real-Postgres coverage for the two execute_* variants nothing in the
+// template calls in production code — forks do (documented in
+// docs/CONVENTIONS.md §3). The unit-level seam only proves they compile and
+// enter the hook (test_database_seam.cpp); these prove the actual semantics.
+
+TEST_F(DatabaseManagerTest, ExecuteTransactionRunsAtRequestedIsolationLevel) {
+    auto isolation = [](auto& txn) {
+        auto r = txn.exec("SHOW transaction_isolation");
+        return r[0][0].template as<std::string>();
+    };
+
+    // Convenience overload = ReadCommitted (the Postgres default — no SET
+    // TRANSACTION is issued on this path).
+    EXPECT_EQ(Database::get().execute_transaction(isolation), "read committed");
+    EXPECT_EQ(Database::get().execute_transaction(Database::IsolationLevel::ReadCommitted, isolation),
+              "read committed");
+    EXPECT_EQ(Database::get().execute_transaction(Database::IsolationLevel::RepeatableRead, isolation),
+              "repeatable read");
+    EXPECT_EQ(Database::get().execute_transaction(Database::IsolationLevel::Serializable, isolation), "serializable");
+}
+
+TEST_F(DatabaseManagerTest, ExecuteTransactionIsAtomicAcrossStatements) {
+    Database::get().execute_write([](auto& txn) {
+        txn.exec("DROP TABLE IF EXISTS test_table");
+        txn.exec("CREATE TABLE test_table (id SERIAL PRIMARY KEY, name TEXT)");
+        return 0;
+    });
+
+    // Happy path: both statements of one transaction land together.
+    const int inserted = Database::get().execute_transaction([](auto& txn) {
+        txn.exec("INSERT INTO test_table (name) VALUES ('a')");
+        txn.exec("INSERT INTO test_table (name) VALUES ('b')");
+        return 2;
+    });
+    EXPECT_EQ(inserted, 2);
+
+    // Failure path: a throw after the first INSERT must roll BOTH back —
+    // a non-pqxx exception is not classified transient, so no retry either.
+    EXPECT_THROW(Database::get().execute_transaction([](auto& txn) {
+        txn.exec("INSERT INTO test_table (name) VALUES ('c')");
+        throw std::runtime_error("boom");
+        return 0;
+    }),
+                 std::runtime_error);
+
+    auto r = Database::get().execute_read([](auto& txn) { return txn.exec("SELECT COUNT(*) FROM test_table"); });
+    EXPECT_EQ(r[0][0].template as<long>(), 2) << "the failed transaction must leave no partial rows";
+}
+
+TEST_F(DatabaseManagerTest, ExecuteWriteIdempotentUpsertConvergesOnReplay) {
+    // execute_write_idempotent uses the liberal (read-style) retry classifier,
+    // which may replay the whole transaction after a connection-class error a
+    // commit could already have survived. That is safe ONLY for writes keyed
+    // by a natural key — replaying converges instead of double-applying.
+    // Exercise exactly that contract: an UPSERT keyed by name.
+    Database::get().execute_write([](auto& txn) {
+        txn.exec("DROP TABLE IF EXISTS test_table");
+        txn.exec("CREATE TABLE test_table (name TEXT PRIMARY KEY, val INT NOT NULL)");
+        return 0;
+    });
+
+    auto upsert = [](int val) {
+        return Database::get().execute_write_idempotent([val](auto& txn) {
+            txn.exec_params(
+                "INSERT INTO test_table (name, val) VALUES ($1, $2) "
+                "ON CONFLICT (name) DO UPDATE SET val = EXCLUDED.val",
+                "job-42",
+                val);
+            return val;
+        });
+    };
+
+    EXPECT_EQ(upsert(1), 1);
+    EXPECT_EQ(upsert(2), 2);  // replay of the same logical write — converges
+
+    auto r =
+        Database::get().execute_read([](auto& txn) { return txn.exec("SELECT COUNT(*), MAX(val) FROM test_table"); });
+    EXPECT_EQ(r[0][0].template as<long>(), 1) << "same key must never duplicate";
+    EXPECT_EQ(r[0][1].template as<int>(), 2) << "last write wins";
+}
+
 TEST_F(DatabaseLifecycleTest, UnreachableReplicaDoesNotBlockBoot) {
     // Replica is an optional read optimization: a dead replica URL must not
     // fail initialization (it used to crash-loop the whole app). Reads then

tests/unit/test_crud_owned.cpp

@@ -0,0 +1,132 @@
+/**
+ * @file test_crud_owned.cpp
+ * @brief Compile-time + behavioral pin for CrudBase's ownership-scoped reads
+ *        (find_owned / list_owned / count_owned, unlocked by kOwnerColumn).
+ *
+ *        These are member templates of a class template: they only
+ *        type-check when something instantiates them. In the template repo
+ *        itself only find_owned is instantiated by production code
+ *        (BillingController.cpp via BillingRepository, kOwnerColumn =
+ *        "user_id"); list_owned/count_owned are instantiated by the output
+ *        of `new-resource.sh <Entity> --owned` (the generated controller
+ *        calls all three) — i.e. by FORK code, not by anything this repo
+ *        compiles. Without this test, a silently broken signature would
+ *        pass CI here and detonate in every fork's first `--owned` resource.
+ *
+ *        Runs against the Database DI seam (Database::install_for_testing,
+ *        same fake shape as test_database_seam.cpp) — no Postgres. The fake
+ *        returns EMPTY pqxx::results, which is enough to prove the owner
+ *        predicate lands in the SQL template and the empty-result paths
+ *        behave (nullopt / empty vector). Pure unit.
+ */
+
+#include <map>
+#include <memory>
+#include <optional>
+#include <string>
+#include <vector>
+
+#include <gtest/gtest.h>
+
+#include "database/Database.hpp"
+#include "repositories/CrudBase.hpp"
+
+namespace {
+
+// ---- Database seam fake (mirrors test_database_seam.cpp) -------------------
+
+class RecordingTxn : public Database::detail::ErasedTxn {
+public:
+    std::vector<std::string> queries;
+
+    pqxx::result exec_params_erased(const std::string& query, const pqxx::params& /*params*/) override {
+        queries.push_back(query);
+        return pqxx::result{};
+    }
+
+    pqxx::result exec_erased(const std::string& query) override {
+        queries.push_back(query);
+        return pqxx::result{};
+    }
+};
+
+class FakeDatabase : public Database::DatabaseManager {
+public:
+    RecordingTxn txn;
+    std::map<std::string, int> entered;  // "<op>@<pool>" -> count
+
+    bool is_initialized() const override { return true; }
+    bool health_check() override { return true; }
+
+protected:
+    Database::detail::ErasedTxn* test_transaction_(const char* op, const char* pool) override {
+        ++entered[std::string(op) + "@" + pool];
+        return &txn;
+    }
+};
+
+// ---- Minimal owned repository (the shape new-resource.sh --owned emits) ----
+
+struct OwnedNote {
+    std::string id;
+    // Templated like every shipped domain struct: pqxx result iteration
+    // yields row_ref, not row& — a concrete `const pqxx::row&` signature
+    // fails to compile inside list_owned (this test proved it live).
+    template <class Row>
+    static OwnedNote from_row(const Row& /*row*/) {
+        return {};
+    }
+};
+
+class OwnedNoteRepository : public Repositories::CrudBase<OwnedNoteRepository, OwnedNote, std::string> {
+public:
+    static constexpr const char* kTable = "owned_notes";
+    static constexpr const char* kColumns = "id, owner_id";
+    static constexpr const char* kIdColumn = "id";
+    static constexpr const char* kOrderBy = "id";
+    static constexpr const char* kOwnerColumn = "owner_id";
+};
+
+class CrudOwnedTest : public ::testing::Test {
+protected:
+    void SetUp() override {
+        auto fake = std::make_unique<FakeDatabase>();
+        fake_ = fake.get();
+        Database::install_for_testing(std::move(fake));
+    }
+    void TearDown() override { Database::reset_for_testing(); }
+
+    FakeDatabase* fake_ = nullptr;
+    OwnedNoteRepository repo_;
+};
+
+TEST_F(CrudOwnedTest, FindOwnedScopesByIdAndOwner) {
+    auto found = repo_.find_owned("note-1", "user-1");
+    EXPECT_FALSE(found.has_value());  // empty fake result → nullopt, no from_row
+    ASSERT_EQ(fake_->txn.queries.size(), 1u);
+    EXPECT_EQ(fake_->txn.queries[0], "SELECT id, owner_id FROM owned_notes WHERE id = $1 AND owner_id = $2");
+    EXPECT_EQ(fake_->entered["db.read@replica"], 1);
+
+    // from_primary=true routes through the read-after-write path.
+    repo_.find_owned("note-1", "user-1", /*from_primary=*/true);
+    EXPECT_EQ(fake_->entered["db.read@primary"], 1);
+}
+
+TEST_F(CrudOwnedTest, ListOwnedScopesByOwner) {
+    auto rows = repo_.list_owned("user-1", 25, 5);
+    EXPECT_TRUE(rows.empty());
+    ASSERT_EQ(fake_->txn.queries.size(), 1u);
+    EXPECT_EQ(fake_->txn.queries[0],
+              "SELECT id, owner_id FROM owned_notes WHERE owner_id = $1 ORDER BY id LIMIT $2 OFFSET $3");
+}
+
+TEST_F(CrudOwnedTest, CountOwnedScopesByOwner) {
+    // The empty fake result can't carry the COUNT(*) row, so reading it throws
+    // (seam limitation — see test_database_seam.cpp header). The template still
+    // fully instantiates and the SQL it issued is observable.
+    EXPECT_THROW((void)repo_.count_owned("user-1"), std::exception);
+    ASSERT_EQ(fake_->txn.queries.size(), 1u);
+    EXPECT_EQ(fake_->txn.queries[0], "SELECT COUNT(*) FROM owned_notes WHERE owner_id = $1");
+}
+
+}  // namespace