Merge pull request #164 from solcre/bugfix/duplicate-filters

Bugfix/duplicate filters

Author: RWOverdijk

src/AssetManager/Service/AssetFilterManager.php

@@ -25,7 +25,12 @@ class AssetFilterManager implements ServiceLocatorAwareInterface, MimeResolverAw
      * @var MimeResolver
      */
     protected $mimeResolver;
-
+    
+    /**
+     * @var FilterInterface[] Filters already instantiated
+     */
+    protected $filterInstances = array();
+    
     /**
      * Construct the AssetFilterManager
      *
@@ -145,7 +150,11 @@ protected function ensureByFilter(AssetInterface $asset, $filter)
             );
         }
 
-        $filterInstance = new $filterClass;
+        if (!isset($this->filterInstances[$filterClass])) {
+            $this->filterInstances[$filterClass] = new $filterClass();
+        }
+
+        $filterInstance = $this->filterInstances[$filterClass];
 
         $asset->ensureFilter($filterInstance);
     }

tests/AssetManagerTest/Service/AssetFilterManagerTest.php

@@ -3,6 +3,7 @@
 namespace AssetManagerTest\Service;
 
 use Assetic\Asset\StringAsset;
+use Assetic\Filter\FilterInterface;
 use PHPUnit_Framework_TestCase;
 use AssetManager\Service\AssetFilterManager;
 use Zend\ServiceManager\ServiceManager;
@@ -93,4 +94,31 @@ public function testensureByInvalid()
 
         $assetFilterManager->setFilters('test/path.test', $asset);
     }
+    
+    public function testFiltersAreInstantiatedOnce()
+    {
+        $assetFilterManager = new AssetFilterManager(array(
+            'test/path.test' => array(
+                array(
+                    'filter' => 'CustomFilter'
+                ),
+            ),
+        ));
+        
+        $filterInstance = null;
+        
+        $asset = $this->getMock('Assetic\Asset\AssetInterface');
+        $asset
+            ->expects($this->any())
+            ->method('ensureFilter')
+            ->with($this->callback(function (FilterInterface $filter) use (&$filterInstance) {
+                if ($filterInstance === null) {
+                    $filterInstance = $filter;
+                }
+                return  $filter === $filterInstance;
+            }));
+        
+        $assetFilterManager->setFilters('test/path.test', $asset);
+        $assetFilterManager->setFilters('test/path.test', $asset);
+    }
 }

tests/AssetManagerTest/Service/AssetManagerTest.php

@@ -9,6 +9,8 @@
 use AssetManager\Cache\FilePathCache;
 use AssetManager\Service\AssetManager;
 use AssetManager\Service\MimeResolver;
+use AssetManager\Resolver\CollectionResolver;
+use AssetManager\Resolver\AggregateResolver;
 use Zend\Http\Response;
 use Zend\Http\PhpEnvironment\Request;
 use Zend\Console\Request as ConsoleRequest;
@@ -23,6 +25,7 @@ public function setUp()
         require_once __DIR__ . '/../../_files/JSMin.inc';
         require_once __DIR__ . '/../../_files/CustomFilter.php';
         require_once __DIR__ . '/../../_files/BrokenFilter.php';
+        require_once __DIR__ . '/../../_files/ReverseFilter.php';
     }
 
     protected function getRequest()
@@ -54,6 +57,24 @@ protected function getResolver($resolveTo = __FILE__)
         return $resolver;
     }
 
+    public function getCollectionResolver()
+    {
+        $aggregateResolver  = new AggregateResolver;
+        $mockedResolver     = $this->getResolver(__DIR__ . '/../../_files/require-jquery.js');
+        $collArr = array(
+            'blah.js' => array(
+                'asset-path'
+            )
+        );
+        $resolver = new CollectionResolver($collArr);
+        $resolver->setAggregateResolver($aggregateResolver);
+
+        $aggregateResolver->attach($mockedResolver, 500);
+        $aggregateResolver->attach($resolver, 1000);
+
+        return $resolver;
+    }
+
     public function testConstruct()
     {
         $resolver     = $this->getMock('AssetManager\Resolver\ResolverInterface');
@@ -209,6 +230,41 @@ public function testSetExtensionFilters()
         $this->assertEquals($minified, $response->getBody());
     }
 
+    public function testSetExtensionFiltersNotDuplicate()
+    {
+        $config = array(
+            'filters' => array(
+                'js' => array(
+                    array(
+                        'filter' => '\ReverseFilter',
+                    ),
+                ),
+            ),
+        );
+
+        $resolver           = $this->getCollectionResolver();
+        $assetFilterManager = new AssetFilterManager($config['filters']);
+        $mimeResolver       = new MimeResolver;
+        $assetFilterManager->setMimeResolver($mimeResolver);
+        $resolver->setAssetFilterManager($assetFilterManager);
+        
+        $response     = new Response;
+        $request      = $this->getRequest();
+        // Have to change uri because asset-path would cause an infinite loop
+        $request->setUri('http://localhost/base-path/blah.js');
+        
+        $assetCacheManager = $this->getAssetCacheManagerMock();
+        $assetManager      = new AssetManager($resolver->getAggregateResolver(), $config);
+        $assetManager->setAssetCacheManager($assetCacheManager);
+        $assetManager->setAssetFilterManager($assetFilterManager);
+
+        $this->assertTrue($assetManager->resolvesToAsset($request));
+        $assetManager->setAssetOnResponse($response);
+
+        $reversedOnlyOnce = '1' . strrev(file_get_contents(__DIR__ . '/../../_files/require-jquery.js'));
+        $this->assertEquals($reversedOnlyOnce, $response->getBody());
+    }
+
     public function testSetMimeTypeFilters()
     {
         $config = array(
@@ -323,7 +379,6 @@ public function testSetFalseClassFilter()
 
     public function testSetAssetOnResponse()
     {
-
         $assetFilterManager = new AssetFilterManager();
         $assetCacheManager  = $this->getAssetCacheManagerMock();
         $mimeResolver       = new MimeResolver;

tests/_files/ReverseFilter.php

@@ -0,0 +1,19 @@
+<?php
+
+
+use Assetic\Asset\AssetInterface;
+
+class ReverseFilter implements Assetic\Filter\FilterInterface
+{
+    private static $executed;
+    public function filterLoad(AssetInterface $asset)
+    {
+    }
+
+    public function filterDump(AssetInterface $asset)
+    {
+        self::$executed++;
+        $content = $asset->getContent();
+        $asset->setContent(self::$executed . strrev($content));
+    }
+}