Users table add id column

lib/Synergy/Environment.pm

@@ -126,31 +126,34 @@ 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,
       json TEXT NOT NULL
     );
   });
 
-  $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)
     );
   });

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})->%*;
     }
   }
 };

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);
     }
   }
 };

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;

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;

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',

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,
   });