8000 [Dotenv] Add Dotenv::bootEnv() to check for .env.local.php before calling Dotenv::loadEnv() by nicolas-grekas · Pull Request #35308 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Dotenv] Add Dotenv::bootEnv() to check for .env.local.php before calling Dotenv::loadEnv() #35308

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
Jan 30, 2020
Merged

[Dotenv] Add Dotenv::bootEnv() to check for .env.local.php before calling Dotenv::loadEnv() #35308

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 UPGRADE-5.1.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ Console

* `Command::setHidden()` is final since Symfony 5.1

Dotenv
------

* Deprecated passing `$usePutenv` argument to Dotenv's constructor, use `Dotenv::usePutenv()` instead.

EventDispatcher
---------------

Expand Down
5 changes: 5 additions & 0 deletions UPGRADE-6.0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ Console

* `Command::setHidden()` has a default value (`true`) for `$hidden` parameter

Dotenv
------

* Removed argument `$usePutenv` from Dotenv's constructor, use `Dotenv::usePutenv()` instead.

EventDispatcher
---------------

Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Bundle/FrameworkBundle/Tests/Secrets/DotenvVaultTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public function testEncryptAndDecrypt()
$vault->seal('foo', $plain);

unset($_SERVER['foo'], $_ENV['foo']);
(new Dotenv(false))->load($this->envFile);
(new Dotenv())->load($this->envFile);

$decrypted = $vault->reveal('foo');
$this->assertSame($plain, $decrypted);
Expand All @@ -50,7 +50,7 @@ public function testEncryptAndDecrypt()
$this->assertFalse($vault->remove('foo'));

unset($_SERVER['foo'], $_ENV['foo']);
(new Dotenv(false))->load($this->envFile);
(new Dotenv())->load($this->envFile);

$this->assertArrayNotHasKey('foo', $vault->list());
}
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Bundle/FrameworkBundle/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
"symfony/console": "^4.4|^5.0",
"symfony/css-selector": "^4.4|^5.0",
"symfony/dom-crawler": "^4.4|^5.0",
"symfony/dotenv": "^4.4|^5.0",
"symfony/dotenv": "^5.1",
"symfony/polyfill-intl-icu": "~1.0",
"symfony/form": "^4.4|^5.0",
"symfony/expression-language": "^4.4|^5.0",
Expand Down Expand Up @@ -71,7 +71,7 @@
"symfony/asset": "<4.4",
"symfony/browser-kit": "<4.4",
"symfony/console": "<4.4",
"symfony/dotenv": "<4.4",
"symfony/dotenv": "<5.1",
"symfony/dom-crawler": "<4.4",
"symfony/http-client": "<4.4",
"symfony/form": "<4.4",
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/Console/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
},
"conflict": {
"symfony/dependency-injection": "<4.4",
"symfony/dotenv": "<5.1",
"symfony/event-dispatcher": "<4.4",
"symfony/lock": "<4.4",
"symfony/process": "<4.4"
Expand Down
9 changes: 9 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,15 @@
CHANGELOG
=========

5.1.0
-----

* added `Dotenv::bootEnv()` to check for `.env.local.php` before calling `Dotenv::loadEnv()`
* added `Dotenv::setProdEnvs()` and `Dotenv::usePutenv()`
* made Dotenv's constructor accept `$envKey` and `$debugKey` arguments, to define
the name of the env vars that configure the env name and debug settings
* deprecated passing `$usePutenv` argument to Dotenv's constructor

5.0.0
-----

Expand Down
84 changes: 72 additions & 12 deletions src/Symfony/Component/Dotenv/Dotenv.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,47 @@ final class Dotenv
private $data;
private $end;
private $values;
private $usePutenv;
private $envKey;
private $debugKey;
private $prodEnvs = ['prod'];
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps for consistency, define $testEnvs = ['test'] + $defaultEnv = 'dev'; as well? Then cleanup the arg list in loadEnv()

Copy link
Member Author

Choose a reason for hiding this comment

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

I considered that, but I think it's not worth the upgrade trouble.

Copy link
Contributor

Choose a reason for hiding this comment

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

it would avoid a half stateful / stateless API, and perhaps is worth it from the component's design POV.

Copy link
Contributor

Choose a reason for hiding this comment

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

do we prefer mutability? or what about new Dotenv('APP_ENV', 'APP_DEBUG', 'dev', ['prod'], ['test']) :/

Copy link
Member Author
@nicolas-grekas nicolas-grekas Jan 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

We have no issues with mutability when its purpose is to configure the instance.
Using many constructor args is a pain when you only want to change the last one.
Setters are better here IMHO.

Copy link
Contributor

Choose a reason for hiding this comment

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

i undertand the technical aspect, im curious if a user should change the context between loading envs (vs using a dedicated instance per context). In practice the context is a fixed concern.

Copy link
Member Author
@nicolas-grekas nicolas-grekas Jan 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

In practice, people will figure out what they need :) I considered alternatives before deciding for this design: it looks the best compromise to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO it's component design first :) i strongly feel the right design is

