Revision compiler revised (#2805)

- revisions now use <asset>.<type>?v=<revision> instead of <asset>-<revision>.<type>- remove deprecated filename for revision method
- reconsider use of cache differentiator and implement something that
prevents recompiling css every single time
- allow force recompilation

src/Frontend/Compiler/CompilerInterface.php

@@ -11,26 +11,14 @@ namespace Flarum\Frontend\Compiler;
 
 interface CompilerInterface
 {
-    /**
-     * @return string
-     */
     public function getFilename(): string;
 
-    /**
-     * @param string $filename
-     */
     public function setFilename(string $filename);
 
-    /**
-     * @param callable $callback
-     */
     public function addSources(callable $callback);
 
-    public function commit();
+    public function commit(bool $force = false);
 
-    /**
-     * @return string|null
-     */
     public function getUrl(): ?string;
 
     public function flush();

src/Frontend/Compiler/JsCompiler.php

@@ -14,9 +14,6 @@ use Flarum\Frontend\Compiler\Source\FileSource;
 
 class JsCompiler extends RevisionCompiler
 {
-    /**
-     * {@inheritdoc}
-     */
     protected function save(string $file, array $sources): bool
     {
         if (empty($sources)) {
@@ -60,9 +57,6 @@ class JsCompiler extends RevisionCompiler
         return true;
     }
 
-    /**
-     * {@inheritdoc}
-     */
     protected function format(string $string): string
     {
         return preg_replace('~//# sourceMappingURL.*$~m', '', $string)."\n";

src/Frontend/Compiler/LessCompiler.php

@@ -24,33 +24,21 @@ class LessCompiler extends RevisionCompiler
      */
     protected $importDirs = [];
 
-    /**
-     * @return string
-     */
     public function getCacheDir(): string
     {
         return $this->cacheDir;
     }
 
-    /**
-     * @param string $cacheDir
-     */
     public function setCacheDir(string $cacheDir)
     {
         $this->cacheDir = $cacheDir;
     }
 
-    /**
-     * @return array
-     */
     public function getImportDirs(): array
     {
         return $this->importDirs;
     }
 
-    /**
-     * @param array $importDirs
-     */
     public function setImportDirs(array $importDirs)
     {
         $this->importDirs = $importDirs;
@@ -84,11 +72,10 @@ class LessCompiler extends RevisionCompiler
         return $parser->getCss();
     }
 
-    /**
-     * @return mixed
-     */
-    protected function getCacheDifferentiator()
+    protected function getCacheDifferentiator(): ?array
     {
-        return time();
+        return [
+            'import_dirs' => $this->importDirs
+        ];
     }
 }

src/Frontend/Compiler/RevisionCompiler.php

@@ -45,26 +45,17 @@ class RevisionCompiler implements CompilerInterface
         $this->filename = $filename;
     }
 
-    /**
-     * {@inheritdoc}
-     */
     public function getFilename(): string
     {
         return $this->filename;
     }
 
-    /**
-     * {@inheritdoc}
-     */
     public function setFilename(string $filename)
     {
         $this->filename = $filename;
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function commit()
+    public function commit(bool $force = false)
     {
         $sources = $this->getSources();
 
@@ -72,12 +63,10 @@ class RevisionCompiler implements CompilerInterface
 
         $newRevision = $this->calculateRevision($sources);
 
-        $oldFile = $oldRevision ? $this->getFilenameForRevision($oldRevision) : null;
-
-        if ($oldRevision !== $newRevision || ($oldFile && ! $this->assetsDir->has($oldFile))) {
-            $newFile = $this->getFilenameForRevision($newRevision);
-
-            if (! $this->save($newFile, $sources)) {
+        // In case the previous and current revisions do not match
+        // Or no file was written yet, let's save the file to disk.
+        if ($force || $oldRevision !== $newRevision || ! $this->assetsDir->has($this->filename)) {
+            if (! $this->save($this->filename, $sources)) {
                 // If no file was written (because the sources were empty), we
                 // will set the revision to a special value so that we can tell
                 // that this file does not have a URL.
@@ -85,16 +74,9 @@ class RevisionCompiler implements CompilerInterface
             }
 
             $this->putRevision($newRevision);
-
-            if ($oldFile && $oldFile !== $newFile) {
-                $this->delete($oldFile);
-            }
         }
     }
 
-    /**
-     * {@inheritdoc}
-     */
     public function addSources(callable $callback)
     {
         $this->sourcesCallbacks[] = $callback;
@@ -103,7 +85,7 @@ class RevisionCompiler implements CompilerInterface
     /**
      * @return SourceInterface[]
      */
-    protected function getSources()
+    protected function getSources(): array
     {
         $sources = new SourceCollector;
 
@@ -114,9 +96,6 @@ class RevisionCompiler implements CompilerInterface
         return $sources->getSources();
     }
 
-    /**
-     * {@inheritdoc}
-     */
     public function getUrl(): ?string
     {
         $revision = $this->getRevision();
@@ -135,9 +114,11 @@ class RevisionCompiler implements CompilerInterface
             return null;
         }
 
-        $file = $this->getFilenameForRevision($revision);
+        $url = $this->assetsDir->url($this->filename);
 
-        return $this->assetsDir->url($file);
+        // Append revision as GET param to signify that there's been
+        // a change to the file and it should be refreshed.
+        return "$url?v=$revision";
     }
 
     /**
@@ -180,22 +161,6 @@ class RevisionCompiler implements CompilerInterface
         return $string;
     }
 
-    /**
-     * Get the filename for the given revision.
-     *
-     * @param string $revision
-     * @return string
-     */
-    protected function getFilenameForRevision(string $revision): string
-    {
-        $ext = pathinfo($this->filename, PATHINFO_EXTENSION);
-
-        return substr_replace($this->filename, '-'.$revision, -strlen($ext) - 1, 0);
-    }
-
-    /**
-     * @return string|null
-     */
     protected function getRevision(): ?string
     {
         if ($this->assetsDir->has(static::REV_MANIFEST)) {
@@ -207,9 +172,6 @@ class RevisionCompiler implements CompilerInterface
         return null;
     }
 
-    /**
-     * @param string|null $revision
-     */
     protected function putRevision(?string $revision)
     {
         if ($this->assetsDir->has(static::REV_MANIFEST)) {
@@ -242,22 +204,15 @@ class RevisionCompiler implements CompilerInterface
         return hash('crc32b', serialize($cacheDifferentiator));
     }
 
-    /**
-     * @return mixed
-     */
-    protected function getCacheDifferentiator()
+    protected function getCacheDifferentiator(): ?array
     {
+        return null;
     }
 
-    /**
-     * {@inheritdoc}
-     */
     public function flush()
     {
-        if ($revision = $this->getRevision()) {
-            $file = $this->getFilenameForRevision($revision);
-
-            $this->delete($file);
+        if ($this->getRevision() !== null) {
+            $this->delete($this->filename);
 
             $this->putRevision(null);
         }

src/Frontend/Content/Assets.php

@@ -49,17 +49,23 @@ class Assets
         ];
 
         if ($this->config->inDebugMode()) {
-            $this->commit(Arr::flatten($compilers));
+            $this->forceCommit(Arr::flatten($compilers));
         }
 
         $document->js = array_merge($document->js, $this->getUrls($compilers['js']));
         $document->css = array_merge($document->css, $this->getUrls($compilers['css']));
     }
 
-    private function commit(array $compilers)
+    /**
+     * Force compilation of assets when in debug mode.
+     *
+     * @param array $compilers
+     */
+    private function forceCommit(array $compilers)
     {
+        /** @var CompilerInterface $compiler */
         foreach ($compilers as $compiler) {
-            $compiler->commit();
+            $compiler->commit(true);
         }
     }