API: ModulePath template global now takes any composer package name.

NEW: URL generation now handled by pluggable ResourceURLGenerator service.
NEW: Requirements::javascript() and Requirements::css() now support “vendor/package:resource” syntax.

These changes will make it easier to us to fully abstract:
 - file access from module location
 - file location from URL generation

API: ModulePath template global now takes any composer package name.
NEW: URL generation now handled by pluggable ResourceURLGenerator service.
NEW: Requirements::javascript() and Requirements::css() now support “vendor/package:resource” syntax.

These changes will make it easier to us to fully abstract:
 - file access from module location
 - file location from URL generation

_config/config.yml

@@ -6,6 +6,10 @@ SilverStripe\Core\Injector\Injector:
     class: SilverStripe\Forms\Schema\FormSchema
   FixtureFactory:
     class: SilverStripe\Dev\FixtureFactory
+  SilverStripe\Core\Manifest\ResourceURLGenerator:
+    class: SilverStripe\Control\SimpleResourceURLGenerator
+    properties:
+      NonceStyle: mtime
 SilverStripe\Control\HTTP:
   cache_control:
     max-age: 0

src/Control/SimpleResourceURLGenerator.php

@@ -0,0 +1,70 @@
+<?php
+
+namespace SilverStripe\Control;
+
+use SilverStripe\Core\Manifest\ResourceURLGenerator;
+
+/**
+ * Generate URLs assuming that BASE_PATH is also the webroot
+ * Standard SilverStripe 3 operation
+ */
+class SimpleResourceURLGenerator implements ResourceURLGenerator
+{
+    /*
+     * @var string
+     */
+    private $nonceStyle;
+
+    /*
+     * Get the style of nonce-suffixes to use, or null if disabled
+     *
+     * @return string|null
+     */
+    public function getNonceStyle()
+    {
+        return $this->nonceStyle;
+    }
+
+    /*
+     * Set the style of nonce-suffixes to use, or null to disable
+     * Currently only "mtime" is allowed
+     *
+     * @param string|null $nonceStyle The style of nonces to apply, or null to disable
+     */
+    public function setNonceStyle($nonceStyle)
+    {
+        if ($nonceStyle && $nonceStyle !== 'mtime') {
+            throw new InvalidArgumentException('The only allowed NonceStyle is mtime');
+        }
+        $this->nonceStyle = $nonceStyle;
+    }
+
+    /**
+     * Return the URL for a resource, prefixing with Director::baseURL() and suffixing with a nonce
+     *
+     * @param string $relativePath File or directory path relative to BASE_PATH
+     * @return string Doman-relative URL
+     * @throws InvalidArgumentException If the resource doesn't exist
+     */
+    public function urlForResource($relativePath)
+    {
+        $absolutePath = preg_replace('/\?.*/', '', Director::baseFolder() . '/' . $relativePath);
+
+        if (!file_exists($absolutePath)) {
+            throw new InvalidArgumentException("File {$relativePath} does not exist");
+        }
+
+        $nonce = '';
+        if ($this->nonceStyle) {
+            $nonce = (strpos($relativePath, '?') === false) ? '?' : '&';
+
+            switch ($this->nonceStyle) {
+                case 'mtime':
+                    $nonce .= "m=" . filemtime($absolutePath);
+                    break;
+            }
+        }
+
+        return Director::baseURL() . $relativePath . $nonce;
+    }
+}

src/Core/Manifest/Module.php

@@ -4,6 +4,7 @@ namespace SilverStripe\Core\Manifest;
 
 use Exception;
 use Serializable;