$dotenv = new Dotenv('APP_ENV', 'APP_DEBUG', 'dev', ['prod'], ['test']);
$dotenv->load('.env', $putenv = true);
$dotenv->loadAll(['.env'], $putenv = true);

but im fine either way, my only use case is a generated one :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I understand, that's what I meant by "compromise". We're not designing from the ground up, and in practice this is the most seamless.

private $usePutenv = false;

/**
* @var bool If `putenv()` should be used to define environment variables or not.
* Beware that `putenv()` is not thread safe, that's why this setting defaults to false
* @param string $envKey
*/
public function __construct(bool $usePutenv = false)
public function __construct($envKey = 'APP_ENV', string $debugKey = 'APP_DEBUG')
{
if (\in_array($envKey = (string) $envKey, ['1', ''], true)) {
@trigger_error(sprintf('Passing a boolean to the constructor of "%s" is deprecated since Symfony 5.1, use "Dotenv::usePutenv()".', __CLASS__), E_USER_DEPRECATED);
$this->usePutenv = (bool) $envKey;
$envKey = 'APP_ENV';
}

$this->envKey = $envKey;
$this->debugKey = $debugKey;
}

/**
* @return $this
*/
public function setProdEnvs(array $prodEnvs): self
{
$this->prodEnvs = $prodEnvs;

return $this;
}

/**
* @param bool $usePutenv If `putenv()` should be used to define environment variables or not.
* Beware that `putenv()` is not thread safe, that's why this setting defaults to false
*
* @return $this
*/
public function usePutenv($usePutenv = true): self
{
$this->usePutenv = $usePutenv;

return $this;
}

/**
Expand All @@ -66,29 +98,31 @@ public function load(string $path, string ...$extraPaths): void
* .env.local is always ignored in test env because tests should produce the same results for everyone.
* .env.dist is loaded when it exists and .env is not found.
*
* @param string $path A file to load
* @param string $varName The name of the env vars that defines the app env
* @param string $defaultEnv The app env to use when none is defined
* @param array $testEnvs A list of app envs for which .env.local should be ignored
* @param string $path A file to load
* @param string $envKey|null The name of the env vars that defines the app env
* @param string $defaultEnv The app env to use when none is defined
* @param array $testEnvs A list of app envs for which .env.local should be ignored
*
* @throws FormatException when a file has a syntax error
* @throws PathException when a file does not exist or is not readable
*/
public function loadEnv(string $path, string $varName = 'APP_ENV', string $defaultEnv = 'dev', array $testEnvs = ['test']): void
public function loadEnv(string $path, string $envKey = null, string $defaultEnv = 'dev', array $testEnvs = ['test']): void
{
$k = $envKey ?? $this->envKey;

if (file_exists($path) || !file_exists($p = "$path.dist")) {
$this->load($path);
} else {
$this->load($p);
}

if (null === $env = $_SERVER[$varName] ?? $_ENV[$varName] ?? null) {
$this->populate([$varName => $env = $defaultEnv]);
if (null === $env = $_SERVER[$k] ?? $_ENV[$k] ?? null) {
$this->populate([$k => $env = $defaultEnv]);
}

if (!\in_array($env, $testEnvs, true) && file_exists($p = "$path.local")) {
$this->load($p);
$env = $_SERVER[$varName] ?? $_ENV[$varName] ?? $env;
$env = $_SERVER[$k] ?? $_ENV[$k] ?? $env;
}

if ('local' === $env) {
Expand All @@ -104,6 +138,32 @@ public function loadEnv(string $path, string $varName = 'APP_ENV', string $defau
}
}

/**
* Loads env vars from .env.local.php if the file exists or from the other .env files otherwise.
*
* This method also configures the APP_DEBUG env var according to the current APP_ENV.
*
* See method loadEnv() for rules related to .env files.
*/
public function bootEnv(string $path, string $defaultEnv = 'dev', array $testEnvs = ['test']): void
{
$p = $path.'.local.php';
$env = (\function_exists('opcache_is_script_cached') && @opcache_is_script_cached($p)) || file_exists($p) ? include $p : null;
$k = $this->envKey;

if (\is_array($env) && (!isset($env[$k]) || ($_SERVER[$k] ?? $_ENV[$k] ?? $env[$k]) === $env[$k])) {
$this->populate($env);
} else {
$this->loadEnv($path, $k, $defaultEnv, $testEnvs);
}

$_SERVER += $_ENV;

$k = $this->debugKey;
$debug = $_SERVER[$k] ?? !\in_array($_SERVER[$this->envKey], $this->prodEnvs, true);
$_SERVER[$k] = $_ENV[$k] = (int) $debug || (!\is_bool($debug) && filter_var($debug, FILTER_VALIDATE_BOOLEAN)) ? '1' : '0';
}

/**
* Loads one or several .env files and enables override existing vars.
*
Expand Down
Loading
0