MAGECLOUD-2288: Fix MIN_LOGGING_LEVEL Variable (#285)

bbatsche · shiftedreality · commit f72f69ed35b6 · 2018-06-12T15:22:12.000-05:00
diff --git a/src/App/Logger/HandlerFactory.php b/src/App/Logger/HandlerFactory.php
@@ -6,6 +6,7 @@
 namespace Magento\MagentoCloud\App\Logger;
 
 use Magento\MagentoCloud\App\Logger\Gelf\HandlerFactory as GelfHandlerFactory;
+use Magento\MagentoCloud\Config\GlobalSection;
 use Magento\MagentoCloud\Config\Log as LogConfig;
 use Monolog\Handler\HandlerInterface;
 use Monolog\Handler\NativeMailerHandler;
@@ -43,37 +44,49 @@ class HandlerFactory
      */
     private $gelfHandlerFactory;
 
+    /**
+     * @var GlobalSection
+     */
+    private $globalConfig;
+
     /**
      * @param LevelResolver $levelResolver
      * @param LogConfig $logConfig
      * @param GelfHandlerFactory $gelfHandlerFactory
+     * @param GlobalSection $globalConfig
      */
     public function __construct(
         LevelResolver $levelResolver,
         LogConfig $logConfig,
-        GelfHandlerFactory $gelfHandlerFactory
+        GelfHandlerFactory $gelfHandlerFactory,
+        GlobalSection $globalConfig
     ) {
         $this->levelResolver = $levelResolver;
         $this->logConfig = $logConfig;
         $this->gelfHandlerFactory = $gelfHandlerFactory;
+        $this->globalConfig = $globalConfig;
     }
 
     /**
      * @param string $handler
      * @return HandlerInterface
      * @throws \Exception
+     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      */
     public function create(string $handler): HandlerInterface
     {
         $configuration = $this->logConfig->get($handler);
-        $minLevel = $this->levelResolver->resolve($configuration->get('min_level', LogConfig::LEVEL_NOTICE));
+        $levelOverride = $this->globalConfig->get(GlobalSection::VAR_MIN_LOGGING_LEVEL);
+        $defaultLevel = $levelOverride ?: LogConfig::LEVEL_NOTICE;
+        $minLevel = $this->levelResolver->resolve($configuration->get('min_level', $defaultLevel));
 
         switch ($handler) {
             case static::HANDLER_STREAM:
             case static::HANDLER_FILE:
+                $defaultLevelStream = $levelOverride ?: LogConfig::LEVEL_INFO;
                 $handlerInstance = new StreamHandler(
                     $configuration->get('stream'),
-                    $this->levelResolver->resolve($configuration->get('min_level', LogConfig::LEVEL_INFO))
+                    $this->levelResolver->resolve($configuration->get('min_level', $defaultLevelStream))
                 );
                 break;
             case static::HANDLER_EMAIL:
diff --git a/src/App/Logger/LevelResolver.php b/src/App/Logger/LevelResolver.php
@@ -5,19 +5,13 @@
  */
 namespace Magento\MagentoCloud\App\Logger;
 
-use Magento\MagentoCloud\Config\Environment;
 use Monolog\Logger;
 
 /**
  * Resolves string level to int level from Logger.
  */
 class LevelResolver
 {
-    /**
-     * @var Environment
-     */
-    private $environment;
-
     /**
      * @var array
      */
@@ -32,22 +26,12 @@ class LevelResolver
         'emergency' => Logger::EMERGENCY,
     ];
 
-    /**
-     * @param Environment $environment
-     */
-    public function __construct(Environment $environment)
-    {
-        $this->environment = $environment;
-    }
-
     /**
      * @param string $level
      * @return int
      */
     public function resolve(string $level): int
     {
-        $level = $this->environment->getMinLoggingLevel() ?: $level;
-
         return $this->mapLevels[strtolower($level)] ?? Logger::NOTICE;
     }
 }
diff --git a/src/Config/Log.php b/src/Config/Log.php
@@ -20,8 +20,14 @@ class Log
     /**
      * Log levels.
      */
+    const LEVEL_DEBUG = 'debug';
     const LEVEL_INFO = 'info';
     const LEVEL_NOTICE = 'notice';
+    const LEVEL_WARNING = 'warning';
+    const LEVEL_ERROR = 'error';
+    const LEVEL_CRITICAL = 'critical';
+    const LEVEL_ALERT = 'alert';
+    const LEVEL_EMERGENCY = 'emergency';
 
     /**
      * @var FileList
diff --git a/src/Config/Schema.php b/src/Config/Schema.php
@@ -191,6 +191,26 @@ public function getSchema()
                     StageConfigInterface::STAGE_GLOBAL => [],
                 ],
             ],
+            StageConfigInterface::VAR_MIN_LOGGING_LEVEL => [
+                self::SCHEMA_TYPE => ['string'],
+                self::SCHEMA_VALUE_VALIDATION => [
+                    '',
+                    Log::LEVEL_DEBUG,
+                    Log::LEVEL_INFO,
+                    Log::LEVEL_NOTICE,
+                    Log::LEVEL_WARNING,
+                    Log::LEVEL_ERROR,
+                    Log::LEVEL_CRITICAL,
+                    Log::LEVEL_ALERT,
+                    Log::LEVEL_EMERGENCY,
+                ],
+                self::SCHEMA_STAGE => [
+                    StageConfigInterface::STAGE_GLOBAL,
+                ],
+                self::SCHEMA_DEFAULT_VALUE => [
+                    StageConfigInterface::STAGE_GLOBAL => '',
+                ],
+            ],
             DeployInterface::VAR_REDIS_USE_SLAVE_CONNECTION => [
                 self::SCHEMA_TYPE => ['boolean'],
                 self::SCHEMA_STAGE => [
diff --git a/src/Config/StageConfigInterface.php b/src/Config/StageConfigInterface.php
@@ -42,6 +42,11 @@ interface StageConfigInterface
     const VAR_DEPLOYED_MAGENTO_VERSION_FROM_GIT = 'DEPLOYED_MAGENTO_VERSION_FROM_GIT';
     const VAR_DEPLOY_FROM_GIT_OPTIONS = 'DEPLOY_FROM_GIT_OPTIONS';
 
+    /**
+     * Default minimum logging level.
+     */
+    const VAR_MIN_LOGGING_LEVEL = 'MIN_LOGGING_LEVEL';
+
     /**
      * Option for enabling merging given configuration with default configuration
      */
diff --git a/src/Test/Unit/App/Logger/HandlerFactoryTest.php b/src/Test/Unit/App/Logger/HandlerFactoryTest.php
@@ -10,6 +10,7 @@
 use Magento\MagentoCloud\App\Logger\Gelf\HandlerFactory as GelfHandlerFactory;
 use Magento\MagentoCloud\App\Logger\HandlerFactory;
 use Magento\MagentoCloud\App\Logger\LevelResolver;
+use Magento\MagentoCloud\Config\GlobalSection;
 use Magento\MagentoCloud\Config\Log as LogConfig;
 use Monolog\Handler\HandlerInterface;
 use Monolog\Handler\NativeMailerHandler;
@@ -18,8 +19,8 @@
 use Monolog\Handler\SyslogHandler;
 use Monolog\Handler\SyslogUdpHandler;
 use Monolog\Logger;
+use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
-use PHPUnit_Framework_MockObject_MockObject as Mock;
 
 /**
  * @inheritdoc
@@ -29,25 +30,30 @@
 class HandlerFactoryTest extends TestCase
 {
     /**
-     * @var LevelResolver|Mock
+     * @var LevelResolver|MockObject
      */
     private $levelResolverMock;
 
     /**
-     * @var LogConfig|Mock
+     * @var LogConfig|MockObject
      */
     private $logConfigMock;
 
     /**
-     * @var Repository|Mock
+     * @var Repository|MockObject
      */
     private $repositoryMock;
 
     /**
-     * @var GelfHandlerFactory|Mock
+     * @var GelfHandlerFactory|MockObject
      */
     private $gelfHandlerFactoryMock;
 
+    /**
+     * @var GlobalSection|MockObject
+     */
+    private $globalConfigMock;
+
     /**
      * @var HandlerFactory
      */
@@ -62,11 +68,13 @@ protected function setUp()
         $this->logConfigMock = $this->createMock(LogConfig::class);
         $this->repositoryMock = $this->createMock(Repository::class);
         $this->gelfHandlerFactoryMock = $this->createMock(GelfHandlerFactory::class);
+        $this->globalConfigMock = $this->createMock(GlobalSection::class);
 
         $this->handlerFactory = new HandlerFactory(
             $this->levelResolverMock,
             $this->logConfigMock,
-            $this->gelfHandlerFactoryMock
+            $this->gelfHandlerFactoryMock,
+            $this->globalConfigMock
         );
     }
 
@@ -128,23 +136,38 @@ public function testCreateGelfHandler()
     public function testCreate(
         string $handler,
         array $repositoryMockReturnMap,
-        string $expectedClass
+        $minLevelOverride,
+        string $expectedClass,
+        int $expectedLevel
     ) {
         $this->logConfigMock->expects($this->once())
             ->method('get')
             ->with($handler)
             ->willReturn($this->repositoryMock);
         $this->repositoryMock->method('get')
             ->willReturnMap($repositoryMockReturnMap);
+        $this->globalConfigMock->expects($this->once())
+            ->method('get')
+            ->with(GlobalSection::VAR_MIN_LOGGING_LEVEL)
+            ->willReturn($minLevelOverride);
+        $this->levelResolverMock
+            ->method('resolve')
+            ->willReturnMap([
+                [LogConfig::LEVEL_NOTICE, Logger::NOTICE],
+                [LogConfig::LEVEL_INFO, Logger::INFO],
+                [LogConfig::LEVEL_WARNING, Logger::WARNING],
+            ]);
 
         $handlerInstance = $this->handlerFactory->create($handler);
 
         $this->assertInstanceOf(HandlerInterface::class, $handlerInstance);
         $this->assertInstanceOf($expectedClass, $handlerInstance);
+        $this->assertSame($expectedLevel, $handlerInstance->getLevel());
     }
 
     /**
      * @return array
+     * @SuppressWarnings(PHPMD.ExcessiveMethodLength)
      */
     public function createDataProvider()
     {
@@ -156,7 +179,19 @@ public function createDataProvider()
                     ['min_level', LogConfig::LEVEL_NOTICE, LogConfig::LEVEL_NOTICE],
                     ['min_level', LogConfig::LEVEL_INFO, LogConfig::LEVEL_INFO],
                 ],
+                'minLevelOverride' => '',
+                'expectedClass' => StreamHandler::class,
+                'expectedLevel' => Logger::INFO,
+            ],
+            [
+                'handler' => HandlerFactory::HANDLER_STREAM,
+                'repositoryMockReturnMap' => [
+                    ['stream', null, 'php://stdout'],
+                    ['min_level', LogConfig::LEVEL_WARNING, LogConfig::LEVEL_WARNING],
+                ],
+                'minLevelOverride' => LogConfig::LEVEL_WARNING,
                 'expectedClass' => StreamHandler::class,
+                'expectedLevel' => Logger::WARNING,
             ],
             [
                 'handler' => HandlerFactory::HANDLER_FILE,
@@ -165,7 +200,9 @@ public function createDataProvider()
                     ['min_level', LogConfig::LEVEL_NOTICE, LogConfig::LEVEL_NOTICE],
                     ['min_level', LogConfig::LEVEL_INFO, LogConfig::LEVEL_INFO],
                 ],
