From 67fe77c987b0422228898754136ae3cfd5111b6f Mon Sep 17 00:00:00 2001 From: Conduction Release Bot Date: Fri, 14 Aug 2026 22:27:43 +0200 Subject: [PATCH 1/4] perf(workspace): only touch the widget registry when the bridge is on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IManager::getWidgets() is not a getter. It calls loadLazyPanels(), which calls load() on EVERY dashboard widget of every app enabled for the user, ignoring the dashboard layout — and load() is where widgets call Util::addScript(). Merely enumerating widgets therefore injects every widget bundle in the instance. index() reached the registry twice per render: once through loadWidgetScripts() and once through getAvailableWidgets() for the initial state. The explicit `foreach ($widgets as $widget) { $widget->load(); }` loop in loadWidgetScripts() was dead weight — getWidgets() had already called load() on all of them before the loop ran — so removing only that loop would have changed nothing. Measured on the workspace page, bridge on vs off: bridge on 161.47 MB JS, 72 files, 24 widget bundles, 36,776 ms bridge off 42.90 MB JS, 21 files, 0 widget bundles, 6,184 ms That is 118.6 MB and ~30s of widget code the workspace never renders, every one of which used to throw because OCA.Dashboard exists only on /apps/dashboard. The registry is now read only when legacyWidgetBridgeEnabled is set. The SPA is unaffected when it is off: Views.vue already fetches the available widget list from GET /api/widgets on boot, and loadInitialState defaults `widgets` to [], so the REQ-INIT-002 reader still never returns undefined. IManager was the only user of the dashboardManager constructor argument, so both the argument and the import go with it. With the bridge on the cost is unchanged — that path is bounded by Nextcloud core, which offers no way to read widget metadata without loadLazyPanels() injecting every script. Fixing that needs an upstream change to OC\Dashboard\Manager. --- lib/Controller/PageController.php | 49 +++++++++++++++---------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 151a69cce..1319ed7e3 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -42,7 +42,6 @@ use OCP\AppFramework\Http\ContentSecurityPolicy; use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Services\IInitialState; -use OCP\Dashboard\IManager; use OCP\IRequest; use OCP\IUserSession; use OCP\Util; @@ -62,7 +61,6 @@ class PageController extends Controller { * Constructor. * * @param IRequest $request The request. - * @param IManager $dashboardManager Nextcloud dashboard widget manager. * @param IInitialState $initialState The Nextcloud initial-state service. * @param IUserSession $userSession Active user session. * @param WidgetService $widgetService Available-widgets descriptor formatter. @@ -95,7 +93,6 @@ class PageController extends Controller { */ public function __construct( IRequest $request, - private readonly IManager $dashboardManager, private readonly IInitialState $initialState, private readonly IUserSession $userSession, private readonly WidgetService $widgetService, @@ -160,8 +157,28 @@ public function index(string $deepLink = ''): TemplateResponse { Util::addScript(application: Application::APP_ID, file: 'launchpad-main'); Util::addStyle(application: Application::APP_ID, file: 'launchpad'); - // Load all widget scripts so legacy widgets can register their callbacks. - $this->loadWidgetScripts(); + // Settings are read once and reused below; getSettings() resolves every + // safe default, so neither read throws on an unset key. + $settings = $this->adminSettingsService->getSettings(); + + // Legacy-widget-bridge spec: touching the Nextcloud widget registry at + // all is what costs us here. `IManager::getWidgets()` is not a getter — + // it calls loadLazyPanels(), which calls load() on EVERY widget of every + // app enabled for the user, ignoring the dashboard layout, and load() is + // where widgets call Util::addScript(). Measured on this instance that is + // ~147 MB of widget JS injected into a page that renders no Nextcloud + // dashboard, every one of which then throws because OCA.Dashboard only + // exists on /apps/dashboard. + // + // So the registry is touched only when the bridge is actually on. When it + // is off, the workspace renders without paying for widgets it will never + // bridge, and the SPA still has everything it needs: Views.vue fetches the + // available-widget list from GET /api/widgets on boot. + $bridgeEnabled = (bool)($settings['legacyWidgetBridgeEnabled'] ?? true); + $bridgedWidgets = []; + if ($bridgeEnabled === true) { + $bridgedWidgets = $this->widgetService->getAvailableWidgets(); + } $userId = $this->resolveUserId(); @@ -193,7 +210,7 @@ public function index(string $deepLink = ''): TemplateResponse { ); $builder - ->setWidgets($this->widgetService->getAvailableWidgets()) + ->setWidgets($bridgedWidgets) ->setLayout($activeState['layout']) ->setPrimaryGroup($primaryGroupId) ->setPrimaryGroupName($primaryGroupName) @@ -217,7 +234,7 @@ public function index(string $deepLink = ''): TemplateResponse { // no-match fallback target. `getSettings()` already resolves the // safe 'none' default when unset/invalid, so this never throws. $quicksearchFallback = (string)( - $this->adminSettingsService->getSettings()['quicksearchFallbackTarget'] ?? AdminSettingsService::DEFAULT_QUICKSEARCH_FALLBACK_TARGET + $settings['quicksearchFallbackTarget'] ?? AdminSettingsService::DEFAULT_QUICKSEARCH_FALLBACK_TARGET ); $builder @@ -550,22 +567,4 @@ public function publicShare(): TemplateResponse { return $response; }//end publicShare() - /** - * Load scripts for all available dashboard widgets. - * - * This ensures legacy widgets can register their callbacks via - * OCA.Dashboard.register. - * - * @return array Map of widget id to widget. - */ - private function loadWidgetScripts(): array { - $widgets = $this->dashboardManager->getWidgets(); - - foreach ($widgets as $widget) { - // Call the widget's load() method to inject its scripts. - $widget->load(); - } - - return $widgets; - }//end loadWidgetScripts() }//end class From 0f3d7c751cf1a5ff6d61c3f32be204065b5e53cf Mon Sep 17 00:00:00 2001 From: Conduction Release Bot Date: Fri, 14 Aug 2026 23:08:36 +0200 Subject: [PATCH 2/4] perf(workspace): default the legacy widget bridge to off MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enabling the bridge makes the workspace read the Nextcloud widget registry, and IManager::getWidgets() injects the scripts of every widget of every enabled app. Measured on the workspace page that is 118.6 MB of JS and ~30s of load for widgets the workspace never renders, so the expensive path should be opt-in rather than the default. With no stored setting the workspace now renders in 4,988 ms with 42.90 MB of JS, 21 files and zero widget bundles, against 36,776 ms / 161.47 MB / 24 bundles when the bridge is on. Instances that rely on bridged placements switch it back on in Beheer; the flag is read on every render, so it takes effect immediately. The stored-value test asserted false, which is the new default and would therefore have passed without reading storage at all. It now stores true — the opposite of the default — so it still discriminates. --- lib/Controller/PageController.php | 2 +- lib/Service/AdminSettingsService.php | 12 +++++++++--- tests/Unit/Service/AdminSettingsServiceTest.php | 12 ++++++++---- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 1319ed7e3..50e4e3cde 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -174,7 +174,7 @@ public function index(string $deepLink = ''): TemplateResponse { // is off, the workspace renders without paying for widgets it will never // bridge, and the SPA still has everything it needs: Views.vue fetches the // available-widget list from GET /api/widgets on boot. - $bridgeEnabled = (bool)($settings['legacyWidgetBridgeEnabled'] ?? true); + $bridgeEnabled = (bool)($settings['legacyWidgetBridgeEnabled'] ?? false); $bridgedWidgets = []; if ($bridgeEnabled === true) { $bridgedWidgets = $this->widgetService->getAvailableWidgets(); diff --git a/lib/Service/AdminSettingsService.php b/lib/Service/AdminSettingsService.php index 1dda62125..6188a6705 100644 --- a/lib/Service/AdminSettingsService.php +++ b/lib/Service/AdminSettingsService.php @@ -130,9 +130,15 @@ public function getSettings(): array { // dashboard default; forced-share groups default to none. 'defaultSharePermissionLevel' => $settings[$sharePermKey] ?? $permDef, 'forcedShareGroups' => array_values($forcedShareGroups), - // Legacy-widget-bridge spec: bridge defaults to ON so existing - // bridged placements keep rendering after upgrade. - 'legacyWidgetBridgeEnabled' => $settings[$bridgeKey] ?? true, + // Legacy-widget-bridge spec: the bridge defaults to OFF because + // turning it on makes the workspace read the Nextcloud widget + // registry, and IManager::getWidgets() injects the scripts of EVERY + // widget of every enabled app — measured at 118.6 MB of JS and ~30s + // of load on this fleet, for widgets the workspace never renders. + // Instances that rely on bridged placements switch it back on in + // Beheer; the setting is read on every render, so it takes effect + // immediately. + 'legacyWidgetBridgeEnabled' => $settings[$bridgeKey] ?? false, // Dashboard-quota-limits REQ-QUOTA-001: numeric governance // quotas. `0` = unlimited (no enforcement). 'maxDashboardsPerUser' => $this->clampQuota(value: $settings[$maxDashKey] ?? 0), diff --git a/tests/Unit/Service/AdminSettingsServiceTest.php b/tests/Unit/Service/AdminSettingsServiceTest.php index 3d014ca0a..97d26194b 100644 --- a/tests/Unit/Service/AdminSettingsServiceTest.php +++ b/tests/Unit/Service/AdminSettingsServiceTest.php @@ -229,8 +229,9 @@ public function testGetSettingsSharingAndBridgeDefaults(): void { // Default share permission mirrors the dashboard default. $this->assertSame(Dashboard::PERMISSION_ADD_ONLY, $settings['defaultSharePermissionLevel']); $this->assertSame([], $settings['forcedShareGroups']); - // Bridge defaults ON so existing placements keep rendering. - $this->assertTrue($settings['legacyWidgetBridgeEnabled']); + // Bridge defaults OFF: enabling it makes the workspace read the + // Nextcloud widget registry, which injects every widget's scripts. + $this->assertFalse($settings['legacyWidgetBridgeEnabled']); }//end testGetSettingsSharingAndBridgeDefaults() public function testGetSettingsSharingAndBridgeStoredValues(): void { @@ -238,7 +239,10 @@ public function testGetSettingsSharingAndBridgeStoredValues(): void { [ AdminSetting::KEY_DEFAULT_SHARE_PERMISSION_LEVEL => 'full', AdminSetting::KEY_FORCED_SHARE_GROUPS => ['marketing', 'sales'], - AdminSetting::KEY_LEGACY_WIDGET_BRIDGE_ENABLED => false, + // Stored TRUE, the opposite of the default, so this assertion + // actually proves the stored value is read rather than passing + // on the default. + AdminSetting::KEY_LEGACY_WIDGET_BRIDGE_ENABLED => true, ] ); @@ -246,7 +250,7 @@ public function testGetSettingsSharingAndBridgeStoredValues(): void { $this->assertSame('full', $settings['defaultSharePermissionLevel']); $this->assertSame(['marketing', 'sales'], $settings['forcedShareGroups']); - $this->assertFalse($settings['legacyWidgetBridgeEnabled']); + $this->assertTrue($settings['legacyWidgetBridgeEnabled']); }//end testGetSettingsSharingAndBridgeStoredValues() public function testUpdateSettingsPersistsForcedShareGroupsDeduplicated(): void { From 9c2eccd046338da416e71bb085eae615fa42c50d Mon Sep 17 00:00:00 2001 From: Conduction Release Bot Date: Sat, 15 Aug 2026 22:42:04 +0200 Subject: [PATCH 3/4] refactor(workspace): extract resolveBridgedWidgets() to satisfy phpmd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught what the local run did not: phpmd ExcessiveMethodLength, because index() went from 88 lines on development to 108 with this PR's guard and its explanatory comment. The threshold is 100. The comment was carrying most of the weight, and it documents the bridge decision rather than the render flow, so it belongs with the logic it explains. Extracting resolveBridgedWidgets() takes both out of index(), which is now 90 lines. No behaviour change: same setting, same default (off), same call to getAvailableWidgets() only when the bridge is on. Verified: phpmd reports no PageController violation, phpcs unchanged at 0 errors for this file, phpstan [OK] No errors, phpunit 1570 tests 0 failures. Worth recording why this was missed — the local sweep ran lint, phpcs, phpstan, psalm and phpunit, but not phpmd, so "gate green" was a claim about a smaller set of checks than CI runs. --- lib/Controller/PageController.php | 52 ++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 50e4e3cde..8e8668df5 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -160,25 +160,7 @@ public function index(string $deepLink = ''): TemplateResponse { // Settings are read once and reused below; getSettings() resolves every // safe default, so neither read throws on an unset key. $settings = $this->adminSettingsService->getSettings(); - - // Legacy-widget-bridge spec: touching the Nextcloud widget registry at - // all is what costs us here. `IManager::getWidgets()` is not a getter — - // it calls loadLazyPanels(), which calls load() on EVERY widget of every - // app enabled for the user, ignoring the dashboard layout, and load() is - // where widgets call Util::addScript(). Measured on this instance that is - // ~147 MB of widget JS injected into a page that renders no Nextcloud - // dashboard, every one of which then throws because OCA.Dashboard only - // exists on /apps/dashboard. - // - // So the registry is touched only when the bridge is actually on. When it - // is off, the workspace renders without paying for widgets it will never - // bridge, and the SPA still has everything it needs: Views.vue fetches the - // available-widget list from GET /api/widgets on boot. - $bridgeEnabled = (bool)($settings['legacyWidgetBridgeEnabled'] ?? false); - $bridgedWidgets = []; - if ($bridgeEnabled === true) { - $bridgedWidgets = $this->widgetService->getAvailableWidgets(); - } + $bridgedWidgets = $this->resolveBridgedWidgets(settings: $settings); $userId = $this->resolveUserId(); @@ -262,6 +244,38 @@ public function index(string $deepLink = ''): TemplateResponse { return $response; }//end index() + /** + * Resolve the widgets to bridge into the workspace, if any. + * + * Legacy-widget-bridge spec: touching the Nextcloud widget registry at all is + * what costs us. `IManager::getWidgets()` is not a getter — it calls + * loadLazyPanels(), which calls load() on EVERY widget of every app enabled + * for the user, ignoring the dashboard layout, and load() is where widgets + * call `Util::addScript()`. Measured on this instance that is ~147 MB of + * widget JS injected into a page rendering no Nextcloud dashboard, every one + * of which then throws because `OCA.Dashboard` exists only on + * `/apps/dashboard`. + * + * So the registry is touched only when the bridge is actually on. When it is + * off the workspace renders without paying for widgets it will never bridge, + * and the SPA still has everything it needs: Views.vue fetches the + * available-widget list from `GET /api/widgets` on boot. + * + * @param array $settings Resolved admin settings (see AdminSettingsService). + * + * @return array The widget descriptors to bridge, empty when the bridge is off. + * + * @spec openspec/specs/runtime-shell/spec.md + */ + private function resolveBridgedWidgets(array $settings): array { + $bridgeEnabled = (bool)($settings['legacyWidgetBridgeEnabled'] ?? false); + if ($bridgeEnabled === false) { + return []; + } + + return $this->widgetService->getAvailableWidgets(); + }//end resolveBridgedWidgets() + /** * Resolve the primary group this request routes through. * From 3411ae7a7c055304acc49386bffc13a36a6782f2 Mon Sep 17 00:00:00 2001 From: Conduction Release Bot Date: Sat, 15 Aug 2026 23:19:18 +0200 Subject: [PATCH 4/4] test(workspace): cover the legacy-widget-bridge decision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI's coverage guard failed the previous commit — not a failing test, the ratchet: Coverage current: 51.68% (11645/22533 statements) Coverage merge base: 51.68% (11645/22532 statements) FAIL: This change adds 1 statements. Adding code without tests drops coverage. Extracting resolveBridgedWidgets() added one statement and covered none of it, because PageController had no test at all. The percentage did not move — only the counts — which is exactly the shape that reads as flake. The decision deserved a test regardless. The assertion that matters is `expects($this->never())` on getAvailableWidgets: the point of this change is not that an empty array comes back, it is that the widget registry is never touched, so no widget's load() runs and no scripts are injected. Asserting on the return value alone would still pass if the registry were read and discarded. Three cases: bridge on reads the registry, bridge off never touches it, and a missing setting defaults to off (the fail-safe direction). Verified load-bearing by mutation rather than by passing: flipping the default from `?? false` to `?? true` fails the third test with "getAvailableWidgets(): array was not expected to be called"; restoring it returns to green. phpunit 1573 tests 0 failures (3 added), phpmd 0 PageController violations, phpcs 0 errors on its configured scope (lib/ — tests/ is deliberately out of scope, matching every existing test file's positional-argument style). --- .../PageControllerBridgedWidgetsTest.php | 152 ++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 tests/Unit/Controller/PageControllerBridgedWidgetsTest.php diff --git a/tests/Unit/Controller/PageControllerBridgedWidgetsTest.php b/tests/Unit/Controller/PageControllerBridgedWidgetsTest.php new file mode 100644 index 000000000..129bb9211 --- /dev/null +++ b/tests/Unit/Controller/PageControllerBridgedWidgetsTest.php @@ -0,0 +1,152 @@ + + * @copyright 2024 Conduction b.v. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * @version GIT:auto + * @link https://conduction.nl + */ + +declare(strict_types=1); + +namespace OCA\LaunchPad\Tests\Unit\Controller; + +use OCA\LaunchPad\Controller\PageController; +use OCA\LaunchPad\Service\AdminSettingsService; +use OCA\LaunchPad\Service\AdminTemplateService; +use OCA\LaunchPad\Service\DashboardService; +use OCA\LaunchPad\Service\DashboardTreeService; +use OCA\LaunchPad\Service\RoleFeaturePermissionService; +use OCA\LaunchPad\Service\WidgetService; +use OCP\AppFramework\Services\IInitialState; +use OCP\IRequest; +use OCP\IUserSession; +use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; +use ReflectionMethod; + +/** + * The workspace renders no Nextcloud dashboard, but reading the widget registry + * to bridge legacy widgets is expensive out of proportion to that: `getWidgets()` + * is not a getter — it calls `loadLazyPanels()`, which calls `load()` on EVERY + * widget of every enabled app, and `load()` is where widgets call + * `Util::addScript()`. Measured at ~147 MB of widget JS on a page that renders + * no dashboard. + * + * So the decision "do we touch the registry at all" is load-bearing, and these + * tests pin both directions of it. + */ +class PageControllerBridgedWidgetsTest extends TestCase { + /** + * Build a controller whose only meaningful collaborator is the widget + * service — every other dependency is an unused mock. + * + * @param WidgetService $widgetService The widget service double. + * + * @return PageController The controller under test. + */ + private function makeController(WidgetService $widgetService): PageController { + return new PageController( + request: $this->createMock(IRequest::class), + initialState: $this->createMock(IInitialState::class), + userSession: $this->createMock(IUserSession::class), + widgetService: $widgetService, + dashboardService: $this->createMock(DashboardService::class), + adminTemplateService: $this->createMock(AdminTemplateService::class), + roleFeaturePerm: $this->createMock(RoleFeaturePermissionService::class), + treeService: $this->createMock(DashboardTreeService::class), + logger: $this->createMock(LoggerInterface::class), + adminSettingsService: $this->createMock(AdminSettingsService::class), + ); + }//end makeController() + + /** + * Invoke the private resolver. + * + * @param PageController $controller The controller under test. + * @param array $settings The resolved admin settings. + * + * @return array The bridged widget descriptors. + */ + private function resolve(PageController $controller, array $settings): array { + $method = new ReflectionMethod( + PageController::class, + 'resolveBridgedWidgets' + ); + $method->setAccessible(accessible: true); + + return $method->invoke($controller, $settings); + }//end resolve() + + /** + * Bridge ON: the registry IS read and its descriptors are returned. + * + * @return void + */ + public function testBridgeEnabledReadsTheWidgetRegistry(): void { + $widgetService = $this->createMock(WidgetService::class); + $widgetService->expects($this->once()) + ->method('getAvailableWidgets') + ->willReturn([['id' => 'deals'], ['id' => 'leads']]); + + $result = $this->resolve( + controller: $this->makeController(widgetService: $widgetService), + settings: ['legacyWidgetBridgeEnabled' => true] + ); + + $this->assertCount(2, $result); + $this->assertSame('deals', $result[0]['id']); + }//end testBridgeEnabledReadsTheWidgetRegistry() + + /** + * Bridge OFF: the registry is NOT touched at all. + * + * `never()` is the assertion that matters — the point of this change is not + * that the returned array is empty, it is that `getAvailableWidgets()` (and + * through it `IManager::getWidgets()`) is never called, so no widget's + * `load()` runs and no scripts are injected. An assertion on the return + * value alone would still pass if the registry were read and discarded. + * + * @return void + */ + public function testBridgeDisabledNeverTouchesTheRegistry(): void { + $widgetService = $this->createMock(WidgetService::class); + $widgetService->expects($this->never()) + ->method('getAvailableWidgets'); + + $result = $this->resolve( + controller: $this->makeController(widgetService: $widgetService), + settings: ['legacyWidgetBridgeEnabled' => false] + ); + + $this->assertSame([], $result); + }//end testBridgeDisabledNeverTouchesTheRegistry() + + /** + * An absent key defaults to OFF, matching AdminSettingsService's default. + * + * This pins the fail-safe direction: a settings row that has never been + * written must not opt an instance into the expensive path. + * + * @return void + */ + public function testMissingSettingDefaultsToBridgeOff(): void { + $widgetService = $this->createMock(WidgetService::class); + $widgetService->expects($this->never()) + ->method('getAvailableWidgets'); + + $resolved = $this->resolve( + controller: $this->makeController(widgetService: $widgetService), + settings: [] + ); + + $this->assertSame([], $resolved); + }//end testMissingSettingDefaultsToBridgeOff() +}//end class