Merge pull request #1351 from ghalse/patch/slave-secondary

Remove master-slave metaphor

config-templates/config.php

@@ -430,19 +430,19 @@ $config = [
     'database.persistent' => false,
 
     /*
-     * Database slave configuration is optional as well. If you are only
+     * Database secondary configuration is optional as well. If you are only
      * running a single database server, leave this blank. If you have
-     * a master/slave configuration, you can define as many slave servers
-     * as you want here. Slaves will be picked at random to be queried from.
+     * a primary/secondary configuration, you can define as many secondary servers
+     * as you want here. Secondaries will be picked at random to be queried from.
      *
-     * Configuration options in the slave array are exactly the same as the
-     * options for the master (shown above) with the exception of the table
+     * Configuration options in the secondary array are exactly the same as the
+     * options for the primary (shown above) with the exception of the table
      * prefix and driver options.
      */
-    'database.slaves' => [
+    'database.secondaries' => [
         /*
         [
-            'dsn' => 'mysql:host=myslave;dbname=saml',
+            'dsn' => 'mysql:host=mysecondary;dbname=saml',
             'username' => 'simplesamlphp',
             'password' => 'secret',
             'persistent' => false,

docs/simplesamlphp-database.md

@@ -27,7 +27,7 @@ If there is a requirement to connect to an alternate database server (ex. authen
 	$config = new \SimpleSAML\Configuration($myconfigarray, "mymodule/lib/Auth/Source/myauth.php");
 	$db = \SimpleSAML\Database::getInstance($config);
 
-That will create a new instance of the database, separate from the global instance, specific to the configuration defined in $myconfigarray. If you are going to specify an alternate config, your configuration array must contain the same keys that exist in the master config (database.dsn, database.username, database.password, database.prefix, etc).
+That will create a new instance of the database, separate from the global instance, specific to the configuration defined in $myconfigarray. If you are going to specify an alternate config, your configuration array must contain the same keys that exist in the primary config (database.dsn, database.username, database.password, database.prefix, etc).
 
 Database Prefix
 ---------------
@@ -42,7 +42,7 @@ Querying The Database
 You can query the database through two public functions read() and write() which are fairly self-explanitory when it comes to determining which one to use when querying.
 
 ### Writing to The Database
-Since the database class allows administrators to configure master and slave database servers, the write function will always use the master database connection.
+Since the database class allows administrators to configure primary and secondary database servers, the write function will always use the primary database connection.
 
 The write function takes 2 parameters: SQL, params.
 
@@ -54,7 +54,7 @@ The write function takes 2 parameters: SQL, params.
 	
 	$query = $db->write("INSERT INTO $table (id, data) VALUES (:id, :data)", $values);
 
-The values specified in the $values array will be bound to the placeholders and will be executed on the master. By default, values are binded as PDO::PARAM_STR. If you need to override this, you can specify it in the values array.
+The values specified in the $values array will be bound to the placeholders and will be executed on the primary. By default, values are binded as PDO::PARAM_STR. If you need to override this, you can specify it in the values array.
 
 	$table = $db->applyPrefix("test");
 	$values = [
@@ -70,7 +70,7 @@ You can also skip usage of prepared statements. You should **only** use this if
 	$query = $db->write("CREATE TABLE IF NOT EXISTS $table (id INT(16) NOT NULL, data TEXT NOT NULL)", false);
 
 ### Reading The Database
-Since the database class allows administrators to configure master and slave database servers, the read function will randomly select a slave server to query. If no slaves are configured, it will read from the master.
+Since the database class allows administrators to configure primary and secondary database servers, the read function will randomly select a secondary server to query. If no secondaries are configured, it will read from the primary.
 
 The read function takes 2 parameters: SQL, params.
 
@@ -81,7 +81,7 @@ The read function takes 2 parameters: SQL, params.
 	
 	$query = $db->read("SELECT * FROM $table WHERE id = :id", $values);
 
-The values specified in the $values array will be bound to the placeholders and will be executed on the selected slave. By default, values are binded as PDO::PARAM_STR. If you need to override this, you can specify it in the values array.
+The values specified in the $values array will be bound to the placeholders and will be executed on the selected secondary. By default, values are binded as PDO::PARAM_STR. If you need to override this, you can specify it in the values array.
 
 	$table = $db->applyPrefix("test");
 	$values = [

lib/SimpleSAML/Database.php

@@ -7,12 +7,13 @@ namespace SimpleSAML;
 use PDO;
 use PDOException;
 use PDOStatement;
+use SimpleSAML\Logger;
 
 /**
  * This file implements functions to read and write to a group of database servers.
  *
- * This database class supports a single database, or a master/slave configuration with as many defined slaves as a
- * user would like.
+ * This database class supports a single database, or a primary/secondary configuration with as many defined secondaries
+ * as a user would like.
  *
  * The goal of this class is to provide a single mechanism to connect to a database that can be reused by any component
  * within SimpleSAMLphp including modules. When using this class, the global configuration should be passed here, but in
@@ -31,14 +32,14 @@ class Database
     private static $instance = [];
 
     /**
-     * PDO Object for the Master database server
+     * PDO Object for the Primary database server
      */
-    private $dbMaster;
+    private $dbPrimary;
 
     /**
-     * Array of PDO Objects for configured database slaves
+     * Array of PDO Objects for configured database secondaries
      */
-    private $dbSlaves = [];
+    private $dbSecondaries = [];
 
     /**
      * Prefix to apply to the tables
@@ -86,28 +87,34 @@ class Database
             $driverOptions = [PDO::ATTR_PERSISTENT => true];
         }
 
-        // connect to the master
-        $this->dbMaster = $this->connect(
+        // connect to the primary
+        $this->dbPrimary = $this->connect(
             $config->getString('database.dsn'),
             $config->getString('database.username', null),
             $config->getString('database.password', null),
             $driverOptions
         );
 
-        // connect to any configured slaves
-        $slaves = $config->getArray('database.slaves', []);
-        foreach ($slaves as $slave) {
+        // TODO: deprecated racism: the "database.slave" terminology is preserved here for backwards compatibility.
+        if ($config->getArray('database.slaves', null) !== null) {
+            Logger::warning(
+                'The "database.slaves" config option is deprecated. ' .
+                'Please update your configuration to use "database.secondaries".'
+            );
+        }
+        // connect to any configured secondaries, preserving legacy config option
+        $secondaries = $config->getArray('database.secondaries', $config->getArray('database.slaves', []));
+        foreach ($secondaries as $secondary) {
             array_push(
-                $this->dbSlaves,
+                $this->dbSecondaries,
                 $this->connect(
-                    $slave['dsn'],
-                    $slave['username'],
-                    $slave['password'],
+                    $secondary['dsn'],
+                    $secondary['username'],
+                    $secondary['password'],
                     $driverOptions
                 )
             );
         }
-
         $this->tablePrefix = $config->getString('database.prefix', '');
     }
 
@@ -122,14 +129,15 @@ class Database
     private static function generateInstanceId(Configuration $config): string
     {
         $assembledConfig = [
-            'master' => [
+            'primary' => [
                 'database.dsn'        => $config->getString('database.dsn'),
                 'database.username'   => $config->getString('database.username', null),
                 'database.password'   => $config->getString('database.password', null),
                 'database.prefix'     => $config->getString('database.prefix', ''),
                 'database.persistent' => $config->getBoolean('database.persistent', false),
             ],
-            'slaves' => $config->getArray('database.slaves', []),
+            // TODO: deprecated racism: the "database.slave" terminology is preserved here for backwards compatibility.
+            'secondaries' => $config->getArray('database.secondaries', $config->getArray('database.slaves', [])),
         ];
 
         return sha1(serialize($assembledConfig));
@@ -161,18 +169,18 @@ class Database
 
 
     /**
-     * This function randomly selects a slave database server to query. In the event no slaves are configured, it will
-     * return the master.
+     * This function randomly selects a secondary database server to query. In the event no secondaries are configured,
+     * it will return the primary.
      *
      * @return \PDO object
      */
-    private function getSlave(): PDO
+    private function getSecondary(): PDO
     {
-        if (count($this->dbSlaves) > 0) {
-            $slaveId = rand(0, count($this->dbSlaves) - 1);
-            return $this->dbSlaves[$slaveId];
+        if (count($this->dbSecondaries) > 0) {
+            $secondaryId = rand(0, count($this->dbSecondaries) - 1);
+            return $this->dbSecondaries[$secondaryId];
         } else {
-            return $this->dbMaster;
+            return $this->dbPrimary;
         }
     }
 
@@ -244,7 +252,7 @@ class Database
 
 
     /**
-     * This executes queries directly on the master.
+     * This executes queries directly on the primary.
      *
      * @param string $stmt Prepared SQL statement
      * @param array  $params Parameters
@@ -253,12 +261,12 @@ class Database
      */
     public function write(string $stmt, array $params = [])
     {
-        return $this->query($this->dbMaster, $stmt, $params)->rowCount();
+        return $this->query($this->dbPrimary, $stmt, $params)->rowCount();
     }
 
 
     /**
-     * This executes queries on a database server that is determined by this::getSlave().
+     * This executes queries on a database server that is determined by this::getSecondary().
      *
      * @param string $stmt Prepared SQL statement
      * @param array  $params Parameters
@@ -267,7 +275,7 @@ class Database
      */
     public function read(string $stmt, array $params = []): PDOStatement
     {
-        $db = $this->getSlave();
+        $db = $this->getSecondary();
 
         return $this->query($db, $stmt, $params);
     }

tests/lib/SimpleSAML/DatabaseTest.php

@@ -68,7 +68,7 @@ class DatabaseTest extends TestCase
             'database.password'   => null,
             'database.prefix'     => 'phpunit_',
             'database.persistent' => true,
-            'database.slaves'     => [],
+            'database.secondaries'     => [],
         ];
 
         $this->config = new Configuration($config, "test/SimpleSAML/DatabaseTest.php");
@@ -97,7 +97,7 @@ class DatabaseTest extends TestCase
             'database.password'   => 'notausersinvalidpassword',
             'database.prefix'     => 'phpunit_',
             'database.persistent' => true,
-            'database.slaves'     => [],
+            'database.secondaries'     => [],
         ];
 
         $this->config = new Configuration($config, "test/SimpleSAML/DatabaseTest.php");
@@ -121,7 +121,7 @@ class DatabaseTest extends TestCase
             'database.password'   => null,
             'database.prefix'     => 'phpunit_',
             'database.persistent' => true,
-            'database.slaves'     => [],
+            'database.secondaries'     => [],
         ];
         $config2 = [
             'database.dsn'        => 'sqlite::memory:',
@@ -129,7 +129,7 @@ class DatabaseTest extends TestCase
             'database.password'   => null,
             'database.prefix'     => 'phpunit2_',
             'database.persistent' => true,
-            'database.slaves'     => [],
+            'database.secondaries'     => [],
         ];
 
         $config1 = new Configuration($config, "test/SimpleSAML/DatabaseTest.php");
@@ -179,22 +179,22 @@ class DatabaseTest extends TestCase
      * @covers SimpleSAML\Database::generateInstanceId
      * @covers SimpleSAML\Database::__construct
      * @covers SimpleSAML\Database::connect
-     * @covers SimpleSAML\Database::getSlave
+     * @covers SimpleSAML\Database::getSecondary
      * @test
      * @return void
      */
-    public function slaves(): void
+    public function secondaries(): void
     {
         $ref = new ReflectionClass($this->db);
-        $dbMaster = $ref->getProperty('dbMaster');
-        $dbMaster->setAccessible(true);
-        $master = spl_object_hash($dbMaster->getValue($this->db));
+        $dbPrimary = $ref->getProperty('dbPrimary');
+        $dbPrimary->setAccessible(true);
+        $primary = spl_object_hash($dbPrimary->getValue($this->db));
 
-        $getSlave = $ref->getMethod('getSlave');
-        $getSlave->setAccessible(true);
-        $slave = spl_object_hash($getSlave->invokeArgs($this->db, []));
+        $getSecondary = $ref->getMethod('getSecondary');
+        $getSecondary->setAccessible(true);
+        $secondary = spl_object_hash($getSecondary->invokeArgs($this->db, []));
 
-        $this->assertTrue(($master == $slave), "getSlave should have returned the master database object");
+        $this->assertTrue(($primary == $secondary), "getSecondary should have returned the primary database object");
 
         $config = [
             'database.dsn'        => 'sqlite::memory:',
@@ -202,7 +202,7 @@ class DatabaseTest extends TestCase
             'database.password'   => null,
             'database.prefix'     => 'phpunit_',
             'database.persistent' => true,
-            'database.slaves'     => [
+            'database.secondaries'     => [
                 [
                     'dsn'      => 'sqlite::memory:',
                     'username' => null,
@@ -215,18 +215,18 @@ class DatabaseTest extends TestCase
         $msdb = Database::getInstance($sspConfiguration);
 
         $ref = new ReflectionClass($msdb);
-        $dbSlaves = $ref->getProperty('dbSlaves');
-        $dbSlaves->setAccessible(true);
-        $slaves = $dbSlaves->getValue($msdb);
+        $dbSecondaries = $ref->getProperty('dbSecondaries');
+        $dbSecondaries->setAccessible(true);
+        $secondaries = $dbSecondaries->getValue($msdb);
 
-        $getSlave = $ref->getMethod('getSlave');
-        $getSlave->setAccessible(true);
-        $gotSlave = spl_object_hash($getSlave->invokeArgs($msdb, []));
+        $getSecondary = $ref->getMethod('getSecondary');
+        $getSecondary->setAccessible(true);
+        $gotSecondary = spl_object_hash($getSecondary->invokeArgs($msdb, []));
 
         $this->assertEquals(
-            spl_object_hash($slaves[0]),
-            $gotSlave,
-            "getSlave should have returned a slave database object"
+            spl_object_hash($secondaries[0]),
+            $gotSecondary,
+            "getSecondary should have returned a secondary database object"
         );
     }
 

tests/modules/saml/lib/IdP/SQLNameIDTest.php

@@ -83,12 +83,12 @@ class SQLNameIDTest extends TestCase
     public function testDatabase(): void
     {
         $config = [
-            'database.dsn'        => 'sqlite::memory:',
-            'database.username'   => null,
-            'database.password'   => null,
-            'database.prefix'     => 'phpunit_',
-            'database.persistent' => true,
-            'database.slaves'     => [
+            'database.dsn'         => 'sqlite::memory:',
+            'database.username'    => null,
+            'database.password'    => null,
+            'database.prefix'      => 'phpunit_',
+            'database.persistent'  => true,
+            'database.secondaries' => [
                 [
                     'dsn'      => 'sqlite::memory:',
                     'username' => null,