+use SilverStripe\Core\Injector\Injector;
 
 class Module implements Serializable
 {
@@ -161,13 +162,44 @@ class Module implements Serializable
      * @param string $path File or directory path relative to module directory
      * @return string Path relative to base directory
      */
-    public function getResourcePath($path)
+    public function getRelativeResourcePath($path)
     {
         $base = trim($this->getRelativePath(), '/\\');
         $path = trim($path, '/\\');
         return trim("{$base}/{$path}", '/\\');
     }
 
+    /**
+     * Gets path to physical file resource relative to base directory.
+     * Directories included
+     *
+     * This method makes no distinction between public / local resources,
+     * which may change in the near future.
+     *
+     * @internal Experimental API and may change
+     * @param string $path File or directory path relative to module directory
+     * @return string Path relative to base directory
+     */
+    public function getResourcePath($path)
+    {
+        return $this->basePath . '/' . $this->getRelativeResourcePath($path);
+    }
+
+    /**
+     * Gets the URL for a given resource.
+     * Relies on the ModuleURLGenerator Injector service to do the heavy lifting
+     *
+     * @internal Experimental API and may change
+     * @param string $path File or directory path relative to module directory
+     * @return string URL, either domain-relative (starting with /) or absolute
+     */
+    public function getResourceURL($path)
+    {
+        return Injector::inst()
+            ->get(ResourceURLGenerator::class)
+            ->urlForResource($this->getRelativeResourcePath($path));
+    }
+
     /**
      * Check if this module has a given resource
      *
@@ -177,7 +209,6 @@ class Module implements Serializable
      */
     public function hasResource($path)
     {
-        $resource = $this->getResourcePath($path);
+        return file_exists($this->getResourcePath($path));
-        return file_exists($this->basePath . '/' . $resource);
     }
 }

src/Core/Manifest/ResourceURLGenerator.php

@@ -0,0 +1,23 @@
+<?php
+
+namespace SilverStripe\Core\Manifest;
+
+use InvalidArgumentException;
+
+/**
+ * Generate URLs for client-side assets and perform any preparation of those assets needed.
+ */
+interface ResourceURLGenerator
+{
+    /**
+     * Return the URL for a given resource within the project.
+     *
+     * As well as returning the URL, this method may also perform any changes needed to ensure that this
+     * URL will resolve, for example, by copying files to another location
+     *
+     * @param string $resource File or directory path relative to BASE_PATH
+     * @return string URL, either domain-relative (starting with /) or absolute
+     * @throws InvalidArgumentException If the resource doesn't exist or can't be sent to the browser
+     */
+    public function urlForResource($resource);
+}

src/Forms/HTMLEditor/TinyMCEConfig.php

@@ -555,10 +555,7 @@ class TinyMCEConfig extends HTMLEditorConfig
         $settings['document_base_url'] = Director::absoluteBaseURL();
 
         // https://www.tinymce.com/docs/api/class/tinymce.editormanager/#baseURL
-        $tinyMCEBaseURL = Controller::join_links(
+        $tinyMCEBaseURL = $this->getAdminModule()->getResourceURL('thirdparty/tinymce');
-            Director::absoluteBaseURL(),
-            $this->getTinyMCEPath()
-        );
         $settings['baseURL'] = $tinyMCEBaseURL;
 
         // map all plugins to absolute urls for loading
@@ -618,7 +615,7 @@ class TinyMCEConfig extends HTMLEditorConfig
         $editor = array();
 
         // Add standard editor.css
-        $editor[] = Director::absoluteURL(ltrim($this->getAdminPath() . '/client/dist/styles/editor.css', '/'));
+        $editor[] = $this->getAdminModule()->getResourceURL('client/dist/styles/editor.css');
 
         // Themed editor.css
         $themedEditor = ThemeResourceLoader::inst()->findThemedCSS('editor', SSViewer::get_themes());
@@ -635,6 +632,7 @@ class TinyMCEConfig extends HTMLEditorConfig
      * so that multiple HTTP requests on the client don't need to be made.
      *
      * @return string
+     * @throws Exception
      */
     public function getScriptURL()
     {
@@ -687,18 +685,7 @@ class TinyMCEConfig extends HTMLEditorConfig
 
     /**
      * @return string|false
-     */
+     * @throws Exception
-    public function getAdminPath()
-    {
-        $module = $this->getAdminModule();
-        if ($module) {
-            return $module->getRelativePath();
-        }
-        return false;
-    }
-
-    /**
-     * @return string|false
      */
     public function getTinyMCEPath()
     {
@@ -708,7 +695,7 @@ class TinyMCEConfig extends HTMLEditorConfig
         }
 
         if ($admin = $this->getAdminModule()) {
-            return $admin->getResourcePath('thirdparty/tinymce');
+            return $admin->getRelativeResourcePath('thirdparty/tinymce');
         }
 
         throw new Exception(sprintf(
@@ -723,6 +710,6 @@ class TinyMCEConfig extends HTMLEditorConfig
      */
     protected function getAdminModule()
     {
-        return ModuleLoader::inst()->getManifest()->getModule('silverstripe/admin');
+        return ModuleLoader::getModule('silverstripe/admin');
     }
 }

src/View/GenericTemplateGlobalProvider.php

@@ -2,7 +2,7 @@
 
 namespace SilverStripe\View;
 
-use InvalidArgumentException;
+use SilverStripe\Core\Manifest\ModuleLoader;
 use SilverStripe\ORM\DataList;
 
 class GenericTemplateGlobalProvider implements TemplateGlobalProvider
@@ -16,16 +16,6 @@ class GenericTemplateGlobalProvider implements TemplateGlobalProvider
         );
     }
 
