Skip to content

Commit 69e2fe6

Browse files
gggeekgggeek
committed
better handling of separate create/drop of users and dbs
1 parent 2bc1a8b commit 69e2fe6

12 files changed

+250
-132
lines changed

‎app/src/Command/BaseCommand.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ protected function writeErrorln($message, $verbosity = OutputInterface::VERBOSIT
3737

3838
// When verbosity is set to quiet, SF swallows the error message in the writeln call
3939
// (unlike for other verbosity levels, which are left for us to handle...)
40-
// We resort to a hackish workaround to _always_ print errors to stdout, even in quiet mode.
41-
// If the end user does not want any error echoed, he can just 2>/dev/null
40+
// We resort to a hackish workaround to _always_ print errors to stderr, even in quiet mode.
41+
// If the end user does not want any error echoed, she can just 2>/dev/null
4242
if ($this->errorOutput->getVerbosity() == OutputInterface::VERBOSITY_QUIET) {
4343
$this->errorOutput->setVerbosity(OutputInterface::VERBOSITY_NORMAL);
4444
$this->errorOutput->writeln($message, $type);

‎app/src/Command/CollationList.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,11 @@ protected function execute(InputInterface $input, OutputInterface $output)
5050
return (int)$results['failed'];
5151
}
5252

53+
/**
54+
* @param string[][] $instanceList
55+
* @return array
56+
* @throws \Exception
57+
*/
5358
protected function listCollations($instanceList)
5459
{
5560
return $this->executeSqlAction(

‎app/src/Command/DatabaseCreate.php‎

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ protected function configure()
1515
$this
1616
->setDescription('Creates a database & associated user in parallel on all configured database instances')
1717
->addOption('database', null, InputOption::VALUE_REQUIRED, 'The name of the database to create')
18-
->addOption('user', null, InputOption::VALUE_REQUIRED, 'The name of the user to create. If omitted, the database name will be used as user name')
19-
->addOption('password', null, InputOption::VALUE_REQUIRED, 'The password. If omitted, a random one will be generated and echoed to stderr')
18+
->addOption('user', null, InputOption::VALUE_REQUIRED, 'The name of a user to create with r/w access to the new database. If omitted, no user will be created')
19+
->addOption('password', null, InputOption::VALUE_REQUIRED, 'The password. If omitted, a random one will be generated and echoed as part of results')
2020
->addOption('charset', null, InputOption::VALUE_REQUIRED, 'The collation/character-set to use for the database. If omitted, the default collation for the instance will be used')
2121
->addCommonOptions()
2222
;
@@ -41,24 +41,32 @@ protected function execute(InputInterface $input, OutputInterface $output)
4141

4242
$instanceList = $this->parseCommonOptions($input);
4343

44+
$dbName = $input->getOption('database');
4445
$userName = $input->getOption('user');
4546
$password = $input->getOption('password');
46-
$dbName = $input->getOption('database');
47+
$charset = $input->getOption('charset');
48+
49+
// BC api
50+
if ($dbName == null && $userName != null) {
51+
$dbName = $userName;
52+
}
4753

4854
if ($dbName == null) {
4955
throw new \Exception("Please provide a database name");
5056
}
5157

5258
if ($userName == null) {
53-
$userName = $dbName;
54-
}
55-
56-
if ($password == null) {
57-
$password = bin2hex(random_bytes(16));
59+
if ($password != null) {
60+
throw new \Exception("Option 'password' is only valid together with option 'user'");
61+
}
62+
} else {
63+
if ($password == null) {
64+
$password = bin2hex(random_bytes(16));
5865

59-
// Should we warn the user always? To avoid breaking non-text-format, we can send it to stderr...
60-
// Otoh we give the password in the structured output that we produce
61-
//$this->writeErrorln("<info>Assigned password to the user: $password</info>");
66+
// Should we warn the user always? To avoid breaking non-text-format, we can send it to stderr...
67+
// Otoh we give the password in the structured output that we produce
68+
//$this->writeErrorln("<info>Assigned password to the user: $password</info>");
69+
}
6270
}
6371

6472
if ($this->outputFormat === 'text') {
@@ -68,14 +76,13 @@ protected function execute(InputInterface $input, OutputInterface $output)
6876
$newDbSpecs = [];
6977
foreach($instanceList as $instanceName => $instanceSpecs) {
7078
$newDbSpecs[$instanceName] = [
71-
'dbname' => $dbName,
72-
'user' => $userName,
73-
'password' => $password,
79+
'dbname' => $dbName
7480
];
75-
}
76-
77-
if (($charset = $input->getOption('charset')) != '') {
78-
foreach($instanceList as $instanceName) {
81+
if ($userName != null) {
82+
$newDbSpecs[$instanceName]['user'] = $userName;
83+
$newDbSpecs[$instanceName]['password'] = $password;
84+
}
85+
if ($charset != '') {
7986
$newDbSpecs[$instanceName]['charset'] = $charset;
8087
}
8188
}

‎app/src/Command/DatabaseDrop.php‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ protected function configure()
1414
{
1515
$this
1616
->setDescription('Drops a database & associated user in parallel on all configured database instances')
17-
->addOption('database', null, InputOption::VALUE_REQUIRED, 'The name of the database to drop.')
18-
->addOption('user', null, InputOption::VALUE_REQUIRED, 'The name of the user to drop. If omitted, the database name will be used as user name')
17+
->addOption('database', null, InputOption::VALUE_REQUIRED, 'The name of the database to drop')
18+
->addOption('user', null, InputOption::VALUE_REQUIRED, 'The name of a user to drop. If omitted, no user will be dropped')
1919
->addCommonOptions()
2020
;
2121
}
@@ -42,6 +42,11 @@ protected function execute(InputInterface $input, OutputInterface $output)
4242
$userName = $input->getOption('user');
4343
$dbName = $input->getOption('database');
4444

45+
// BC api
46+
if ($dbName == null && $userName != null) {
47+
$dbName = $userName;
48+
}
49+
4550
if ($dbName == null) {
4651
throw new \Exception("Please provide a database name");
4752
}
@@ -53,13 +58,16 @@ protected function execute(InputInterface $input, OutputInterface $output)
5358
$dbToDropSpecs = [];
5459
foreach($instanceList as $instanceName => $instanceSpecs) {
5560
$dbToDropSpecs[$instanceName] = [
56-
'user' => $userName,
5761
'dbname' => $dbName
5862
];
63+
if ($userName != null) {
64+
$dbToDropSpecs[$instanceName]['user'] = $userName;
65+
}
5966
}
67+
6068
$results = $this->dropDatabases($instanceList, $dbToDropSpecs);
6169

62-
// BC with versions < 0.8
70+
// q: shall we print something more useful?
6371
$results['data'] = null;
6472

6573
$time = microtime(true) - $start;

‎app/src/Command/DatabaseList.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,11 @@ protected function execute(InputInterface $input, OutputInterface $output)
5050
return (int)$results['failed'];
5151
}
5252

53+
/**
54+
* @param string[][] $instanceList
55+
* @return array
56+
* @throws \Exception
57+
*/
5358
protected function listDatabases($instanceList)
5459
{
5560
return $this->executeSqlAction(

‎app/src/Command/DatabaseManagingCommand.php‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ abstract class DatabaseManagingCommand extends SQLExecutingCommand
1010
* @param string[][] $instanceList
1111
* @param array[] $dbSpecList key: db name (as used to identify configured databases), value: array('user': mandatory, 'dbname': mandatory, 'password': mandatory)
1212
* @return array 'succeeded': int, 'failed': int, 'results': same format as dbConfigurationManager::getInstanceConfiguration
13+
* @throws \Exception
1314
*/
1415
protected function createDatabases($instanceList, $dbSpecList)
1516
{
@@ -24,8 +25,8 @@ function ($schemaManager, $instanceName) use ($dbSpecList) {
2425
/** @var DatabaseSchemaManager $schemaManager */
2526
return $schemaManager->getCreateDatabaseSqlAction(
2627
$dbConnectionSpec['dbname'],
27-
$dbConnectionSpec['user'],
28-
$dbConnectionSpec['password'],
28+
isset($dbConnectionSpec['user']) ? $dbConnectionSpec['user'] : null,
29+
isset($dbConnectionSpec['password']) ? $dbConnectionSpec['password'] : null,
2930
(isset($dbConnectionSpec['charset']) && $dbConnectionSpec['charset'] != '') ? $dbConnectionSpec['charset'] : null
3031
);
3132
}
@@ -34,14 +35,17 @@ function ($schemaManager, $instanceName) use ($dbSpecList) {
3435
$finalData = [];
3536
foreach($results['data'] as $instanceName => $data) {
3637
$dbConnectionSpec = $dbSpecList[$instanceName];
37-
$finalData[$instanceName] = array_merge(
38-
$instanceList[$instanceName],
39-
[
40-
'user' => $dbConnectionSpec['user'],
41-
'password' => $dbConnectionSpec['password'],
42-
'dbname' => (isset($dbConnectionSpec['dbname']) && $dbConnectionSpec['dbname'] != '') ? $dbConnectionSpec['dbname'] : $dbConnectionSpec['user']
43-
]
44-
);
38+
$finalData[$instanceName] = $instanceList[$instanceName];
39+
$finalData[$instanceName]['dbname'] = $dbConnectionSpec['dbname'];
40+
if (isset($dbConnectionSpec['user'])) {
41+
$finalData[$instanceName]['user'] = $dbConnectionSpec['user'];
42+
}
43+
if (isset($dbConnectionSpec['password'])) {
44+
$finalData[$instanceName]['password'] = $dbConnectionSpec['password'];
45+
}
46+
if (isset($dbConnectionSpec['charset'])) {
47+
$finalData[$instanceName]['charset'] = $dbConnectionSpec['charset'];
48+
}
4549
}
4650

4751
$results['data'] = $finalData;
@@ -51,19 +55,22 @@ function ($schemaManager, $instanceName) use ($dbSpecList) {
5155
/**
5256
* @param string[][] $instanceList
5357
* @param array[] $dbSpecList key: db name (as used to identify configured databases), value: array('user': mandatory, 'dbname': mandatory, if unspecified assumed same as user)
58+
* @param bool $ifExists
5459
* @return array 'succeeded': int, 'failed': int, 'results': string[]
60+
* @throws \Exception
5561
*/
56-
protected function dropDatabases($instanceList, $dbSpecList)
62+
protected function dropDatabases($instanceList, $dbSpecList, $ifExists = false)
5763
{
5864
return $this->executeSqlAction(
5965
$instanceList,
6066
'Dropping of database & user',
61-
function ($schemaManager, $instanceName) use ($dbSpecList) {
67+
function ($schemaManager, $instanceName) use ($dbSpecList, $ifExists) {
6268
$dbConnectionSpec = $dbSpecList[$instanceName];
6369
/** @var DatabaseSchemaManager $schemaManager */
6470
return $schemaManager->getDropDatabaseSqlAction(
6571
$dbConnectionSpec['dbname'],
66-
$dbConnectionSpec['user']
72+
isset($dbConnectionSpec['user']) ? $dbConnectionSpec['user'] : null,
73+
$ifExists
6774
);
6875
}
6976
);

‎app/src/Command/InstanceList.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ protected function configure()
1616
{
1717
$this
1818
->setDescription('Lists all configured database servers')
19-
->addOption('output-type', null, InputOption::VALUE_REQUIRED, 'The format for the output: json, php, text or yml', 'text')
2019
->addCommonOptions()
2120
;
2221
}

‎app/src/Command/SQLExecutingCommand.php‎

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,12 @@ public function __construct(
4545
protected function addCommonOptions()
4646
{
4747
$this
48-
->addOption('output-type', null, InputOption::VALUE_REQUIRED, 'The format for the output: json, php, text or yml', self::DEFAULT_OUTPUT_FORMAT)
49-
->addOption('timeout', null, InputOption::VALUE_REQUIRED, 'The maximum time to wait for execution (secs)', self::DEFAULT_PROCESS_TIMEOUT)
50-
->addOption('max-parallel', null, InputOption::VALUE_REQUIRED, 'The maximum number of processes to run in parallel', self::DEFAULT_PARALLEL_PROCESSES)
51-
->addOption('dont-force-enabled-sigchild', null, InputOption::VALUE_NONE, "When using a separate php process to run each sql command, do not force Symfony to believe that php was compiled with --enable-sigchild option")
5248
->addOption('only-instances', null, InputOption::VALUE_REQUIRED, 'Filter the database servers to run this command against. Usage of * and ? wildcards is allowed. To see all instances available, use `instance:list`', null)
53-
->addOption('except-instances', null, InputOption::VALUE_REQUIRED, 'Filter the database servers to run this command against.', null)
49+
->addOption('except-instances', null, InputOption::VALUE_REQUIRED, 'Filter the database servers to run this command against', null)
50+
->addOption('output-type', null, InputOption::VALUE_REQUIRED, 'The format for the output: json, php, text or yml', self::DEFAULT_OUTPUT_FORMAT)
51+
->addOption('timeout', null, InputOption::VALUE_REQUIRED, 'The maximum time to wait for subprocess execution (secs)', self::DEFAULT_PROCESS_TIMEOUT)
52+
->addOption('max-parallel', null, InputOption::VALUE_REQUIRED, 'The maximum number of subprocesses to run in parallel', self::DEFAULT_PARALLEL_PROCESSES)
53+
->addOption('dont-force-enabled-sigchild', null, InputOption::VALUE_NONE, "When using a separate process to run each sql command, do not force Symfony to believe that php was compiled with --enable-sigchild option")
5454
;
5555
}
5656

@@ -214,35 +214,43 @@ protected function executeSqlAction($instanceList, $actionName, $getSqlActionCal
214214
}
215215

216216
/**
217-
* @param array $results should contain elements: succeeded(int) failed(int), data(mixed)
218-
* @param float $time execution time in seconds
219-
* @throws \Exception for unsupported formats
217+
* @param array $results
218+
* @return string
219+
* @throws \OutOfBoundsException for unsupported output formats
220220
*/
221-
protected function writeResults(array $results, $time)
221+
protected function formatResults(array $results)
222222
{
223223
switch ($this->outputFormat) {
224224
case 'json':
225-
$data = json_encode($results['data'], JSON_PRETTY_PRINT);
226-
break;
225+
return json_encode($results['data'], JSON_PRETTY_PRINT);
227226
case 'php':
228-
$data = var_export($results['data'], true);
229-
break;
227+
return var_export($results['data'], true);
230228
case 'text':
231229
case 'yml':
232230
case 'yaml':
233-
$data = Yaml::dump($results['data'], 2, 4, Yaml::DUMP_MULTI_LINE_LITERAL_BLOCK);
234-
break;
231+
return Yaml::dump($results['data'], 2, 4, Yaml::DUMP_MULTI_LINE_LITERAL_BLOCK);
235232
default:
236-
throw new \Exception("Unsupported output format: '{$this->outputFormat}'");
233+
throw new \OutOfBoundsException("Unsupported output format: '{$this->outputFormat}'");
237234
break;
238235
}
239-
$this->writeln($data, OutputInterface::VERBOSITY_QUIET, OutputInterface::OUTPUT_RAW);
236+
}
237+
238+
/**
239+
* @param array $results should contain elements: succeeded(int) failed(int), data(mixed)
240+
* @param float $time execution time in seconds
241+
* @throws \Exception for unsupported formats
242+
* @todo since we use could be using forked processes, we can not measure total memory used... is it worth measuring just ours?
243+
*/
244+
protected function writeResults(array $results, $time = null)
245+
{
246+
$this->writeln($this->formatResults($results), OutputInterface::VERBOSITY_QUIET, OutputInterface::OUTPUT_RAW);
240247

241248
if ($this->outputFormat === 'text') {
242249
$this->writeln($results['succeeded'] . ' succeeded, ' . $results['failed'] . ' failed');
243250

244-
// since we use forked processes, we can not measure max memory used
245-
$this->writeln("<info>Time taken: ".sprintf('%.2f', $time)." secs</info>");
251+
if ($time !== null) {
252+
$this->writeln("<info>Time taken: ".sprintf('%.2f', $time)." secs</info>");
253+
}
246254
}
247255
}
248256
}

‎app/src/Command/SqlExecute.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ protected function configure()
2222
->addOption('sql', null, InputOption::VALUE_REQUIRED, 'The sql command(s) string to execute')
2323
->addOption('file', null, InputOption::VALUE_REQUIRED, "A file with sql commands to execute. The tokens '{dbtype}' and '{instancename}' will be replaced with actual values")
2424

25-
->addOption('database', null, InputOption::VALUE_REQUIRED, 'The name of an existing the database to use. If omitted a dedicated temporary database will be created on the fly and disposed after use')
25+
->addOption('database', null, InputOption::VALUE_REQUIRED, 'The name of an existing database to use. If omitted, a dedicated temporary database will be created on the fly and disposed after use')
2626
->addOption('user', null, InputOption::VALUE_REQUIRED, 'The name of the user to use for connecting to the existing database. Temporary databases get created with a temp user and random password')
2727
->addOption('password', null, InputOption::VALUE_REQUIRED, 'The user password')
2828
->addOption('charset', null, InputOption::VALUE_REQUIRED, 'The collation/character-set to use, _only_ for the dedicated temporary database. If omitted, the default collation for the instance will be used')
@@ -65,6 +65,8 @@ protected function execute(InputInterface $input, OutputInterface $output)
6565

6666
if (($dbName == null) xor ($userName == null)) {
6767

68+
/// @todo is it possible to let the user log in to the 'root' db, regardless of db type ?
69+
6870
throw new \Exception("Please provide both a custom database name and associated user account");
6971
}
7072

@@ -136,7 +138,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
136138
unset($instanceList[$instanceName]);
137139
}
138140
}
139-
$this->dropDatabases($instanceList, $dbConnectionSpecs);
141+
$this->dropDatabases($instanceList, $dbConnectionSpecs, true);
140142
}
141143
} else {
142144
$results = ['succeeded' => 0, 'failed' => 0, 'data' => null];

‎app/src/Command/UserDrop.php‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,24 @@ protected function execute(InputInterface $input, OutputInterface $output)
6767
return (int)$results['failed'];
6868
}
6969

70-
protected function dropUsers($instanceList, $userToDropSpecs)
70+
/**
71+
* @param $instanceList
72+
* @param $userToDropSpecs
73+
* @param bool $ifExists
74+
* @return array
75+
* @throws \Exception
76+
*/
77+
protected function dropUsers($instanceList, $userToDropSpecs, $ifExists = false)
7178
{
7279
return $this->executeSqlAction(
7380
$instanceList,
7481
'Dropping of user',
75-
function ($schemaManager, $instanceName) use ($userToDropSpecs) {
82+
function ($schemaManager, $instanceName) use ($userToDropSpecs, $ifExists) {
7683
$dbConnectionSpec = $userToDropSpecs[$instanceName];
7784
/** @var DatabaseSchemaManager $schemaManager */
7885
return $schemaManager->getDropUserSqlAction(
79-
$dbConnectionSpec['user']
86+
$dbConnectionSpec['user'],
87+
$ifExists
8088
);
8189
}
8290
);

0 commit comments

Comments
 (0)