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..eb480262 100644 --- a/lib/Synergy/UserDirectory.pm +++ b/lib/Synergy/UserDirectory.pm @@ -114,6 +114,10 @@ sub user_by_channel_and_address ($self, $channel_name, $address) { return undef; } +sub user_by_id ($self, $id) { + return first { $_->id == $id } $self->all_users; +} + # NOTE: This is a hack. It's here because I want to make sure for reactors # with preferences, you never have to remember to call ->fetch_state on # startup to load the prefs (...because I never remember to do so). That means @@ -128,25 +132,30 @@ sub register_with_hub {} sub load_users_from_database ($self) { my $dbh = $self->env->state_dbh; my %users; - - # load prefs - $self->fetch_state($self->name); + my %users_by_id; 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; + $users_by_id{$row->{id}} = $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 +170,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 +216,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 +233,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 +278,7 @@ sub reload_user ($self, $username, $data) { my $new_user = Synergy::User->new({ %$old, %$data, + id => $old->id, username => $username, });