Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 38 additions & 25 deletions lib/Controller/PageController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -160,8 +157,10 @@ 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();
$bridgedWidgets = $this->resolveBridgedWidgets(settings: $settings);

$userId = $this->resolveUserId();

Expand Down Expand Up @@ -193,7 +192,7 @@ public function index(string $deepLink = ''): TemplateResponse {
);

$builder
->setWidgets($this->widgetService->getAvailableWidgets())
->setWidgets($bridgedWidgets)
->setLayout($activeState['layout'])
->setPrimaryGroup($primaryGroupId)
->setPrimaryGroupName($primaryGroupName)
Expand All @@ -217,7 +216,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
Expand Down Expand Up @@ -245,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.
*
Expand Down Expand Up @@ -550,22 +581,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<string, \OCP\Dashboard\IWidget> 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
12 changes: 9 additions & 3 deletions lib/Service/AdminSettingsService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
152 changes: 152 additions & 0 deletions tests/Unit/Controller/PageControllerBridgedWidgetsTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
<?php

/**
* PageControllerBridgedWidgetsTest
*
* Covers the legacy-widget-bridge decision on the workspace render path.
*
* @category Test
* @package OCA\LaunchPad\Tests\Unit\Controller
* @author Conduction b.v. <info@conduction.nl>
* @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
12 changes: 8 additions & 4 deletions tests/Unit/Service/AdminSettingsServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -229,24 +229,28 @@ 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 {
$this->settingMapper->method('getAllAsArray')->willReturn(
[
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,
]
);

$settings = $this->service->getSettings();

$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 {
Expand Down
Loading