Skip to content

Commit

Permalink
Merge pull request #95 from creative-commoners/pulls/3/db-replica
Browse files Browse the repository at this point in the history
FIX Ensure primary connection is updated with temporary database
  • Loading branch information
GuySartorelli authored Oct 10, 2024
2 parents 3eb61e3 + 402a9f9 commit a247a44
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 17 deletions.
2 changes: 1 addition & 1 deletion _config.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
*/
if (class_exists('Resque_Event') && class_exists('SSResqueRun')) {
Resque_Event::listen('beforeFork', function ($data) {
$databaseConfig = DB::getConfig();
$databaseConfig = DB::getConfig(DB::CONN_PRIMARY);

// Reconnect to the database - this may connect to the old DB first, but is required because these processes
// are long-lived, and MySQL connections often get closed in between worker runs. We need to connect before
Expand Down
2 changes: 1 addition & 1 deletion src/TestSessionController.php
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ public function clear()
}

/**
* As with {@link self::start()}, if you want to extend the functionality of this, then look at
* As with {@link TestSessionController::start()}, if you want to extend the functionality of this, then look at
* {@link TestSessionEnvironent::endTestSession()} as the extension points have moved to there now that the logic
* is there.
*/
Expand Down
36 changes: 24 additions & 12 deletions src/TestSessionEnvironment.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class TestSessionEnvironment
/**
* @var string The original database name, before we overrode it with our tmpdb.
*
* Used in {@link self::resetDatabaseName()} when we want to restore the normal DB connection.
* Used in {@link TestSessionEnvironment::resetDatabaseName()} when we want to restore the normal DB connection.
*/
private $oldDatabaseName;

Expand Down Expand Up @@ -137,14 +137,16 @@ public function getId()

/**
* Creates a temp database, sets up any extra requirements, and writes the state file. The database will be
* connected to as part of {@link self::applyState()}, so if you're continuing script execution after calling this
* connected to as part of {@link TestSessionEnvironment::applyState()}, so if you're continuing script
* execution after calling this
* method, be aware that the database will be different - so various things may break (e.g. administrator logins
* using the SS_DEFAULT_USERNAME / SS_DEFAULT_PASSWORD constants).
*
* If something isn't explicitly handled here, and needs special handling, then it should be taken care of by an
* extension to TestSessionEnvironment. You can either extend onBeforeStartTestSession() or
* onAfterStartTestSession(). Alternatively, for more fine-grained control, you can also extend
* onBeforeApplyState() and onAfterApplyState(). See the {@link self::applyState()} method for more.
* onBeforeApplyState() and onAfterApplyState(). See the {@link TestSessionEnvironment::applyState()}
* method for more.
*
* @param array $state An array of test state options to write.
* @param mixed $id
Expand Down Expand Up @@ -270,7 +272,7 @@ public function applyState($state)
$this->extend('onBeforeApplyState', $state);

// back up source
$databaseConfig = DB::getConfig();
$databaseConfig = DB::getConfig(DB::CONN_PRIMARY);
$this->oldDatabaseName = $databaseConfig['database'];

// Load existing state from $this->state into $state, if there is any
Expand Down Expand Up @@ -445,7 +447,8 @@ private function removeStateFile()

/**
* Cleans up the test session state by restoring the normal database connect (for the rest of this request, if any)
* and removes the {@link self::$test_state_file} so that future requests don't use this test state.
* and removes the {@link TestSessionEnvironment::$test_state_file} so that future requests don't use this
* test state.
*
* Can be extended by implementing either onBeforeEndTestSession() or onAfterEndTestSession().
*
Expand Down Expand Up @@ -514,14 +517,15 @@ public function loadFixtureIntoDb($fixtureFile)
}

/**
* Reset the database connection to use the original database. Called by {@link self::endTestSession()}.
* Reset the database connection to use the original database.
* Called by {@link TestSessionEnvironment::endTestSession()}.
*/
public function resetDatabaseName()
{
if ($this->oldDatabaseName) {
$databaseConfig = DB::getConfig();
$databaseConfig = DB::getConfig(DB::CONN_PRIMARY);
$databaseConfig['database'] = $this->oldDatabaseName;
DB::setConfig($databaseConfig);
DB::setConfig($databaseConfig, DB::CONN_PRIMARY);

$conn = DB::get_conn();

Expand All @@ -532,7 +536,7 @@ public function resetDatabaseName()
}

/**
* @return stdClass Data as taken from the JSON object in {@link self::loadFromFile()}
* @return stdClass Data as taken from the JSON object in {@link TestSessionEnvironment::loadFromFile()}
*/
public function getState()
{
Expand Down Expand Up @@ -564,7 +568,7 @@ public function connectToDatabase($state = null)
$state = $this->getState();
}

$databaseConfig = DB::getConfig();
$databaseConfig = DB::getConfig(DB::CONN_PRIMARY);

if (isset($state->database) && $state->database) {
if (!DB::get_conn()) {
Expand All @@ -575,14 +579,14 @@ public function connectToDatabase($state = null)
}

// Connect to database
DB::connect($databaseConfig);
$this->connectToDB($databaseConfig);
} else {
// We've already connected to the database, do a fast check to see what database we're currently using
$db = DB::get_conn()->getSelectedDatabase();
if (isset($state->database) && $db != $state->database) {
$this->oldDatabaseName = $databaseConfig['database'];
$databaseConfig['database'] = $state->database;
DB::connect($databaseConfig);
$this->connectToDB($databaseConfig);
}
}
}
Expand Down Expand Up @@ -618,4 +622,12 @@ public function waitForPendingRequests($await = 700, $timeout = 10000)

return true;
}

private function connectToDB(array $databaseConfig): void
{
// Ensure we connect the primary connection and not a replica
// which can happen if we use the default value of DB::CONN_DYNAMIC
// and there is a replica database configured
DB::connect($databaseConfig, DB::CONN_PRIMARY);
}
}
6 changes: 3 additions & 3 deletions src/TestSessionState.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public static function incrementState()
{
$schema = DataObject::getSchema();

$update = SQLUpdate::create(sprintf('"%s"', $schema->tableName(self::class)))
$update = SQLUpdate::create(sprintf('"%s"', $schema->tableName(TestSessionState::class)))
->addWhere(['ID' => 1])
->assignSQL('"PendingRequests"', '"PendingRequests" + 1');

Expand All @@ -50,10 +50,10 @@ public static function decrementState()
{
$schema = DataObject::getSchema();

$update = SQLUpdate::create(sprintf('"%s"', $schema->tableName(self::class)))
$update = SQLUpdate::create(sprintf('"%s"', $schema->tableName(TestSessionState::class)))
->addWhere(['ID' => 1])
->assignSQL('"PendingRequests"', '"PendingRequests" - 1')
->assign('"LastResponseTimestamp"', self::millitime());
->assign('"LastResponseTimestamp"', TestSessionState::millitime());

$update->execute();
}
Expand Down

0 comments on commit a247a44

Please sign in to comment.