feature #27421 CacheWarmerAggregate handle deprecations logs (ScullWM)

nicolas-grekas · nicolas-grekas · commit 4fde3bb4f39e · 2018-06-19T15:15:00.000+02:00
This PR was merged into the 4.2-dev branch. Discussion ---------- CacheWarmerAggregate handle deprecations logs | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes  | Fixed tickets | #27387 | License | MIT Actually the Web Debug Toolbar warning you about the deprecation messages thrown during the container built (#21502). Cache warmup can throw deprecated message without any persistance, it may cause issue like #27387 This PR reproduce the same job for the cache warmer, and so on, handle deprecated messages during the warmup of Twig, Translator, Validator, Security and all `kernel.cache_warmer` services. Here are the point that may be improvable in this PR: 1. Actually I've "duplicate" the callable used in the `set_error_handler` of the Kernel. IMHO I think that Kernel and CacheWarmerAggregate have differents jobs and a trait may be a good solution to share this error handler setter without duplicating the code, but I'm a little bit lost about the repercussion of adding a Trait in the Kernel. 2. I've think about extending the `CacheWarmerAggregate` into a `DeprecatedLogHandlingCacheWarmerAggregate` to add the debug and containerClass argument, and declare it as the `cache_warmer` service only in debug mode (by declaring it in the DebugBundle/Resources/config/services.xml). Commits ------- f03b8bb CacheWarmerAggregate handle deprecations logs
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/services.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/services.xml
@@ -32,6 +32,8 @@
 
         <service id="cache_warmer" class="Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerAggregate" public="true">
             <argument type="tagged" tag="kernel.cache_warmer" />
+            <argument>%kernel.debug%</argument>
+            <argument>%kernel.cache_dir%/%kernel.container_class%Deprecations.log</argument>
         </service>
 
         <service id="cache_clearer" class="Symfony\Component\HttpKernel\CacheClearer\ChainCacheClearer" public="true">
diff --git a/src/Symfony/Component/HttpKernel/CacheWarmer/CacheWarmerAggregate.php b/src/Symfony/Component/HttpKernel/CacheWarmer/CacheWarmerAggregate.php
@@ -21,12 +21,16 @@
 class CacheWarmerAggregate implements CacheWarmerInterface
 {
     private $warmers;
+    private $debug;
+    private $deprecationLogsFilepath;
     private $optionalsEnabled = false;
     private $onlyOptionalsEnabled = false;
 
-    public function __construct(iterable $warmers = array())
+    public function __construct(iterable $warmers = array(), bool $debug = false, string $deprecationLogsFilepath = null)
     {
         $this->warmers = $warmers;
+        $this->debug = $debug;
+        $this->deprecationLogsFilepath = $deprecationLogsFilepath;
     }
 
     public function enableOptionalWarmers()
@@ -46,15 +50,62 @@ public function enableOnlyOptionalWarmers()
      */
     public function warmUp($cacheDir)
     {
-        foreach ($this->warmers as $warmer) {
-            if (!$this->optionalsEnabled && $warmer->isOptional()) {
-                continue;
-            }
-            if ($this->onlyOptionalsEnabled && !$warmer->isOptional()) {
-                continue;
+        if ($this->debug) {
+            $collectedLogs = array();
+            $previousHandler = defined('PHPUNIT_COMPOSER_INSTALL');
+            $previousHandler = $previousHandler ?: set_error_handler(function ($type, $message, $file, $line) use (&$collectedLogs, &$previousHandler) {
+                if (E_USER_DEPRECATED !== $type && E_DEPRECATED !== $type) {
+                    return $previousHandler ? $previousHandler($type, $message, $file, $line) : false;
+                }
+
+                if (isset($collectedLogs[$message])) {
+                    ++$collectedLogs[$message]['count'];
+
+                    return;
+                }
+
+                $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 3);
+                // Clean the trace by removing first frames added by the error handler itself.
+                for ($i = 0; isset($backtrace[$i]); ++$i) {
+                    if (isset($backtrace[$i]['file'], $backtrace[$i]['line']) && $backtrace[$i]['line'] === $line && $backtrace[$i]['file'] === $file) {
+                        $backtrace = array_slice($backtrace, 1 + $i);
+                        break;
+                    }
+                }
+
+                $collectedLogs[$message] = array(
+                    'type' => $type,
+                    'message' => $message,
+                    'file' => $file,
+                    'line' => $line,
+                    'trace' => $backtrace,
+                    'count' => 1,
+                );
+            });
+        }
+
+        try {
+            foreach ($this->warmers as $warmer) {
+                if (!$this->optionalsEnabled && $warmer->isOptional()) {
+                    continue;
+                }
+                if ($this->onlyOptionalsEnabled && !$warmer->isOptional()) {
+                    continue;
+                }
+
+                $warmer->warmUp($cacheDir);
             }
+        } finally {
+            if ($this->debug && true !== $previousHandler) {
+                restore_error_handler();
 
-            $warmer->warmUp($cacheDir);
+                if (file_exists($this->deprecationLogsFilepath)) {
+                    $previousLogs = unserialize(file_get_contents($this->deprecationLogsFilepath));
+                    $collectedLogs = array_merge($previousLogs, $collectedLogs);
+                }
+
+                file_put_contents($this->deprecationLogsFilepath, serialize(array_values($collectedLogs)));
+            }
         }
     }
 
