Merge pull request #30279 from nextcloud/fix/30278/script_deps

2 files changed, 86 insertions, 45 deletions

diff --git a/lib/public/Util.php b/lib/public/Util.php
index d0b23ddd3e0..06f99bb6029 100644
--- a/lib/public/Util.php
+++ b/lib/public/Util.php
@@ -78,6 +78,9 @@ class Util {
 	/** @var array */
 	private static $scripts = [];
 
+	/** @var array */
+	private static $scriptDeps = [];
+
 	/**
 	 * get the current installed version of Nextcloud
 	 * @return array
@@ -195,39 +198,12 @@ class Util {
 			self::addTranslations($application);
 		}
 
-		// manage priorities if defined
-		// we store the data like this, then flatten everything
-		// [
-		// 	'core' => [
-		// 		'first' => [
-		// 			'/core/js/main.js',
-		// 		],
-		// 		'last' => [
-		// 			'/apps/viewer/js/viewer-main.js',
-		// 		]
-		// 	],
-		// 	'viewer' => [
-		// 		'first' => [
-		// 			'/apps/viewer/js/viewer-public.js',
-		// 		],
-		// 		'last' => [
-		// 			'/apps/files_pdfviewer/js/files_pdfviewer-main.js',
-		// 		]
-		// 	]
-		// ]
+		// store dependency if defined
 		if (!empty($afterAppId)) {
-			// init afterAppId app array if it doesn't exists
-			if (!array_key_exists($afterAppId, self::$scripts)) {
-				self::$scripts[$afterAppId] = ['first' => [], 'last' => []];
-			}
-			self::$scripts[$afterAppId]['last'][] = $path;
-		} else {
-			// init app array if it doesn't exists
-			if (!array_key_exists($application, self::$scripts)) {
-				self::$scripts[$application] = ['first' => [], 'last' => []];
-			}
-			self::$scripts[$application]['first'][] = $path;
+			self::$scriptDeps[$application] = $afterAppId;
 		}
+
+		self::$scripts[$application][] = $path;
 	}
 
 	/**
@@ -236,15 +212,32 @@ class Util {
 	 * @since 24.0.0
 	 */
 	public static function getScripts(): array {
-		// merging first and last data set
-		$mapFunc = function (array $scriptsArray): array {
-			return array_merge(...array_values($scriptsArray));
+		// Sort by dependency if any
+		$sortByDeps = static function (string $app1, string $app2): int {
+			// Always sort core first
+			if ($app1 === 'core') {
+				return -1;
+			}
+			if ($app2 === 'core') {
+				return 1;
+			}
+
+			// If app1 has a dependency
+			if (array_key_exists($app1, self::$scriptDeps)) {
+				$apps = array_keys(self::$scripts);
+				// Move app1 backwards if dep comes afterwards
+				if (array_search($app1, $apps, true) <
+					array_search(self::$scriptDeps[$app1], $apps, true)) {
+					return 1;
+				}
+			}
+
+			return 0;
 		};
-		$appScripts = array_map($mapFunc, self::$scripts);
-		// sort core first
-		$scripts = array_merge(isset($appScripts['core']) ? $appScripts['core'] : [], ...array_values($appScripts));
-		// remove duplicates
-		return array_unique($scripts);
+		uksort(self::$scripts, $sortByDeps);
+
+		// Flatten array and remove duplicates
+		return self::$scripts ? array_unique(array_merge(...array_values(self::$scripts))) : [];
 	}
 
 	/**
@@ -262,7 +255,7 @@ class Util {
 		} else {
 			$path = "l10n/$languageCode";
 		}
-		self::$scripts[$application]['first'][] = $path;
+		self::$scripts[$application][] = $path;
 	}
 
 	/**
diff --git a/tests/lib/UtilTest.php b/tests/lib/UtilTest.php
index 1b430fd7ecd..1b244d29dd5 100644
--- a/tests/lib/UtilTest.php
+++ b/tests/lib/UtilTest.php
@@ -238,22 +238,70 @@ class UtilTest extends \Test\TestCase {
 	public function testAddScript() {
 		\OCP\Util::addScript('core', 'myFancyJSFile1');
 		\OCP\Util::addScript('files', 'myFancyJSFile2', 'core');
+		\OCP\Util::addScript('myApp5', 'myApp5JSFile', 'myApp2');
 		\OCP\Util::addScript('myApp', 'myFancyJSFile3');
 		\OCP\Util::addScript('core', 'myFancyJSFile4');
 		// after itself
 		\OCP\Util::addScript('core', 'myFancyJSFile5', 'core');
 		// add duplicate
 		\OCP\Util::addScript('core', 'myFancyJSFile1');
-
-		$this->assertEquals([
+		// dependency chain
+		\OCP\Util::addScript('myApp4', 'myApp4JSFile', 'myApp3');
+		\OCP\Util::addScript('myApp3', 'myApp3JSFile', 'myApp2');
+		\OCP\Util::addScript('myApp2', 'myApp2JSFile', 'myApp');
+
+		// Core should appear first
+		$this->assertEquals(
+			0,
+			array_search('core/js/myFancyJSFile1', \OCP\Util::getScripts(), true)
+		);
+		$this->assertEquals(
+			1,
+			array_search('core/js/myFancyJSFile4', \OCP\Util::getScripts(), true)
+		);
+
+		// Dependencies should appear before their children
+		$this->assertLessThan(
+			array_search('files/js/myFancyJSFile2', \OCP\Util::getScripts(), true),
+			array_search('core/js/myFancyJSFile3', \OCP\Util::getScripts(), true)
+		);
+		$this->assertLessThan(
+			array_search('myApp2/js/myApp2JSFile', \OCP\Util::getScripts(), true),
+			array_search('myApp/js/myFancyJSFile3', \OCP\Util::getScripts(), true)
+		);
+		$this->assertLessThan(
+			array_search('myApp3/js/myApp3JSFile', \OCP\Util::getScripts(), true),
+			array_search('myApp2/js/myApp2JSFile', \OCP\Util::getScripts(), true)
+		);
+		$this->assertLessThan(
+			array_search('myApp4/js/myApp4JSFile', \OCP\Util::getScripts(), true),
+			array_search('myApp3/js/myApp3JSFile', \OCP\Util::getScripts(), true)
+		);
+		$this->assertLessThan(
+			array_search('myApp5/js/myApp5JSFile', \OCP\Util::getScripts(), true),
+			array_search('myApp2/js/myApp2JSFile', \OCP\Util::getScripts(), true)
+		);
+
+		// No duplicates
+		$this->assertEquals(
+			\OCP\Util::getScripts(),
+			array_unique(\OCP\Util::getScripts())
+		);
+
+		// All scripts still there
+		$scripts = [
 			'core/js/myFancyJSFile1',
 			'core/js/myFancyJSFile4',
 			'files/js/myFancyJSFile2',
 			'core/js/myFancyJSFile5',
-			'files/l10n/en',
-			'myApp/l10n/en',
 			'myApp/js/myFancyJSFile3',
-		], array_values(\OCP\Util::getScripts()));
+			'myApp2/js/myApp2JSFile',
+			'myApp3/js/myApp3JSFile',
+			'myApp4/js/myApp4JSFile',
+		];
+		foreach ($scripts as $script) {
+			$this->assertContains($script, \OCP\Util::getScripts());
+		}
 	}
 
 	public function testAddVendorScript() {