+                'minLevelOverride' => '',
                 'expectedClass' => StreamHandler::class,
+                'expectedLevel' => Logger::INFO,
             ],
             [
                 'handler' => HandlerFactory::HANDLER_SLACK,
@@ -176,7 +213,21 @@ public function createDataProvider()
                     ['min_level', LogConfig::LEVEL_NOTICE, LogConfig::LEVEL_NOTICE],
                     ['min_level', LogConfig::LEVEL_INFO, LogConfig::LEVEL_INFO],
                 ],
+                'minLevelOverride' => '',
                 'expectedClass' => SlackHandler::class,
+                'expectedLevel' => Logger::NOTICE,
+            ],
+            [
+                'handler' => HandlerFactory::HANDLER_SLACK,
+                'repositoryMockReturnMap' => [
+                    ['token', null, 'someToken'],
+                    ['channel', 'general', 'someChannel'],
+                    ['username', 'Slack Log Notifier', 'someUser'],
+                    ['min_level', LogConfig::LEVEL_WARNING, LogConfig::LEVEL_WARNING],
+                ],
+                'minLevelOverride' => LogConfig::LEVEL_WARNING,
+                'expectedClass' => SlackHandler::class,
+                'expectedLevel' => Logger::WARNING,
             ],
             [
                 'handler' => HandlerFactory::HANDLER_EMAIL,
@@ -187,7 +238,9 @@ public function createDataProvider()
                     ['min_level', LogConfig::LEVEL_NOTICE, LogConfig::LEVEL_NOTICE],
                     ['min_level', LogConfig::LEVEL_INFO, LogConfig::LEVEL_INFO],
                 ],
