mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #23851 from nextcloud/enh/read_env_only_once 

Read the env variables only once
This commit is contained in:
Morris Jobke 2020-11-06 22:35:00 +01:00 committed by GitHub
parent fb5d1b40e4 f7a2b71627
commit 659ea6ad79
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 56 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
lib/private/Config.php
View File

@ -47,6 +47,8 @@ class Config {
/** @var array Associative array ($key => $value) */ /** @var array Associative array ($key => $value) */
protected $cache = []; protected $cache = [];
/** @var array */
protected $envCache = [];
/** @var string */ /** @var string */
protected $configDir; protected $configDir;
/** @var string */ /** @var string */
@ -88,9 +90,9 @@ class Config {
* @return mixed the value or $default * @return mixed the value or $default
*/ */
public function getValue($key, $default = null) { public function getValue($key, $default = null) {
$envValue = getenv(self::ENV_PREFIX . $key); $envKey = self::ENV_PREFIX . $key;
if ($envValue !== false) { if (isset($this->envCache[$envKey])) {
return $envValue; return $this->envCache[$envKey];
} }
if (isset($this->cache[$key])) { if (isset($this->cache[$key])) {
@ -222,6 +224,8 @@ class Config {
flock($filePointer, LOCK_UN); flock($filePointer, LOCK_UN);
fclose($filePointer); fclose($filePointer);
} }
$this->envCache = getenv();
} }
/** /**

82
tests/lib/ConfigTest.php
View File

@ -8,6 +8,8 @@
namespace Test; namespace Test;
use OC\Config;
class ConfigTest extends TestCase { class ConfigTest extends TestCase {
public const TESTCONTENT = '<?php $CONFIG=array("foo"=>"bar", "beers" => array("Appenzeller", "Guinness", "Kölsch"), "alcohol_free" => false);'; public const TESTCONTENT = '<?php $CONFIG=array("foo"=>"bar", "beers" => array("Appenzeller", "Guinness", "Kölsch"), "alcohol_free" => false);';
@ -15,8 +17,6 @@ class ConfigTest extends TestCase {
private $initialConfig = ['foo' => 'bar', 'beers' => ['Appenzeller', 'Guinness', 'Kölsch'], 'alcohol_free' => false]; private $initialConfig = ['foo' => 'bar', 'beers' => ['Appenzeller', 'Guinness', 'Kölsch'], 'alcohol_free' => false];
/** @var string */ /** @var string */
private $configFile; private $configFile;
/** @var \OC\Config */
private $config;
/** @var string */ /** @var string */
private $randomTmpDir; private $randomTmpDir;
@ -26,7 +26,6 @@ class ConfigTest extends TestCase {
$this->randomTmpDir = \OC::$server->getTempManager()->getTemporaryFolder(); $this->randomTmpDir = \OC::$server->getTempManager()->getTemporaryFolder();
$this->configFile = $this->randomTmpDir.'testconfig.php'; $this->configFile = $this->randomTmpDir.'testconfig.php';
file_put_contents($this->configFile, self::TESTCONTENT); file_put_contents($this->configFile, self::TESTCONTENT);
$this->config = new \OC\Config($this->randomTmpDir, 'testconfig.php');
} }
protected function tearDown(): void { protected function tearDown(): void {
@ -34,54 +33,69 @@ class ConfigTest extends TestCase {
parent::tearDown(); parent::tearDown();
} }
private function getConfig(): Config {
return new \OC\Config($this->randomTmpDir, 'testconfig.php');
}
public function testGetKeys() { public function testGetKeys() {
$expectedConfig = ['foo', 'beers', 'alcohol_free']; $expectedConfig = ['foo', 'beers', 'alcohol_free'];
$this->assertSame($expectedConfig, $this->config->getKeys()); $this->assertSame($expectedConfig, $this->getConfig()->getKeys());
} }
public function testGetValue() { public function testGetValue() {
$this->assertSame('bar', $this->config->getValue('foo')); $config = $this->getConfig();
$this->assertSame(null, $this->config->getValue('bar')); $this->assertSame('bar', $config->getValue('foo'));
$this->assertSame('moo', $this->config->getValue('bar', 'moo')); $this->assertSame(null, $config->getValue('bar'));
$this->assertSame(false, $this->config->getValue('alcohol_free', 'someBogusValue')); $this->assertSame('moo', $config->getValue('bar', 'moo'));
$this->assertSame(['Appenzeller', 'Guinness', 'Kölsch'], $this->config->getValue('beers', 'someBogusValue')); $this->assertSame(false, $config->getValue('alcohol_free', 'someBogusValue'));
$this->assertSame(['Appenzeller', 'Guinness', 'Kölsch'], $this->config->getValue('beers')); $this->assertSame(['Appenzeller', 'Guinness', 'Kölsch'], $config->getValue('beers', 'someBogusValue'));
$this->assertSame(['Appenzeller', 'Guinness', 'Kölsch'], $config->getValue('beers'));
} }
public function testGetValueReturnsEnvironmentValueIfSet() { public function testGetValueReturnsEnvironmentValueIfSet() {
$this->assertEquals('bar', $this->config->getValue('foo')); $config = $this->getConfig();
$this->assertEquals('bar', $config->getValue('foo'));
putenv('NC_foo=baz'); putenv('NC_foo=baz');
$this->assertEquals('baz', $this->config->getValue('foo')); $config = $this->getConfig();
$this->assertEquals('baz', $config->getValue('foo'));
putenv('NC_foo'); // unset the env variable putenv('NC_foo'); // unset the env variable
} }
public function testGetValueReturnsEnvironmentValueIfSetToZero() { public function testGetValueReturnsEnvironmentValueIfSetToZero() {
$this->assertEquals('bar', $this->config->getValue('foo')); $config = $this->getConfig();
$this->assertEquals('bar', $config->getValue('foo'));
putenv('NC_foo=0'); putenv('NC_foo=0');
$this->assertEquals('0', $this->config->getValue('foo')); $config = $this->getConfig();
$this->assertEquals('0', $config->getValue('foo'));
putenv('NC_foo'); // unset the env variable putenv('NC_foo'); // unset the env variable
} }
public function testGetValueReturnsEnvironmentValueIfSetToFalse() { public function testGetValueReturnsEnvironmentValueIfSetToFalse() {
$this->assertEquals('bar', $this->config->getValue('foo')); $config = $this->getConfig();
$this->assertEquals('bar', $config->getValue('foo'));
putenv('NC_foo=false'); putenv('NC_foo=false');
$this->assertEquals('false', $this->config->getValue('foo')); $config = $this->getConfig();
$this->assertEquals('false', $config->getValue('foo'));
putenv('NC_foo'); // unset the env variable putenv('NC_foo'); // unset the env variable
} }
public function testSetValue() { public function testSetValue() {
$this->config->setValue('foo', 'moo'); $config = $this->getConfig();
$this->assertSame('moo', $this->config->getValue('foo')); $config->setValue('foo', 'moo');
$this->assertSame('moo', $config->getValue('foo'));
$content = file_get_contents($this->configFile); $content = file_get_contents($this->configFile);
$expected = "<?php\n\$CONFIG = array (\n 'foo' => 'moo',\n 'beers' => \n array (\n 0 => 'Appenzeller',\n " . $expected = "<?php\n\$CONFIG = array (\n 'foo' => 'moo',\n 'beers' => \n array (\n 0 => 'Appenzeller',\n " .
" 1 => 'Guinness',\n 2 => 'Kölsch',\n ),\n 'alcohol_free' => false,\n);\n"; " 1 => 'Guinness',\n 2 => 'Kölsch',\n ),\n 'alcohol_free' => false,\n);\n";
$this->assertEquals($expected, $content); $this->assertEquals($expected, $content);
$this->config->setValue('bar', 'red'); $config->setValue('bar', 'red');
$this->config->setValue('apps', ['files', 'gallery']); $config->setValue('apps', ['files', 'gallery']);
$this->assertSame('red', $this->config->getValue('bar')); $this->assertSame('red', $config->getValue('bar'));
$this->assertSame(['files', 'gallery'], $this->config->getValue('apps')); $this->assertSame(['files', 'gallery'], $config->getValue('apps'));
$content = file_get_contents($this->configFile); $content = file_get_contents($this->configFile);
@ -92,27 +106,28 @@ class ConfigTest extends TestCase {
} }
public function testSetValues() { public function testSetValues() {
$config = $this->getConfig();
$content = file_get_contents($this->configFile); $content = file_get_contents($this->configFile);
$this->assertEquals(self::TESTCONTENT, $content); $this->assertEquals(self::TESTCONTENT, $content);
// Changing configs to existing values and deleting non-existing once // Changing configs to existing values and deleting non-existing once
// should not rewrite the config.php // should not rewrite the config.php
$this->config->setValues([ $config->setValues([
'foo' => 'bar', 'foo' => 'bar',
'not_exists' => null, 'not_exists' => null,
]); ]);
$this->assertSame('bar', $this->config->getValue('foo')); $this->assertSame('bar', $config->getValue('foo'));
$this->assertSame(null, $this->config->getValue('not_exists')); $this->assertSame(null, $config->getValue('not_exists'));
$content = file_get_contents($this->configFile); $content = file_get_contents($this->configFile);
$this->assertEquals(self::TESTCONTENT, $content); $this->assertEquals(self::TESTCONTENT, $content);
$this->config->setValues([ $config->setValues([
'foo' => 'moo', 'foo' => 'moo',
'alcohol_free' => null, 'alcohol_free' => null,
]); ]);
$this->assertSame('moo', $this->config->getValue('foo')); $this->assertSame('moo', $config->getValue('foo'));
$this->assertSame(null, $this->config->getValue('not_exists')); $this->assertSame(null, $config->getValue('not_exists'));
$content = file_get_contents($this->configFile); $content = file_get_contents($this->configFile);
$expected = "<?php\n\$CONFIG = array (\n 'foo' => 'moo',\n 'beers' => \n array (\n 0 => 'Appenzeller',\n " . $expected = "<?php\n\$CONFIG = array (\n 'foo' => 'moo',\n 'beers' => \n array (\n 0 => 'Appenzeller',\n " .
@ -121,8 +136,9 @@ class ConfigTest extends TestCase {
} }
public function testDeleteKey() { public function testDeleteKey() {
$this->config->deleteKey('foo'); $config = $this->getConfig();
$this->assertSame('this_was_clearly_not_set_before', $this->config->getValue('foo', 'this_was_clearly_not_set_before')); $config->deleteKey('foo');
$this->assertSame('this_was_clearly_not_set_before', $config->getValue('foo', 'this_was_clearly_not_set_before'));
$content = file_get_contents($this->configFile); $content = file_get_contents($this->configFile);
$expected = "<?php\n\$CONFIG = array (\n 'beers' => \n array (\n 0 => 'Appenzeller',\n " . $expected = "<?php\n\$CONFIG = array (\n 'beers' => \n array (\n 0 => 'Appenzeller',\n " .
@ -137,14 +153,14 @@ class ConfigTest extends TestCase {
file_put_contents($additionalConfigPath, $additionalConfig); file_put_contents($additionalConfigPath, $additionalConfig);
// Reinstantiate the config to force a read-in of the additional configs // Reinstantiate the config to force a read-in of the additional configs
$this->config = new \OC\Config($this->randomTmpDir, 'testconfig.php'); $config = new \OC\Config($this->randomTmpDir, 'testconfig.php');
// Ensure that the config value can be read and the config has not been modified // Ensure that the config value can be read and the config has not been modified
$this->assertSame('totallyOutdated', $this->config->getValue('php53', 'bogusValue')); $this->assertSame('totallyOutdated', $config->getValue('php53', 'bogusValue'));
$this->assertEquals(self::TESTCONTENT, file_get_contents($this->configFile)); $this->assertEquals(self::TESTCONTENT, file_get_contents($this->configFile));
// Write a new value to the config // Write a new value to the config
$this->config->setValue('CoolWebsites', ['demo.owncloud.org', 'owncloud.org', 'owncloud.com']); $config->setValue('CoolWebsites', ['demo.owncloud.org', 'owncloud.org', 'owncloud.com']);
$expected = "<?php\n\$CONFIG = array (\n 'foo' => 'bar',\n 'beers' => \n array (\n 0 => 'Appenzeller',\n " . $expected = "<?php\n\$CONFIG = array (\n 'foo' => 'bar',\n 'beers' => \n array (\n 0 => 'Appenzeller',\n " .
" 1 => 'Guinness',\n 2 => 'Kölsch',\n ),\n 'alcohol_free' => false,\n 'php53' => 'totallyOutdated',\n 'CoolWebsites' => \n array (\n " . " 1 => 'Guinness',\n 2 => 'Kölsch',\n ),\n 'alcohol_free' => false,\n 'php53' => 'totallyOutdated',\n 'CoolWebsites' => \n array (\n " .
" 0 => 'demo.owncloud.org',\n 1 => 'owncloud.org',\n 2 => 'owncloud.com',\n ),\n);\n"; " 0 => 'demo.owncloud.org',\n 1 => 'owncloud.org',\n 2 => 'owncloud.com',\n ),\n);\n";