8000 [Dotenv] add a flag to allow env vars override by fmata · Pull Request #26859 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Dotenv] add a flag to allow env vars override #26859

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 4, 2018
Merged

[Dotenv] add a flag to allow env vars override #26859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/Symfony/Component/Dotenv/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
CHANGELOG
=========

4.2.0
-----

* added `Dotenv::overload()` and `$overrideExistingVars` as optional parameter of `Dotenv::populate()`

3.3.0
-----

Expand Down
44 changes: 31 additions & 13 deletions src/Symfony/Component/Dotenv/Dotenv.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,33 +47,38 @@ final class Dotenv
*/
public function load(string $path, string ...$paths): void
{
array_unshift($paths, $path);

foreach ($paths as $path) {
if (!is_readable($path) || is_dir($path)) {
throw new PathException($path);
}
$this->doLoad(false, $path, ...$paths);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of creating a PHP array (with the variadic argument) to spread it again (here) to create another PHP array (variadic argument in the private method), it might make sense to pass an array to the private method instead of making it variadic.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, we don't need a variadic argument in the private method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stof & @fabpot new PR #28359 created :)

}

$this->populate($this->parse(file_get_contents($path), $path));
}
/**
* Loads one or several .env files and enables override existing vars.
*
* @param string $path A file to load
* @param ...string $paths A list of additional files to load
*
* @throws FormatException when a file has a syntax error
* @throws PathException when a file does not exist or is not readable
*/
public function overload(string $path, string ...$paths): void
{
$this->doLoad(true, $path, ...$paths);
}

/**
* Sets values as environment variables (via putenv, $_ENV, and $_SERVER).
*
* Note that existing environment variables are not overridden.
*
* @param array $values An array of env variables
* @param array $values An array of env variables
* @param bool $overrideExistingVars true when existing environment variables must be overridden
*/
public function populate(array $values): void
public function populate(array $values, bool $overrideExistingVars = false): void
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be reverted IMHO: it makes no sense to provide two public interfaces to do the same thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree but I think the same thing of populate() method, I don't know why it is public because the purpose of Dotenv is to use a file, no directly an array of vars it's so low level IMHO. So I added here to be consistent but let me know if you have no doubt about this revert, I can do it :)

{
$loadedVars = array_flip(explode(',', getenv('SYMFONY_DOTENV_VARS')));
unset($loadedVars['']);

foreach ($values as $name => $value) {
$notHttpName = 0 !== strpos($name, 'HTTP_');
// don't check existence with getenv() because of thread safety issues
if (!isset($loadedVars[$name]) && (isset($_ENV[$name]) || (isset($_SERVER[$name]) && $notHttpName))) {
if (!isset($loadedVars[$name]) && (!$overrideExistingVars && (isset($_ENV[$name]) || (isset($_SERVER[$name]) && $notHttpName)))) {
continue;
}

Expand Down Expand Up @@ -399,4 +404,17 @@ private function createFormatException($message)
{
return new FormatException($message, new FormatExceptionContext($this->data, $this->path, $this->lineno, $this->cursor));
}

private function doLoad(bool $overrideExistingVars, string $path, string ...$paths): void
{
array_unshift($paths, $path);

foreach ($paths as $path) {
if (!is_readable($path) || is_dir($path)) {
throw new PathException($path);
}

$this->populate($this->parse(file_get_contents($path), $path), $overrideExistingVars);
}
}
}
47 changes: 47 additions & 0 deletions src/Symfony/Component/Dotenv/Tests/DotenvTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,41 @@ public function testLoad()
$this->assertSame('BAZ', $bar);
}

public function testOverload()
{
unset($_ENV['FOO']);
unset($_ENV['BAR']);
unset($_SERVER['FOO']);
unset($_SERVER['BAR']);

putenv('FOO=initial_foo_value');
putenv('BAR=initial_bar_value');
$_ENV['FOO'] = 'initial_foo_value';
$_ENV['BAR'] = 'initial_bar_value';

@mkdir($tmpdir = sys_get_temp_dir().'/dotenv');

$path1 = tempnam($tmpdir, 'sf-');
$path2 = tempnam($tmpdir, 'sf-');

file_put_contents($path1, 'FOO=BAR');
file_put_contents($path2, 'BAR=BAZ');

(new DotEnv())->overload($path1, $path2);

$foo = getenv('FOO');
$bar = getenv('BAR');

putenv('FOO');
putenv('BAR');
unlink($path1);
unlink($path2);
rmdir($tmpdir);

$this->assertSame('BAR', $foo);
$this->assertSame('BAZ', $bar);
}

/**
* @expectedException \Symfony\Component\Dotenv\Exception\PathException
*/
Expand Down Expand Up @@ -228,6 +263,18 @@ public function testHttpVarIsPartiallyOverriden()
$this->assertSame('http_value', $_SERVER['HTTP_TEST_ENV_VAR']);
}

public function testEnvVarIsOverriden()
{
putenv('TEST_ENV_VAR_OVERRIDEN=original_value');

$dotenv = new DotEnv();
$dotenv->populate(array('TEST_ENV_VAR_OVERRIDEN' => 'new_value'), true);

$this->assertSame('new_value', getenv('TEST_ENV_VAR_OVERRIDEN'));
$this->assertSame('new_value', $_ENV['TEST_ENV_VAR_OVERRIDEN']);
$this->assertSame('new_value', $_SERVER['TEST_ENV_VAR_OVERRIDEN']);
}

public function testMemorizingLoadedVarsNamesInSpecialVar()
{
// Special variable not exists
Expand Down
0