-    /**
-     * @var array Module paths
-     */
-    public static $modules = array(
-        'framework' => FRAMEWORK_DIR,
-        'frameworkadmin' => FRAMEWORK_ADMIN_DIR,
-        'thirdparty' => THIRDPARTY_DIR,
-        'assets' => ASSETS_DIR
-    );
-
     /**
      * Given some pre-defined modules, return the filesystem path of the module.
      * @param string $name Name of module to find path of
@@ -33,15 +23,17 @@ class GenericTemplateGlobalProvider implements TemplateGlobalProvider
      */
     public static function ModulePath($name)
     {
-        if (isset(self::$modules[$name])) {
+        // BC for a couple fo the key modules in the old syntax. Reduces merge brittleness but can
-            return self::$modules[$name];
+        // be removed before 4.0 stable
-        } else {
+        $legacyMapping = [
-            throw new InvalidArgumentException(sprintf(
+            'framework' => 'silverstripe/framework',
-                '%s is not a supported argument. Possible values: %s',
+            'frameworkadmin' => 'silverstripe/admin',
-                $name,
+        ];
-                implode(', ', self::$modules)
+        if (isset($legacyMapping[$name])) {
-            ));
+            $name = $legacyMapping[$name];
         }
+
+        return ModuleLoader::getModule($name)->getRelativePath();
     }
 
     /**

src/View/Requirements_Backend.php

@@ -11,6 +11,9 @@ use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Convert;
 use SilverStripe\Core\Injector\Injectable;
+use SilverStripe\Core\Injector\Injector;
+use SilverStripe\Core\Manifest\ResourceURLGenerator;
+use SilverStripe\Core\Manifest\ModuleLoader;
 use SilverStripe\Dev\Debug;
 use SilverStripe\Dev\Deprecation;
 use SilverStripe\i18n\i18n;
@@ -389,7 +392,7 @@ class Requirements_Backend
     /**
      * Register the given JavaScript file as required.
      *
-     * @param string $file Relative to docroot
+     * @param string $file Either relative to docroot or in the form "vendor/package:resource"
      * @param array $options List of options. Available options include:
      * - 'provides' : List of scripts files included in this file
      * - 'async' : Boolean value to set async attribute to script tag
@@ -398,6 +401,8 @@ class Requirements_Backend
      */
     public function javascript($file, $options = array())
     {
+        $file = $this->parseModuleResourceReference($file);
+
         // Get type
         $type = null;
         if (isset($this->javascript[$file]['type'])) {
@@ -621,11 +626,34 @@ class Requirements_Backend
      */
     public function css($file, $media = null)
     {
+        $file = $this->parseModuleResourceReference($file);
+
         $this->css[$file] = array(
             "media" => $media
         );
     }
 
+    /**
+     * Convert a file of the form "vendor/package:resource" into a BASE_PATH-relative file
+     * For other files, reutrn original value
+     *
+     * @param string $file
+     * @return string
+     */
+    protected function parseModuleResourceReference($file)
+    {
+        // String of the form vendor/package:resource. Excludes "http://bla" as that's an absolute URL
+        if (preg_match('#([^ ]*/[^ ]*) *: *([^ ]*)#', $file, $matches)) {
+            list(, $module, $resource) = $matches;
+            $moduleObj = ModuleLoader::getModule($module);
+            if (!$moduleObj) {
+                throw new \InvalidArgumentException("Can't find module '$module'");
+            }
+            return $moduleObj->getRelativeResourcePath($resource);
+        }
+        return $file;
+    }
+
     /**
      * Remove a css requirement
      *
@@ -1019,27 +1047,8 @@ class Requirements_Backend
         // Since combined urls could be root relative, treat them as urls here.
         if (preg_match('{^(//)|(http[s]?:)}', $fileOrUrl) || Director::is_root_relative_url($fileOrUrl)) {
             return $fileOrUrl;
-        } elseif (Director::fileExists($fileOrUrl)) {
-            $filePath = preg_replace('/\?.*/', '', Director::baseFolder() . '/' . $fileOrUrl);
-            $prefix = Director::baseURL();
-            $mtimesuffix = "";
-            $suffix = '';
-            if ($this->getSuffixRequirements()) {
-                $mtimesuffix = "?m=" . filemtime($filePath);
-                $suffix = '&';
-            }
-            if (strpos($fileOrUrl, '?') !== false) {
-                if (strlen($suffix) == 0) {
-                    $suffix = '?';
-                }
-                $suffix .= substr($fileOrUrl, strpos($fileOrUrl, '?') + 1);
-                $fileOrUrl = substr($fileOrUrl, 0, strpos($fileOrUrl, '?'));
-            } else {
-                $suffix = '';
-            }
-            return "{$prefix}{$fileOrUrl}{$mtimesuffix}{$suffix}";
         } else {
-            throw new InvalidArgumentException("File {$fileOrUrl} does not exist");
+            return Injector::inst()->get(ResourceURLGenerator::class)->urlForResource($fileOrUrl);
         }
     }
 

tests/php/Core/Manifest/ModuleManifestTest.php

@@ -82,7 +82,7 @@ class ModuleManifestTest extends SapphireTest
         $this->assertFalse($module->hasResource('package.json'));
         $this->assertEquals(
             'moduleb/composer.json',
-            $module->getResourcePath('composer.json')
+            $module->getRelativeResourcePath('composer.json')
         );
     }
 
