diff --git a/UPGRADING.md b/UPGRADING.md index b9725d3d..924561f3 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -20,6 +20,7 @@ Perform EACH version specific task between your version and the new one, otherwi ### Developer +- **BC break**: removed registration of `.env` with `putenv()` - do not rely on `getenv()` as it is not thread-safe - **BC break**: interface method signature `HtmlToPdfConverter::convertToPdf()` changed - **BC break**: the macros `badge` and `label` do not apply the `|trans` filter any more - **BC Break**: removed `getVisible()` (deprecated since 1.4) method on Customer, Project and Activity (use `isVisible()` instead, templates are still working) diff --git a/bin/console b/bin/console index 8ccf63f3..4c52dd80 100755 --- a/bin/console +++ b/bin/console @@ -9,11 +9,10 @@ use Symfony\Component\Dotenv\Dotenv; set_time_limit(0); -require __DIR__.'/../vendor/autoload.php'; +require dirname(__DIR__) . '/vendor/autoload.php'; -if (!isset($_SERVER['APP_ENV'])) { - (new Dotenv(true))->load(__DIR__.'/../.env'); -} +// do NOT rely on calls to getenv() as it is not thread safe +(new Dotenv(false))->loadEnv(dirname(__DIR__).'/.env'); $input = new ArgvInput(); $env = $input->getParameterOption(['--env', '-e'], $_SERVER['APP_ENV'] ?? 'prod'); @@ -22,9 +21,7 @@ $debug = (bool) ($_SERVER['APP_DEBUG'] ?? (in_array($env, ['dev', 'test']))) && if ($debug) { umask(0000); - if (class_exists(Debug::class)) { - Debug::enable(); - } + Debug::enable(); } $kernel = new Kernel($env, $debug); diff --git a/config/services.yaml b/config/services.yaml index a8c82260..e0cc556f 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -14,6 +14,7 @@ services: # The best practice is to be explicit about your dependencies anyway. bind: $projectDirectory: '%kernel.project_dir%' + $kernelEnvironment: '%kernel.environment%' # makes classes in src/ available to be used as services # this creates a service per class whose id is the fully-qualified class name @@ -42,6 +43,9 @@ services: arguments: $dashboard: '%kimai.dashboard%' + App\Configuration\MailConfiguration: + arguments: ['%env(MAILER_FROM)%'] + App\Configuration\LanguageFormattings: arguments: ['%kimai.languages%'] diff --git a/public/index.php b/public/index.php index 400c0ce6..dba4405b 100644 --- a/public/index.php +++ b/public/index.php @@ -5,12 +5,10 @@ use Symfony\Component\ErrorHandler\Debug; use Symfony\Component\Dotenv\Dotenv; use Symfony\Component\HttpFoundation\Request; -require __DIR__.'/../vendor/autoload.php'; +require dirname(__DIR__) . '/vendor/autoload.php'; -// The check is to ensure we don't use .env in production -if (!isset($_SERVER['APP_ENV'])) { - (new Dotenv(true))->load(__DIR__.'/../.env'); -} +// do NOT rely on calls to getenv() as it is not thread safe +(new Dotenv(false))->loadEnv(dirname(__DIR__) . '/.env'); $env = $_SERVER['APP_ENV'] ?? 'prod'; $debug = (bool) ($_SERVER['APP_DEBUG'] ?? (in_array($env, ['dev', 'test']))); diff --git a/src/Command/CreateReleaseCommand.php b/src/Command/CreateReleaseCommand.php index 8ae0ab46..04041b08 100644 --- a/src/Command/CreateReleaseCommand.php +++ b/src/Command/CreateReleaseCommand.php @@ -28,14 +28,16 @@ class CreateReleaseCommand extends Command /** * @var string */ - protected $rootDir = ''; - + private $rootDir = ''; /** - * @param string $projectDirectory + * @var string */ - public function __construct(string $projectDirectory) + private $environment; + + public function __construct(string $projectDirectory, string $kernelEnvironment) { $this->rootDir = realpath($projectDirectory); + $this->environment = $kernelEnvironment; parent::__construct(); } @@ -51,14 +53,16 @@ class CreateReleaseCommand extends Command ->addOption('directory', null, InputOption::VALUE_OPTIONAL, 'Directory where the release package will be stored', '/tmp/') ->addOption('release', null, InputOption::VALUE_OPTIONAL, 'The version that should be zipped', Constants::VERSION) ; + } - /* - * Hide this command in production. - * Maybe it should be de-activated completely?! - */ - if (getenv('APP_ENV') === 'prod') { - $this->setHidden(true); - } + /** + * Make sure that this command CANNOT be executed in production. + * + * @return bool + */ + public function isEnabled() + { + return $this->environment !== 'prod'; } /** @@ -70,12 +74,6 @@ class CreateReleaseCommand extends Command { $io = new SymfonyStyle($input, $output); - if (getenv('APP_ENV') === 'prod') { - $io->error('kimai:create-release is not allowed in production'); - - return -2; - } - $directory = $input->getOption('directory'); if ($directory[0] === '/') { diff --git a/src/Command/ResetCommand.php b/src/Command/ResetCommand.php index 6f742dcc..df535453 100644 --- a/src/Command/ResetCommand.php +++ b/src/Command/ResetCommand.php @@ -29,6 +29,17 @@ use Symfony\Component\Console\Style\SymfonyStyle; */ class ResetCommand extends Command { + /** + * @var string + */ + private $environment; + + public function __construct(string $kernelEnvironment) + { + $this->environment = $kernelEnvironment; + parent::__construct(); + } + /** * {@inheritdoc} */ @@ -55,7 +66,7 @@ EOT */ public function isEnabled() { - return getenv('APP_ENV') !== 'prod'; + return $this->environment !== 'prod'; } /** diff --git a/src/Configuration/MailConfiguration.php b/src/Configuration/MailConfiguration.php index c9512179..fd186fc4 100644 --- a/src/Configuration/MailConfiguration.php +++ b/src/Configuration/MailConfiguration.php @@ -11,14 +11,22 @@ namespace App\Configuration; class MailConfiguration { + /** + * @var string + */ + private $mailFrom; + + public function __construct(string $mailFrom) + { + $this->mailFrom = $mailFrom; + } + public function getFromAddress(): ?string { - $from = getenv('MAILER_FROM'); - - if ($from === false || empty($from)) { + if (empty($this->mailFrom)) { return null; } - return $from; + return $this->mailFrom; } } diff --git a/src/Controller/DoctorController.php b/src/Controller/DoctorController.php index f7269bd9..b2f9e2ec 100644 --- a/src/Controller/DoctorController.php +++ b/src/Controller/DoctorController.php @@ -49,10 +49,15 @@ class DoctorController extends AbstractController * @var string */ private $projectDirectory; + /** + * @var string + */ + private $environment; - public function __construct(string $projectDirectory) + public function __construct(string $projectDirectory, string $kernelEnvironment) { $this->projectDirectory = $projectDirectory; + $this->environment = $kernelEnvironment; } /** @@ -90,7 +95,7 @@ class DoctorController extends AbstractController return $this->render('doctor/index.html.twig', array_merge( [ 'modules' => get_loaded_extensions(), - 'dotenv' => $this->getEnvVars(), + 'environment' => $this->environment, 'info' => $this->getPhpInfo(), 'settings' => $this->getIniSettings(), 'extensions' => $this->getLoadedExtensions(), @@ -124,36 +129,26 @@ class DoctorController extends AbstractController return $results; } - private function getLogSize() + private function getLogSize(): int { - $logfileName = 'var/log/' . getenv('APP_ENV') . '.log'; - $logfile = $this->projectDirectory . '/' . $logfileName; + $logfile = $this->getLogFilename(); - return filesize($logfile); + return file_exists($logfile) ? filesize($logfile) : 0; } private function getLogFilename(): string { - // why is this check here ??? - if (!\in_array(getenv('APP_ENV'), ['test', 'dev', 'prod'])) { - throw new \RuntimeException('Unsupported log environment'); - } - - $logfileName = 'var/log/' . getenv('APP_ENV') . '.log'; + $logfileName = 'var/log/' . $this->environment . '.log'; return $this->projectDirectory . '/' . $logfileName; } - private function getLog(int $lines = 100) + private function getLog(int $lines = 100): array { - try { - $logfile = $this->getLogFilename(); - } catch (\Exception $ex) { - return ['ATTENTION: ' . $ex->getMessage()]; - } + $logfile = $this->getLogFilename(); if (!file_exists($logfile)) { - return ['ATTENTION: Missing logfile']; + return ['Missing logfile']; } if (!is_readable($logfile)) { @@ -163,7 +158,7 @@ class DoctorController extends AbstractController $file = new \SplFileObject($logfile, 'r'); if ($file->getSize() === 0) { - return ['Empty log']; + return ['Empty logfile']; } $file->seek($file->getSize()); @@ -173,8 +168,6 @@ class DoctorController extends AbstractController } $iterator = new \LimitIterator($file, $last_line - $lines, $last_line); - $result = []; - try { $result = iterator_to_array($iterator); } catch (\Exception $ex) { @@ -182,7 +175,7 @@ class DoctorController extends AbstractController } if (!is_writable($logfile)) { - $result[] = 'ATTENTION: Cannot write log file'; + $result[] = 'ATTENTION: Logfile is not writable'; } return $result; @@ -210,14 +203,6 @@ class DoctorController extends AbstractController return $results; } - private function getEnvVars() - { - return [ - 'APP_ENV' => getenv('APP_ENV'), - 'CORS_ALLOW_ORIGIN' => getenv('CORS_ALLOW_ORIGIN'), - ]; - } - private function getIniSettings() { $ini = [ diff --git a/src/DependencyInjection/Compiler/DoctrineCompilerPass.php b/src/DependencyInjection/Compiler/DoctrineCompilerPass.php index 30d7f5d8..b0253e38 100644 --- a/src/DependencyInjection/Compiler/DoctrineCompilerPass.php +++ b/src/DependencyInjection/Compiler/DoctrineCompilerPass.php @@ -21,7 +21,7 @@ class DoctrineCompilerPass implements CompilerPassInterface /** * @var string[] */ - protected $allowedEngines = [ + private $allowedEngines = [ 'mysql', 'sqlite' ]; @@ -33,20 +33,32 @@ class DoctrineCompilerPass implements CompilerPassInterface protected function findEngine() { $engine = null; + $databaseUrl = null; - if (null === $engine) { - $dbConfig = explode('://', getenv('DATABASE_URL')); - $engine = $dbConfig['0'] ?: null; + if (null === $databaseUrl && isset($_ENV['DATABASE_URL'])) { + $databaseUrl = $_ENV['DATABASE_URL']; + } + + if (null === $databaseUrl && isset($_SERVER['DATABASE_URL'])) { + $databaseUrl = $_SERVER['DATABASE_URL']; + } + + if (null === $databaseUrl && (false !== $envDbUrl = getenv('DATABASE_URL'))) { + $databaseUrl = $envDbUrl; + } + + if (null !== $databaseUrl) { + $urlParts = explode('://', $databaseUrl); + $engine = $urlParts[0] ?: null; } if (null === $engine) { $engine = getenv('DATABASE_ENGINE'); } - if (false === $engine) { + if (empty($engine)) { throw new \Exception( - 'Could not detect database engine. Please set the environment config DATABASE_ENGINE ' . - 'to one of: "' . implode(', ', $this->allowedEngines) . '" in your .env file, e.g. DATABASE_ENGINE=sqlite' + 'Could not detect database engine, make sure DATABASE_URL is available from $_SERVER or $_ENV. Check your .env file.' ); } diff --git a/templates/doctor/index.html.twig b/templates/doctor/index.html.twig index bf1ade74..882cfac3 100644 --- a/templates/doctor/index.html.twig +++ b/templates/doctor/index.html.twig @@ -2,6 +2,7 @@ {% import "doctor/actions.html.twig" as actions %} {% block page_title %}{{ 'menu.doctor'|trans }}{% endblock %} +{% block page_subtitle %}Environment: {{ environment }}{% endblock %} {% block page_actions %}{{ actions.doctor('index') }}{% endblock %} {% block main %} @@ -101,8 +102,9 @@ {% embed '@AdminLTE/Widgets/box-widget.html.twig' with {collapsed: true} %} {% block box_title %}Composer packages{% endblock %} + {% block box_body_class %}no-padding{% endblock %} {% block box_body %} - +
{% for name, value in composer %} @@ -113,24 +115,11 @@ {% endblock %} {% endembed %} - {% embed '@AdminLTE/Widgets/box-widget.html.twig' with {collapsed: true} %} - {% block box_title %}Environment variables{% endblock %} - {% block box_body %} -
{{ name }}
- {% for name, value in dotenv %} - - - - - {% endfor %} -
{{ name }}{{ value }}
- {% endblock %} - {% endembed %} - {% embed '@AdminLTE/Widgets/box-widget.html.twig' with {collapsed: true} %} {% block box_title %}PHP{% endblock %} + {% block box_body_class %}no-padding{% endblock %} {% block box_body %} - +
@@ -157,8 +146,9 @@ {% embed '@AdminLTE/Widgets/box-widget.html.twig' with {collapsed: true} %} {% block box_title %}Server{% endblock %} + {% block box_body_class %}no-padding{% endblock %} {% block box_body %} -
Version {{ constant('PHP_VERSION') }}
+
{% for name, value in info %} diff --git a/tests/Command/CreateReleaseCommandTest.php b/tests/Command/CreateReleaseCommandTest.php index 8c14af8e..db5fb844 100644 --- a/tests/Command/CreateReleaseCommandTest.php +++ b/tests/Command/CreateReleaseCommandTest.php @@ -19,21 +19,20 @@ use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; */ class CreateReleaseCommandTest extends KernelTestCase { - /** - * @var Application - */ - protected $application; - - protected function setUp(): void - { - $kernel = self::bootKernel(); - $this->application = new Application($kernel); - $this->application->add(new CreateReleaseCommand(realpath(__DIR__ . '/../../'))); - } - public function testCommandName() { - $command = $this->application->find('kimai:create-release'); + $kernel = self::bootKernel(['environment' => 'test']); + $application = new Application($kernel); + $application->add(new CreateReleaseCommand(realpath(__DIR__ . '/../../'), 'test')); + + $command = $application->find('kimai:create-release'); + self::assertTrue($command->isEnabled()); self::assertInstanceOf(CreateReleaseCommand::class, $command); } + + public function testCommandNameIsNotAvailableInProd() + { + $command = new CreateReleaseCommand(realpath(__DIR__ . '/../../'), 'prod'); + self::assertFalse($command->isEnabled()); + } } diff --git a/tests/Command/ResetCommandTest.php b/tests/Command/ResetCommandTest.php index 6e6655b8..13a94436 100644 --- a/tests/Command/ResetCommandTest.php +++ b/tests/Command/ResetCommandTest.php @@ -19,21 +19,19 @@ use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; */ class ResetCommandTest extends KernelTestCase { - /** - * @var Application - */ - protected $application; - - protected function setUp(): void - { - $kernel = self::bootKernel(); - $this->application = new Application($kernel); - $this->application->add(new ResetCommand()); - } - public function testCommandName() { - $command = $this->application->find('kimai:reset-dev'); + $kernel = self::bootKernel(); + $application = new Application($kernel); + $application->add(new ResetCommand('test')); + + $command = $application->find('kimai:reset-dev'); self::assertInstanceOf(ResetCommand::class, $command); } + + public function testCommandNameIsNotEnabledInProd() + { + $command = new ResetCommand('prod'); + self::assertFalse($command->isEnabled()); + } } diff --git a/tests/Configuration/MailConfigurationTest.php b/tests/Configuration/MailConfigurationTest.php index 603bbf9e..e60bab63 100644 --- a/tests/Configuration/MailConfigurationTest.php +++ b/tests/Configuration/MailConfigurationTest.php @@ -19,15 +19,13 @@ class MailConfigurationTest extends TestCase { public function testGetFromAddress() { - $previous = getenv('MAILER_FROM'); - putenv('MAILER_FROM=foo-bar123@example.com'); - - $sut = new MailConfiguration(); + $sut = new MailConfiguration('foo-bar123@example.com'); self::assertEquals('foo-bar123@example.com', $sut->getFromAddress()); + } - putenv('MAILER_FROM='); + public function testGetFromAddressWithEmptyAddressReturnsNull() + { + $sut = new MailConfiguration(''); self::assertNull($sut->getFromAddress()); - - putenv('MAILER_FROM=' . $previous); } } diff --git a/tests/Controller/DoctorControllerTest.php b/tests/Controller/DoctorControllerTest.php index aaa93f0f..32987333 100644 --- a/tests/Controller/DoctorControllerTest.php +++ b/tests/Controller/DoctorControllerTest.php @@ -32,6 +32,6 @@ class DoctorControllerTest extends ControllerBaseTest $this->assertAccessIsGranted($client, '/doctor'); $result = $client->getCrawler()->filter('.content .box-header'); - $this->assertEquals(7, \count($result)); + self::assertCount(6, $result); } }
{{ name }}