Skip to content

Commit 97e01a5

Browse files
committed
Minor improvements; add test.
1 parent f0ad00e commit 97e01a5

File tree

2 files changed

+60
-14
lines changed

2 files changed

+60
-14
lines changed

module/VuFind/src/VuFind/Db/DbBuilder.php

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ protected function getRootDatabaseConnection(
203203
* @param string $newUser Username for connecting to new database (will be created)
204204
* @param string $newPass Password for new user
205205
* @param string $driver Database driver to use
206-
* @param string $dbHost Name of database host
206+
* @param string $dbHost Name of database host (may include port number, e.g. localhost:3306)
207207
* @param string $vufindHost Name of VuFind host (for use in creating users)
208208
* @param string $rootUser Root username for connecting to database
209209
* @param string $rootPass Root password for connecting to database
@@ -226,16 +226,14 @@ public function build(
226226
bool $returnSqlOnly = false,
227227
array $steps = []
228228
): string {
229-
$dbPort = null;
230-
if (str_contains($dbHost, ':')) {
231-
$dbHost = explode(':', $dbHost);
232-
$dbPort = $dbHost[1];
233-
$dbHost = $dbHost[0];
234-
}
229+
// Account for possibility of port number attached to host:
230+
[$dbHost, $dbPort] = str_contains($dbHost, ':')
231+
? explode(':', $dbHost)
232+
: [$dbHost, null];
235233
try {
236-
$db = $returnSqlOnly ?
237-
null :
238-
$this->getRootDatabaseConnection(
234+
$db = $returnSqlOnly
235+
? null
236+
: $this->getRootDatabaseConnection(
239237
$driver,
240238
$dbHost,
241239
$rootUser,
@@ -247,7 +245,7 @@ public function build(
247245
'Problem initializing database adapter; '
248246
. 'check for missing ' . $driver
249247
. ' library. Details: ' . $e->getMessage(),
250-
'error',
248+
$e->getCode(),
251249
$e
252250
);
253251
}

module/VuFind/tests/unit-tests/src/VuFindTest/Db/DbBuilderTest.php

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929

3030
namespace VuFindTest\Db;
3131

32+
use PHPUnit\Framework\MockObject\MockObject;
3233
use VuFind\Config\Version;
3334
use VuFind\Db\Connection;
3435
use VuFind\Db\ConnectionFactory;
@@ -48,6 +49,55 @@
4849
*/
4950
class DbBuilderTest extends \PHPUnit\Framework\TestCase
5051
{
52+
/**
53+
* Get a mock database connection with a working quote method.
54+
*
55+
* @return MockObject&Connection
56+
*/
57+
protected function getMockConnectionWithQuote(): MockObject&Connection
58+
{
59+
$mockConnection = $this->createMock(Connection::class);
60+
$mockConnection->expects($this->once())->method('quote')->willReturnCallback(fn ($str) => "'$str'");
61+
return $mockConnection;
62+
}
63+
64+
/**
65+
* Data provider for testPortHandling().
66+
*
67+
* @return array[]
68+
*/
69+
public static function portHandlingProvider(): array
70+
{
71+
return [
72+
'port' => ['localhost:1234', 'localhost', '1234'],
73+
'no port' => ['localhost', 'localhost', null],
74+
];
75+
}
76+
77+
/**
78+
* Test port number processing.
79+
*
80+
* @param string $host Host string
81+
* @param string $expectedHost Expected hostname parsed from string
82+
* @param ?string $expectedPort Expected port number (or null) parsed from string
83+
*
84+
* @return void
85+
*
86+
* @dataProvider portHandlingProvider
87+
*/
88+
public function testPortHandling(string $host, string $expectedHost, ?string $expectedPort): void
89+
{
90+
$mockConnectionFactory = $this->createMock(ConnectionFactory::class);
91+
$mockLoader = $this->createMock(MigrationLoader::class);
92+
$builder = $this->getMockBuilder(DbBuilder::class)->onlyMethods(['getRootDatabaseConnection'])
93+
->setConstructorArgs([$mockConnectionFactory, $mockLoader])
94+
->getMock();
95+
$builder->expects($this->exactly(2))->method('getRootDatabaseConnection')
96+
->with('mysql', $expectedHost, 'root', '', $expectedPort)
97+
->willReturn($this->getMockConnectionWithQuote());
98+
$builder->build('newName', 'newUser', 'newPass', 'mysql', $host);
99+
}
100+
51101
/**
52102
* Data provider for testPreCommands().
53103
*
@@ -95,10 +145,8 @@ public function testPreCommands(string $driver, array $expectedCommands, bool $s
95145
if ($sqlOnly) {
96146
$factory->expects($this->never())->method('getConnectionFromOptions');
97147
} else {
98-
$mockConnection = $this->createMock(Connection::class);
148+
$mockConnection = $this->getMockConnectionWithQuote();
99149
$mockConnection->expects($this->exactly(count($expectedCommands)))->method('executeQuery');
100-
$mockConnection->expects($this->exactly(1))->method('quote')
101-
->willReturnCallback(fn ($str) => "'$str'");
102150
$factory->expects($this->exactly(1))->method('getConnectionFromOptions')->willReturn($mockConnection);
103151
}
104152
$builder = new DbBuilder($factory, $this->createMock(MigrationLoader::class));

0 commit comments

Comments
 (0)