@@ -96,7 +96,7 @@ class ModuleManifestTest extends SapphireTest
         $this->assertTrue($module->hasResource('composer.json'));
         $this->assertEquals(
             'composer.json',
-            $module->getResourcePath('composer.json')
+            $module->getRelativeResourcePath('composer.json')
         );
     }
 }

tests/php/Forms/HTMLEditor/HTMLEditorConfigTest.php

@@ -5,10 +5,13 @@ namespace SilverStripe\Forms\Tests\HTMLEditor;
 use Exception;
 use PHPUnit_Framework_MockObject_MockObject;
 use SilverStripe\Control\Director;
+use SilverStripe\Control\SimpleResourceURLGenerator;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Convert;
+use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Core\Manifest\ModuleLoader;
 use SilverStripe\Core\Manifest\ModuleManifest;
+use SilverStripe\Core\Manifest\ResourceURLGenerator;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\Forms\HTMLEditor\HTMLEditorConfig;
 use SilverStripe\Forms\HTMLEditor\HTMLEditorField;
@@ -49,6 +52,10 @@ class HTMLEditorConfigTest extends SapphireTest
 
     public function testEnablePluginsByArrayWithPaths()
     {
+        // Disable nonces
+        $urlGenerator = new SimpleResourceURLGenerator();
+        Injector::inst()->registerService($urlGenerator, ResourceURLGenerator::class);
+
         Config::modify()->set(Director::class, 'alternate_base_url', 'http://mysite.com/subdir');
         $c = new TinyMCEConfig();
         $c->setTheme('modern');
@@ -92,7 +99,7 @@ class HTMLEditorConfigTest extends SapphireTest
         // Plugin specified with standard location
         $this->assertContains('plugin4', array_keys($plugins));
         $this->assertEquals(
-            'http://mysite.com/subdir/test/thirdparty/tinymce/plugins/plugin4/plugin.min.js',
+            '/subdir/silverstripe-admin/thirdparty/tinymce/plugins/plugin4/plugin.min.js',
             $plugins['plugin4']
         );
 

tests/php/View/RequirementsTest.php

@@ -11,6 +11,8 @@ use SilverStripe\View\Requirements;
 use SilverStripe\View\ArrayData;
 use SilverStripe\Assets\Tests\Storage\AssetStoreTest\TestAssetStore;
 use SilverStripe\View\Requirements_Backend;
+use SilverStripe\Core\Manifest\ResourceURLGenerator;
+use SilverStripe\Control\SimpleResourceURLGenerator;
 
 /**
  * @todo Test that order of combine_files() is correct
@@ -634,14 +636,14 @@ class RequirementsTest extends SapphireTest
 
         /* Javascript has correct path */
         $this->assertRegExp(
-            '/src=".*\/RequirementsTest_a\.js\?m=\d\d+&test=1&test=2&test=3/',
+            '/src=".*\/RequirementsTest_a\.js\?test=1&test=2&test=3&m=\d\d+/',
             $html,
             'javascript has correct path'
         );
 
         /* CSS has correct path */
         $this->assertRegExp(
-            '/href=".*\/RequirementsTest_a\.css\?m=\d\d+&test=1&test=2&test=3/',
+            '/href=".*\/RequirementsTest_a\.css\?test=1&test=2&test=3&m=\d\d+/',
             $html,
             'css has correct path'
         );
@@ -804,6 +806,9 @@ class RequirementsTest extends SapphireTest
 
     public function testCommentedOutScriptTagIsIgnored()
     {
+        /// Disable nonce
+        Injector::inst()->registerService(new SimpleResourceURLGenerator(), ResourceURLGenerator::class);
+
         $template = '<html><head></head><body><!--<script>alert("commented out");</script>-->'
             . '<h1>more content</h1></body></html>';
         /**
@@ -811,7 +816,7 @@ class RequirementsTest extends SapphireTest
 */
         $backend = Injector::inst()->create(Requirements_Backend::class);
         $this->setupRequirements($backend);
-        $backend->setSuffixRequirements(false);
+
         $src = $this->getThemeRoot() . '/javascript/RequirementsTest_a.js';
         $urlSrc = Controller::join_links(Director::baseURL(), $src);
         $backend->javascript($src);
@@ -889,6 +894,10 @@ EOS
 
     public function testSuffix()
     {
+        /// Disable nonce
+        $urlGenerator = new SimpleResourceURLGenerator();
+        Injector::inst()->registerService($urlGenerator, ResourceURLGenerator::class);
+
         $template = '<html><head></head><body><header>My header</header><p>Body</p></body></html>';
         $basePath = $this->getThemeRoot();
 
@@ -901,20 +910,20 @@ EOS
         $backend->css($basePath .'/css/RequirementsTest_a.css');
         $backend->css($basePath .'/css/RequirementsTest_b.css?foo=bar&bla=blubb');
 
-        $backend->setSuffixRequirements(true);
+        $urlGenerator->setNonceStyle('mtime');
         $html = $backend->includeInHTML($template);
         $this->assertRegExp('/RequirementsTest_a\.js\?m=[\d]*"/', $html);
-        $this->assertRegExp('/RequirementsTest_b\.js\?m=[\d]*&amp;foo=bar&amp;bla=blubb"/', $html);
+        $this->assertRegExp('/RequirementsTest_b\.js\?foo=bar&amp;bla=blubb&amp;m=[\d]*"/', $html);
         $this->assertRegExp('/RequirementsTest_a\.css\?m=[\d]*"/', $html);
-        $this->assertRegExp('/RequirementsTest_b\.css\?m=[\d]*&amp;foo=bar&amp;bla=blubb"/', $html);
+        $this->assertRegExp('/RequirementsTest_b\.css\?foo=bar&amp;bla=blubb&amp;m=[\d]*"/', $html);
 
-        $backend->setSuffixRequirements(false);
+        $urlGenerator->setNonceStyle(null);
         $html = $backend->includeInHTML($template);
         $this->assertNotContains('RequirementsTest_a.js=', $html);
         $this->assertNotRegExp('/RequirementsTest_a\.js\?m=[\d]*"/', $html);
-        $this->assertNotRegExp('/RequirementsTest_b\.js\?m=[\d]*&amp;foo=bar&amp;bla=blubb"/', $html);
+        $this->assertNotRegExp('/RequirementsTest_b\.js\?foo=bar&amp;bla=blubb&amp;m=[\d]*"/', $html);
         $this->assertNotRegExp('/RequirementsTest_a\.css\?m=[\d]*"/', $html);
-        $this->assertNotRegExp('/RequirementsTest_b\.css\?m=[\d]*&amp;foo=bar&amp;bla=blubb"/', $html);
+        $this->assertNotRegExp('/RequirementsTest_b\.css\?foo=bar&amp;bla=blubb&amp;m=[\d]*"/', $html);
     }
 
     /**