feat(sidecar)!: guild rosters on the guild board, and a way to migrate the store
Protocol 4 gives the guild board a real member list instead of the member *count* that was all Protocol 2 could express. `guild.roster` carries the set; `guild.leave` is forwarded but deliberately not projected. The roster lives in its own `members` column rather than as a field folded into `json`. That column holds the verbatim `guild.update` line, so a roster write into it would clobber the snapshot — name, abbreviation, leader, online count — that `guild.update` owns. Two writers across two columns of one row means both stay plain upserts: neither reads the other's value first, so there is no read-modify-write and no ordering requirement between the two kinds. `GET /guilds` folds the roster back in as `roster` at read time. `guild.leave` gets no board arm on purpose. The shard re-emits `guild.roster` whenever the member set changes, so the board self-corrects within one sweep, and keeping the delta out of the projection is what keeps the sidecar a forwarder rather than a thing that maintains state. This is also the repo's first store migration, and the reason it needed one: `SCHEMA` is `CREATE TABLE IF NOT EXISTS`, which can add a table but cannot add a column to a table that already exists. Every schema change up to and including Protocol 3.0 happened to add whole tables, so `ALTER TABLE` appears nowhere in this repo's history and the gap was invisible. `guilds.members` is the first column added to an existing table, so without a mechanism the column would simply never reach an installed sidecar and every roster write would fail. The counter is SQLite's own `PRAGMA user_version` — an integer in the database header, so it costs no table and cannot drift from the file it describes. Each step runs in a transaction together with the bump recording it, so a step lands completely or not at all. A database written by a *newer* sidecar warns and continues rather than failing: every step is additive, so a newer schema has only columns an older reader ignores, and refusing to start would turn rolling the binary back — a recovery path — into a dead end. A migration failure aborts startup, which was already the behaviour and is the right one: a half-migrated store answers the website with confusing partial data, and the shard dials *out*, so a sidecar that refuses to start never stalls the game. store.rs had no tests before this. The six added here cover the upgrade path that matters (an existing pre-Protocol-4 database gains the column and lands at the current version), that a restart re-running the migration is a no-op, that a roster does not clobber the snapshot, that the two writers work in either order, and that a guild with no roster yet has no `roster` key at all — "not known" and "known to be empty" must not be conflated, or a website renders an empty roster as fact. Also gates PRs into `edge`, not just `main`. This workstream lands ten phases there, and gating only the `main` hop would run these checks for the first time at the cutover. The precedent and the reasoning are already in RunicGateway/installer's copy of this workflow. Refs: docs/website/TEAMS.md Part 12 Phase 1 Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -161,6 +161,29 @@ where
|
||||
}
|
||||
}
|
||||
}
|
||||
// Guild roster (Protocol 4): the member list that `guild.update`'s counts cannot
|
||||
// express. It writes a *different column* of the same row, so it never races
|
||||
// guild.update. `guild.leave` deliberately has no arm here — the sidecar
|
||||
// forwards it (persisted and broadcast below, like any event) and the board's
|
||||
// roster self-corrects on the next `guild.roster`, which the shard re-emits
|
||||
// whenever the member set changes. Keeping the delta out of the board is what
|
||||
// keeps the sidecar a forwarder rather than a thing that maintains state.
|
||||
"guild.roster" => {
|
||||
if let Some(id) = ev.value.get("id").and_then(|v| v.as_i64()) {
|
||||
let members = ev
|
||||
.value
|
||||
.get("members")
|
||||
.cloned()
|
||||
.unwrap_or_else(|| serde_json::json!([]));
|
||||
|
||||
if let Err(e) = event_store
|
||||
.upsert_guild_roster(id, &members.to_string(), t)
|
||||
.await
|
||||
{
|
||||
tracing::warn!(error = %e, "failed to upsert guild roster");
|
||||
}
|
||||
}
|
||||
}
|
||||
"guild.remove" => {
|
||||
if let Some(id) = ev.value.get("id").and_then(|v| v.as_i64()) {
|
||||
if let Err(e) = event_store.delete_guild(id).await {
|
||||
|
||||
@@ -46,7 +46,13 @@ use tracing_subscriber::EnvFilter;
|
||||
/// `vendor.listing.remove`, with the `GET /ruleset`, `/points` and `/market` reads that serve them
|
||||
/// from the store. Same shape as the v2 bump — the kinds are additive, the endpoints are not — and
|
||||
/// there is deliberately no feature-negotiation array: v3 implies all three kinds.
|
||||
pub const PROTOCOL_VERSION: u32 = 3;
|
||||
///
|
||||
/// v4 (Protocol 4): adds `guild.roster` and `guild.leave`, giving the guild board a real member list
|
||||
/// instead of the member *count* that was all v2 could express. Additive in the same way again — the
|
||||
/// kinds are new, `GET /guilds` grows a `roster` key, and nothing existing changed shape. This is the
|
||||
/// first bump that also needed a **store migration** (`guilds.members`), because it is the first to
|
||||
/// add a column to a table that already exists rather than a whole new table; see `store::migrate`.
|
||||
pub const PROTOCOL_VERSION: u32 = 4;
|
||||
|
||||
// Not `#[tokio::main]`: on Windows the SCM dispatcher takes over this thread and starts the runtime
|
||||
// itself, on its own thread, once the service actually begins. The runtime is built by whichever
|
||||
|
||||
@@ -45,6 +45,7 @@ impl Store {
|
||||
.await?;
|
||||
|
||||
sqlx::query(SCHEMA).execute(&pool).await?;
|
||||
migrate(&pool).await?;
|
||||
info!(%path, "store ready");
|
||||
Ok(Self { pool })
|
||||
}
|
||||
@@ -236,12 +237,61 @@ impl Store {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Upserts one guild's member roster (Protocol 4), keyed by guild id, touching **only** the
|
||||
/// `members` column.
|
||||
///
|
||||
/// Deliberately not a write to `json`. That column holds the verbatim `guild.update` line, and a
|
||||
/// roster arriving as its own event must not clobber the snapshot — name, abbreviation, leader,
|
||||
/// online count — that `guild.update` owns. Splitting the two writers across two columns of one
|
||||
/// row is what lets both be plain upserts: neither needs to read the other's value first, so
|
||||
/// there is no read-modify-write and no ordering requirement between the two kinds.
|
||||
///
|
||||
/// The `INSERT` half is not redundant: a roster can arrive before the first `guild.update` for a
|
||||
/// guild, and the row it creates then carries `'{}'` until that update fills it in.
|
||||
pub async fn upsert_guild_roster(
|
||||
&self,
|
||||
id: i64,
|
||||
members_json: &str,
|
||||
t: i64,
|
||||
) -> anyhow::Result<()> {
|
||||
sqlx::query(
|
||||
"INSERT INTO guilds (id, name, json, updated_t, members) VALUES (?, NULL, '{}', ?, ?)
|
||||
ON CONFLICT(id) DO UPDATE SET members = excluded.members, updated_t = excluded.updated_t",
|
||||
)
|
||||
.bind(id)
|
||||
.bind(t)
|
||||
.bind(members_json)
|
||||
.execute(&self.pool)
|
||||
.await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// The full guild board: every guild's latest snapshot, ordered by name.
|
||||
///
|
||||
/// The roster is stored in its own column (see [`Self::upsert_guild_roster`]) and folded into
|
||||
/// the projected object as `roster` here, at read time. A guild that has had a `guild.update`
|
||||
/// but no `guild.roster` yet simply has no `roster` key, which is the honest representation of
|
||||
/// "not known" and distinct from a guild whose roster is genuinely empty.
|
||||
pub async fn guilds_all(&self) -> anyhow::Result<Vec<Value>> {
|
||||
let rows = sqlx::query("SELECT json FROM guilds ORDER BY name, id")
|
||||
let rows = sqlx::query("SELECT json, members FROM guilds ORDER BY name, id")
|
||||
.fetch_all(&self.pool)
|
||||
.await?;
|
||||
Ok(parse_json_column(rows))
|
||||
|
||||
Ok(rows
|
||||
.into_iter()
|
||||
.filter_map(|r| {
|
||||
let mut v: Value = serde_json::from_str(&r.get::<String, _>("json")).ok()?;
|
||||
let members: Option<String> = r.get("members");
|
||||
|
||||
if let (Some(obj), Some(raw)) = (v.as_object_mut(), members) {
|
||||
if let Ok(list) = serde_json::from_str::<Value>(&raw) {
|
||||
obj.insert("roster".into(), list);
|
||||
}
|
||||
}
|
||||
|
||||
Some(v)
|
||||
})
|
||||
.collect())
|
||||
}
|
||||
|
||||
// ---- governor board (Protocol 2.0) ----
|
||||
@@ -513,6 +563,75 @@ impl Store {
|
||||
}
|
||||
}
|
||||
|
||||
/// The schema version this build expects. Bump it, and add the matching arm to [`migrate`], for
|
||||
/// every change that `SCHEMA` alone cannot make to a database that already exists.
|
||||
const SCHEMA_VERSION: i64 = 1;
|
||||
|
||||
/// Brings an existing database forward to [`SCHEMA_VERSION`].
|
||||
///
|
||||
/// `SCHEMA` is `CREATE TABLE IF NOT EXISTS` only, which is enough to *add a table* but cannot add a
|
||||
/// column to a table that is already there. Every schema change up to and including Protocol 3.0
|
||||
/// happened to add whole tables, so this never mattered and `ALTER TABLE` appears nowhere in this
|
||||
/// repo's history. `guilds.members` (Protocol 4) is the first column added to an existing table, so
|
||||
/// the mechanism has to exist now.
|
||||
///
|
||||
/// The version counter is SQLite's own `PRAGMA user_version`: an integer in the database header, so
|
||||
/// it needs no table of its own and cannot be separated from the file it describes. Each step runs
|
||||
/// in a transaction **together with** the bump that records it, so a step either lands completely or
|
||||
/// not at all, and an interrupted run resumes at the right place rather than re-applying half of one.
|
||||
///
|
||||
/// A failure here propagates and aborts startup, deliberately. A half-migrated store answers the
|
||||
/// website with confusing partial data, which is worse than being plainly absent — and the shard
|
||||
/// dials *out* to the sidecar, so a sidecar that refuses to start never stalls the game.
|
||||
async fn migrate(pool: &SqlitePool) -> anyhow::Result<()> {
|
||||
let mut version: i64 = sqlx::query_scalar("PRAGMA user_version")
|
||||
.fetch_one(pool)
|
||||
.await?;
|
||||
|
||||
// A database written by a *newer* sidecar than this binary. This is not an error: every step
|
||||
// here is additive, so a newer schema has only columns and tables an older reader ignores, and
|
||||
// refusing to start would turn "roll the binary back" — a recovery path — into a dead end.
|
||||
if version > SCHEMA_VERSION {
|
||||
tracing::warn!(
|
||||
found = version,
|
||||
expected = SCHEMA_VERSION,
|
||||
"store was written by a newer sidecar; continuing, as migrations are additive"
|
||||
);
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
while version < SCHEMA_VERSION {
|
||||
let next = version + 1;
|
||||
let mut tx = pool.begin().await?;
|
||||
|
||||
match next {
|
||||
// Protocol 4: the guild board carries a member roster. Its own column rather than a
|
||||
// field folded into `json`, because `json` holds the verbatim `guild.update` line and
|
||||
// the two writers must not overwrite each other — see `upsert_guild_roster`.
|
||||
1 => {
|
||||
sqlx::query("ALTER TABLE guilds ADD COLUMN members TEXT")
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
}
|
||||
// Unreachable while SCHEMA_VERSION and this match are edited together, which is the
|
||||
// point of failing loudly rather than silently leaving the counter short.
|
||||
n => anyhow::bail!("no migration step defined for schema version {n}"),
|
||||
}
|
||||
|
||||
// `PRAGMA` takes no bind parameters, so this is formatted — safe because `next` is an i64
|
||||
// this loop produced, never anything from outside the process.
|
||||
sqlx::query(&format!("PRAGMA user_version = {next}"))
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
|
||||
tx.commit().await?;
|
||||
info!(version = next, "schema migration applied");
|
||||
version = next;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn parse_json_column(rows: Vec<sqlx::sqlite::SqliteRow>) -> Vec<Value> {
|
||||
rows.into_iter()
|
||||
.filter_map(|r| serde_json::from_str(&r.get::<String, _>("json")).ok())
|
||||
@@ -611,3 +730,195 @@ CREATE TABLE IF NOT EXISTS ruleset (
|
||||
updated_t INTEGER NOT NULL
|
||||
);
|
||||
"#;
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// A unique scratch database path. Matches `config`'s idiom — `std::env::temp_dir()` plus the
|
||||
/// test name — so the cases stay independent under the parallel test runner.
|
||||
fn scratch(name: &str) -> String {
|
||||
let dir = std::env::temp_dir().join(format!("uo-link-store-test-{name}"));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
std::fs::create_dir_all(&dir).expect("create scratch dir");
|
||||
dir.join("uo-link.db").to_string_lossy().into_owned()
|
||||
}
|
||||
|
||||
/// The `guilds` table exactly as a pre-Protocol-4 sidecar left it: no `members` column, and
|
||||
/// `user_version` still 0. This is the shape a real operator's database is in before an update,
|
||||
/// and the only starting point where the migration does anything.
|
||||
async fn legacy_db(path: &str) -> SqlitePool {
|
||||
let pool = SqlitePoolOptions::new()
|
||||
.max_connections(1)
|
||||
.connect_with(
|
||||
SqliteConnectOptions::new()
|
||||
.filename(path)
|
||||
.create_if_missing(true),
|
||||
)
|
||||
.await
|
||||
.expect("open legacy db");
|
||||
|
||||
sqlx::query(
|
||||
"CREATE TABLE guilds (
|
||||
id INTEGER PRIMARY KEY,
|
||||
name TEXT,
|
||||
json TEXT NOT NULL,
|
||||
updated_t INTEGER NOT NULL
|
||||
)",
|
||||
)
|
||||
.execute(&pool)
|
||||
.await
|
||||
.expect("create legacy guilds table");
|
||||
|
||||
pool.close().await;
|
||||
pool
|
||||
}
|
||||
|
||||
async fn user_version(store: &Store) -> i64 {
|
||||
sqlx::query_scalar("PRAGMA user_version")
|
||||
.fetch_one(&store.pool)
|
||||
.await
|
||||
.expect("read user_version")
|
||||
}
|
||||
|
||||
async fn guild_columns(store: &Store) -> Vec<String> {
|
||||
sqlx::query("PRAGMA table_info(guilds)")
|
||||
.fetch_all(&store.pool)
|
||||
.await
|
||||
.expect("table_info")
|
||||
.into_iter()
|
||||
.map(|r| r.get::<String, _>("name"))
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn an_existing_pre_protocol_4_database_gains_the_members_column() {
|
||||
// The case that matters: `SCHEMA`'s CREATE TABLE IF NOT EXISTS is a no-op against a table
|
||||
// that is already there, so without `migrate` this database would never get the column and
|
||||
// every roster write would fail against a live install.
|
||||
let path = scratch("legacy-upgrade");
|
||||
legacy_db(&path).await;
|
||||
|
||||
let store = Store::open(&path).await.expect("open migrates");
|
||||
|
||||
assert!(
|
||||
guild_columns(&store).await.contains(&"members".to_string()),
|
||||
"the migration must add guilds.members to a database that already had the table"
|
||||
);
|
||||
assert_eq!(user_version(&store).await, SCHEMA_VERSION);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn a_fresh_database_lands_at_the_current_version() {
|
||||
let path = scratch("fresh");
|
||||
let store = Store::open(&path).await.expect("open");
|
||||
|
||||
assert!(guild_columns(&store).await.contains(&"members".to_string()));
|
||||
assert_eq!(user_version(&store).await, SCHEMA_VERSION);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reopening_an_already_migrated_database_is_a_no_op() {
|
||||
// Every sidecar restart re-runs this path, so a second run must not attempt the ALTER again
|
||||
// — which would fail with "duplicate column name" and, since a migration failure aborts
|
||||
// startup, would leave the sidecar unable to start at all after its first upgrade.
|
||||
let path = scratch("idempotent");
|
||||
legacy_db(&path).await;
|
||||
|
||||
Store::open(&path).await.expect("first open");
|
||||
let store = Store::open(&path).await.expect("second open must succeed");
|
||||
|
||||
assert_eq!(user_version(&store).await, SCHEMA_VERSION);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn a_roster_does_not_clobber_the_guild_update_snapshot() {
|
||||
// The invariant the two-column split exists to give. `json` holds the verbatim guild.update
|
||||
// line; if a roster write touched it, name/abbr/online would vanish from the board.
|
||||
let path = scratch("no-clobber");
|
||||
let store = Store::open(&path).await.expect("open");
|
||||
|
||||
store
|
||||
.upsert_guild(
|
||||
7,
|
||||
Some("The Cartographers"),
|
||||
r#"{"kind":"guild.update","id":7,"name":"The Cartographers","abbr":"MAP","members":2,"online":1}"#,
|
||||
100,
|
||||
)
|
||||
.await
|
||||
.expect("upsert guild");
|
||||
|
||||
store
|
||||
.upsert_guild_roster(
|
||||
7,
|
||||
r#"[{"serial":"0x1","name":"Ada"},{"serial":"0x2","name":"Bo"}]"#,
|
||||
200,
|
||||
)
|
||||
.await
|
||||
.expect("upsert roster");
|
||||
|
||||
let guilds = store.guilds_all().await.expect("read board");
|
||||
assert_eq!(guilds.len(), 1);
|
||||
let g = &guilds[0];
|
||||
|
||||
assert_eq!(
|
||||
g["name"], "The Cartographers",
|
||||
"guild.update's name survived"
|
||||
);
|
||||
assert_eq!(g["abbr"], "MAP", "guild.update's abbr survived");
|
||||
assert_eq!(g["online"], 1, "guild.update's online count survived");
|
||||
assert_eq!(g["roster"].as_array().expect("roster is an array").len(), 2);
|
||||
assert_eq!(g["roster"][0]["name"], "Ada");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn the_two_writers_are_order_independent() {
|
||||
// A roster can arrive before the first guild.update for a guild — on a reconnect the shard
|
||||
// re-emits both and nothing orders them. Neither write may depend on the other's row.
|
||||
let path = scratch("either-order");
|
||||
let store = Store::open(&path).await.expect("open");
|
||||
|
||||
store
|
||||
.upsert_guild_roster(9, r#"[{"serial":"0x3","name":"Cy"}]"#, 100)
|
||||
.await
|
||||
.expect("roster first");
|
||||
store
|
||||
.upsert_guild(
|
||||
9,
|
||||
Some("Late Arrivals"),
|
||||
r#"{"kind":"guild.update","id":9,"name":"Late Arrivals","abbr":"LTE"}"#,
|
||||
200,
|
||||
)
|
||||
.await
|
||||
.expect("update second");
|
||||
|
||||
let guilds = store.guilds_all().await.expect("read board");
|
||||
assert_eq!(guilds.len(), 1, "one row, not two");
|
||||
assert_eq!(guilds[0]["name"], "Late Arrivals");
|
||||
assert_eq!(guilds[0]["roster"].as_array().expect("roster").len(), 1);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn a_guild_with_no_roster_yet_has_no_roster_key() {
|
||||
// "Not known" and "known to be empty" are different, and the board must not conflate them:
|
||||
// a website reading `roster: []` would render an empty roster as fact.
|
||||
let path = scratch("absent-roster");
|
||||
let store = Store::open(&path).await.expect("open");
|
||||
|
||||
store
|
||||
.upsert_guild(
|
||||
11,
|
||||
Some("Unswept"),
|
||||
r#"{"kind":"guild.update","id":11,"name":"Unswept"}"#,
|
||||
100,
|
||||
)
|
||||
.await
|
||||
.expect("upsert guild");
|
||||
|
||||
let guilds = store.guilds_all().await.expect("read board");
|
||||
assert!(
|
||||
guilds[0].get("roster").is_none(),
|
||||
"a guild with no roster event must not grow a roster key"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user