+                'minLevelOverride' => '',
                 'expectedClass' => NativeMailerHandler::class,
+                'expectedLevel' => Logger::NOTICE,
             ],
             [
                 'handler' => HandlerFactory::HANDLER_SYSLOG,
@@ -199,7 +252,9 @@ public function createDataProvider()
                     ['min_level', LogConfig::LEVEL_NOTICE, LogConfig::LEVEL_NOTICE],
                     ['min_level', LogConfig::LEVEL_INFO, LogConfig::LEVEL_INFO],
                 ],
+                'minLevelOverride' => '',
                 'expectedClass' => SyslogHandler::class,
+                'expectedLevel' => Logger::NOTICE,
             ],
             [
                 'handler' => HandlerFactory::HANDLER_SYSLOG_UDP,
@@ -212,8 +267,10 @@ public function createDataProvider()
                     ['min_level', LogConfig::LEVEL_NOTICE, LogConfig::LEVEL_NOTICE],
                     ['min_level', LogConfig::LEVEL_INFO, LogConfig::LEVEL_INFO],
                 ],
+                'minLevelOverride' => '',
                 'expectedClass' => SyslogUdpHandler::class,
-            ]
+                'expectedLevel' => Logger::NOTICE,
+            ],
         ];
     }
 }
diff --git a/src/Test/Unit/App/Logger/LevelResolverTest.php b/src/Test/Unit/App/Logger/LevelResolverTest.php
@@ -72,29 +72,4 @@ public function resolveDataProvider(): array
             ['level' => 'invalid', Logger::NOTICE],
         ];
     }
