From 7e15f1d8e9e3e94bc86a54e9d1fe33904303db06 Mon Sep 17 00:00:00 2001 From: Andrew Davis Date: Fri, 20 Mar 2026 20:30:33 +1100 Subject: [PATCH 1/3] Database: add an id column to the users table And key all reactor prefs on it instead of username Claude code assisted with this change. --- lib/Synergy/Environment.pm | 17 +++--- lib/Synergy/Reactor/Status.pm | 29 ++++++++-- lib/Synergy/Reactor/TimeClock.pm | 18 +++++-- lib/Synergy/Reactor/Vestaboard.pm | 20 +++++-- lib/Synergy/Role/HasPreferences.pm | 33 +++++++++++- lib/Synergy/User.pm | 6 +++ lib/Synergy/UserDirectory.pm | 86 ++++++++++++++++++++++++------ 7 files changed, 173 insertions(+), 36 deletions(-) diff --git a/lib/Synergy/Environment.pm b/lib/Synergy/Environment.pm index a584faf2..97759c28 100644 --- a/lib/Synergy/Environment.pm +++ b/lib/Synergy/Environment.pm @@ -126,7 +126,9 @@ has state_dbh => ( ); sub _maybe_create_state_tables ($self) { - $self->state_dbh->do(q{ + my $dbh = $self->state_dbh; + + $dbh->do(q{ CREATE TABLE IF NOT EXISTS synergy_state ( reactor_name TEXT PRIMARY KEY, stored_at INTEGER NOT NULL, @@ -134,23 +136,24 @@ sub _maybe_create_state_tables ($self) { ); }); - $self->state_dbh->do(q{ + $dbh->do(q{ CREATE TABLE IF NOT EXISTS users ( - username TEXT PRIMARY KEY, + id INTEGER PRIMARY KEY, + username TEXT UNIQUE NOT NULL, is_master INTEGER DEFAULT 0, is_virtual INTEGER DEFAULT 0, is_deleted INTEGER DEFAULT 0 ); }); - $self->state_dbh->do(q{ + $dbh->do(q{ CREATE TABLE IF NOT EXISTS user_identities ( id INTEGER PRIMARY KEY, - username TEXT NOT NULL, + user_id INTEGER NOT NULL, identity_name TEXT NOT NULL, identity_value TEXT NOT NULL, - FOREIGN KEY (username) REFERENCES users(username) ON DELETE CASCADE, - CONSTRAINT constraint_username_identity UNIQUE (username, identity_name), + FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE, + CONSTRAINT constraint_user_identity UNIQUE (user_id, identity_name), UNIQUE (identity_name, identity_value) ); }); diff --git a/lib/Synergy/Reactor/Status.pm b/lib/Synergy/Reactor/Status.pm index f8e74382..3ef0b30f 100644 --- a/lib/Synergy/Reactor/Status.pm +++ b/lib/Synergy/Reactor/Status.pm @@ -50,23 +50,42 @@ listener chatter => async sub ($self, $event) { return; }; +sub _remap_to_user_ids ($self, $hashref) { + my $ud = $self->hub->user_directory; + my %id_keyed; + for my $username (keys %$hashref) { + my $user = $ud->user_named($username) or next; + $id_keyed{ $user->id } = $hashref->{$username}; + } + return \%id_keyed; +} + +sub _remap_from_user_ids ($self, $hashref) { + my $ud = $self->hub->user_directory; + my %username_keyed; + for my $user_id (keys %$hashref) { + my $user = $ud->user_by_id($user_id) or next; + $username_keyed{ $user->username } = $hashref->{$user_id}; + } + return \%username_keyed; +} + sub state ($self) { return { - chatter => $self->_last_chatter, - doings => $self->_user_doings, + chatter => $self->_remap_to_user_ids($self->_last_chatter), + doings => $self->_remap_to_user_ids($self->_user_doings), }; } after register_with_hub => sub ($self, @) { if (my $state = $self->fetch_state) { if ($state->{chatter}) { - $self->_last_chatter->%* = $state->{chatter}->%*; + $self->_last_chatter->%* = $self->_remap_from_user_ids($state->{chatter})->%*; } if ($state->{doings}) { my $doings = $self->_user_doings; - - %$doings = $state->{doings}->%*; + %$doings = $self->_remap_from_user_ids($state->{doings})->%*; } } }; diff --git a/lib/Synergy/Reactor/TimeClock.pm b/lib/Synergy/Reactor/TimeClock.pm index 9e73cf5f..3df7781c 100644 --- a/lib/Synergy/Reactor/TimeClock.pm +++ b/lib/Synergy/Reactor/TimeClock.pm @@ -98,9 +98,15 @@ has clock_out_channel => ( ); sub state ($self) { + my $ud = $self->hub->user_directory; + my %times_by_id; + for my $username (keys %{ $self->user_last_report_times }) { + my $user = $ud->user_named($username) or next; + $times_by_id{ $user->id } = $self->user_last_report_times->{$username}; + } return { - last_report_time => $self->last_report_time, - user_last_report_times => $self->user_last_report_times, + last_report_time => $self->last_report_time, + user_last_report_times => \%times_by_id, }; } @@ -127,7 +133,13 @@ after register_with_hub => sub ($self, @) { } if (my $times = $state->{user_last_report_times}) { - $self->_set_user_last_report_times($times); + my $ud = $self->hub->user_directory; + my %times_by_username; + for my $user_id (keys %$times) { + my $user = $ud->user_by_id($user_id) or next; + $times_by_username{ $user->username } = $times->{$user_id}; + } + $self->_set_user_last_report_times(\%times_by_username); } } }; diff --git a/lib/Synergy/Reactor/Vestaboard.pm b/lib/Synergy/Reactor/Vestaboard.pm index 7efdc46c..150688b5 100644 --- a/lib/Synergy/Reactor/Vestaboard.pm +++ b/lib/Synergy/Reactor/Vestaboard.pm @@ -418,9 +418,15 @@ sub _validate_design ($self, $design) { # secret: { expires_at: EPOCH-SEC, value: STRING } sub state ($self) { + my $ud = $self->hub->user_directory; + my %user_state_by_id; + for my $username (keys %{ $self->_user_state }) { + my $user = $ud->user_named($username) or next; + $user_state_by_id{ $user->id } = $self->_user_state->{$username}; + } return { - user => $self->_user_state, - lock => $self->_lock_state, + user => \%user_state_by_id, + lock => $self->_lock_state, current_characters => $self->_current_characters, }; } @@ -477,7 +483,15 @@ after register_with_hub => sub ($self, @) { } if (my $state = $self->fetch_state) { - $self->_set_user_state($state->{user}); + if (my $user_state = $state->{user}) { + my $ud = $self->hub->user_directory; + my %by_username; + for my $user_id (keys %$user_state) { + my $user = $ud->user_by_id($user_id) or next; + $by_username{ $user->username } = $user_state->{$user_id}; + } + $self->_set_user_state(\%by_username); + } $self->_set_lock_state($state->{lock}); $self->_set_current_characters($state->{current_characters}) if $self->_current_characters; diff --git a/lib/Synergy/Role/HasPreferences.pm b/lib/Synergy/Role/HasPreferences.pm index 1c4f082a..3f25a20c 100644 --- a/lib/Synergy/Role/HasPreferences.pm +++ b/lib/Synergy/Role/HasPreferences.pm @@ -169,9 +169,28 @@ role { return $value; }; + # Returns the user directory for preference key translation. For reactors + # this is hub->user_directory; for UserDirectory itself, it is $self. + method _pref_user_directory => sub ($self) { + return $self->hub->user_directory if $self->can('hub'); + return $self; + }; + around state => sub ($orig, $self, @rest) { my $state = $self->$orig(@rest); - $state->{preferences} = $self->user_preferences; + + my $ud = $self->_pref_user_directory; + my %id_keyed; + for my $username (keys %all_user_prefs) { + my $user = $ud->user_named($username); + unless ($user) { + $Logger->log(["HasPreferences: skipping prefs for unknown user %s during save", $username]); + next; + } + $id_keyed{ $user->id } = $all_user_prefs{$username}; + } + + $state->{preferences} = \%id_keyed; return $state; }; @@ -186,7 +205,17 @@ role { my $state = $self->$orig(@rest); if (my $prefs = $state->{preferences}) { - $self->_load_preferences($prefs); + my $ud = $self->_pref_user_directory; + my %username_keyed; + for my $user_id (keys %$prefs) { + my $user = $ud->user_by_id($user_id); + unless ($user) { + $Logger->log(["HasPreferences: skipping prefs for unknown user id %s during load", $user_id]); + next; + } + $username_keyed{ $user->username } = $prefs->{$user_id}; + } + $self->_load_preferences(\%username_keyed); } return $state; diff --git a/lib/Synergy/User.pm b/lib/Synergy/User.pm index 194087d5..5a7e9f99 100644 --- a/lib/Synergy/User.pm +++ b/lib/Synergy/User.pm @@ -31,6 +31,12 @@ has is_virtual => ( default => 0, ); +has id => ( + is => 'ro', + isa => 'Int', + writer => '_set_id', +); + has username => ( is => 'ro', isa => 'Str', diff --git a/lib/Synergy/UserDirectory.pm b/lib/Synergy/UserDirectory.pm index 279def9d..10fb8723 100644 --- a/lib/Synergy/UserDirectory.pm +++ b/lib/Synergy/UserDirectory.pm @@ -18,7 +18,7 @@ use Synergy::User; use Synergy::Util qw(known_alphabets read_config_file day_name_from_abbr); use Synergy::Logger '$Logger'; use Lingua::EN::Inflect qw(WORDLIST); -use List::Util qw(first shuffle all); +use List::Util qw(shuffle all); use DateTime; use Defined::KV; use Try::Tiny; @@ -76,8 +76,32 @@ has _active_users => ( }, ); -after _set_user => sub ($self, @) { $self->_clear_active_users }; -after _set_users => sub ($self, @) { $self->_clear_active_users }; +has _users_by_id => ( + isa => 'HashRef', + traits => [ 'Hash' ], + handles => { + user_by_id => 'get', + }, + lazy => 1, + clearer => '_clear_users_by_id', + default => sub ($self) { + my %by_id; + for my $user ($self->all_users) { + next unless defined $user->id; + $by_id{ $user->id } = $user; + } + return \%by_id; + }, +); + +after _set_user => sub ($self, @) { + $self->_clear_active_users; + $self->_clear_users_by_id; +}; +after _set_users => sub ($self, @) { + $self->_clear_active_users; + $self->_clear_users_by_id; +}; sub state ($self) { return {} } @@ -129,24 +153,27 @@ sub load_users_from_database ($self) { my $dbh = $self->env->state_dbh; my %users; - # load prefs - $self->fetch_state($self->name); - my $user_sth = $dbh->prepare('SELECT * FROM users'); $user_sth->execute; while (my $row = $user_sth->fetchrow_hashref) { my $username = $row->{username}; - $users{$username} = Synergy::User->new({ + my $user = Synergy::User->new({ directory => $self, + defined_kv(id => $row->{id}), username => $username, defined_kv(is_master => $row->{is_master}), defined_kv(is_virtual => $row->{is_virtual}), defined_kv(deleted => $row->{is_deleted}), }); + $users{$username} = $user; } - my $identity_sth = $dbh->prepare('SELECT * FROM user_identities'); + my $identity_sth = $dbh->prepare( + 'SELECT u.username, ui.identity_name, ui.identity_value + FROM user_identities ui + JOIN users u ON ui.user_id = u.id' + ); $identity_sth->execute; while (my $row = $identity_sth->fetchrow_hashref) { @@ -161,7 +188,13 @@ sub load_users_from_database ($self) { $user->add_identity($row->{identity_name}, $row->{identity_value}); } + # _set_users must come before fetch_state so HasPreferences can translate + # stored user ids back to usernames when loading preferences. $self->_set_users(\%users); + + # load prefs + $self->fetch_state($self->name); + return \%users; } @@ -201,8 +234,14 @@ sub register_user ($self, $user) { q{VALUES (?,?,?,?)} )); + state $user_insert_with_id_sth = $dbh->prepare(join(q{ }, + q{INSERT INTO users}, + q{ (id, username, is_master, is_virtual, is_deleted)}, + q{VALUES (?,?,?,?,?)} + )); + state $identity_insert_sth = $dbh->prepare(join(q{ }, - q{INSERT INTO user_identities (username, identity_name, identity_value)}, + q{INSERT INTO user_identities (user_id, identity_name, identity_value)}, q{VALUES (?,?,?)} )); @@ -212,15 +251,29 @@ sub register_user ($self, $user) { my $ok = 0; $dbh->begin_work; try { - $user_insert_sth->execute( - $user->username, - $user->is_master, - $user->is_virtual, - $user->is_deleted, - ); + my $user_id; + if ($user->id) { + $user_insert_with_id_sth->execute( + $user->id, + $user->username, + $user->is_master, + $user->is_virtual, + $user->is_deleted, + ); + $user_id = $user->id; + } else { + $user_insert_sth->execute( + $user->username, + $user->is_master, + $user->is_virtual, + $user->is_deleted, + ); + $user_id = $dbh->last_insert_id; + $user->_set_id($user_id); + } for my $pair ($user->identity_pairs) { - $identity_insert_sth->execute($user->username, $pair->[0], $pair->[1]); + $identity_insert_sth->execute($user_id, $pair->[0], $pair->[1]); } $self->_set_user($user->username, $user); @@ -243,6 +296,7 @@ sub reload_user ($self, $username, $data) { my $new_user = Synergy::User->new({ %$old, %$data, + id => $old->id, username => $username, }); From 11802d4e18c001eaa746d1f915389104b893d299 Mon Sep 17 00:00:00 2001 From: Andrew Davis Date: Fri, 20 Mar 2026 20:31:58 +1100 Subject: [PATCH 2/3] database migration script performs the migration of the database to have user ids, and updates all the reactor state to re-key their preferences on an id instead of username. Claude code assisted with this change. --- bin/migrate-user-ids | 251 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 251 insertions(+) create mode 100755 bin/migrate-user-ids diff --git a/bin/migrate-user-ids b/bin/migrate-user-ids new file mode 100755 index 00000000..1eed8d78 --- /dev/null +++ b/bin/migrate-user-ids @@ -0,0 +1,251 @@ +#!/usr/bin/env perl +use v5.36.0; + +# Migration: convert users table to integer primary key, and remap all +# username-keyed JSON state blobs to use integer user ids instead. +# +# Run this once against your production database before deploying the code +# changes that expect the new schema. +# +# Usage: bin/migrate-user-ids --db synergy.sqlite +# or: bin/migrate-user-ids --config synergy.toml + +use DBI; +use Getopt::Long::Descriptive; +use JSON::MaybeXS; +use Path::Tiny; + +my ($opt, $usage) = describe_options( + '%c %o', + [ 'db|d=s', 'path to synergy.sqlite' ], + [ 'config|c=s', 'path to synergy config file (to find the db path)' ], + [ 'dry-run|n', 'print what would be done without making changes' ], + [ 'help|h', 'print usage', { shortcircuit => 1 } ], +); + +print($usage->text), exit if $opt->help; + +my $dbfile; +if ($opt->db) { + $dbfile = $opt->db; +} elsif ($opt->config) { + my $text = path($opt->config)->slurp; + ($dbfile) = $text =~ /state_dbfile\s*=\s*['"]?([^'"\n]+)['"]?/; + die "Couldn't find state_dbfile in config\n" unless $dbfile; +} else { + $dbfile = 'synergy.sqlite'; + warn "No --db or --config given, trying $dbfile\n"; +} + +die "Database file not found: $dbfile\n" unless -f $dbfile; + +my $dbh = DBI->connect( + "dbi:SQLite:dbname=$dbfile", + undef, undef, + { RaiseError => 1, AutoCommit => 1 }, +) or die $DBI::errstr; + +$dbh->do('PRAGMA foreign_keys = OFF'); + +# --------------------------------------------------------------------------- +# 1. Check current schema +# --------------------------------------------------------------------------- + +my ($users_pk) = $dbh->selectrow_array( + q{SELECT type FROM pragma_table_info('users') WHERE name = 'id'} +); + +if ($users_pk) { + say "users table already has an 'id' column — migration may have already run."; + say "Check the schema before re-running."; + exit 1; +} + +# --------------------------------------------------------------------------- +# 2. Read current users and build username -> id mapping (we assign ids now) +# --------------------------------------------------------------------------- + +my @users = @{ $dbh->selectall_arrayref( + 'SELECT username, is_master, is_virtual, is_deleted FROM users ORDER BY rowid', + { Slice => {} }, +) }; + +my @identities = @{ $dbh->selectall_arrayref( + 'SELECT username, identity_name, identity_value FROM user_identities', + { Slice => {} }, +) }; + +say "Found " . scalar(@users) . " user(s) and " . scalar(@identities) . " identity row(s)."; + +if ($opt->dry_run) { + # These ids are just a preview based on current rowid order; the real run + # lets SQLite assign ids via INSERT, so the final numbering may differ. + say "[dry-run] Would assign ids (approximate — SQLite picks the actual ids):"; + my $id = 1; + for my $u (@users) { + say " $id: $u->{username}"; + $id++; + } +} + +# --------------------------------------------------------------------------- +# 3. Rebuild users and user_identities tables +# --------------------------------------------------------------------------- + +unless ($opt->dry_run) { + $dbh->begin_work; + + eval { + $dbh->do('DROP TABLE IF EXISTS user_identities'); + $dbh->do('DROP TABLE IF EXISTS users'); + + $dbh->do(q{ + CREATE TABLE users ( + id INTEGER PRIMARY KEY, + username TEXT UNIQUE NOT NULL, + is_master INTEGER DEFAULT 0, + is_virtual INTEGER DEFAULT 0, + is_deleted INTEGER DEFAULT 0 + ) + }); + + $dbh->do(q{ + CREATE TABLE user_identities ( + id INTEGER PRIMARY KEY, + user_id INTEGER NOT NULL, + identity_name TEXT NOT NULL, + identity_value TEXT NOT NULL, + FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE, + CONSTRAINT constraint_user_identity UNIQUE (user_id, identity_name), + UNIQUE (identity_name, identity_value) + ) + }); + + my $ins_user = $dbh->prepare( + 'INSERT INTO users (username, is_master, is_virtual, is_deleted) VALUES (?,?,?,?)' + ); + for my $u (@users) { + $ins_user->execute($u->{username}, $u->{is_master}, $u->{is_virtual}, $u->{is_deleted}); + } + + $dbh->commit; + }; + if ($@) { + $dbh->rollback; + die "Failed rebuilding user tables: $@\n"; + } + + say "Rebuilt users and user_identities tables."; +} + +# Build username -> id map from the new (or dry-run simulated) data +my %username_to_id; +{ + my $rows = $dbh->selectall_arrayref( + 'SELECT id, username FROM users', + { Slice => {} }, + ); + %username_to_id = map { $_->{username} => $_->{id} } @$rows; +} + +unless ($opt->dry_run) { + $dbh->begin_work; + eval { + my $ins_id = $dbh->prepare( + 'INSERT INTO user_identities (user_id, identity_name, identity_value) VALUES (?,?,?)' + ); + for my $row (@identities) { + my $uid = $username_to_id{ $row->{username} }; + unless ($uid) { + warn "No user id found for identity username '$row->{username}', skipping\n"; + next; + } + $ins_id->execute($uid, $row->{identity_name}, $row->{identity_value}); + } + $dbh->commit; + }; + if ($@) { + $dbh->rollback; + die "Failed reinserting identities: $@\n"; + } + say "Reinserted " . scalar(@identities) . " identity row(s)."; +} + +# --------------------------------------------------------------------------- +# 4. Migrate synergy_state JSON blobs +# +# These keys are known to be username-keyed hashes that need remapping: +# - preferences (all reactors with HasPreferences, plus _user_directory) +# - chatter (Status reactor) +# - doings (Status reactor) +# - user_last_report_times (TimeClock reactor) +# - user (Vestaboard reactor) +# --------------------------------------------------------------------------- + +my @state_rows = @{ $dbh->selectall_arrayref( + 'SELECT reactor_name, json FROM synergy_state', + { Slice => {} }, +) }; + +my $json = JSON::MaybeXS->new->utf8->canonical; + +my %USERNAME_KEYED_KEYS = map { $_ => 1 } qw( + preferences + chatter + doings + user_last_report_times + user +); + +for my $row (@state_rows) { + my $name = $row->{reactor_name}; + my $state = eval { $json->decode($row->{json}) }; + unless ($state && ref $state eq 'HASH') { + warn "Couldn't decode JSON for $name, skipping\n"; + next; + } + + my $changed = 0; + for my $key (keys %USERNAME_KEYED_KEYS) { + next unless exists $state->{$key} && ref $state->{$key} eq 'HASH'; + + my $orig = $state->{$key}; + my %remapped; + for my $username (keys %$orig) { + # Skip if already looks like an integer id + if ($username =~ /\A[0-9]+\z/) { + $remapped{$username} = $orig->{$username}; + next; + } + my $uid = $username_to_id{$username}; + unless ($uid) { + warn "[$name/$key] No id found for username '$username', skipping\n"; + next; + } + $remapped{$uid} = $orig->{$username}; + $changed = 1; + } + $state->{$key} = \%remapped; + } + + if ($changed) { + if ($opt->dry_run) { + say "[dry-run] Would remap $name state keys: " . + join(', ', grep { exists $state->{$_} } keys %USERNAME_KEYED_KEYS); + } else { + $dbh->do( + 'UPDATE synergy_state SET json = ? WHERE reactor_name = ?', + undef, + $json->encode($state), + $name, + ); + say "Updated state for $name."; + } + } else { + say "No changes needed for $name."; + } +} + +$dbh->do('PRAGMA foreign_keys = ON'); + +say $opt->dry_run ? "Dry run complete — no changes made." : "Migration complete."; From fe4978e82b55dd7a27912738818f96562423fa93 Mon Sep 17 00:00:00 2001 From: Ricardo Signes Date: Fri, 24 Apr 2026 14:12:14 +1000 Subject: [PATCH 3/3] migrate-user-ids: add a PODNAME comment --- bin/migrate-user-ids | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bin/migrate-user-ids b/bin/migrate-user-ids index 1eed8d78..39382a0c 100755 --- a/bin/migrate-user-ids +++ b/bin/migrate-user-ids @@ -1,6 +1,8 @@ #!/usr/bin/env perl use v5.36.0; +# PODNAME: migrate-user-ids + # Migration: convert users table to integer primary key, and remap all # username-keyed JSON state blobs to use integer user ids instead. #