From e9d12e943c41d8d7664c28d80ed78be3a5cf805d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Auswo=CC=88ger?= Date: Mon, 23 Jan 2017 14:12:09 +0100 Subject: [PATCH 1/5] Use the contao.web_dir parameter in the Combiner --- .../contao/library/Contao/Combiner.php | 40 ++++++++++++++----- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/src/Resources/contao/library/Contao/Combiner.php b/src/Resources/contao/library/Contao/Combiner.php index 19c41582b9..aed754d0c7 100644 --- a/src/Resources/contao/library/Contao/Combiner.php +++ b/src/Resources/contao/library/Contao/Combiner.php @@ -73,12 +73,29 @@ class Combiner extends \System */ protected $arrFiles = array(); + /** + * Webdir relative to TL_ROOT + * @var string + */ + protected $strWebDir; + /** * Public constructor required */ public function __construct() { + $strWebDir = \System::getContainer()->getParameter('contao.web_dir'); + + if (strncmp($strWebDir, TL_ROOT . '/', strlen(TL_ROOT . '/')) === 0) + { + $this->strWebDir = substr($strWebDir, strlen(TL_ROOT . '/')) . '/'; + } + else + { + throw new \InvalidArgumentException(sprintf('Webdir is not inside TL_ROOT "%s"', $strWebDir)); + } + parent::__construct(); } @@ -115,19 +132,14 @@ public function add($strFile, $strVersion=null, $strMedia='all') throw new \LogicException('You cannot mix different file types. Create another Combiner object instead.'); } - // Prevent duplicates - if (isset($this->arrFiles[$strFile])) - { - return; - } - // Check the source file if (!file_exists(TL_ROOT . '/' . $strFile)) { - // Handle public bundle resources - if (file_exists(TL_ROOT . '/web/' . $strFile)) + // Handle public bundle resources in web/ + if (file_exists(TL_ROOT . '/' . $this->strWebDir . $strFile)) { - $strFile = 'web/' . $strFile; + @trigger_error('Paths relative to the webdir are deprecated and will no longer work in Contao 5.0.', E_USER_DEPRECATED); + $strFile = $this->strWebDir . $strFile; } else { @@ -135,6 +147,12 @@ public function add($strFile, $strVersion=null, $strMedia='all') } } + // Prevent duplicates + if (isset($this->arrFiles[$strFile])) + { + return; + } + // Default version if ($strVersion === null) { @@ -212,9 +230,9 @@ public function getFileUrls() $name = $arrFile['name']; // Strip the web/ prefix (see #328) - if (strncmp($name, 'web/', 4) === 0) + if (strncmp($name, $this->strWebDir, strlen($this->strWebDir)) === 0) { - $name = substr($name, 4); + $name = substr($name, strlen($this->strWebDir)); } // Add the media query (see #7070) From 5105e90d83268938461c670bf0c63601966db3e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Auswo=CC=88ger?= Date: Mon, 23 Jan 2017 15:52:18 +0100 Subject: [PATCH 2/5] Add tests for Combiner --- .../contao/library/Contao/Combiner.php | 2 +- tests/Contao/CombinerTest.php | 214 ++++++++++++++++++ tests/TestCase.php | 1 + 3 files changed, 216 insertions(+), 1 deletion(-) create mode 100644 tests/Contao/CombinerTest.php diff --git a/src/Resources/contao/library/Contao/Combiner.php b/src/Resources/contao/library/Contao/Combiner.php index aed754d0c7..78613aeb4f 100644 --- a/src/Resources/contao/library/Contao/Combiner.php +++ b/src/Resources/contao/library/Contao/Combiner.php @@ -93,7 +93,7 @@ public function __construct() } else { - throw new \InvalidArgumentException(sprintf('Webdir is not inside TL_ROOT "%s"', $strWebDir)); + throw new \RuntimeException(sprintf('Webdir is not inside TL_ROOT "%s"', $strWebDir)); } parent::__construct(); diff --git a/tests/Contao/CombinerTest.php b/tests/Contao/CombinerTest.php new file mode 100644 index 0000000000..201c007fef --- /dev/null +++ b/tests/Contao/CombinerTest.php @@ -0,0 +1,214 @@ + + * + * @runTestsInSeparateProcesses + * @preserveGlobalState disabled + * @group legacy + */ +class CombinerTest extends TestCase +{ + /** + * @var string + */ + private static $rootDir; + + /** + * @var ContainerInterface + */ + private $container; + + /** + * {@inheritdoc} + */ + public static function setUpBeforeClass() + { + self::$rootDir = __DIR__.'/../../tmp'; + + $fs = new Filesystem(); + $fs->mkdir(self::$rootDir); + $fs->mkdir(self::$rootDir.'/assets'); + $fs->mkdir(self::$rootDir.'/assets/css'); + $fs->mkdir(self::$rootDir.'/system'); + $fs->mkdir(self::$rootDir.'/system/tmp'); + $fs->mkdir(self::$rootDir.'/web'); + } + + /** + * {@inheritdoc} + */ + public static function tearDownAfterClass() + { + $fs = new Filesystem(); + $fs->remove(self::$rootDir); + } + + /** + * {@inheritdoc} + */ + protected function setUp() + { + parent::setUp(); + + define('TL_ERROR', 'ERROR'); + define('TL_ROOT', self::$rootDir); + define('TL_ASSETS_URL', ''); + + $this->container = $this->mockContainerWithContaoScopes(); + $this->container->setParameter('contao.web_dir', self::$rootDir . '/web'); + + System::setContainer($this->container); + } + + /** + * Tests the object instantiation. + */ + public function testConstruct() + { + $this->assertInstanceOf('Contao\Combiner', new Combiner()); + } + + /** + * Tests the object instantiation with a web dir outside TL_ROOT. + */ + public function testWebDirOutsideRoot() + { + $this->container->setParameter('contao.web_dir', '/not/in/root/dir'); + + $this->setExpectedException('RuntimeException', '/not/in/root/dir'); + + new Combiner(); + } + + /** + * Tests the CSS combiner. + */ + public function testCombineCss() + { + file_put_contents(static::$rootDir.'/file1.css', 'file1 { background: url("foo.bar") }'); + file_put_contents(static::$rootDir.'/web/file2.css', 'web/file2'); + file_put_contents(static::$rootDir.'/file3.css', 'file3'); + file_put_contents(static::$rootDir.'/web/file3.css', 'web/file3'); + + $combiner = new Combiner(); + $combiner->add('file1.css'); + $combiner->addMultiple(['file2.css', 'file3.css']); + + $this->assertEquals([ + 'file1.css', + 'file2.css" media="screen', + 'file3.css" media="screen', + ], $combiner->getFileUrls()); + + $combinedFile = $combiner->getCombinedFile(); + + $this->assertRegExp('/^assets\/css\/[a-z0-9]+\.css$/', $combinedFile); + + $this->assertEquals( + "file1 { background: url(\"../../foo.bar\") }\n@media screen{\nweb/file2\n}\n@media screen{\nfile3\n}\n", + file_get_contents(static::$rootDir.'/'.$combinedFile) + ); + + Config::set('debugMode', true); + + $markup = $combiner->getCombinedFile(); + + $this->assertEquals( + 'file1.css">add('file1.scss'); + $combiner->add('file2.scss'); + + $this->assertEquals([ + 'assets/css/file1.scss.css', + 'assets/css/file2.scss.css', + ], $combiner->getFileUrls()); + + $combinedFile = $combiner->getCombinedFile(); + + $this->assertRegExp('/^assets\/css\/[a-z0-9]+\.css$/', $combinedFile); + + $this->assertEquals( + "body{color:red}\nbody{color:green}\n", + file_get_contents(static::$rootDir.'/'.$combinedFile) + ); + + Config::set('debugMode', true); + + $markup = $combiner->getCombinedFile(); + + $this->assertEquals( + 'assets/css/file1.scss.css">getCombinedFile(); + + $this->assertEquals( + 'file1.js">