-
-    /**
-     * @param string $level
-     * @param int $expectedResult
-     * @dataProvider resolveDataProvider
-     * @dataProvider resolveOverrideDataProvider
-     */
-    public function testResolveOverride(string $level, int $expectedResult)
-    {
-        $this->environmentMock
-            ->method('getMinLoggingLevel')
-            ->willReturn($level);
-
-        $this->assertSame($expectedResult, $this->levelResolver->resolve('some level'));
-    }
-
-    /**
-     * @return array
-     */
-    public function resolveOverrideDataProvider(): array
-    {
-        return [
-            ['level' => '', Logger::NOTICE],
-        ];
-    }
 }
diff --git a/src/Test/Unit/Config/SchemaTest.php b/src/Test/Unit/Config/SchemaTest.php
@@ -94,6 +94,7 @@ public function testGetDefaultsForGlobalSection()
                 StageConfigInterface::VAR_SKIP_HTML_MINIFICATION => false,
                 StageConfigInterface::VAR_DEPLOYED_MAGENTO_VERSION_FROM_GIT => false,
                 StageConfigInterface::VAR_DEPLOY_FROM_GIT_OPTIONS => [],
+                StageConfigInterface::VAR_MIN_LOGGING_LEVEL => '',
             ],
             $this->schema->getDefaults(StageConfigInterface::STAGE_GLOBAL)
         );
diff --git a/tests/unit/code-coverage.php b/tests/unit/code-coverage.php
@@ -3,7 +3,7 @@
  * @author Marco Pivetta
  * @link https://ocramius.github.io/blog/automated-code-coverage-check-for-github-pull-requests-with-travis/
  */
-const MIN_COVERAGE = 90;
+const MIN_COVERAGE = 91;
 
 $inputFile = $argv[1];
 $percentage = min(100, max(0, MIN_COVERAGE));
