fix(JSResourceLocator): Consider configured app roots for files

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

diff --git a/lib/private/Template/JSResourceLocator.php b/lib/private/Template/JSResourceLocator.php
index 68a83fa4b733a4bb4d121c410652baaa5343a62a..c73b3efe997007df095492672f79e63b761d0d61 100644 (file)
--- a/lib/private/Template/JSResourceLocator.php
+++ b/lib/private/Template/JSResourceLocator.php
@@ -51,6 +51,22 @@ class JSResourceLocator extends ResourceLocator {
                // Extracting the appId and the script file name
                $app = substr($script, 0, strpos($script, '/'));
                $scriptName = basename($script);
+               // Get the app root path
+               $appRoot = $this->serverroot . 'apps/';
+               $appWebRoot = null;
+               try {
+                       // We need the dir name as getAppPath appends the appid
+                       $appRoot = dirname($this->appManager->getAppPath($app));
+                       // Only do this if $app_path is set, because an empty argument to realpath gets turned into cwd.
+                       if ($appRoot) {
+                               // Handle symlinks
+                               $appRoot = realpath($appRoot);
+                       }
+                       // Get the app webroot
+                       $appWebRoot = dirname($this->appManager->getAppWebPath($app));
+               } catch (AppPathNotFoundException $e) {
+                       // ignore
+               }
 
                if (str_contains($script, '/l10n/')) {
                        // For language files we try to load them all, so themes can overwrite
@@ -60,13 +76,8 @@ class JSResourceLocator extends ResourceLocator {
                        $found += $this->appendScriptIfExist($this->serverroot, $theme_dir.'core/'.$script);
                        $found += $this->appendScriptIfExist($this->serverroot, $script);
                        $found += $this->appendScriptIfExist($this->serverroot, $theme_dir.$script);
-
-                       foreach (\OC::$APPSROOTS as $appRoot) {
-                               $dirName = basename($appRoot['path']);
-                               $rootPath = dirname($appRoot['path']);
-                               $found += $this->appendScriptIfExist($rootPath, $dirName.'/'.$script);
-                               $found += $this->appendScriptIfExist($this->serverroot, $theme_dir.$dirName.'/'.$script);
-                       }
+                       $found += $this->appendScriptIfExist($appRoot, $script, $appWebRoot);
+                       $found += $this->appendScriptIfExist($this->serverroot, $theme_dir.'apps/'.$script);
 
                        if ($found) {
                                return;
@@ -76,8 +87,9 @@ class JSResourceLocator extends ResourceLocator {
                        || $this->appendScriptIfExist($this->serverroot, $script)
                        || $this->appendScriptIfExist($this->serverroot, $theme_dir."dist/$app-$scriptName")
                        || $this->appendScriptIfExist($this->serverroot, "dist/$app-$scriptName")
-                       || $this->appendScriptIfExist($this->serverroot, 'apps/'.$script)
+                       || $this->appendScriptIfExist($appRoot, $script, $appWebRoot)
                        || $this->cacheAndAppendCombineJsonIfExist($this->serverroot, $script.'.json')
+                       || $this->cacheAndAppendCombineJsonIfExist($appRoot, $script.'.json', $appWebRoot)
                        || $this->appendScriptIfExist($this->serverroot, $theme_dir.'core/'.$script)
                        || $this->appendScriptIfExist($this->serverroot, 'core/'.$script)
                        || (strpos($scriptName, '/') === -1 && ($this->appendScriptIfExist($this->serverroot, $theme_dir."dist/core-$scriptName")
@@ -87,43 +99,13 @@ class JSResourceLocator extends ResourceLocator {
                        return;
                }
 
-               $script = substr($script, strpos($script, '/') + 1);
-               $app_url = null;
-
-               try {
-                       $app_url = $this->appManager->getAppWebPath($app);
-               } catch (AppPathNotFoundException) {
-                       // pass
-               }
-
-               try {
-                       $app_path = $this->appManager->getAppPath($app);
-
-                       // Account for the possibility of having symlinks in app path. Only
-                       // do this if $app_path is set, because an empty argument to realpath
-                       // gets turned into cwd.
-                       $app_path = realpath($app_path);
-
-                       // check combined files
-                       if (!str_starts_with($script, 'l10n/') && $this->cacheAndAppendCombineJsonIfExist($app_path, $script.'.json', $app)) {
-                               return;
-                       }
-
-                       // fallback to plain file location
-                       if ($this->appendScriptIfExist($app_path, $script, $app_url)) {
-                               return;
-                       }
-               } catch (AppPathNotFoundException) {
-                       // pass (general error handling happens below)
-               }
-
                // missing translations files will be ignored
-               if (str_starts_with($script, 'l10n/')) {
+               if (str_contains($script, '/l10n/')) {
                        return;
                }
 
                $this->logger->error('Could not find resource {resource} to load', [
-                       'resource' => $app . '/' . $script . '.js',
+                       'resource' => $script . '.js',
                        'app' => 'jsresourceloader',
                ]);
        }

diff --git a/tests/lib/Template/JSResourceLocatorTest.php b/tests/lib/Template/JSResourceLocatorTest.php
index f5af138060a198c02971be259b144515912be2dd..ae104d4d6ea9c95f64e988d06f9c6a29e9bf3533 100644 (file)
--- a/tests/lib/Template/JSResourceLocatorTest.php
+++ b/tests/lib/Template/JSResourceLocatorTest.php
@@ -127,9 +127,9 @@ class JSResourceLocatorTest extends \Test\TestCase {
                $webRoot = $resource[1];
                $file = $resource[2];
 
-               $expectedRoot = $new_apps_path . '/test-js-app';
-               $expectedWebRoot = \OC::$WEBROOT . '/js-apps-test/test-js-app';
-               $expectedFile = 'test-file.js';
+               $expectedRoot = $new_apps_path;
+               $expectedWebRoot = \OC::$WEBROOT . '/js-apps-test';
+               $expectedFile = $appName . '/test-file.js';
 
                $this->assertEquals($expectedRoot, $root,
                        'Ensure the app path symlink is resolved into the real path');
@@ -145,7 +145,7 @@ class JSResourceLocatorTest extends \Test\TestCase {
                        ->method('getAppPath')
                        ->with('core')
                        ->willThrowException(new AppPathNotFoundException());
-               $this->appManager->expects($this->once())
+               $this->appManager->expects($this->atMost(1))
                        ->method('getAppWebPath')
                        ->with('core')
                        ->willThrowException(new AppPathNotFoundException());