diff --git a/src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts b/src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts index 024c029d4b97..c6e6066b8f8b 100644 --- a/src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts +++ b/src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts @@ -14,17 +14,62 @@ import type { Guard, UmbRoute } from '@umbraco-cms/backoffice/router'; import { pathWithoutBasePath } from '@umbraco-cms/backoffice/router'; import { RuntimeLevelModel } from '@umbraco-cms/backoffice/external/backend-api'; import { UmbContextDebugController } from '@umbraco-cms/backoffice/debug'; -import { UmbBundleExtensionInitializer, UmbServerExtensionRegistrator } from '@umbraco-cms/backoffice/extension-api'; +import { + UmbBundleExtensionInitializer, + UmbServerExtensionRegistrator, + type ManifestBase, +} from '@umbraco-cms/backoffice/extension-api'; import { UmbAppEntryPointExtensionInitializer, umbExtensionsRegistry, + type UmbExtensionManifestKind, } from '@umbraco-cms/backoffice/extension-registry'; -import { firstValueFrom } from '@umbraco-cms/backoffice/external/rxjs'; import { redirectToStoredPath } from '@umbraco-cms/backoffice/utils'; import { umbHttpClient } from '@umbraco-cms/backoffice/http-client'; import { UmbViewContext } from '@umbraco-cms/backoffice/view'; import './app-logo.element.js'; +import { UMB_CURRENT_USER_CONTEXT } from '@umbraco-cms/backoffice/current-user'; + +const CORE_PACKAGES: Array }>> = [ + import('../../packages/block/umbraco-package.js'), + import('../../packages/clipboard/umbraco-package.js'), + import('../../packages/code-editor/umbraco-package.js'), + import('../../packages/content/umbraco-package.js'), + import('../../packages/data-type/umbraco-package.js'), + import('../../packages/dictionary/umbraco-package.js'), + import('../../packages/documents/umbraco-package.js'), + import('../../packages/embedded-media/umbraco-package.js'), + import('../../packages/extension-insights/umbraco-package.js'), + import('../../packages/health-check/umbraco-package.js'), + import('../../packages/help/umbraco-package.js'), + import('../../packages/language/umbraco-package.js'), + import('../../packages/log-viewer/umbraco-package.js'), + import('../../packages/management-api/umbraco-package.js'), + import('../../packages/markdown-editor/umbraco-package.js'), + import('../../packages/media/umbraco-package.js'), + import('../../packages/members/umbraco-package.js'), + import('../../packages/models-builder/umbraco-package.js'), + import('../../packages/multi-url-picker/umbraco-package.js'), + import('../../packages/packages/umbraco-package.js'), + import('../../packages/performance-profiling/umbraco-package.js'), + import('../../packages/property-editors/umbraco-package.js'), + import('../../packages/publish-cache/umbraco-package.js'), + import('../../packages/relations/umbraco-package.js'), + import('../../packages/rte/umbraco-package.js'), + import('../../packages/settings/umbraco-package.js'), + import('../../packages/static-file/umbraco-package.js'), + import('../../packages/sysinfo/umbraco-package.js'), + import('../../packages/tags/umbraco-package.js'), + import('../../packages/telemetry/umbraco-package.js'), + import('../../packages/templating/umbraco-package.js'), + import('../../packages/tiptap/umbraco-package.js'), + import('../../packages/translation/umbraco-package.js'), + import('../../packages/ufm/umbraco-package.js'), + import('../../packages/umbraco-news/umbraco-package.js'), + import('../../packages/user/umbraco-package.js'), + import('../../packages/webhook/umbraco-package.js'), +]; @customElement('umb-app') export class UmbAppElement extends UmbLitElement { @@ -129,18 +174,22 @@ export class UmbAppElement extends UmbLitElement { { path: '**', component: () => import('../backoffice/backoffice.element.js'), - guards: [this.#isAuthorizedGuard()], + guards: [this.#isAuthorizedGuard(), this.#loadedGuard()], }, ]; #authContext?: typeof UMB_AUTH_CONTEXT.TYPE; #serverConnection?: UmbServerConnection; #authController = new UmbAppAuthController(this); + #bundleInitializer: UmbBundleExtensionInitializer; + + #currentUser?: typeof UMB_CURRENT_USER_CONTEXT.TYPE; + #packageModules?: Promise }>>; constructor() { super(); - new UmbBundleExtensionInitializer(this, umbExtensionsRegistry); + this.#bundleInitializer = new UmbBundleExtensionInitializer(this, umbExtensionsRegistry); new UUIIconRegistryEssential().attach(this); @@ -149,6 +198,13 @@ export class UmbAppElement extends UmbLitElement { new UmbNetworkConnectionStatusManager(this); new UmbViewContext(this, null); + + this.consumeContext(UMB_CURRENT_USER_CONTEXT, (userContext) => { + this.#currentUser = userContext; + if (userContext) { + this.#loadCurrentUser(); + } + }); } override connectedCallback(): void { @@ -166,6 +222,21 @@ export class UmbAppElement extends UmbLitElement { ); this.#authContext.configureClient(umbHttpClient); + this.observe( + this.#authContext.isAuthorized, + async (isAuthorized) => { + if (isAuthorized === undefined) return; + if (isAuthorized) { + // TODO: Remove dependency on current user context from the app element in future [MR] + this.#loadCurrentUser(); + } else { + // TODO: Unregistering all extensions from v.18 [NL] + //void this.#unregisterExtensions(); + } + }, + null, + ); + this.#serverConnection = await new UmbServerConnection(this, this.serverUrl).connect(); new UmbServerContext(this, { backofficePath: this.backofficePath, @@ -178,8 +249,7 @@ export class UmbAppElement extends UmbLitElement { // Register public extensions (login extensions) await new UmbServerExtensionRegistrator(this, umbExtensionsRegistry).registerPublicExtensions(); - const initializer = new UmbAppEntryPointExtensionInitializer(this, umbExtensionsRegistry); - await firstValueFrom(initializer.loaded); + new UmbAppEntryPointExtensionInitializer(this, umbExtensionsRegistry); // Try to initialise the auth flow and get the runtime status try { @@ -238,6 +308,33 @@ export class UmbAppElement extends UmbLitElement { await this.#authContext.setInitialState(); } + async #registerExtensions() { + if (this.#packageModules === undefined) { + this.#packageModules = Promise.all(CORE_PACKAGES); + this.#packageModules.then(() => { + this.#loadCurrentUser(); + }); + } + + umbExtensionsRegistry.registerMany((await this.#packageModules).flatMap((modules) => modules.extensions)); + } + + // TODO (V18): Unregister extensions on sign-out. [NL] + /* + async #unregisterExtensions() { + if (!this.#packageModules) return; + (await this.#packageModules).forEach((packageModule) => { + const aliases = packageModule.extensions.map((extension) => extension.alias); + umbExtensionsRegistry.unregisterMany(aliases); + }); + } + */ + + #loadCurrentUser() { + if (!this.#currentUser || !this.#packageModules) return; + this.#currentUser.load(); + } + #redirect() { const pathname = pathWithoutBasePath({ start: true, end: false }); @@ -292,6 +389,26 @@ export class UmbAppElement extends UmbLitElement { return () => this.#authController.isAuthorized() ?? false; } + #loadedGuard(): Guard { + return async () => { + const results = await Promise.allSettled([ + this.observe(this.#bundleInitializer?.loaded).asPromise(), + this.#registerExtensions(), + new UmbServerExtensionRegistrator(this, umbExtensionsRegistry).registerPrivateExtensions(), + ]); + + const result = results.reduce((acc, curr) => acc && curr.status === 'fulfilled', true); + if (result === false) { + this.#errorPage( + 'Extensions failed loading, this might be due to a network issue or a server error. Check that extensions registered on the server are valid.', + undefined, + { headline: 'Failed to load extensions' }, + ); + } + return result; + }; + } + #errorPage(errorMsg: string, error?: unknown, options?: { headline?: string; hideBackButton?: boolean }) { // Redirect to the error page this._routes = [ @@ -312,7 +429,9 @@ export class UmbAppElement extends UmbLitElement { } override render() { - return html``; + return html`
`; } static override styles = css` @@ -327,6 +446,20 @@ export class UmbAppElement extends UmbLitElement { width: 100%; height: 100vh; } + + #loader { + display: flex; + height: 100%; + justify-content: center; + align-items: center; + opacity: 0; + animation: fadeIn 240ms forwards; + } + @keyframes fadeIn { + to { + opacity: 1; + } + } `; } diff --git a/src/Umbraco.Web.UI.Client/src/apps/backoffice/backoffice.context.ts b/src/Umbraco.Web.UI.Client/src/apps/backoffice/backoffice.context.ts index 3ff37a0af90f..1bf14c3f593a 100644 --- a/src/Umbraco.Web.UI.Client/src/apps/backoffice/backoffice.context.ts +++ b/src/Umbraco.Web.UI.Client/src/apps/backoffice/backoffice.context.ts @@ -6,7 +6,6 @@ import { UmbContextBase } from '@umbraco-cms/backoffice/class-api'; import { UmbContextToken } from '@umbraco-cms/backoffice/context-api'; import { UmbExtensionsManifestInitializer } from '@umbraco-cms/backoffice/extension-api'; import { UmbSysinfoRepository } from '@umbraco-cms/backoffice/sysinfo'; -import { UMB_AUTH_CONTEXT } from '@umbraco-cms/backoffice/auth'; import { UMB_CURRENT_USER_CONTEXT } from '@umbraco-cms/backoffice/current-user'; import type { ManifestSection } from '@umbraco-cms/backoffice/section'; import type { UmbControllerHost } from '@umbraco-cms/backoffice/controller-api'; @@ -26,13 +25,7 @@ export class UmbBackofficeContext extends UmbContextBase { constructor(host: UmbControllerHost) { super(host, UMB_BACKOFFICE_CONTEXT); - // TODO: We need to ensure this request is called every time the user logs in, but this should be done somewhere across the app and not here [JOV] - this.consumeContext(UMB_AUTH_CONTEXT, (authContext) => { - this.observe(authContext?.isAuthorized, (isAuthorized) => { - if (!isAuthorized) return; - this.#getVersion(); - }); - }); + this.#getVersion(); this.consumeContext(UMB_CURRENT_USER_CONTEXT, (userContext) => { this.observe( diff --git a/src/Umbraco.Web.UI.Client/src/apps/backoffice/backoffice.element.ts b/src/Umbraco.Web.UI.Client/src/apps/backoffice/backoffice.element.ts index 65e4b5998992..d1361630d39a 100644 --- a/src/Umbraco.Web.UI.Client/src/apps/backoffice/backoffice.element.ts +++ b/src/Umbraco.Web.UI.Client/src/apps/backoffice/backoffice.element.ts @@ -5,52 +5,10 @@ import { UmbEntryPointExtensionInitializer, umbExtensionsRegistry, } from '@umbraco-cms/backoffice/extension-registry'; -import { UmbServerExtensionRegistrator } from '@umbraco-cms/backoffice/extension-api'; import { UmbLitElement } from '@umbraco-cms/backoffice/lit-element'; -import { UMB_AUTH_CONTEXT } from '@umbraco-cms/backoffice/auth'; import './components/index.js'; -const CORE_PACKAGES = [ - import('../../packages/block/umbraco-package.js'), - import('../../packages/clipboard/umbraco-package.js'), - import('../../packages/code-editor/umbraco-package.js'), - import('../../packages/content/umbraco-package.js'), - import('../../packages/data-type/umbraco-package.js'), - import('../../packages/dictionary/umbraco-package.js'), - import('../../packages/documents/umbraco-package.js'), - import('../../packages/embedded-media/umbraco-package.js'), - import('../../packages/extension-insights/umbraco-package.js'), - import('../../packages/health-check/umbraco-package.js'), - import('../../packages/help/umbraco-package.js'), - import('../../packages/language/umbraco-package.js'), - import('../../packages/log-viewer/umbraco-package.js'), - import('../../packages/management-api/umbraco-package.js'), - import('../../packages/markdown-editor/umbraco-package.js'), - import('../../packages/media/umbraco-package.js'), - import('../../packages/members/umbraco-package.js'), - import('../../packages/models-builder/umbraco-package.js'), - import('../../packages/multi-url-picker/umbraco-package.js'), - import('../../packages/packages/umbraco-package.js'), - import('../../packages/performance-profiling/umbraco-package.js'), - import('../../packages/property-editors/umbraco-package.js'), - import('../../packages/publish-cache/umbraco-package.js'), - import('../../packages/relations/umbraco-package.js'), - import('../../packages/rte/umbraco-package.js'), - import('../../packages/settings/umbraco-package.js'), - import('../../packages/static-file/umbraco-package.js'), - import('../../packages/sysinfo/umbraco-package.js'), - import('../../packages/tags/umbraco-package.js'), - import('../../packages/telemetry/umbraco-package.js'), - import('../../packages/templating/umbraco-package.js'), - import('../../packages/tiptap/umbraco-package.js'), - import('../../packages/translation/umbraco-package.js'), - import('../../packages/ufm/umbraco-package.js'), - import('../../packages/umbraco-news/umbraco-package.js'), - import('../../packages/user/umbraco-package.js'), - import('../../packages/webhook/umbraco-package.js'), -]; - @customElement('umb-backoffice') export class UmbBackofficeElement extends UmbLitElement { /** @@ -61,33 +19,11 @@ export class UmbBackofficeElement extends UmbLitElement { constructor() { super(); - new UmbBackofficeContext(this); - new UmbBackofficeEntryPointExtensionInitializer(this, umbExtensionsRegistry); new UmbEntryPointExtensionInitializer(this, umbExtensionsRegistry); } - override async firstUpdated() { - // TODO: Move this logic into the Context? [NL] - await this.#extensionsAfterAuth(); - - // So far local packages are this simple to register, so no need for a manager to do that: - CORE_PACKAGES.forEach(async (packageImport) => { - const packageModule = await packageImport; - umbExtensionsRegistry.registerMany(packageModule.extensions); - }); - } - - async #extensionsAfterAuth() { - const authContext = await this.getContext(UMB_AUTH_CONTEXT, { preventTimeout: true }); - if (!authContext) { - throw new Error('UmbBackofficeElement requires the UMB_AUTH_CONTEXT to be set.'); - } - await this.observe(authContext.isAuthorized).asPromise(); - await new UmbServerExtensionRegistrator(this, umbExtensionsRegistry).registerPrivateExtensions(); - } - override render() { return html` diff --git a/src/Umbraco.Web.UI.Client/src/apps/backoffice/components/backoffice-main.element.ts b/src/Umbraco.Web.UI.Client/src/apps/backoffice/components/backoffice-main.element.ts index d3026ec3392b..6882962a3938 100644 --- a/src/Umbraco.Web.UI.Client/src/apps/backoffice/components/backoffice-main.element.ts +++ b/src/Umbraco.Web.UI.Client/src/apps/backoffice/components/backoffice-main.element.ts @@ -10,7 +10,7 @@ import type { PageComponent, UmbRoute } from '@umbraco-cms/backoffice/router'; @customElement('umb-backoffice-main') export class UmbBackofficeMainElement extends UmbLitElement { @state() - private _routes: Array = []; + private _routes?: Array; @state() private _sections: Array = []; @@ -82,7 +82,9 @@ export class UmbBackofficeMainElement extends UmbLitElement { } override render() { - if (!this._routes.length) return; + if (!this._routes || !this._routes.length) { + return html`
`; + } return html``; } @@ -96,6 +98,19 @@ export class UmbBackofficeMainElement extends UmbLitElement { 100% - 60px ); /* 60 => top header height, TODO: Make sure this comes from somewhere so it is maintainable and eventually responsive. */ } + + #loader { + display: flex; + justify-content: center; + align-items: center; + opacity: 0; + animation: fadeIn 240ms forwards; + } + @keyframes fadeIn { + to { + opacity: 1; + } + } `, ]; } diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extension-initializer.controller.test.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extension-initializer.controller.test.ts index ffc084f3fa81..702b94c9e8fa 100644 --- a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extension-initializer.controller.test.ts +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extension-initializer.controller.test.ts @@ -14,6 +14,7 @@ import { UmbControllerBase } from '@umbraco-cms/backoffice/class-api'; import { customElement, html } from '@umbraco-cms/backoffice/external/lit'; import { UmbSwitchCondition } from '@umbraco-cms/backoffice/extension-registry'; import type { UmbControllerHost } from '@umbraco-cms/backoffice/controller-api'; +import type { SwitchConditionConfig } from 'src/packages/core/extension-registry/conditions/switch.condition.js'; @customElement('umb-test-controller-host') // Element is used in tests @@ -31,7 +32,7 @@ class UmbTestExtensionController extends UmbBaseExtensionInitializer { this._init(); } - protected async _conditionsAreGood() { + protected async _conditionsAreGood(_signal: AbortSignal) { return true; } @@ -487,7 +488,7 @@ describe('UmbBaseExtensionController', () => { describe('Manifest with one condition that changes over time', () => { let hostElement: UmbControllerHostElement; let extensionRegistry: UmbExtensionRegistry; - let manifest: ManifestWithDynamicConditions; + let manifest: ManifestWithDynamicConditions; beforeEach(async () => { hostElement = await fixture(html``); @@ -499,8 +500,8 @@ describe('UmbBaseExtensionController', () => { alias: 'Umb.Test.Section.1', conditions: [ { - alias: 'Umb.Test.Condition.Delay', - value: '100', + alias: 'Umb.Condition.Switch', + frequency: '100', }, ], }; @@ -511,8 +512,8 @@ describe('UmbBaseExtensionController', () => { const conditionManifest = { type: 'condition', - name: 'test-condition-delay', - alias: 'Umb.Test.Condition.Delay', + name: 'test-condition-switch', + alias: 'Umb.Condition.Switch', api: UmbSwitchCondition, }; @@ -545,8 +546,8 @@ describe('UmbBaseExtensionController', () => { describe('Manifest with multiple conditions that changes over time', () => { let hostElement: UmbControllerHostElement; - let extensionRegistry: UmbExtensionRegistry; - let manifest: ManifestWithDynamicConditions; + let extensionRegistry: UmbExtensionRegistry>; + let manifest: ManifestWithDynamicConditions; beforeEach(async () => { hostElement = await fixture(html``); @@ -558,12 +559,12 @@ describe('UmbBaseExtensionController', () => { alias: 'Umb.Test.Section.1', conditions: [ { - alias: 'Umb.Test.Condition.Delay', - value: '10', + alias: 'Umb.Condition.Switch', + frequency: '10', }, { - alias: 'Umb.Test.Condition.Delay', - value: '20', + alias: 'Umb.Condition.Switch', + frequency: '20', }, ], }; @@ -576,8 +577,8 @@ describe('UmbBaseExtensionController', () => { const conditionManifest = { type: 'condition', - name: 'test-condition-delay', - alias: 'Umb.Test.Condition.Delay', + name: 'test-condition-switch', + alias: 'Umb.Condition.Switch', api: UmbSwitchCondition, }; diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extension-initializer.controller.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extension-initializer.controller.ts index 4059e5dd8a73..3b4bc72e0868 100644 --- a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extension-initializer.controller.ts +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extension-initializer.controller.ts @@ -83,6 +83,7 @@ export abstract class UmbBaseExtensionInitializer< this.#gotManifest(extensionManifest); } else { this.#manifest = undefined; + this._isConditionsPositive = undefined; this.#clearPermittedState(); this.#overwrites = []; this.#cleanConditions(); @@ -169,8 +170,8 @@ export abstract class UmbBaseExtensionInitializer< } } - #gotConditions = (manifests: ManifestCondition[]) => { - manifests.forEach(this.#gotCondition); + #gotConditions = (manifests: ManifestCondition[] | undefined) => { + manifests?.forEach(this.#gotCondition); }; #gotCondition = async (conditionManifest: ManifestCondition) => { @@ -237,12 +238,29 @@ export abstract class UmbBaseExtensionInitializer< return undefined; } - #conditionsAreInitialized() { + #checkConditionsAreGood() { // Not good if we don't have a manifest. + if (this.#manifest === undefined) return false; // Only good if conditions of manifest is equal to the amount of condition controllers (one for each condition). [NL] - return ( - this.#manifest !== undefined && this.#conditionControllers.length === (this.#manifest.conditions ?? []).length + const hasAllConditions = (this.#manifest.conditions ?? []).length === this.#conditionControllers.length; + if (hasAllConditions === false) return false; + // Compare all manifest conditions with the condition controllers configs to be sure we have the right ones, as we might end up in a state where we have the same amount of controllers as conditions, but they are not the right ones. [NL] + const allConditionsHaveControllers = (this.#manifest.conditions ?? []).every((condition) => + this.#conditionControllers.some((controller) => controller.config === condition), ); + if (allConditionsHaveControllers === false) return false; + // Only good if all the conditions are permitted: + return this.#conditionControllers.some((condition) => condition.permitted === false) === false; + } + + // The currently-pending `_conditionsAreGood()` promise and manifest, to detect if we can reuse it. + #pendingGoodCall?: { manifest: ManifestType; promise: Promise; abortController: AbortController }; + + #abortPendingGoodCall() { + if (this.#pendingGoodCall) { + this.#pendingGoodCall.abortController.abort(); + this.#pendingGoodCall = undefined; + } } #onConditionsChangedCallback = async () => { @@ -251,38 +269,55 @@ export abstract class UmbBaseExtensionInitializer< // When writing this the only plausible case is a call from the conditionController to the onChange callback. return; } + // Find a condition that is not permitted (Notice how no conditions, means that this extension is permitted) + const isPositive = this.#checkConditionsAreGood(); + + if (this._isConditionsPositive === isPositive) { + // No change in the conditions, so we don't need to do anything, this is an optimization to prevent multiple calls to the callback when there is no change. [NL] + return; + } // We will collect old value here, but we need to re-collect it after a async method have been called, as it could have changed in the mean time. [NL] let oldValue = this.#isPermitted ?? false; - // Find a condition that is not permitted (Notice how no conditions, means that this extension is permitted) - const isPositive = - this.#conditionsAreInitialized() && - this.#conditionControllers.some((condition) => condition.permitted === false) === false; - this._isConditionsPositive = isPositive; if (isPositive === true) { if (this.#isPermitted !== true) { - const newPermission = await this._conditionsAreGood(); - // Only set new permission if we are still positive, otherwise it means that we have been destroyed in the mean time. [NL] - if (newPermission === false || this._isConditionsPositive === false) { - // Then we need to revert the above work: - this._conditionsAreBad(); + let newPermission: boolean; + // check current pending _conditionsAreGood call: [NL] + let entry = this.#pendingGoodCall; + if (entry && entry.manifest === this.#manifest) { + newPermission = await entry.promise; + } else { + // Since this is a new call, then lets abort the pending call. [NL] + this.#abortPendingGoodCall(); + await this._conditionsAreBad(); + const abortController = new AbortController(); + const promise = this._conditionsAreGood(abortController.signal); + entry = { manifest: this.#manifest, promise, abortController }; + this.#pendingGoodCall = entry; + newPermission = await promise; + } + if (this.#pendingGoodCall !== entry) { return; + } else { + this.#pendingGoodCall = undefined; + } + + // Only set new permission if we are still positive, otherwise it means that we have been destroyed in the mean time, or we got destroyed. [NL] + if (newPermission === false || this._isConditionsPositive !== true) { + // Then we need to revert the above work. Sync call — see the note in + // the else-if branch on why we don't await. [NL] + this._conditionsAreBad(); + newPermission = false; } // We update the oldValue as this point, cause in this way we are sure its the value at this point, when doing async code someone else might have changed the state in the mean time. [NL] oldValue = this.#isPermitted ?? false; this.#isPermitted = newPermission; } } else if (this.#isPermitted !== false) { - // Clean up: - await this._conditionsAreBad(); - - // Only continue if we are still negative, otherwise it means that something changed in the mean time. [NL] - if (this._isConditionsPositive === true) { - return; - } - // We update the oldValue as this point, cause in this way we are sure its the value at this point, when doing async code someone else might have changed the state in the mean time. [NL] + // clean-up any current [NL] + this._conditionsAreBad(); oldValue = this.#isPermitted ?? false; this.#isPermitted = false; } @@ -295,7 +330,7 @@ export abstract class UmbBaseExtensionInitializer< } }; - protected abstract _conditionsAreGood(): Promise; + protected abstract _conditionsAreGood(signal: AbortSignal): Promise; protected abstract _conditionsAreBad(): Promise; @@ -335,9 +370,12 @@ export abstract class UmbBaseExtensionInitializer< if (!this.#extensionRegistry) return; this.#manifest = undefined; this.#promiseResolvers = []; + // Abort any pending good-call so its subclass run refuses to commit (sees + // `signal.aborted`) instead of trying to assign into a destroyed initializer. [NL] + this.#abortPendingGoodCall(); this.#clearPermittedState(); // This fires the callback as not permitted, if it was permitted before. [NL] this.#isPermitted = undefined; - this._isConditionsPositive = false; + this._isConditionsPositive = undefined; this.#overwrites = []; this.#cleanConditions(); this.#onPermissionChanged = undefined; diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extension-initializer.race.test.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extension-initializer.race.test.ts new file mode 100644 index 000000000000..c4f8c50bce03 --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extension-initializer.race.test.ts @@ -0,0 +1,388 @@ +import type { ManifestCondition, ManifestWithDynamicConditions, UmbConditionConfigBase } from '../types/index.js'; +import type { UmbConditionControllerArguments } from '../condition/condition-controller-arguments.type.js'; +import { UmbExtensionRegistry } from '../registry/extension.registry.js'; +import { UmbBaseExtensionInitializer } from './index.js'; +import { UmbConditionBase } from '@umbraco-cms/backoffice/extension-registry'; +import { expect, fixture } from '@open-wc/testing'; +import { UmbControllerHostElementMixin } from '@umbraco-cms/backoffice/controller-api'; +import type { UmbControllerHostElement, UmbControllerHost } from '@umbraco-cms/backoffice/controller-api'; +import { customElement, html } from '@umbraco-cms/backoffice/external/lit'; + +// NOTE: These tests target the race condition in `#onConditionsChangedCallback` where +// `_conditionsAreGood()` is genuinely async (for UmbExtensionApiInitializer it dynamically +// imports the API module). While that await is in flight the underlying condition can flip +// multiple times — we want the initializer to settle at whatever the condition's *final* +// permitted state is. See the analysis in this branch for the full trace. + +@customElement('umb-test-race-controller-host') +// eslint-disable-next-line @typescript-eslint/no-unused-vars +class UmbTestRaceControllerHostElement extends UmbControllerHostElementMixin(HTMLElement) {} + +// A condition whose permitted state we can flip from the outside. We stash the most +// recently created instance in a module-level variable so a test can reach it — the +// initializer owns the controller internally otherwise. +let lastManualCondition: UmbManualCondition | undefined; + +class UmbManualCondition extends UmbConditionBase { + constructor(host: UmbControllerHost, args: UmbConditionControllerArguments) { + super(host, args); + lastManualCondition = this; + } + + flipTo(value: boolean) { + this.permitted = value; + } +} + +class UmbSlowGoodExtensionController extends UmbBaseExtensionInitializer { + static goodDelayMs = 40; + goodCalls = 0; + badCalls = 0; + + constructor( + host: UmbControllerHost, + extensionRegistry: UmbExtensionRegistry, + alias: string, + onPermissionChanged?: (isPermitted: boolean) => void, + ) { + super(host, extensionRegistry, 'slow-good', alias, onPermissionChanged); + this._init(); + } + + protected async _conditionsAreGood(signal: AbortSignal) { + this.goodCalls++; + await new Promise((r) => setTimeout(r, UmbSlowGoodExtensionController.goodDelayMs)); + if (signal.aborted) return false; + return true; + } + + protected async _conditionsAreBad() { + this.badCalls++; + } +} + +async function wait(ms: number) { + await new Promise((r) => setTimeout(r, ms)); +} + +describe('UmbBaseExtensionInitializer — condition-flip race with slow _conditionsAreGood', () => { + let hostElement: UmbControllerHostElement; + let extensionRegistry: UmbExtensionRegistry; + + const manifest: ManifestWithDynamicConditions = { + type: 'section', + name: 'race-section', + alias: 'Umb.Test.Race.Section', + conditions: [{ alias: 'Umb.Test.Condition.Manual' }], + }; + const conditionManifest: ManifestCondition = { + type: 'condition', + name: 'race-condition-manual', + alias: 'Umb.Test.Condition.Manual', + api: UmbManualCondition, + }; + + beforeEach(async () => { + hostElement = await fixture(html``); + extensionRegistry = new UmbExtensionRegistry(); + lastManualCondition = undefined; + + extensionRegistry.register(manifest); + extensionRegistry.register(conditionManifest); + }); + + // Baseline — sanity check. Flip once to true with a slow `_conditionsAreGood`; we should + // land at permitted=true after one good-call. + it('settles at true after a single flip to true', async () => { + const history: boolean[] = []; + const controller = new UmbSlowGoodExtensionController(hostElement, extensionRegistry, manifest.alias, (p) => + history.push(p), + ); + + // Wait for manifest + condition wiring. + await wait(0); + expect(lastManualCondition, 'condition instance must exist').to.exist; + + lastManualCondition!.flipTo(true); + await wait(UmbSlowGoodExtensionController.goodDelayMs + 30); + + expect(controller.permitted, `history: ${JSON.stringify(history)}`).to.be.true; + controller.destroy(); + }); + + // Repro for the user-reported symptom: "very fast goes from good to bad to good, then it + // ends up not being good." The final condition value is TRUE, so the extension must + // settle at permitted=true. + it('settles at true after flipping true → false → true while _conditionsAreGood is in flight', async () => { + const history: boolean[] = []; + const controller = new UmbSlowGoodExtensionController(hostElement, extensionRegistry, manifest.alias, (p) => + history.push(p), + ); + + await wait(0); + + // Flip synchronously to maximise overlap with the pending `_conditionsAreGood` await. + // Each call is dedup'd by the condition setter (different value vs. previous), so all + // three onChange notifications fire and trigger `#onConditionsChangedCallback`. + lastManualCondition!.flipTo(true); + lastManualCondition!.flipTo(false); + lastManualCondition!.flipTo(true); + + // 2 × good-delay + buffer, enough for all pending awaits to settle. + await wait(UmbSlowGoodExtensionController.goodDelayMs * 2 + 60); + + expect( + controller.permitted, + `expected permitted=true (final condition state). callback history: ${JSON.stringify(history)}`, + ).to.be.true; + expect(lastManualCondition!.permitted, 'condition final state').to.be.true; + controller.destroy(); + }); + + // Same scenario, one more round of flips. With 5 rapid transitions (true,false,true, + // false,true) the setter-dedup + the "#isPermitted !== false" guard in the callback can + // skip an intermediate callback, leaving `_isConditionsPositive` in a stale state when + // the earlier in-flight `_conditionsAreGood` finally resolves. Final should still be true. + it('settles at true after flipping true → false → true → false → true', async () => { + const history: boolean[] = []; + const controller = new UmbSlowGoodExtensionController(hostElement, extensionRegistry, manifest.alias, (p) => + history.push(p), + ); + + await wait(0); + + lastManualCondition!.flipTo(true); + lastManualCondition!.flipTo(false); + lastManualCondition!.flipTo(true); + lastManualCondition!.flipTo(false); + lastManualCondition!.flipTo(true); + + await wait(UmbSlowGoodExtensionController.goodDelayMs * 3 + 80); + + expect(controller.permitted, `expected permitted=true. callback history: ${JSON.stringify(history)}`).to.be.true; + expect(lastManualCondition!.permitted, 'condition final state').to.be.true; + controller.destroy(); + }); + + // Inverse — final condition value is FALSE, extension must settle at false. This is the + // "negative" side of the same race: the `_conditionsAreGood` in-flight from an earlier + // +true flip must not clobber the later -false decision. + it('settles at false after flipping true → false → true → false', async () => { + const history: boolean[] = []; + const controller = new UmbSlowGoodExtensionController(hostElement, extensionRegistry, manifest.alias, (p) => + history.push(p), + ); + + await wait(0); + + lastManualCondition!.flipTo(true); + lastManualCondition!.flipTo(false); + lastManualCondition!.flipTo(true); + lastManualCondition!.flipTo(false); + + await wait(UmbSlowGoodExtensionController.goodDelayMs * 3 + 80); + + expect(controller.permitted, `expected permitted=false. callback history: ${JSON.stringify(history)}`).to.be.false; + expect(lastManualCondition!.permitted, 'condition final state').to.be.false; + controller.destroy(); + }); + + // Flips with small microtask gaps between them (not synchronous). This mirrors the + // real observable pipeline where `mergeObservables` + `shareReplay` emissions are + // spread across microtasks. + it('settles at true when flips are separated by microtask gaps (true → false → true)', async () => { + const history: boolean[] = []; + const controller = new UmbSlowGoodExtensionController(hostElement, extensionRegistry, manifest.alias, (p) => + history.push(p), + ); + + await wait(0); + + lastManualCondition!.flipTo(true); + await Promise.resolve(); + lastManualCondition!.flipTo(false); + await Promise.resolve(); + lastManualCondition!.flipTo(true); + + await wait(UmbSlowGoodExtensionController.goodDelayMs * 2 + 60); + + expect(controller.permitted, `expected permitted=true. history: ${JSON.stringify(history)}`).to.be.true; + controller.destroy(); + }); + + // Flips that arrive WHILE a previous _conditionsAreGood is already resolving. We force + // this by waiting just a touch less than good-delay before the next flip, so two awaits + // overlap with different pending states. + it('settles at true when a flip arrives mid-resolution of a previous good-call', async () => { + const history: boolean[] = []; + const controller = new UmbSlowGoodExtensionController(hostElement, extensionRegistry, manifest.alias, (p) => + history.push(p), + ); + + await wait(0); + + lastManualCondition!.flipTo(true); + await wait(UmbSlowGoodExtensionController.goodDelayMs - 15); + lastManualCondition!.flipTo(false); + await wait(5); + lastManualCondition!.flipTo(true); + + await wait(UmbSlowGoodExtensionController.goodDelayMs * 2 + 80); + + expect(controller.permitted, `expected permitted=true. history: ${JSON.stringify(history)}`).to.be.true; + controller.destroy(); + }); + + // Ten rapid flips ending at true. If there's any cumulative drift in state tracking + // across callbacks, this should expose it. + it('settles at true after ten rapid flips ending at true', async () => { + const history: boolean[] = []; + const controller = new UmbSlowGoodExtensionController(hostElement, extensionRegistry, manifest.alias, (p) => + history.push(p), + ); + + await wait(0); + + for (let i = 0; i < 10; i++) { + lastManualCondition!.flipTo(i % 2 === 0); // t,f,t,f,... ending at... let's compute + } + // Loop ends at i=9 (odd) → flipTo(false). Add one more to end at true. + lastManualCondition!.flipTo(true); + + await wait(UmbSlowGoodExtensionController.goodDelayMs * 4 + 100); + + expect(controller.permitted, `expected permitted=true. history: ${JSON.stringify(history)}`).to.be.true; + controller.destroy(); + }); + + // Flips that straddle a longer gap than the good-delay: earlier callbacks have fully + // resolved before the next flip comes in. This is the "clean" serial case and should + // obviously work — it's the control against which the above race tests stand out. + it('settles at the final value when flips are spaced out longer than the good-delay', async () => { + const history: boolean[] = []; + const controller = new UmbSlowGoodExtensionController(hostElement, extensionRegistry, manifest.alias, (p) => + history.push(p), + ); + + await wait(0); + + lastManualCondition!.flipTo(true); + await wait(UmbSlowGoodExtensionController.goodDelayMs + 20); + lastManualCondition!.flipTo(false); + await wait(UmbSlowGoodExtensionController.goodDelayMs + 20); + lastManualCondition!.flipTo(true); + await wait(UmbSlowGoodExtensionController.goodDelayMs + 20); + + expect(controller.permitted, `expected permitted=true (well-spaced flips). history: ${JSON.stringify(history)}`).to + .be.true; + controller.destroy(); + }); +}); + +// --------------------------------------------------------------------------- +// Same scenarios, but using the real `UmbExtensionApiInitializer` — this is what the block +// workspace submit button actually uses. Its `_conditionsAreGood()` dynamically imports the +// API class, which in production is what turns "fast flip" into a bug. +// --------------------------------------------------------------------------- + +import { UmbExtensionApiInitializer } from './extension-api-initializer.controller.js'; +import type { ManifestApi } from '../types/index.js'; + +// A minimal API class whose construction we can slow down — this stands in for the cost of +// `await import('./submit.action.js')` in the real world. +class UmbSlowApiClass { + public ready = false; + constructor() { + // The real initializer awaits `createExtensionApi`, which awaits the dynamic + // import. We simulate the import delay by not doing anything here — the delay + // lives in a custom factory below, not the constructor. + this.ready = true; + } + destroy() { + /* no-op */ + } +} + +describe('UmbExtensionApiInitializer — condition-flip race with a slow API factory', () => { + let hostElement: UmbControllerHostElement; + let extensionRegistry: UmbExtensionRegistry; + + const apiManifest: ManifestApi & ManifestWithDynamicConditions = { + type: 'test-type' as 'test-type', + name: 'race-api', + alias: 'Umb.Test.Race.Api', + // A factory that returns the API class, but only after a delay, emulating a dynamic + // module import. `loadManifestApi` expects either `{ api }` or `{ default }`. + api: async () => { + await new Promise((r) => setTimeout(r, 30)); + return { api: UmbSlowApiClass }; + }, + conditions: [{ alias: 'Umb.Test.Condition.Manual' }], + } as any; + + const conditionManifest: ManifestCondition = { + type: 'condition', + name: 'race-condition-manual', + alias: 'Umb.Test.Condition.Manual', + api: UmbManualCondition, + }; + + beforeEach(async () => { + hostElement = await fixture(html``); + extensionRegistry = new UmbExtensionRegistry(); + lastManualCondition = undefined; + + extensionRegistry.register(apiManifest as any); + extensionRegistry.register(conditionManifest); + }); + + it('settles at permitted=true when flipping true → false → true during slow API construction', async () => { + const history: boolean[] = []; + const controller = new UmbExtensionApiInitializer( + hostElement, + extensionRegistry as any, + apiManifest.alias, + [hostElement], + (p) => history.push(p), + ); + + await wait(0); + expect(lastManualCondition, 'condition instance must exist').to.exist; + + lastManualCondition!.flipTo(true); + lastManualCondition!.flipTo(false); + lastManualCondition!.flipTo(true); + + await wait(200); + + expect(controller.permitted, `expected permitted=true. callback history: ${JSON.stringify(history)}`).to.be.true; + expect(controller.api, 'api instance should exist when permitted').to.exist; + + controller.destroy(); + }); + + it('settles at permitted=false when flipping true → false → true → false during slow API construction', async () => { + const history: boolean[] = []; + const controller = new UmbExtensionApiInitializer( + hostElement, + extensionRegistry as any, + apiManifest.alias, + [hostElement], + (p) => history.push(p), + ); + + await wait(0); + + lastManualCondition!.flipTo(true); + lastManualCondition!.flipTo(false); + lastManualCondition!.flipTo(true); + lastManualCondition!.flipTo(false); + + await wait(200); + + expect(controller.permitted, `expected permitted=false. callback history: ${JSON.stringify(history)}`).to.be.false; + expect(controller.api, 'api instance should be destroyed when not permitted').to.be.undefined; + + controller.destroy(); + }); +}); diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extension-initializer.two-conditions.race.test.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extension-initializer.two-conditions.race.test.ts new file mode 100644 index 000000000000..90581a54176d --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extension-initializer.two-conditions.race.test.ts @@ -0,0 +1,290 @@ +import type { + ManifestCondition, + ManifestWithDynamicConditions, + UmbConditionConfigBase, +} from '../types/index.js'; +import type { UmbConditionControllerArguments } from '../condition/condition-controller-arguments.type.js'; +import { UmbExtensionRegistry } from '../registry/extension.registry.js'; +import { UmbBaseExtensionInitializer } from './index.js'; +import { UmbConditionBase } from '@umbraco-cms/backoffice/extension-registry'; +import { expect, fixture } from '@open-wc/testing'; +import { UmbControllerHostElementMixin } from '@umbraco-cms/backoffice/controller-api'; +import type { UmbControllerHost, UmbControllerHostElement } from '@umbraco-cms/backoffice/controller-api'; +import { customElement, html } from '@umbraco-cms/backoffice/external/lit'; + +// The real `Umb.WorkspaceAction.Block.SubmitUpdate` has TWO conditions: +// 1. UMB_WORKSPACE_CONDITION_ALIAS matching UMB_BLOCK_WORKSPACE_ALIAS +// 2. Umb.Condition.BlockWorkspaceIsReadOnly with match: false +// All prior race tests used a single condition. These tests reproduce the two-condition +// shape and exercise various orderings of when each condition first emits relative to +// the readonly condition flipping true → false → true during a slow API load. + +@customElement('umb-test-two-cond-race-host') +// eslint-disable-next-line @typescript-eslint/no-unused-vars +class UmbTestTwoCondRaceHost extends UmbControllerHostElementMixin(HTMLElement) {} + +interface ManualConfig extends UmbConditionConfigBase { + uniqueKey: string; +} + +const conditionsByKey = new Map(); + +class UmbManualCondition extends UmbConditionBase { + constructor(host: UmbControllerHost, args: UmbConditionControllerArguments) { + super(host, args); + conditionsByKey.set(args.config.uniqueKey, this); + } + flipTo(value: boolean) { + this.permitted = value; + } +} + +const conditionManifest: ManifestCondition = { + type: 'condition', + name: 'two-cond-manual', + alias: 'Umb.Test.TwoCond.Manual', + api: UmbManualCondition, +}; + +// Slow good emulates the dynamic-import cost of a real workspace-action API factory. +class UmbSlowGoodController extends UmbBaseExtensionInitializer { + static goodDelayMs = 30; + + constructor( + host: UmbControllerHost, + extensionRegistry: UmbExtensionRegistry, + alias: string, + onPermissionChanged: (isPermitted: boolean, controller: UmbSlowGoodController) => void, + ) { + super(host, extensionRegistry, 'two-cond-slow', alias, onPermissionChanged); + this._init(); + } + + protected async _conditionsAreGood(signal: AbortSignal) { + await new Promise((r) => setTimeout(r, UmbSlowGoodController.goodDelayMs)); + if (signal.aborted) return false; + return true; + } + + protected async _conditionsAreBad() { + /* no-op */ + } +} + +async function wait(ms: number) { + await new Promise((r) => setTimeout(r, ms)); +} + +describe('UmbBaseExtensionInitializer — two-condition race', () => { + let hostElement: UmbControllerHostElement; + let extensionRegistry: UmbExtensionRegistry; + + const manifest: ManifestWithDynamicConditions = { + type: 'section', + name: 'two-cond', + alias: 'Umb.Test.TwoCond.Ext', + conditions: [ + { alias: conditionManifest.alias, uniqueKey: 'gate' } as UmbConditionConfigBase, + { alias: conditionManifest.alias, uniqueKey: 'readonly' } as UmbConditionConfigBase, + ], + }; + + beforeEach(async () => { + hostElement = await fixture(html``); + extensionRegistry = new UmbExtensionRegistry(); + conditionsByKey.clear(); + + extensionRegistry.register(conditionManifest); + extensionRegistry.register(manifest); + }); + + // Baseline — both conditions become true and stay true; extension must be permitted. + it('permits when both conditions become true serially', async () => { + const history: boolean[] = []; + const controller = new UmbSlowGoodController( + hostElement, + extensionRegistry, + manifest.alias, + (p) => history.push(p), + ); + + await wait(0); + conditionsByKey.get('gate')!.flipTo(true); + conditionsByKey.get('readonly')!.flipTo(true); + + await wait(UmbSlowGoodController.goodDelayMs + 80); + + expect(controller.permitted, `history: ${JSON.stringify(history)}`).to.be.true; + controller.destroy(); + }); + + // The actual user-reported shape. `gate` settles true early; `readonly` flips + // true → false → true while the gate is stable. Final expected: permitted=true. + it('permits when gate=true and readonly flips true → false → true', async () => { + const history: boolean[] = []; + const controller = new UmbSlowGoodController( + hostElement, + extensionRegistry, + manifest.alias, + (p) => history.push(p), + ); + + await wait(0); + + conditionsByKey.get('gate')!.flipTo(true); + // Let the gate's positive transition finish settling (rAF / microtasks) before + // the readonly burst begins — matches real life where the workspace condition + // resolves before the readonly rules get assembled. + await wait(UmbSlowGoodController.goodDelayMs + 40); + + const readonly = conditionsByKey.get('readonly')!; + readonly.flipTo(true); + readonly.flipTo(false); + readonly.flipTo(true); + + await wait(UmbSlowGoodController.goodDelayMs * 2 + 100); + + expect( + controller.permitted, + `final permitted should be true. history: ${JSON.stringify(history)}`, + ).to.be.true; + controller.destroy(); + }); + + // Both conditions race — gate becomes true AFTER readonly has already completed its + // flip burst. If the initializer latched something during the burst it needs to + // re-evaluate when the gate finally becomes true. + it('permits when readonly flips true → false → true BEFORE gate becomes true', async () => { + const history: boolean[] = []; + const controller = new UmbSlowGoodController( + hostElement, + extensionRegistry, + manifest.alias, + (p) => history.push(p), + ); + + await wait(0); + + const readonly = conditionsByKey.get('readonly')!; + readonly.flipTo(true); + readonly.flipTo(false); + readonly.flipTo(true); + + // Readonly ends true. Gate is still false — extension cannot be permitted yet. + await wait(UmbSlowGoodController.goodDelayMs + 40); + + conditionsByKey.get('gate')!.flipTo(true); + + await wait(UmbSlowGoodController.goodDelayMs + 60); + + expect( + controller.permitted, + `final permitted should be true. history: ${JSON.stringify(history)}`, + ).to.be.true; + controller.destroy(); + }); + + // Gate and readonly are BOTH flipping during the same burst. Final state of each is + // true. The initializer has two in-flight good-calls and several dead-zone callbacks + // to juggle. + it('permits when gate and readonly both flip concurrently, both end true', async () => { + const history: boolean[] = []; + const controller = new UmbSlowGoodController( + hostElement, + extensionRegistry, + manifest.alias, + (p) => history.push(p), + ); + + await wait(0); + + const gate = conditionsByKey.get('gate')!; + const readonly = conditionsByKey.get('readonly')!; + + gate.flipTo(true); + readonly.flipTo(true); + readonly.flipTo(false); + gate.flipTo(false); + readonly.flipTo(true); + gate.flipTo(true); + + await wait(UmbSlowGoodController.goodDelayMs * 3 + 120); + + expect( + controller.permitted, + `final permitted should be true. history: ${JSON.stringify(history)}`, + ).to.be.true; + controller.destroy(); + }); + + // Same synchronous burst pattern as the user-observed sequence, but starting from a + // clean slate where BOTH conditions default to false and emit their first value in + // quick succession — closer to what happens on initial workspace mount. + it('permits after synchronous initial burst: gate+true, readonly true→false→true', async () => { + const history: boolean[] = []; + const controller = new UmbSlowGoodController( + hostElement, + extensionRegistry, + manifest.alias, + (p) => history.push(p), + ); + + await wait(0); + + // Everything fires within one synchronous tick — gate resolves first, then the + // readonly condition gets an emission burst from guard rules being assembled. + conditionsByKey.get('gate')!.flipTo(true); + const readonly = conditionsByKey.get('readonly')!; + readonly.flipTo(true); + readonly.flipTo(false); + readonly.flipTo(true); + + await wait(UmbSlowGoodController.goodDelayMs * 2 + 120); + + expect( + controller.permitted, + `final permitted should be true. history: ${JSON.stringify(history)}`, + ).to.be.true; + controller.destroy(); + }); + + // Dead-zone stress — a known suspicious area: when `#isPermitted === false` and the + // callback fires with `isPositive === false`, the else-if guard + // `#isPermitted !== false` evaluates FALSE and the callback falls through without + // setting `_isConditionsPositive` in the else-if branch. (It *does* set it + // unconditionally at the top though — let's see if that's enough.) + it('permits when readonly flips many times while gate flickers', async () => { + const history: boolean[] = []; + const controller = new UmbSlowGoodController( + hostElement, + extensionRegistry, + manifest.alias, + (p) => history.push(p), + ); + + await wait(0); + + const gate = conditionsByKey.get('gate')!; + const readonly = conditionsByKey.get('readonly')!; + + // Alternate gate and readonly flips, ending both at true. + gate.flipTo(true); + readonly.flipTo(true); + gate.flipTo(false); + readonly.flipTo(false); + gate.flipTo(true); + readonly.flipTo(true); + gate.flipTo(false); + readonly.flipTo(false); + gate.flipTo(true); + readonly.flipTo(true); + + await wait(UmbSlowGoodController.goodDelayMs * 4 + 160); + + expect( + controller.permitted, + `final permitted should be true. history: ${JSON.stringify(history)}`, + ).to.be.true; + controller.destroy(); + }); +}); diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extensions-initializer.controller.test.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extensions-initializer.controller.test.ts index 4be62ea2ad2f..52d947ca8fa4 100644 --- a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extensions-initializer.controller.test.ts +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extensions-initializer.controller.test.ts @@ -25,7 +25,7 @@ class UmbTestExtensionController extends UmbBaseExtensionInitializer { this._init(); } - protected async _conditionsAreGood() { + protected async _conditionsAreGood(_signal: AbortSignal) { return true; } diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extensions-initializer.race.test.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extensions-initializer.race.test.ts new file mode 100644 index 000000000000..bd1b861093af --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extensions-initializer.race.test.ts @@ -0,0 +1,515 @@ +import type { + ManifestCondition, + ManifestWithDynamicConditions, + UmbConditionConfigBase, +} from '../types/index.js'; +import type { UmbConditionControllerArguments } from '../condition/condition-controller-arguments.type.js'; +import { UmbExtensionRegistry } from '../registry/extension.registry.js'; +import type { PermittedControllerType, UmbBaseExtensionsInitializerArgs } from './index.js'; +import { UmbBaseExtensionInitializer, UmbBaseExtensionsInitializer } from './index.js'; +import { UmbConditionBase } from '@umbraco-cms/backoffice/extension-registry'; +import { expect, fixture } from '@open-wc/testing'; +import { UmbControllerHostElementMixin } from '@umbraco-cms/backoffice/controller-api'; +import type { UmbControllerHost, UmbControllerHostElement } from '@umbraco-cms/backoffice/controller-api'; +import { customElement, html } from '@umbraco-cms/backoffice/external/lit'; + +// These tests target the extra machinery that `UmbBaseExtensionsInitializer` layers on top +// of `UmbBaseExtensionInitializer`: +// * `_extensionChanged` which mutates `#permittedExts` on every single-initializer flip; +// * `#notifyChange` debounced via `requestAnimationFrame` (one batched onChange per frame); +// * overwrite/single-mode post-processing. +// If an individual extension's condition flips rapidly while its API factory is loading, +// the plural initializer must end up with `#exposedPermittedExts` matching the *final* +// permitted state — not a stale intermediate one. + +@customElement('umb-test-plural-race-host') +// eslint-disable-next-line @typescript-eslint/no-unused-vars +class UmbTestPluralRaceHost extends UmbControllerHostElementMixin(HTMLElement) {} + +// Track condition instances per alias so tests can drive each extension's condition +// independently — we can't use the "last created" trick when multiple extensions share the +// same condition alias. +const conditionByUnique = new Map(); + +interface ManualConfig extends UmbConditionConfigBase { + uniqueKey: string; +} + +class UmbManualCondition extends UmbConditionBase { + constructor(host: UmbControllerHost, args: UmbConditionControllerArguments) { + super(host, args); + conditionByUnique.set(args.config.uniqueKey, this); + } + + flipTo(value: boolean) { + this.permitted = value; + } +} + +const conditionManifest: ManifestCondition = { + type: 'condition', + name: 'plural-race-condition-manual', + alias: 'Umb.Test.Plural.Race.Condition.Manual', + api: UmbManualCondition, +}; + +// Individual extension controller with a genuinely async `_conditionsAreGood` so flips can +// overlap the await. Mirrors what the real `UmbExtensionApiInitializer` does with dynamic +// imports. +class UmbSlowGoodController extends UmbBaseExtensionInitializer { + static goodDelayMs = 30; + + constructor( + host: UmbControllerHost, + extensionRegistry: UmbExtensionRegistry, + alias: string, + onPermissionChanged: (isPermitted: boolean, controller: UmbSlowGoodController) => void, + ) { + super(host, extensionRegistry, 'plural-slow-good', alias, onPermissionChanged); + this._init(); + } + + protected async _conditionsAreGood(signal: AbortSignal) { + await new Promise((r) => setTimeout(r, UmbSlowGoodController.goodDelayMs)); + if (signal.aborted) return false; + return true; + } + + protected async _conditionsAreBad() { + /* no-op */ + } +} + +type TestManifests = ManifestWithDynamicConditions | ManifestCondition; + +class UmbTestPluralController extends UmbBaseExtensionsInitializer< + TestManifests, + 'test-plural-race', + ManifestWithDynamicConditions, + UmbSlowGoodController, + PermittedControllerType +> { + #registry: UmbExtensionRegistry; + + constructor( + host: UmbControllerHost, + extensionRegistry: UmbExtensionRegistry, + onChange: (permitted: Array>) => void, + args?: UmbBaseExtensionsInitializerArgs, + ) { + super(host, extensionRegistry, 'test-plural-race', null, onChange, 'testPluralController', args); + this.#registry = extensionRegistry; + this._init(); + } + + protected _createController(manifest: ManifestWithDynamicConditions) { + return new UmbSlowGoodController(this, this.#registry, manifest.alias, this._extensionChanged); + } +} + +async function wait(ms: number) { + await new Promise((r) => setTimeout(r, ms)); +} + +// Wait until rAF has had a chance to flush the debounced notify — a couple of rAFs to be +// safe against microtask ordering. +async function waitForDebouncedNotify() { + await new Promise((r) => + requestAnimationFrame(() => requestAnimationFrame(() => r())), + ); +} + +describe('UmbBaseExtensionsInitializer — condition-flip race', () => { + let hostElement: UmbControllerHostElement; + let extensionRegistry: UmbExtensionRegistry; + + const makeManifest = (alias: string, uniqueKey: string, hasCondition = true): ManifestWithDynamicConditions => ({ + type: 'test-plural-race', + name: alias, + alias, + weight: 100, + ...(hasCondition + ? { + conditions: [ + { + alias: conditionManifest.alias, + uniqueKey, + } as UmbConditionConfigBase, + ], + } + : {}), + }); + + beforeEach(async () => { + hostElement = await fixture(html``); + extensionRegistry = new UmbExtensionRegistry(); + conditionByUnique.clear(); + extensionRegistry.register(conditionManifest); + }); + + // Baseline — one extension with a condition. Single flip to true should land in the + // exposed list. + it('exposes the extension after a single flip to true', async () => { + const manifest = makeManifest('Umb.Test.Plural.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = new UmbTestPluralController(hostElement, extensionRegistry, (permitted) => + changes.push(permitted.map((p) => p.alias)), + ); + + await wait(0); + conditionByUnique.get('A')!.flipTo(true); + await wait(UmbSlowGoodController.goodDelayMs + 40); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect(finalState, `exposed list history: ${JSON.stringify(changes)}`).to.deep.equal([manifest.alias]); + + plural.destroy(); + }); + + // The "stuck at bad" repro — final condition state is true, so the plural initializer's + // exposed list must include the extension. + it('exposes the extension after condition flips true → false → true', async () => { + const manifest = makeManifest('Umb.Test.Plural.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = new UmbTestPluralController(hostElement, extensionRegistry, (permitted) => + changes.push(permitted.map((p) => p.alias)), + ); + + await wait(0); + + const cond = conditionByUnique.get('A')!; + cond.flipTo(true); + cond.flipTo(false); + cond.flipTo(true); + + await wait(UmbSlowGoodController.goodDelayMs * 2 + 60); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `final exposed should include extension. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifest.alias]); + expect(cond.permitted, 'condition final state').to.be.true; + + plural.destroy(); + }); + + // Inverse — final condition is false, the extension must be absent from the list. + it('excludes the extension after condition flips true → false → true → false', async () => { + const manifest = makeManifest('Umb.Test.Plural.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = new UmbTestPluralController(hostElement, extensionRegistry, (permitted) => + changes.push(permitted.map((p) => p.alias)), + ); + + await wait(0); + + const cond = conditionByUnique.get('A')!; + cond.flipTo(true); + cond.flipTo(false); + cond.flipTo(true); + cond.flipTo(false); + + await wait(UmbSlowGoodController.goodDelayMs * 3 + 80); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `final exposed should NOT include extension. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([]); + + plural.destroy(); + }); + + // Five flips — ending at true. Longer sequence to surface any cumulative drift. + it('exposes the extension after five flips ending at true', async () => { + const manifest = makeManifest('Umb.Test.Plural.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = new UmbTestPluralController(hostElement, extensionRegistry, (permitted) => + changes.push(permitted.map((p) => p.alias)), + ); + + await wait(0); + + const cond = conditionByUnique.get('A')!; + cond.flipTo(true); + cond.flipTo(false); + cond.flipTo(true); + cond.flipTo(false); + cond.flipTo(true); + + await wait(UmbSlowGoodController.goodDelayMs * 3 + 100); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `final exposed should include extension. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifest.alias]); + + plural.destroy(); + }); + + // Rapid synchronous flips should be coalesced by the rAF debouncer into at most one + // onChange per state transition visible to the consumer. + it('debounces onChange when flips all happen inside a single frame', async () => { + const manifest = makeManifest('Umb.Test.Plural.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = new UmbTestPluralController(hostElement, extensionRegistry, (permitted) => + changes.push(permitted.map((p) => p.alias)), + ); + + await wait(0); + + const cond = conditionByUnique.get('A')!; + cond.flipTo(true); + cond.flipTo(false); + cond.flipTo(true); + cond.flipTo(false); + cond.flipTo(true); + + // Wait for all `_conditionsAreGood` awaits to complete, then wait for rAF flush. + await wait(UmbSlowGoodController.goodDelayMs * 3 + 100); + await waitForDebouncedNotify(); + + // The final state is "permitted=true" so the extension must be in the list. The + // number of onChange emissions is not fixed (debouncer coalesces within frames), + // but the final emission must reflect truth. + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `final exposed should include extension. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifest.alias]); + + plural.destroy(); + }); + + // Two extensions, each with its own manually-controlled condition, flipping concurrently. + // Final state — A=true, B=false — must be reflected in the exposed list. + it('tracks two extensions independently when their conditions flip concurrently', async () => { + const manifestA = makeManifest('Umb.Test.Plural.Race.A', 'A'); + const manifestB = makeManifest('Umb.Test.Plural.Race.B', 'B'); + extensionRegistry.register(manifestA); + extensionRegistry.register(manifestB); + + const changes: Array = []; + const plural = new UmbTestPluralController(hostElement, extensionRegistry, (permitted) => + changes.push(permitted.map((p) => p.alias).sort()), + ); + + await wait(0); + + const condA = conditionByUnique.get('A')!; + const condB = conditionByUnique.get('B')!; + + // Interleave flips so the two initializers' awaits overlap. + condA.flipTo(true); + condB.flipTo(true); + condA.flipTo(false); + condB.flipTo(false); + condA.flipTo(true); + condB.flipTo(true); + condB.flipTo(false); // B ends at false + + await wait(UmbSlowGoodController.goodDelayMs * 3 + 100); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `only A should be permitted. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifestA.alias]); + + plural.destroy(); + }); + + // Both extensions flipping in lockstep, both end at true. Must both be in the list. + it('exposes both extensions when both final states are true after flips', async () => { + const manifestA = makeManifest('Umb.Test.Plural.Race.A', 'A'); + const manifestB = makeManifest('Umb.Test.Plural.Race.B', 'B'); + extensionRegistry.register(manifestA); + extensionRegistry.register(manifestB); + + const changes: Array = []; + const plural = new UmbTestPluralController(hostElement, extensionRegistry, (permitted) => + changes.push(permitted.map((p) => p.alias).sort()), + ); + + await wait(0); + + const condA = conditionByUnique.get('A')!; + const condB = conditionByUnique.get('B')!; + + condA.flipTo(true); + condB.flipTo(true); + condA.flipTo(false); + condB.flipTo(false); + condA.flipTo(true); + condB.flipTo(true); + + await wait(UmbSlowGoodController.goodDelayMs * 3 + 100); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `both extensions should be in the list. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifestA.alias, manifestB.alias].sort()); + + plural.destroy(); + }); + + // Flip happens DURING the await window of a previous _conditionsAreGood — a more + // adversarial scenario than synchronous flips (since the flip can change + // `_isConditionsPositive` between when an older callback read it and when the newer + // callback sets it). + it('exposes the extension when a flip lands mid-resolution of a previous good-call', async () => { + const manifest = makeManifest('Umb.Test.Plural.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = new UmbTestPluralController(hostElement, extensionRegistry, (permitted) => + changes.push(permitted.map((p) => p.alias)), + ); + + await wait(0); + const cond = conditionByUnique.get('A')!; + + cond.flipTo(true); + await wait(UmbSlowGoodController.goodDelayMs - 10); // in flight + cond.flipTo(false); + await wait(5); + cond.flipTo(true); + + await wait(UmbSlowGoodController.goodDelayMs * 2 + 100); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `final exposed should include extension. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifest.alias]); + + plural.destroy(); + }); + + // Unregister the extension's manifest mid-race. After unregister the list must be empty + // regardless of what the condition was doing. + it('drops the extension when its manifest is unregistered during a flip race', async () => { + const manifest = makeManifest('Umb.Test.Plural.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = new UmbTestPluralController(hostElement, extensionRegistry, (permitted) => + changes.push(permitted.map((p) => p.alias)), + ); + + await wait(0); + const cond = conditionByUnique.get('A')!; + + cond.flipTo(true); + cond.flipTo(false); + cond.flipTo(true); + + // Unregister while good-calls are still pending. + await wait(10); + extensionRegistry.unregister(manifest.alias); + + await wait(UmbSlowGoodController.goodDelayMs * 2 + 100); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `extension should be absent after unregister. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([]); + + plural.destroy(); + }); + + // Single-mode: only the highest-weighted extension should be exposed. Final state of + // both conditions is true, but `single: true` collapses the list to just one entry. + it('exposes only the highest-weight extension in single mode when both end at true', async () => { + const manifestA = { ...makeManifest('Umb.Test.Plural.Race.A', 'A'), weight: 1 }; + const manifestB = { ...makeManifest('Umb.Test.Plural.Race.B', 'B'), weight: 99 }; + extensionRegistry.register(manifestA); + extensionRegistry.register(manifestB); + + const changes: Array = []; + const plural = new UmbTestPluralController( + hostElement, + extensionRegistry, + (permitted) => changes.push(permitted.map((p) => p.alias)), + { single: true }, + ); + + await wait(0); + + const condA = conditionByUnique.get('A')!; + const condB = conditionByUnique.get('B')!; + + condA.flipTo(true); + condB.flipTo(true); + condA.flipTo(false); + condB.flipTo(false); + condA.flipTo(true); + condB.flipTo(true); + + await wait(UmbSlowGoodController.goodDelayMs * 3 + 100); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `only the highest-weight extension should be exposed. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifestB.alias]); + + plural.destroy(); + }); + + // The consumer only sees emissions via `#notifyChange`, so after a burst of flips the + // final emission must match `#permittedExts` at the time of that rAF. If there's any + // desync between `#permittedExts` and the final `onChange` payload the final exposed + // list will lie about the internal state. + it('final onChange payload matches the true permitted state after ten rapid flips', async () => { + const manifest = makeManifest('Umb.Test.Plural.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = new UmbTestPluralController(hostElement, extensionRegistry, (permitted) => + changes.push(permitted.map((p) => p.alias)), + ); + + await wait(0); + const cond = conditionByUnique.get('A')!; + + for (let i = 0; i < 10; i++) { + cond.flipTo(i % 2 === 0); + } + cond.flipTo(true); // ensure final is true + + await wait(UmbSlowGoodController.goodDelayMs * 4 + 120); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `final exposed should include extension. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifest.alias]); + + plural.destroy(); + }); +}); diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-api-initializer.controller.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-api-initializer.controller.ts index 1e995a2fd63c..2ce79dbe240c 100644 --- a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-api-initializer.controller.ts +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-api-initializer.controller.ts @@ -83,7 +83,7 @@ export class UmbExtensionApiInitializer< }; */ - protected async _conditionsAreGood() { + protected async _conditionsAreGood(signal: AbortSignal) { const manifest = this.manifest!; // In this case we are sure its not undefined. const newApi = await createExtensionApi( @@ -91,10 +91,19 @@ export class UmbExtensionApiInitializer< manifest as unknown as ManifestApi, this.#constructorArguments as any, ); - if (!this._isConditionsPositive) { - // We are not positive anymore, so we will back out of this creation. + + if (signal.aborted || !this._isConditionsPositive) { + newApi?.destroy?.(); return false; } + // A previous _conditionsAreGood() on this same initializer may have already + // assigned this.#api and resolved before us. Without cleanup that instance would + // keep running until this._host is destroyed — not a hard leak, but any + // subscriptions / context consumers / async setup it started stay alive. + if (this.#api && this.#api !== newApi) { + this.#api.destroy?.(); + } + this.#api = newApi; if (this.#api) { diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-api-initializer.test.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-api-initializer.test.ts index 11f5b9aad908..c2c5d11b094d 100644 --- a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-api-initializer.test.ts +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-api-initializer.test.ts @@ -1,5 +1,5 @@ import { UmbExtensionRegistry } from '../registry/extension.registry.js'; -import type { ManifestApi, ManifestWithDynamicConditions } from '../types/index.js'; +import type { ManifestApi, ManifestBase, ManifestWithDynamicConditions } from '../types/index.js'; import { UmbExtensionApiInitializer } from './index.js'; import { expect, fixture } from '@open-wc/testing'; import { UmbControllerBase } from '@umbraco-cms/backoffice/class-api'; @@ -8,6 +8,7 @@ import type { UmbControllerHostElement, UmbControllerHost } from '@umbraco-cms/b import { customElement, html } from '@umbraco-cms/backoffice/external/lit'; import { UmbSwitchCondition } from '@umbraco-cms/backoffice/extension-registry'; import type { ManifestSection } from '@umbraco-cms/backoffice/section'; +import type { SwitchConditionConfig } from 'src/packages/core/extension-registry/conditions/switch.condition.js'; @customElement('umb-test-controller-host') // Element is used in tests @@ -91,8 +92,10 @@ describe('UmbExtensionApiController', () => { describe('Manifest with multiple conditions that changes over time', () => { let hostElement: UmbControllerHostElement; - let extensionRegistry: UmbExtensionRegistry; - let manifest: ManifestSection; + let extensionRegistry: UmbExtensionRegistry< + ManifestApi & ManifestWithDynamicConditions + >; + let manifest: ManifestApi & ManifestWithDynamicConditions; beforeEach(async () => { hostElement = await fixture(html``); @@ -105,15 +108,15 @@ describe('UmbExtensionApiController', () => { api: UmbTestApiController, conditions: [ { - alias: 'Umb.Test.Condition.Delay', + alias: 'Umb.Condition.Switch', frequency: '100', }, { - alias: 'Umb.Test.Condition.Delay', + alias: 'Umb.Condition.Switch', frequency: '200', }, ], - } as any; + }; // A ASCII timeline for the conditions, when allowed and then not allowed: // Condition 0ms 100ms 200ms 300ms 400ms 500ms @@ -123,8 +126,8 @@ describe('UmbExtensionApiController', () => { const conditionManifest = { type: 'condition', - name: 'test-condition-delay', - alias: 'Umb.Test.Condition.Delay', + name: 'test-condition-switch', + alias: 'Umb.Condition.Switch', api: UmbSwitchCondition, }; diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-element-and-api-initializer.controller.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-element-and-api-initializer.controller.ts index 4757c6b97bcf..fdc9a9c6f55c 100644 --- a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-element-and-api-initializer.controller.ts +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-element-and-api-initializer.controller.ts @@ -132,7 +132,7 @@ export class UmbExtensionElementAndApiInitializer< }); }; - protected async _conditionsAreGood() { + protected async _conditionsAreGood(signal: AbortSignal) { const manifest = this.manifest!; // In this case we are sure its not undefined. const { element: newComponent, api: newApi } = await createExtensionElementWithApi< @@ -140,15 +140,25 @@ export class UmbExtensionElementAndApiInitializer< ExtensionApiInterface >(manifest, this.#constructorArguments as any, this.#defaultElement, this.#defaultApi); - if (!this._isConditionsPositive) { + if (signal.aborted || !this._isConditionsPositive) { newApi?.destroy?.(); if (newComponent && 'destroy' in newComponent) { (newComponent as unknown as { destroy: () => void }).destroy(); } - // We are not positive anymore, so we will back out of this creation. return false; } + // A previous _conditionsAreGood() on this same initializer may have already + // assigned this.#api / this.#component and resolved before us. The API's host is + // the transient element (see createExtensionElementWithApi), not this initializer, + // so nothing else in the controller-host chain will clean an orphaned pair up. + if (this.#api && this.#api !== newApi) { + this.#api.destroy?.(); + } + if (this.#component && this.#component !== newComponent && 'destroy' in this.#component) { + (this.#component as unknown as { destroy: () => void }).destroy(); + } + this.#api = newApi; if (this.#api) { this.#assignApiProps(); @@ -156,7 +166,6 @@ export class UmbExtensionElementAndApiInitializer< } else { console.warn('Manifest did not provide any useful data for a api to be created.'); } - this.#component = newComponent; if (this.#component) { this.#assignElProps(); diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-element-and-api-initializer.race.test.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-element-and-api-initializer.race.test.ts new file mode 100644 index 000000000000..64506fe91137 --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-element-and-api-initializer.race.test.ts @@ -0,0 +1,460 @@ +import type { + ManifestCondition, + ManifestElementAndApi, + ManifestWithDynamicConditions, + UmbApi, + UmbConditionConfigBase, +} from '../index.js'; +import type { UmbConditionControllerArguments } from '../condition/condition-controller-arguments.type.js'; +import { UmbExtensionRegistry } from '../registry/extension.registry.js'; +import { UmbExtensionElementAndApiInitializer } from './extension-element-and-api-initializer.controller.js'; +import { UmbConditionBase } from '@umbraco-cms/backoffice/extension-registry'; +import { expect, fixture } from '@open-wc/testing'; +import { UmbControllerBase } from '@umbraco-cms/backoffice/class-api'; +import { UmbControllerHostElementMixin } from '@umbraco-cms/backoffice/controller-api'; +import type { UmbControllerHost, UmbControllerHostElement } from '@umbraco-cms/backoffice/controller-api'; +import { customElement, html } from '@umbraco-cms/backoffice/external/lit'; + +// Same race as `base-extension-initializer.race.test.ts`, but running through the real +// `UmbExtensionElementAndApiInitializer`. This initializer is what element-bearing workspace +// extensions (headers, views, action renderers) use — its `_conditionsAreGood()` awaits both +// the element and the api loaders in parallel via `createExtensionElementWithApi`, so the +// await window is typically the longest of any initializer and most likely to overlap +// condition flips. + +@customElement('umb-test-race-ea-host') +// eslint-disable-next-line @typescript-eslint/no-unused-vars +class UmbTestRaceElementAndApiHost extends UmbControllerHostElementMixin(HTMLElement) {} + +@customElement('umb-test-race-ea-element') +// eslint-disable-next-line @typescript-eslint/no-unused-vars +class UmbTestRaceElementAndApiElement extends UmbControllerHostElementMixin(HTMLElement) {} + +// Module-level handle to the most recently constructed manual condition — the same trick +// used in the base-initializer race test so a test can flip state from the outside. +let lastManualCondition: UmbManualCondition | undefined; + +class UmbManualCondition extends UmbConditionBase { + constructor(host: UmbControllerHost, args: UmbConditionControllerArguments) { + super(host, args); + lastManualCondition = this; + } + + flipTo(value: boolean) { + this.permitted = value; + } +} + +// A simple API whose construction we track, so we can assert that the initializer creates / +// destroys us in sync with permitted transitions. +let apiCtorCount = 0; +let apiDestroyCount = 0; + +class UmbRaceTestApi extends UmbControllerBase implements UmbApi { + // UmbControllerBase auto-registers on the host, and destroy() may be called by several + // chains (the extension's `_conditionsAreBad`, the overwrite-cleanup path, the host's + // own teardown). Guard the counter so each instance is counted at most once — a real + // leak still surfaces as ctor > destroy. + #destroyed = false; + + constructor(host: UmbControllerHost) { + super(host); + apiCtorCount++; + } + + override destroy() { + if (!this.#destroyed) { + this.#destroyed = true; + apiDestroyCount++; + } + super.destroy(); + } +} + +interface TestManifest extends ManifestWithDynamicConditions, ManifestElementAndApi { + type: 'test-type'; +} + +async function wait(ms: number) { + await new Promise((r) => setTimeout(r, ms)); +} + +describe('UmbExtensionElementAndApiInitializer — condition-flip race with a slow API factory', () => { + let hostElement: UmbControllerHostElement; + let extensionRegistry: UmbExtensionRegistry; + + const conditionManifest: ManifestCondition = { + type: 'condition', + name: 'race-condition-manual', + alias: 'Umb.Test.Condition.Manual', + api: UmbManualCondition, + }; + + // Manifest with: + // - an `elementName` (cheap — just a custom element tag) + // - an `api` factory that awaits 30ms before returning the class, emulating a dynamic + // module import. This is the realistic shape of a workspace action / view / header + // extension that code-splits its API module. + const baseManifest: TestManifest = { + type: 'test-type' as 'test-type', + name: 'race-ea', + alias: 'Umb.Test.Race.ElementAndApi', + elementName: 'umb-test-race-ea-element', + api: async () => { + await new Promise((r) => setTimeout(r, 30)); + return { api: UmbRaceTestApi }; + }, + conditions: [{ alias: 'Umb.Test.Condition.Manual' }], + }; + + beforeEach(async () => { + hostElement = await fixture(html``); + extensionRegistry = new UmbExtensionRegistry(); + lastManualCondition = undefined; + apiCtorCount = 0; + apiDestroyCount = 0; + + extensionRegistry.register(baseManifest); + extensionRegistry.register(conditionManifest); + }); + + it('settles at permitted=true after a single flip to true', async () => { + const history: boolean[] = []; + const controller = new UmbExtensionElementAndApiInitializer( + hostElement, + extensionRegistry, + baseManifest.alias, + [hostElement], + (p) => history.push(p), + ); + + await wait(0); + expect(lastManualCondition, 'condition instance must exist').to.exist; + + lastManualCondition!.flipTo(true); + + await wait(120); + + expect(controller.permitted, `history: ${JSON.stringify(history)}`).to.be.true; + expect(controller.component, 'element should be created').to.exist; + expect(controller.api, 'api should be created').to.exist; + controller.destroy(); + }); + + // The "stuck at bad" repro scenario. Final condition value is true; the extension MUST + // settle at permitted=true with a live element + api. + it('settles at permitted=true after flipping true → false → true during slow element+api construction', async () => { + const history: boolean[] = []; + const controller = new UmbExtensionElementAndApiInitializer( + hostElement, + extensionRegistry, + baseManifest.alias, + [hostElement], + (p) => history.push(p), + ); + + await wait(0); + + lastManualCondition!.flipTo(true); + lastManualCondition!.flipTo(false); + lastManualCondition!.flipTo(true); + + await wait(200); + + expect( + controller.permitted, + `expected permitted=true. callback history: ${JSON.stringify(history)}`, + ).to.be.true; + expect(controller.component, 'element should exist when permitted').to.exist; + expect(controller.api, 'api should exist when permitted').to.exist; + expect(lastManualCondition!.permitted, 'condition final state').to.be.true; + + controller.destroy(); + }); + + it('settles at permitted=true after flipping true → false → true → false → true', async () => { + const history: boolean[] = []; + const controller = new UmbExtensionElementAndApiInitializer( + hostElement, + extensionRegistry, + baseManifest.alias, + [hostElement], + (p) => history.push(p), + ); + + await wait(0); + + lastManualCondition!.flipTo(true); + lastManualCondition!.flipTo(false); + lastManualCondition!.flipTo(true); + lastManualCondition!.flipTo(false); + lastManualCondition!.flipTo(true); + + await wait(250); + + expect( + controller.permitted, + `expected permitted=true. callback history: ${JSON.stringify(history)}`, + ).to.be.true; + expect(controller.component, 'element should exist').to.exist; + expect(controller.api, 'api should exist').to.exist; + + controller.destroy(); + }); + + it('settles at permitted=false after flipping true → false → true → false', async () => { + const history: boolean[] = []; + const controller = new UmbExtensionElementAndApiInitializer( + hostElement, + extensionRegistry, + baseManifest.alias, + [hostElement], + (p) => history.push(p), + ); + + await wait(0); + + lastManualCondition!.flipTo(true); + lastManualCondition!.flipTo(false); + lastManualCondition!.flipTo(true); + lastManualCondition!.flipTo(false); + + await wait(250); + + expect( + controller.permitted, + `expected permitted=false. callback history: ${JSON.stringify(history)}`, + ).to.be.false; + expect(controller.component, 'element should NOT exist when not permitted').to.be.undefined; + expect(controller.api, 'api should NOT exist when not permitted').to.be.undefined; + + controller.destroy(); + }); + + it('settles at the final value when flips are spaced out longer than the factory delay', async () => { + const history: boolean[] = []; + const controller = new UmbExtensionElementAndApiInitializer( + hostElement, + extensionRegistry, + baseManifest.alias, + [hostElement], + (p) => history.push(p), + ); + + await wait(0); + + lastManualCondition!.flipTo(true); + await wait(80); + lastManualCondition!.flipTo(false); + await wait(80); + lastManualCondition!.flipTo(true); + await wait(120); + + expect( + controller.permitted, + `expected permitted=true. history: ${JSON.stringify(history)}`, + ).to.be.true; + expect(controller.component).to.exist; + expect(controller.api).to.exist; + + controller.destroy(); + }); + + it('settles at true when flips are separated by microtask gaps', async () => { + const history: boolean[] = []; + const controller = new UmbExtensionElementAndApiInitializer( + hostElement, + extensionRegistry, + baseManifest.alias, + [hostElement], + (p) => history.push(p), + ); + + await wait(0); + + lastManualCondition!.flipTo(true); + await Promise.resolve(); + lastManualCondition!.flipTo(false); + await Promise.resolve(); + lastManualCondition!.flipTo(true); + + await wait(200); + + expect( + controller.permitted, + `expected permitted=true. history: ${JSON.stringify(history)}`, + ).to.be.true; + controller.destroy(); + }); + + it('settles at true when a flip arrives mid-resolution of a previous element+api load', async () => { + const history: boolean[] = []; + const controller = new UmbExtensionElementAndApiInitializer( + hostElement, + extensionRegistry, + baseManifest.alias, + [hostElement], + (p) => history.push(p), + ); + + await wait(0); + + lastManualCondition!.flipTo(true); + await wait(15); // < 30ms factory delay, so first _conditionsAreGood is still pending + lastManualCondition!.flipTo(false); + await wait(5); + lastManualCondition!.flipTo(true); + + await wait(200); + + expect( + controller.permitted, + `expected permitted=true. history: ${JSON.stringify(history)}`, + ).to.be.true; + expect(controller.api).to.exist; + controller.destroy(); + }); + + it('settles at true after ten rapid flips ending at true', async () => { + const history: boolean[] = []; + const controller = new UmbExtensionElementAndApiInitializer( + hostElement, + extensionRegistry, + baseManifest.alias, + [hostElement], + (p) => history.push(p), + ); + + await wait(0); + + for (let i = 0; i < 10; i++) { + lastManualCondition!.flipTo(i % 2 === 0); + } + lastManualCondition!.flipTo(true); // ensure final state is true + + await wait(300); + + expect( + controller.permitted, + `expected permitted=true. history: ${JSON.stringify(history)}`, + ).to.be.true; + expect(controller.api).to.exist; + controller.destroy(); + }); + + // Leak check: every API the initializer constructs should end up destroyed, either by a + // later `_conditionsAreBad` or by the `_conditionsAreGood` back-out path (which now + // explicitly destroys a stillborn API — see extension-element-and-api-initializer.ts:144). + // If ctor-count != destroy-count after the final destroy, there's a leak. + it('does not leak API instances when flips overlap element+api construction', async () => { + const controller = new UmbExtensionElementAndApiInitializer( + hostElement, + extensionRegistry, + baseManifest.alias, + [hostElement], + () => { + /* we only care about ctor/destroy accounting here */ + }, + ); + + await wait(0); + + lastManualCondition!.flipTo(true); + lastManualCondition!.flipTo(false); + lastManualCondition!.flipTo(true); + lastManualCondition!.flipTo(false); + lastManualCondition!.flipTo(true); + + await wait(250); + + controller.destroy(); + // Give destroy's own _conditionsAreBad call a tick to land. + await wait(10); + + expect(apiCtorCount, 'some APIs were constructed').to.be.greaterThan(0); + expect( + apiDestroyCount, + `API leak: ${apiCtorCount} constructed, only ${apiDestroyCount} destroyed`, + ).to.equal(apiCtorCount); + }); + + // Regression test for the "bad-path restored-during-await" bug that made the submit + // button fail to show after a brief readonly flip. Sequence: + // + // 1. Good-call completes — `this.#api` / `this.#component` assigned, `#isPermitted=true`. + // 2. Condition flips `false` — base enters the else-if, `_conditionsAreBad()` runs + // synchronously and destroys `this.#component`. Before the fix, the base then + // `await`-ed that call. The await yielded a microtask. + // 3. Condition flips `true` during that microtask — a new callback fired synchronously, + // saw `_isConditionsPositive` flip back to `true` and `#isPermitted` still `true` + // (the bad-path hadn't committed yet), hit the `#isPermitted !== true` dead-zone + // and did nothing. + // 4. Bad-path resumed, saw `_isConditionsPositive !== false`, and bailed out early + // without committing `#isPermitted = false`. + // 5. End state: `#isPermitted = true`, `#component = undefined`. Lit's `ext.component` + // was `undefined` — button visible in the permitted list but with nothing to render. + // + // The fix dropped the `await` on `_conditionsAreBad` and removed the "restored during + // await" early-return so `#isPermitted = false` is always committed after destruction. + // Any subsequent restore-callback then enters the good-branch and rebuilds. + // + // This test asserts the INVARIANT: after the flip burst settles, `permitted` and + // `component` must agree. Either both present (rebuilt) or both absent (stayed negative). + // Before the fix, `permitted=true` with `component=undefined` was the observed failure. + it('keeps permitted/component state consistent after a false→true flip during bad-path', async () => { + const controller = new UmbExtensionElementAndApiInitializer( + hostElement, + extensionRegistry, + baseManifest.alias, + [hostElement], + () => { + /* state check at the end; onChange noise is irrelevant */ + }, + ); + + await wait(0); + expect(lastManualCondition, 'condition must exist').to.exist; + + // Drive to permitted=true with component assigned. + lastManualCondition!.flipTo(true); + await wait(120); // factory delay + buffer + expect(controller.permitted, 'setup: should be permitted after initial true flip').to.be.true; + expect(controller.component, 'setup: component should exist after initial true flip').to.exist; + + // Now the race. `flipTo(false)` → bad-path destroys component synchronously (before + // the fix, then awaited). `flipTo(true)` happens in the same sync turn — this is the + // microtask-interleaving that used to dead-zone and strand `#isPermitted=true` with + // a destroyed component. + lastManualCondition!.flipTo(false); + lastManualCondition!.flipTo(true); + + // Allow any pending rebuild good-call + rAF notifications to settle. + await wait(200); + + // The invariant: permitted and component must be consistent. Before the fix, the + // failure was `permitted=true` with `component=undefined`. + if (controller.permitted) { + expect( + controller.component, + 'state inconsistency: permitted=true but component is undefined — ' + + 'bad-path destroyed component then bailed out without committing isPermitted=false, ' + + 'so nothing ever triggered a rebuild.', + ).to.exist; + expect(controller.api, 'state inconsistency: permitted=true but api is undefined').to.exist; + } else { + expect(controller.component, 'consistency: not permitted but component lingered').to.be.undefined; + expect(controller.api, 'consistency: not permitted but api lingered').to.be.undefined; + } + + // The condition's final state was `true`, so the happy path is to end permitted with + // a rebuilt component. Explicit check so a silently-negative-ending regression also + // surfaces. + expect( + controller.permitted, + 'after a false→true burst the controller should settle at permitted=true (rebuilt)', + ).to.be.true; + expect(controller.component, 'component should have been rebuilt after the burst').to.exist; + + controller.destroy(); + }); +}); diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-element-and-api-initializer.test.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-element-and-api-initializer.test.ts index 82c6fd52d537..3b633894781d 100644 --- a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-element-and-api-initializer.test.ts +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-element-and-api-initializer.test.ts @@ -7,6 +7,7 @@ import { UmbControllerHostElementMixin } from '@umbraco-cms/backoffice/controlle import { customElement } from '@umbraco-cms/backoffice/external/lit'; import { UmbSwitchCondition } from '@umbraco-cms/backoffice/extension-registry'; import { UmbControllerBase } from '@umbraco-cms/backoffice/class-api'; +import type { SwitchConditionConfig } from 'src/packages/core/extension-registry/conditions/switch.condition.js'; @customElement('umb-test-controller-host') class UmbTestControllerHostElement extends UmbControllerHostElementMixin(HTMLElement) {} @@ -25,7 +26,8 @@ class UmbTestApiController extends UmbControllerBase implements UmbApi { } interface TestManifest - extends ManifestWithDynamicConditions, + extends + ManifestWithDynamicConditions, ManifestElementAndApi { type: 'test-type'; } @@ -103,8 +105,12 @@ describe('UmbExtensionElementAndApiController', () => { describe('Manifest with multiple conditions that changes over time', () => { let hostElement: UmbControllerHostElement; - let extensionRegistry: UmbExtensionRegistry; - let manifest: TestManifest; + let extensionRegistry: UmbExtensionRegistry< + ManifestWithDynamicConditions & + ManifestElementAndApi + >; + let manifest: ManifestWithDynamicConditions & + ManifestElementAndApi; beforeEach(async () => { hostElement = new UmbTestControllerHostElement(); @@ -118,13 +124,13 @@ describe('UmbExtensionElementAndApiController', () => { api: UmbTestApiController, conditions: [ { - alias: 'Umb.Test.Condition.Delay', + alias: 'Umb.Condition.Switch', frequency: '100', - } as any, + }, { - alias: 'Umb.Test.Condition.Delay', + alias: 'Umb.Condition.Switch', frequency: '200', - } as any, + }, ], }; @@ -136,8 +142,8 @@ describe('UmbExtensionElementAndApiController', () => { const conditionManifest = { type: 'condition', - name: 'test-condition-delay', - alias: 'Umb.Test.Condition.Delay', + name: 'test-condition-switch', + alias: 'Umb.Condition.Switch', api: UmbSwitchCondition, }; @@ -251,11 +257,11 @@ describe('UmbExtensionElementAndApiController', () => { api: UmbTestApiController, conditions: [ { - alias: 'Umb.Test.Condition.Delay', + alias: 'Umb.Condition.Switch', frequency: '100', } as any, { - alias: 'Umb.Test.Condition.Delay', + alias: 'Umb.Condition.Switch', frequency: '200', } as any, ], @@ -269,8 +275,8 @@ describe('UmbExtensionElementAndApiController', () => { const conditionManifest = { type: 'condition', - name: 'test-condition-delay', - alias: 'Umb.Test.Condition.Delay', + name: 'test-condition-switch', + alias: 'Umb.Condition.Switch', api: UmbSwitchCondition, }; diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-element-initializer.controller.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-element-initializer.controller.ts index 91ff3e2a86b5..cb1984390dae 100644 --- a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-element-initializer.controller.ts +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-element-initializer.controller.ts @@ -76,14 +76,24 @@ export class UmbExtensionElementInitializer< }); }; - protected async _conditionsAreGood() { + protected async _conditionsAreGood(signal: AbortSignal) { const manifest = this.manifest!; // In this case we are sure its not undefined. const newComponent = await createExtensionElement(manifest, this.#defaultElement); - if (!this._isConditionsPositive) { - // We are not positive anymore, so we will back out of this creation. + + if (signal.aborted || !this._isConditionsPositive) { + if (newComponent && 'destroy' in newComponent) { + (newComponent as unknown as { destroy: () => void }).destroy(); + } return false; } + if (this.#component && this.#component !== newComponent) { + if ('destroy' in this.#component) { + (this.#component as unknown as { destroy: () => void }).destroy(); + } + this.#component = undefined; + } + this.#component = newComponent as ExtensionElementInterface; if (this.#component) { this.#assignProperties(); diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-element-initializer.test.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-element-initializer.test.ts index bae2173bbef9..fb33edb6ac36 100644 --- a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-element-initializer.test.ts +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-element-initializer.test.ts @@ -97,11 +97,11 @@ describe('UmbExtensionElementController', () => { elementName: 'section', conditions: [ { - alias: 'Umb.Test.Condition.Delay', + alias: 'Umb.Condition.Switch', frequency: '100', }, { - alias: 'Umb.Test.Condition.Delay', + alias: 'Umb.Condition.Switch', frequency: '200', }, ], @@ -115,8 +115,8 @@ describe('UmbExtensionElementController', () => { const conditionManifest = { type: 'condition', - name: 'test-condition-delay', - alias: 'Umb.Test.Condition.Delay', + name: 'test-condition-switch', + alias: 'Umb.Condition.Switch', api: UmbSwitchCondition, }; diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-manifest-initializer.controller.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-manifest-initializer.controller.ts index 372af05abd01..45ef880f116b 100644 --- a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-manifest-initializer.controller.ts +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extension-manifest-initializer.controller.ts @@ -26,7 +26,7 @@ export class UmbExtensionManifestInitializer< this._init(); } - protected async _conditionsAreGood() { + protected async _conditionsAreGood(_signal: AbortSignal) { return true; } diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extensions-api-initializer.race.test.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extensions-api-initializer.race.test.ts new file mode 100644 index 000000000000..37a68c7bad51 --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extensions-api-initializer.race.test.ts @@ -0,0 +1,520 @@ +import type { + ManifestApi, + ManifestCondition, + ManifestWithDynamicConditions, + UmbApi, + UmbConditionConfigBase, +} from '../index.js'; +import type { UmbConditionControllerArguments } from '../condition/condition-controller-arguments.type.js'; +import { UmbExtensionRegistry } from '../registry/extension.registry.js'; +import { UmbExtensionsApiInitializer } from './extensions-api-initializer.controller.js'; +import type { UmbExtensionApiInitializer } from './extension-api-initializer.controller.js'; +import { UmbConditionBase } from '@umbraco-cms/backoffice/extension-registry'; +import { expect, fixture } from '@open-wc/testing'; +import { UmbControllerBase } from '@umbraco-cms/backoffice/class-api'; +import { UmbControllerHostElementMixin } from '@umbraco-cms/backoffice/controller-api'; +import type { UmbControllerHost, UmbControllerHostElement } from '@umbraco-cms/backoffice/controller-api'; +import { customElement, html } from '@umbraco-cms/backoffice/external/lit'; + +// Sibling of `extensions-element-and-api-initializer.race.test.ts`, but for API-only +// workspace contexts / hooks / services that run through `UmbExtensionsApiInitializer`. +// The per-extension `_conditionsAreGood` here dynamically imports the API class via +// `createExtensionApi` (no element-load in parallel), and the plural layer debounces +// permitted-state changes via rAF. + +@customElement('umb-test-plural-api-race-host') +// eslint-disable-next-line @typescript-eslint/no-unused-vars +class UmbTestPluralApiRaceHost extends UmbControllerHostElementMixin(HTMLElement) {} + +const conditionByUnique = new Map(); + +interface ManualConfig extends UmbConditionConfigBase { + uniqueKey: string; +} + +class UmbManualCondition extends UmbConditionBase { + constructor(host: UmbControllerHost, args: UmbConditionControllerArguments) { + super(host, args); + conditionByUnique.set(args.config.uniqueKey, this); + } + + flipTo(value: boolean) { + this.permitted = value; + } +} + +const conditionManifest: ManifestCondition = { + type: 'condition', + name: 'plural-api-race-condition-manual', + alias: 'Umb.Test.Plural.Api.Race.Condition.Manual', + api: UmbManualCondition, +}; + +let apiCtorCount = 0; +let apiDestroyCount = 0; +let nextApiId = 1; +const liveApis = new Set(); + +class UmbRaceTestApi extends UmbControllerBase implements UmbApi { + // UmbControllerBase auto-registers on the host (= the plural initializer) and the + // several teardown chains may call destroy() more than once per instance. Guard so + // each instance counts once — a real leak still surfaces as ctor > destroy. + #destroyed = false; + readonly id: string; + + constructor(host: UmbControllerHost) { + super(host); + this.id = 'api-' + nextApiId++; + liveApis.add(this.id); + apiCtorCount++; + } + + override destroy() { + if (!this.#destroyed) { + this.#destroyed = true; + liveApis.delete(this.id); + apiDestroyCount++; + } + super.destroy(); + } +} + +interface TestManifest extends ManifestWithDynamicConditions, ManifestApi { + type: 'test-plural-api-race'; +} + +async function wait(ms: number) { + await new Promise((r) => setTimeout(r, ms)); +} + +async function waitForDebouncedNotify() { + await new Promise((r) => requestAnimationFrame(() => requestAnimationFrame(() => r()))); +} + +const FACTORY_DELAY_MS = 30; + +// Factory that emulates `await import(...)` returning `{ api: ClassConstructor }`, the +// shape `loadManifestApi` accepts for async module-style loaders. +const slowApiFactory = async () => { + await new Promise((r) => setTimeout(r, FACTORY_DELAY_MS)); + return { api: UmbRaceTestApi }; +}; + +function makeManifest(alias: string, uniqueKey: string, weight = 100): TestManifest { + return { + type: 'test-plural-api-race', + name: alias, + alias, + weight, + api: slowApiFactory, + conditions: [ + { + alias: conditionManifest.alias, + uniqueKey, + } as UmbConditionConfigBase, + ], + }; +} + +describe('UmbExtensionsApiInitializer — condition-flip race with slow API factory', () => { + let hostElement: UmbControllerHostElement; + let extensionRegistry: UmbExtensionRegistry; + + beforeEach(async () => { + hostElement = await fixture(html``); + extensionRegistry = new UmbExtensionRegistry(); + conditionByUnique.clear(); + apiCtorCount = 0; + apiDestroyCount = 0; + nextApiId = 1; + liveApis.clear(); + + extensionRegistry.register(conditionManifest as any); + }); + + const makePlural = ( + onChange: (permitted: Array>) => void, + args?: { single?: boolean }, + ) => + new UmbExtensionsApiInitializer( + hostElement, + extensionRegistry as any, + 'test-plural-api-race', + [hostElement], + null, + onChange as any, + 'testPluralApiRaceController', + args, + ); + + it('exposes the extension after a single flip to true', async () => { + const manifest = makeManifest('Umb.Test.Plural.Api.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = makePlural((permitted) => changes.push(permitted.map((p) => p.alias))); + + await wait(0); + conditionByUnique.get('A')!.flipTo(true); + + await wait(FACTORY_DELAY_MS + 60); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect(finalState, `history: ${JSON.stringify(changes)}`).to.deep.equal([manifest.alias]); + + plural.destroy(); + }); + + // The "stuck at bad" repro, going through the full stack (plural → api → base). + it('exposes the extension after flipping true → false → true during slow API load', async () => { + const manifest = makeManifest('Umb.Test.Plural.Api.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = makePlural((permitted) => changes.push(permitted.map((p) => p.alias))); + + await wait(0); + + const cond = conditionByUnique.get('A')!; + cond.flipTo(true); + cond.flipTo(false); + cond.flipTo(true); + + await wait(FACTORY_DELAY_MS * 2 + 80); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `final exposed should include extension. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifest.alias]); + expect(cond.permitted, 'condition final state').to.be.true; + + plural.destroy(); + }); + + it('exposes the extension after flipping true → false → true → false → true', async () => { + const manifest = makeManifest('Umb.Test.Plural.Api.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = makePlural((permitted) => changes.push(permitted.map((p) => p.alias))); + + await wait(0); + + const cond = conditionByUnique.get('A')!; + cond.flipTo(true); + cond.flipTo(false); + cond.flipTo(true); + cond.flipTo(false); + cond.flipTo(true); + + await wait(FACTORY_DELAY_MS * 3 + 100); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `final exposed should include extension. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifest.alias]); + + plural.destroy(); + }); + + it('excludes the extension after flipping true → false → true → false', async () => { + const manifest = makeManifest('Umb.Test.Plural.Api.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = makePlural((permitted) => changes.push(permitted.map((p) => p.alias))); + + await wait(0); + + const cond = conditionByUnique.get('A')!; + cond.flipTo(true); + cond.flipTo(false); + cond.flipTo(true); + cond.flipTo(false); + + await wait(FACTORY_DELAY_MS * 3 + 100); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `final exposed should NOT include extension. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([]); + + plural.destroy(); + }); + + it('tracks two extensions independently when conditions flip concurrently', async () => { + const manifestA = makeManifest('Umb.Test.Plural.Api.Race.A', 'A', 10); + const manifestB = makeManifest('Umb.Test.Plural.Api.Race.B', 'B', 5); + extensionRegistry.register(manifestA); + extensionRegistry.register(manifestB); + + const changes: Array = []; + const plural = makePlural((permitted) => changes.push(permitted.map((p) => p.alias).sort())); + + await wait(0); + + const condA = conditionByUnique.get('A')!; + const condB = conditionByUnique.get('B')!; + + condA.flipTo(true); + condB.flipTo(true); + condA.flipTo(false); + condB.flipTo(false); + condA.flipTo(true); + condB.flipTo(true); + condB.flipTo(false); // B ends false, A ends true + + await wait(FACTORY_DELAY_MS * 3 + 120); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `only A should be permitted. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifestA.alias]); + + plural.destroy(); + }); + + it('exposes both extensions sorted by weight when both end at true', async () => { + const manifestA = makeManifest('Umb.Test.Plural.Api.Race.A', 'A', 10); + const manifestB = makeManifest('Umb.Test.Plural.Api.Race.B', 'B', 99); + extensionRegistry.register(manifestA); + extensionRegistry.register(manifestB); + + const changes: Array = []; + const plural = makePlural((permitted) => changes.push(permitted.map((p) => p.alias))); + + await wait(0); + + const condA = conditionByUnique.get('A')!; + const condB = conditionByUnique.get('B')!; + + condA.flipTo(true); + condB.flipTo(true); + condA.flipTo(false); + condB.flipTo(false); + condA.flipTo(true); + condB.flipTo(true); + + await wait(FACTORY_DELAY_MS * 3 + 120); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + // B (weight 99) should come before A (weight 10). + expect( + finalState, + `B should come first. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifestB.alias, manifestA.alias]); + + plural.destroy(); + }); + + it('exposes the extension when a flip lands mid-resolution of a previous good-call', async () => { + const manifest = makeManifest('Umb.Test.Plural.Api.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = makePlural((permitted) => changes.push(permitted.map((p) => p.alias))); + + await wait(0); + const cond = conditionByUnique.get('A')!; + + cond.flipTo(true); + await wait(FACTORY_DELAY_MS - 10); + cond.flipTo(false); + await wait(5); + cond.flipTo(true); + + await wait(FACTORY_DELAY_MS * 2 + 120); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `final exposed should include extension. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifest.alias]); + + plural.destroy(); + }); + + it('final onChange matches the true permitted state after ten rapid flips', async () => { + const manifest = makeManifest('Umb.Test.Plural.Api.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = makePlural((permitted) => changes.push(permitted.map((p) => p.alias))); + + await wait(0); + const cond = conditionByUnique.get('A')!; + + for (let i = 0; i < 10; i++) { + cond.flipTo(i % 2 === 0); + } + cond.flipTo(true); // force final to true + + await wait(FACTORY_DELAY_MS * 4 + 140); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `final exposed should include extension. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifest.alias]); + + plural.destroy(); + }); + + it('drops the extension cleanly when its manifest is unregistered during a flip race', async () => { + const manifest = makeManifest('Umb.Test.Plural.Api.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = makePlural((permitted) => changes.push(permitted.map((p) => p.alias))); + + await wait(0); + const cond = conditionByUnique.get('A')!; + + cond.flipTo(true); + cond.flipTo(false); + cond.flipTo(true); + + await wait(10); + extensionRegistry.unregister(manifest.alias); + + await wait(FACTORY_DELAY_MS * 2 + 120); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `extension should be absent after unregister. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([]); + + plural.destroy(); + await wait(10); + + expect( + apiDestroyCount, + `API leak after unregister+destroy: ${apiCtorCount} constructed, ${apiDestroyCount} destroyed. Leaked: ${[...liveApis].join(', ')}`, + ).to.equal(apiCtorCount); + }); + + it('exposes only the highest-weight extension in single mode after races', async () => { + const manifestA = makeManifest('Umb.Test.Plural.Api.Race.A', 'A', 1); + const manifestB = makeManifest('Umb.Test.Plural.Api.Race.B', 'B', 99); + extensionRegistry.register(manifestA); + extensionRegistry.register(manifestB); + + const changes: Array = []; + const plural = makePlural( + (permitted) => changes.push(permitted.map((p) => p.alias)), + { single: true }, + ); + + await wait(0); + + const condA = conditionByUnique.get('A')!; + const condB = conditionByUnique.get('B')!; + + condA.flipTo(true); + condB.flipTo(true); + condA.flipTo(false); + condB.flipTo(false); + condA.flipTo(true); + condB.flipTo(true); + + await wait(FACTORY_DELAY_MS * 3 + 120); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `only B (weight 99) should be exposed. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifestB.alias]); + + plural.destroy(); + }); + + // Unlike the element-and-api case, a stale API here is still a controller of the + // plural initializer, so `plural.destroy()` sweeps it up at the end — the final leak + // counter will balance even without the overwrite-cleanup fix. What the fix actually + // buys is that orphans don't stay alive *during* the plural's lifetime. We assert that + // after a flip burst settles there's exactly one live API per currently-permitted + // extension, not several. + it('keeps only one live API per permitted extension after a flip burst', async () => { + const manifest = makeManifest('Umb.Test.Plural.Api.Race.A', 'A'); + extensionRegistry.register(manifest); + + const plural = makePlural(() => { + /* not observing emissions here */ + }); + + await wait(0); + + const cond = conditionByUnique.get('A')!; + // Burst of flips ending at true — the bug scenario where two good-calls both + // resolve with `_isConditionsPositive=true` and race to assign this.#api. + cond.flipTo(true); + cond.flipTo(false); + cond.flipTo(true); + + await wait(FACTORY_DELAY_MS * 2 + 120); + await waitForDebouncedNotify(); + + // Final state: A is permitted. Exactly one live API expected. Without the fix we + // see 2 here (the leaked one plus the current one), both still running. + expect( + liveApis.size, + `expected exactly 1 live API for the 1 permitted extension. live: ${[...liveApis].join(', ')}`, + ).to.equal(1); + + plural.destroy(); + }); + + // The leak accounting — this is what caught the element-and-api bug. Two extensions, + // one ending at true (overwrite path), one ending at false (back-out path). Every API + // constructed must be destroyed before we're done. + it('does not leak API instances across flip races and destroy', async () => { + const manifestA = makeManifest('Umb.Test.Plural.Api.Race.A', 'A'); + const manifestB = makeManifest('Umb.Test.Plural.Api.Race.B', 'B'); + extensionRegistry.register(manifestA); + extensionRegistry.register(manifestB); + + const plural = makePlural(() => { + /* only accounting matters here */ + }); + + await wait(0); + + const condA = conditionByUnique.get('A')!; + const condB = conditionByUnique.get('B')!; + + condA.flipTo(true); + condB.flipTo(true); + condA.flipTo(false); + condB.flipTo(false); + condA.flipTo(true); + condB.flipTo(true); + condA.flipTo(false); // A ends false, B ends true + + await wait(FACTORY_DELAY_MS * 3 + 140); + await waitForDebouncedNotify(); + + plural.destroy(); + await wait(10); + + expect(apiCtorCount, 'some APIs were constructed during the race').to.be.greaterThan(0); + expect( + apiDestroyCount, + `API leak: ${apiCtorCount} constructed, only ${apiDestroyCount} destroyed. Leaked: ${[...liveApis].join(', ')}`, + ).to.equal(apiCtorCount); + }); +}); diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extensions-element-and-api-initializer.race.test.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extensions-element-and-api-initializer.race.test.ts new file mode 100644 index 000000000000..56ed298362ee --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/extensions-element-and-api-initializer.race.test.ts @@ -0,0 +1,516 @@ +import type { + ManifestCondition, + ManifestElementAndApi, + ManifestWithDynamicConditions, + UmbApi, + UmbConditionConfigBase, +} from '../index.js'; +import type { UmbConditionControllerArguments } from '../condition/condition-controller-arguments.type.js'; +import { UmbExtensionRegistry } from '../registry/extension.registry.js'; +import { UmbExtensionsElementAndApiInitializer } from './extensions-element-and-api-initializer.controller.js'; +import type { UmbExtensionElementAndApiInitializer } from './extension-element-and-api-initializer.controller.js'; +import { UmbConditionBase } from '@umbraco-cms/backoffice/extension-registry'; +import { expect, fixture } from '@open-wc/testing'; +import { UmbControllerBase } from '@umbraco-cms/backoffice/class-api'; +import { UmbControllerHostElementMixin } from '@umbraco-cms/backoffice/controller-api'; +import type { UmbControllerHost, UmbControllerHostElement } from '@umbraco-cms/backoffice/controller-api'; +import { customElement, html } from '@umbraco-cms/backoffice/external/lit'; + +// This is the closest test to the real user-reported scenario: workspace actions are +// rendered by a plural element-and-api initializer. Each action has its own element + api, +// constructed via an async `api: () => import(...)` factory. The plural layer then debounces +// permission changes via rAF and exposes the final list. +// +// We stress two layers of race at once: +// 1. Per-extension: rapid condition flips overlap `_conditionsAreGood` (async element+api +// construction). +// 2. Plural: many `_extensionChanged` calls land in a single frame and must coalesce into +// a correct final `onChange` payload. + +@customElement('umb-test-plural-ea-race-host') +// eslint-disable-next-line @typescript-eslint/no-unused-vars +class UmbTestPluralEaRaceHost extends UmbControllerHostElementMixin(HTMLElement) {} + +@customElement('umb-test-plural-ea-race-element') +// eslint-disable-next-line @typescript-eslint/no-unused-vars +class UmbTestPluralEaRaceElement extends UmbControllerHostElementMixin(HTMLElement) {} + +// Keyed condition registry so each extension's condition can be driven independently. +const conditionByUnique = new Map(); + +interface ManualConfig extends UmbConditionConfigBase { + uniqueKey: string; +} + +class UmbManualCondition extends UmbConditionBase { + constructor(host: UmbControllerHost, args: UmbConditionControllerArguments) { + super(host, args); + conditionByUnique.set(args.config.uniqueKey, this); + } + + flipTo(value: boolean) { + this.permitted = value; + } +} + +const conditionManifest: ManifestCondition = { + type: 'condition', + name: 'plural-ea-race-condition-manual', + alias: 'Umb.Test.Plural.Ea.Race.Condition.Manual', + api: UmbManualCondition, +}; + +// Count API ctor / destroy across the test so we can assert no leaks. +let apiCtorCount = 0; +let apiDestroyCount = 0; + +let nextApiId = 1; +const liveApis = new Set(); + +class UmbRaceTestApi extends UmbControllerBase implements UmbApi { + // UmbControllerBase auto-registers on the host, so the host's teardown will also call + // destroy(). We guard so our counter reflects the *first* teardown per instance — any + // leak would show as ctor > destroy; a missing first-teardown still surfaces. + #destroyed = false; + readonly id: string; + + constructor(host: UmbControllerHost) { + super(host); + this.id = 'api-' + nextApiId++; + liveApis.add(this.id); + apiCtorCount++; + } + + override destroy() { + if (!this.#destroyed) { + this.#destroyed = true; + liveApis.delete(this.id); + apiDestroyCount++; + } + super.destroy(); + } +} + +interface TestManifest + extends ManifestWithDynamicConditions, + ManifestElementAndApi { + type: 'test-plural-ea-race'; +} + +async function wait(ms: number) { + await new Promise((r) => setTimeout(r, ms)); +} + +async function waitForDebouncedNotify() { + // Two rAFs to be safe across microtask ordering. + await new Promise((r) => requestAnimationFrame(() => requestAnimationFrame(() => r()))); +} + +const FACTORY_DELAY_MS = 30; + +// A factory that emulates a dynamic `import()` — resolves to `{ api: UmbRaceTestApi }` after +// a delay. `loadManifestApi` expects `{ api }` or `{ default }`. +const slowApiFactory = async () => { + await new Promise((r) => setTimeout(r, FACTORY_DELAY_MS)); + return { api: UmbRaceTestApi }; +}; + +function makeManifest(alias: string, uniqueKey: string, weight = 100): TestManifest { + return { + type: 'test-plural-ea-race', + name: alias, + alias, + weight, + elementName: 'umb-test-plural-ea-race-element', + api: slowApiFactory, + conditions: [ + { + alias: conditionManifest.alias, + uniqueKey, + } as UmbConditionConfigBase, + ], + }; +} + +describe('UmbExtensionsElementAndApiInitializer — condition-flip race with slow element+api factory', () => { + let hostElement: UmbControllerHostElement; + let extensionRegistry: UmbExtensionRegistry; + + beforeEach(async () => { + hostElement = await fixture(html``); + extensionRegistry = new UmbExtensionRegistry(); + conditionByUnique.clear(); + apiCtorCount = 0; + apiDestroyCount = 0; + nextApiId = 1; + liveApis.clear(); + + extensionRegistry.register(conditionManifest as any); + }); + + const makePlural = ( + onChange: (permitted: Array>) => void, + args?: { single?: boolean }, + ) => + new UmbExtensionsElementAndApiInitializer( + hostElement, + extensionRegistry as any, + 'test-plural-ea-race', + [hostElement], + null, + onChange as any, + 'testPluralEaRaceController', + undefined, + undefined, + args, + ); + + it('exposes the extension after a single flip to true', async () => { + const manifest = makeManifest('Umb.Test.Plural.Ea.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = makePlural((permitted) => changes.push(permitted.map((p) => p.alias))); + + await wait(0); + conditionByUnique.get('A')!.flipTo(true); + + await wait(FACTORY_DELAY_MS + 60); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect(finalState, `history: ${JSON.stringify(changes)}`).to.deep.equal([manifest.alias]); + + plural.destroy(); + }); + + // The "stuck at bad" repro, through the full stack (plural → element-and-api → base). + it('exposes the extension after flipping true → false → true during slow element+api load', async () => { + const manifest = makeManifest('Umb.Test.Plural.Ea.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = makePlural((permitted) => changes.push(permitted.map((p) => p.alias))); + + await wait(0); + + const cond = conditionByUnique.get('A')!; + cond.flipTo(true); + cond.flipTo(false); + cond.flipTo(true); + + await wait(FACTORY_DELAY_MS * 2 + 80); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `final exposed should include extension. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifest.alias]); + expect(cond.permitted, 'condition final state').to.be.true; + + plural.destroy(); + }); + + it('exposes the extension after flipping true → false → true → false → true', async () => { + const manifest = makeManifest('Umb.Test.Plural.Ea.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = makePlural((permitted) => changes.push(permitted.map((p) => p.alias))); + + await wait(0); + + const cond = conditionByUnique.get('A')!; + cond.flipTo(true); + cond.flipTo(false); + cond.flipTo(true); + cond.flipTo(false); + cond.flipTo(true); + + await wait(FACTORY_DELAY_MS * 3 + 100); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `final exposed should include extension. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifest.alias]); + + plural.destroy(); + }); + + it('excludes the extension after flipping true → false → true → false', async () => { + const manifest = makeManifest('Umb.Test.Plural.Ea.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = makePlural((permitted) => changes.push(permitted.map((p) => p.alias))); + + await wait(0); + + const cond = conditionByUnique.get('A')!; + cond.flipTo(true); + cond.flipTo(false); + cond.flipTo(true); + cond.flipTo(false); + + await wait(FACTORY_DELAY_MS * 3 + 100); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `final exposed should NOT include extension. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([]); + + plural.destroy(); + }); + + // Multiple extensions, each with its own condition, flipping concurrently. The plural + // layer must end at { A: true, B: false } → only A in the list. + it('tracks two extensions independently when conditions flip concurrently', async () => { + const manifestA = makeManifest('Umb.Test.Plural.Ea.Race.A', 'A', 10); + const manifestB = makeManifest('Umb.Test.Plural.Ea.Race.B', 'B', 5); + extensionRegistry.register(manifestA); + extensionRegistry.register(manifestB); + + const changes: Array = []; + const plural = makePlural((permitted) => changes.push(permitted.map((p) => p.alias).sort())); + + await wait(0); + + const condA = conditionByUnique.get('A')!; + const condB = conditionByUnique.get('B')!; + + condA.flipTo(true); + condB.flipTo(true); + condA.flipTo(false); + condB.flipTo(false); + condA.flipTo(true); + condB.flipTo(true); + condB.flipTo(false); // B ends false, A ends true + + await wait(FACTORY_DELAY_MS * 3 + 120); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `only A should be permitted. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifestA.alias]); + + plural.destroy(); + }); + + // Both extensions end at true — both must be exposed, sorted by weight (higher first). + it('exposes both extensions sorted by weight when both end at true', async () => { + const manifestA = makeManifest('Umb.Test.Plural.Ea.Race.A', 'A', 10); + const manifestB = makeManifest('Umb.Test.Plural.Ea.Race.B', 'B', 99); + extensionRegistry.register(manifestA); + extensionRegistry.register(manifestB); + + const changes: Array = []; + const plural = makePlural((permitted) => changes.push(permitted.map((p) => p.alias))); + + await wait(0); + + const condA = conditionByUnique.get('A')!; + const condB = conditionByUnique.get('B')!; + + condA.flipTo(true); + condB.flipTo(true); + condA.flipTo(false); + condB.flipTo(false); + condA.flipTo(true); + condB.flipTo(true); + + await wait(FACTORY_DELAY_MS * 3 + 120); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + // B (weight 99) should come before A (weight 10). + expect( + finalState, + `B should come first. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifestB.alias, manifestA.alias]); + + plural.destroy(); + }); + + // Flip mid-resolution of a previous factory load — adversarial for the condition's + // `_isConditionsPositive` latch. + it('exposes the extension when a flip lands mid-resolution of a previous good-call', async () => { + const manifest = makeManifest('Umb.Test.Plural.Ea.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = makePlural((permitted) => changes.push(permitted.map((p) => p.alias))); + + await wait(0); + const cond = conditionByUnique.get('A')!; + + cond.flipTo(true); + await wait(FACTORY_DELAY_MS - 10); + cond.flipTo(false); + await wait(5); + cond.flipTo(true); + + await wait(FACTORY_DELAY_MS * 2 + 120); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `final exposed should include extension. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifest.alias]); + + plural.destroy(); + }); + + // Ten rapid flips — if any cumulative drift exists between per-extension + // `#isPermitted` and the plural's `#permittedExts`, this is where it surfaces. + it('final onChange matches the true permitted state after ten rapid flips', async () => { + const manifest = makeManifest('Umb.Test.Plural.Ea.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = makePlural((permitted) => changes.push(permitted.map((p) => p.alias))); + + await wait(0); + const cond = conditionByUnique.get('A')!; + + for (let i = 0; i < 10; i++) { + cond.flipTo(i % 2 === 0); + } + cond.flipTo(true); // force final to true + + await wait(FACTORY_DELAY_MS * 4 + 140); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `final exposed should include extension. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifest.alias]); + + plural.destroy(); + }); + + // Unregister during a race: the extension's manifest is pulled out of the registry + // while its condition is still oscillating and a factory load is pending. After all + // this noise the final list must be empty and the API must not leak. + it('drops the extension cleanly when its manifest is unregistered during a flip race', async () => { + const manifest = makeManifest('Umb.Test.Plural.Ea.Race.A', 'A'); + extensionRegistry.register(manifest); + + const changes: Array = []; + const plural = makePlural((permitted) => changes.push(permitted.map((p) => p.alias))); + + await wait(0); + const cond = conditionByUnique.get('A')!; + + cond.flipTo(true); + cond.flipTo(false); + cond.flipTo(true); + + // Pull the manifest mid-race. + await wait(10); + extensionRegistry.unregister(manifest.alias); + + await wait(FACTORY_DELAY_MS * 2 + 120); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `extension should be absent after unregister. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([]); + + plural.destroy(); + await wait(10); + + expect( + apiDestroyCount, + `API leak after unregister+destroy: ${apiCtorCount} constructed, ${apiDestroyCount} destroyed`, + ).to.equal(apiCtorCount); + }); + + // Single-mode collapses the list to the single highest-weight permitted extension. + // Both end at true → only the higher-weight one should be exposed. + it('exposes only the highest-weight extension in single mode after races', async () => { + const manifestA = makeManifest('Umb.Test.Plural.Ea.Race.A', 'A', 1); + const manifestB = makeManifest('Umb.Test.Plural.Ea.Race.B', 'B', 99); + extensionRegistry.register(manifestA); + extensionRegistry.register(manifestB); + + const changes: Array = []; + const plural = makePlural( + (permitted) => changes.push(permitted.map((p) => p.alias)), + { single: true }, + ); + + await wait(0); + + const condA = conditionByUnique.get('A')!; + const condB = conditionByUnique.get('B')!; + + condA.flipTo(true); + condB.flipTo(true); + condA.flipTo(false); + condB.flipTo(false); + condA.flipTo(true); + condB.flipTo(true); + + await wait(FACTORY_DELAY_MS * 3 + 120); + await waitForDebouncedNotify(); + + const finalState = changes[changes.length - 1] ?? []; + expect( + finalState, + `only B (weight 99) should be exposed. history: ${JSON.stringify(changes)}`, + ).to.deep.equal([manifestB.alias]); + + plural.destroy(); + }); + + // Leak check — every API the plural layer caused to be constructed must be destroyed + // either by a later `_conditionsAreBad`, by the good-call back-out (which explicitly + // destroys stillborn APIs), or by the final plural destroy. + it('does not leak API instances across flip races and destroy', async () => { + const manifestA = makeManifest('Umb.Test.Plural.Ea.Race.A', 'A'); + const manifestB = makeManifest('Umb.Test.Plural.Ea.Race.B', 'B'); + extensionRegistry.register(manifestA); + extensionRegistry.register(manifestB); + + const plural = makePlural(() => { + /* ignore emissions for this accounting test */ + }); + + await wait(0); + + const condA = conditionByUnique.get('A')!; + const condB = conditionByUnique.get('B')!; + + condA.flipTo(true); + condB.flipTo(true); + condA.flipTo(false); + condB.flipTo(false); + condA.flipTo(true); + condB.flipTo(true); + condA.flipTo(false); // A ends false + // B stays true + + await wait(FACTORY_DELAY_MS * 3 + 140); + await waitForDebouncedNotify(); + + plural.destroy(); + await wait(10); + + expect(apiCtorCount, 'some APIs were constructed during the race').to.be.greaterThan(0); + expect( + apiDestroyCount, + `API leak: ${apiCtorCount} constructed, only ${apiDestroyCount} destroyed. Leaked: ${[...liveApis].join(', ')}`, + ).to.equal(apiCtorCount); + }); +}); diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/initializers/extension-initializer-base.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/initializers/extension-initializer-base.ts index 7911e1c8f759..131e9ebd16fe 100644 --- a/src/Umbraco.Web.UI.Client/src/libs/extension-api/initializers/extension-initializer-base.ts +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/initializers/extension-initializer-base.ts @@ -3,7 +3,7 @@ import type { UmbExtensionRegistry } from '../registry/extension.registry.js'; import type { SpecificManifestTypeOrManifestBase } from '../types/map.types.js'; import { UmbControllerBase } from '@umbraco-cms/backoffice/class-api'; import type { UmbElement } from '@umbraco-cms/backoffice/element-api'; -import { ReplaySubject } from '@umbraco-cms/backoffice/external/rxjs'; +import { UmbBooleanState } from '@umbraco-cms/backoffice/observable-api'; /** * Base class for extension initializers, which are responsible for loading and unloading extensions. @@ -12,10 +12,12 @@ export abstract class UmbExtensionInitializerBase< Key extends string, T extends ManifestBase = SpecificManifestTypeOrManifestBase, > extends UmbControllerBase { - protected host; - protected extensionRegistry; + protected host: UmbElement; + protected extensionRegistry: UmbExtensionRegistry; #extensionMap = new Map(); - #loaded = new ReplaySubject(1); + + // Use the value `undefined`, as that would not resolve a observation promise. [NL] + #loaded = new UmbBooleanState(undefined); loaded = this.#loaded.asObservable(); constructor(host: UmbElement, extensionRegistry: UmbExtensionRegistry, manifestType: Key) { @@ -38,7 +40,9 @@ export abstract class UmbExtensionInitializerBase< }), ); - this.#loaded.next(); + if (extensions.length > 0) { + this.#loaded.setValue(true); + } }); } diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/registry/extension.registry.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/registry/extension.registry.ts index 655c06ce8fda..52b5cb26fa10 100644 --- a/src/Umbraco.Web.UI.Client/src/libs/extension-api/registry/extension.registry.ts +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/registry/extension.registry.ts @@ -233,8 +233,32 @@ export class UmbExtensionRegistry< * @memberof UmbExtensionRegistry */ registerMany(manifests: Array>): void { - // we have to register extensions individually, so we ensure a manifest is valid before continuing to the next one - manifests.forEach((manifest) => this.register(manifest)); + const toAdd: ManifestTypes[] = []; + + for (const manifest of manifests) { + // TODO: refactor this so this code is not duplicated between this and single extension register + if (!this.#validateExtension(manifest)) continue; + + if (manifest.type === 'kind') { + this.defineKind(manifest as ManifestKind); + continue; + } + + // TODO: Revisit this, it could use the isExtensionApproved, but this code also checks alias duplication against the toAdd array. [NL] + if (!this.#acceptExtension(manifest as ManifestTypes)) continue; + + const alias = (manifest as ManifestTypes).alias; + if (this._extensions.getValue().find((e) => e.alias === alias) || toAdd.find((e) => e.alias === alias)) { + console.error(`Extension with alias ${alias} is already registered`); + continue; + } + + toAdd.push(this.#appendAdditionalConditions(manifest as ManifestTypes)); + } + + if (toAdd.length) { + this._extensions.setValue([...this._extensions.getValue(), ...toAdd]); + } } /** diff --git a/src/Umbraco.Web.UI.Client/src/packages/block/block-custom-view/types.ts b/src/Umbraco.Web.UI.Client/src/packages/block/block-custom-view/types.ts index f885e5b63d29..86c94ce2fdac 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/block/block-custom-view/types.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/block/block-custom-view/types.ts @@ -29,10 +29,11 @@ export interface UmbBlockEditorCustomViewProperties< settingsInvalid?: boolean; unsupported?: boolean; unpublished?: boolean; + readonly?: boolean; } export interface UmbBlockEditorCustomViewElement< LayoutType extends UmbBlockLayoutBaseModel = UmbBlockLayoutBaseModel, BlockType extends UmbBlockTypeBaseModel = UmbBlockTypeBaseModel, -> extends UmbBlockEditorCustomViewProperties, - HTMLElement {} +> + extends UmbBlockEditorCustomViewProperties, HTMLElement {} diff --git a/src/Umbraco.Web.UI.Client/src/packages/block/block-grid/components/block-grid-entry/block-grid-entry.element.ts b/src/Umbraco.Web.UI.Client/src/packages/block/block-grid/components/block-grid-entry/block-grid-entry.element.ts index ef343a773ebc..342f0fef3eb9 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/block/block-grid/components/block-grid-entry/block-grid-entry.element.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/block/block-grid/components/block-grid-entry/block-grid-entry.element.ts @@ -283,7 +283,10 @@ export class UmbBlockGridEntryElement extends UmbLitElement implements UmbProper this.observe( this.#context.readOnlyGuard.permitted, - (isReadOnly) => (this._isReadOnly = isReadOnly), + (isReadOnly) => { + this._isReadOnly = isReadOnly; + this.#updateBlockViewProps({ readonly: isReadOnly }); + }, 'umbReadOnlyObserver', ); } @@ -582,7 +585,6 @@ export class UmbBlockGridEntryElement extends UmbLitElement implements UmbProper } #renderEditAction() { - if (this._isReadOnly) return nothing; return html` ${when( this._showContentEdit && this._workspaceEditContentPath, @@ -616,7 +618,6 @@ export class UmbBlockGridEntryElement extends UmbLitElement implements UmbProper } #renderEditSettingsAction() { - if (this._isReadOnly) return nothing; return html` ${this._hasSettings && this._workspaceEditSettingsPath ? html` diff --git a/src/Umbraco.Web.UI.Client/src/packages/block/block-grid/property-editors/block-grid-editor/property-editor-ui-block-grid.element.ts b/src/Umbraco.Web.UI.Client/src/packages/block/block-grid/property-editors/block-grid-editor/property-editor-ui-block-grid.element.ts index 290b941ac2ec..f88402d923ba 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/block/block-grid/property-editors/block-grid-editor/property-editor-ui-block-grid.element.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/block/block-grid/property-editors/block-grid-editor/property-editor-ui-block-grid.element.ts @@ -175,9 +175,9 @@ export class UmbPropertyEditorUIBlockGridElement } }).passContextAliasMatches(); - this.consumeContext(UMB_PROPERTY_CONTEXT, (context) => { + this.consumeContext(UMB_PROPERTY_CONTEXT, (propertyContext) => { this.observe( - context?.dataPath, + propertyContext?.dataPath, (dataPath) => { if (dataPath) { // Set the data path for the local validation context: @@ -187,10 +187,7 @@ export class UmbPropertyEditorUIBlockGridElement }, 'observeDataPath', ); - }); - // TODO: Prevent initial notification from these observes - this.consumeContext(UMB_PROPERTY_CONTEXT, (propertyContext) => { this.observe( observeMultiple([ this.#managerContext.layouts, diff --git a/src/Umbraco.Web.UI.Client/src/packages/block/block-list/components/block-list-entry/block-list-entry.element.ts b/src/Umbraco.Web.UI.Client/src/packages/block/block-list/components/block-list-entry/block-list-entry.element.ts index 4427002c20e2..cea8a4faf9e2 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/block/block-list/components/block-list-entry/block-list-entry.element.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/block/block-list/components/block-list-entry/block-list-entry.element.ts @@ -242,7 +242,10 @@ export class UmbBlockListEntryElement extends UmbLitElement implements UmbProper ); this.observe( this.#context.readOnlyGuard.permitted, - (isReadOnly) => (this._isReadOnly = isReadOnly), + (isReadOnly) => { + this._isReadOnly = isReadOnly; + this.#updateBlockViewProps({ readonly: isReadOnly }); + }, 'umbReadOnlyObserver', ); } @@ -377,6 +380,7 @@ export class UmbBlockListEntryElement extends UmbLitElement implements UmbProper .icon=${this._icon} .index=${this._blockViewProps.index} .unpublished=${!this._exposed} + .readOnly=${this._isReadOnly} .config=${this._blockViewProps.config} .content=${this._blockViewProps.content} .settings=${this._blockViewProps.settings} @@ -391,6 +395,7 @@ export class UmbBlockListEntryElement extends UmbLitElement implements UmbProper .icon=${this._icon} .index=${this._blockViewProps.index} .unpublished=${!this._exposed} + .readOnly=${this._isReadOnly} .config=${this._blockViewProps.config} .content=${this._blockViewProps.content} .settings=${this._blockViewProps.settings} @@ -455,7 +460,6 @@ export class UmbBlockListEntryElement extends UmbLitElement implements UmbProper } #renderEditContentAction() { - if (this._isReadOnly) return nothing; return this._showContentEdit && this._workspaceEditContentPath ? html` this.#context.requestDelete()} title=${this.localize.term('general_delete')}> + return html` this.#context.requestDelete()} + title=${this.localize.term('general_delete')}> `; } diff --git a/src/Umbraco.Web.UI.Client/src/packages/block/block-rte/components/block-rte-entry/block-rte-entry.element.ts b/src/Umbraco.Web.UI.Client/src/packages/block/block-rte/components/block-rte-entry/block-rte-entry.element.ts index 3189c9147fcc..9fce1e5ef828 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/block/block-rte/components/block-rte-entry/block-rte-entry.element.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/block/block-rte/components/block-rte-entry/block-rte-entry.element.ts @@ -231,7 +231,10 @@ export class UmbBlockRteEntryElement extends UmbLitElement implements UmbPropert ); this.observe( this.#context.readOnlyGuard.permitted, - (isReadOnly) => (this._isReadOnly = isReadOnly), + (isReadOnly) => { + this._isReadOnly = isReadOnly; + this.#updateBlockViewProps({ readonly: isReadOnly }); + }, 'umbReadOnlyObserver', ); } @@ -373,7 +376,6 @@ export class UmbBlockRteEntryElement extends UmbLitElement implements UmbPropert } #renderEditAction() { - if (this._isReadOnly) return nothing; return this._showContentEdit && this._workspaceEditContentPath ? html` (this._isReadOnly = isReadOnly), + (isReadOnly) => { + this._isReadOnly = isReadOnly; + this.#updateBlockViewProps({ readonly: isReadOnly }); + }, 'umbReadOnlyObserver', ); } @@ -441,7 +444,6 @@ export class UmbBlockSingleEntryElement extends UmbLitElement implements UmbProp } #renderEditContentAction() { - if (this._isReadOnly) return nothing; return this._showContentEdit && this._workspaceEditContentPath ? html` { this.observe( - context?.readOnlyGuard.permitted, + context?.readOnlyGuard.isPermittedForObservableVariant(context.variantId), (isReadOnly) => { + // Only react to positives or negatives: if (isReadOnly !== undefined) { - this.permitted = isReadOnly === (this.config.match !== undefined ? this.config.match : true); + const match = args.config.match ?? true; + this.permitted = isReadOnly === match; } }, 'observeIsReadOnly', diff --git a/src/Umbraco.Web.UI.Client/src/packages/block/block/context/block-entry.context.ts b/src/Umbraco.Web.UI.Client/src/packages/block/block/context/block-entry.context.ts index bb8b2f3fa59e..407095358545 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/block/block/context/block-entry.context.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/block/block/context/block-entry.context.ts @@ -601,15 +601,13 @@ export abstract class UmbBlockEntryContext< // TODO: Here is a potential future issue. This is parsing on the read only state of the variant that this is opened from, that is problematic when we enable switching variant within a Block. [NL] // TODO: This could benefit from a more dynamic approach, where we inherit all non-variant and variant scoped states. [NL] this.observe( - // TODO: Instead transfer all variant states. - this._manager.readOnlyState.isPermittedForObservableVariant(this._variantId), + this._manager.readOnlyState.permitted, (isReadOnly) => { const unique = 'UMB_BLOCK_MANAGER_CONTEXT'; if (isReadOnly) { const rule = { unique, - variantId: this.#variantId.getValue(), }; this.readOnlyGuard?.addRule(rule); diff --git a/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-element-manager.ts b/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-element-manager.ts index bd3bf92958bd..2aff5670b38d 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-element-manager.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-element-manager.ts @@ -96,17 +96,25 @@ export class UmbBlockElementManager { - if (id) { - this.structure.loadType(id); - } - }); - - this.observe(this.unique, (key) => { - if (key) { - this.validation.setDataPath('$.' + dataPathPropertyName + `[?(@.key == '${key}')]`); - } - }); + this.observe( + this.contentTypeId, + (id) => { + if (id) { + this.structure.loadType(id); + } + }, + null, + ); + + this.observe( + this.unique, + (key) => { + if (key) { + this.validation.setDataPath('$.' + dataPathPropertyName + `[?(@.key == '${key}')]`); + } + }, + null, + ); this.observe( this.structure.contentTypeDataTypeUniques, diff --git a/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-workspace-editor.element.ts b/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-workspace-editor.element.ts index 9ebd8991c52f..9068c1542c1a 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-workspace-editor.element.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-workspace-editor.element.ts @@ -1,5 +1,5 @@ import { UMB_BLOCK_WORKSPACE_CONTEXT } from './index.js'; -import { css, customElement, html, state } from '@umbraco-cms/backoffice/external/lit'; +import { css, customElement, html, nothing, state } from '@umbraco-cms/backoffice/external/lit'; import { UmbLitElement } from '@umbraco-cms/backoffice/lit-element'; @customElement('umb-block-workspace-editor') @@ -7,25 +7,40 @@ export class UmbBlockWorkspaceEditorElement extends UmbLitElement { constructor() { super(); this.consumeContext(UMB_BLOCK_WORKSPACE_CONTEXT, (context) => { - if (context) { - this.observe( - context.name, - (name) => { + this.observe( + context?.name, + (name) => { + if (name) { this._headline = this.localize.string(name); - }, - 'observeOwnerContentElementTypeName', - ); - } else { - this.removeUmbControllerByAlias('observeOwnerContentElementTypeName'); - } + } + }, + 'observeOwnerContentElementTypeName', + ); + this.observe( + context?.readOnlyGuard.isPermittedForObservableVariant(context.variantId), + (isReadOnly) => { + this._readOnly = isReadOnly; + }, + 'observeIsReadOnly', + ); }); } @state() private _headline: string = ''; + @state() + private _readOnly?: boolean; + override render() { - return html``; + return html`
+

${this._headline}

+ ${this._readOnly + ? html`${this.localize.term('general_readOnly')}` + : nothing} +
`; } static override readonly styles = [ @@ -35,6 +50,18 @@ export class UmbBlockWorkspaceEditorElement extends UmbLitElement { width: 100%; height: 100%; } + div { + display: flex; + align-items: center; + gap: var(--uui-size-3); + } + #headline { + display: block; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + min-width: 0; + } `, ]; } diff --git a/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-workspace-language-access.controller.test.ts b/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-workspace-language-access.controller.test.ts new file mode 100644 index 000000000000..07bde49d2c8c --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-workspace-language-access.controller.test.ts @@ -0,0 +1,267 @@ +import { UMB_BLOCK_MANAGER_CONTEXT } from '../context/block-manager.context-token.js'; +import { UMB_BLOCK_WORKSPACE_CONTEXT } from './block-workspace.context-token.js'; +import { UmbBlockLanguageAccessWorkspaceController } from './block-workspace-language-access.controller.js'; +import { expect, fixture } from '@open-wc/testing'; +import { customElement, html } from '@umbraco-cms/backoffice/external/lit'; +import { UmbControllerHostElementMixin } from '@umbraco-cms/backoffice/controller-api'; +import type { UmbControllerHost, UmbControllerHostElement } from '@umbraco-cms/backoffice/controller-api'; +import { UmbContextBase } from '@umbraco-cms/backoffice/class-api'; +import { UMB_CURRENT_USER_CONTEXT } from '@umbraco-cms/backoffice/current-user'; +import { UmbReadOnlyVariantGuardManager } from '@umbraco-cms/backoffice/utils'; +import { UmbVariantId } from '@umbraco-cms/backoffice/variant'; +import { UmbArrayState, UmbBasicState, UmbBooleanState } from '@umbraco-cms/backoffice/observable-api'; + +class UmbBlockWorkspaceContextStub extends UmbContextBase { + public readonly IS_BLOCK_WORKSPACE_CONTEXT = true; + readonly #variantId = new UmbBasicState(undefined); + readonly variantId = this.#variantId.asObservable(); + public readonly readOnlyGuard = new UmbReadOnlyVariantGuardManager(this); + public readonly content = { readOnlyGuard: new UmbReadOnlyVariantGuardManager(this) }; + public readonly settings = { readOnlyGuard: new UmbReadOnlyVariantGuardManager(this) }; + + constructor(host: UmbControllerHost) { + super(host, UMB_BLOCK_WORKSPACE_CONTEXT.toString()); + } + + setVariantId(variantId: UmbVariantId | undefined) { + this.#variantId.setValue(variantId); + } +} + +class UmbBlockManagerContextStub extends UmbContextBase { + readonly #permitted = new UmbBooleanState(undefined); + public readonly readOnlyState = { permitted: this.#permitted.asObservable() }; + + constructor(host: UmbControllerHost) { + super(host, UMB_BLOCK_MANAGER_CONTEXT.toString()); + } + + setPermitted(value: boolean) { + this.#permitted.setValue(value); + } +} + +class UmbCurrentUserContextStub extends UmbContextBase { + readonly #languages = new UmbArrayState([], (x) => x); + public readonly languages = this.#languages.asObservable(); + readonly #hasAccessToAllLanguages = new UmbBooleanState(undefined); + public readonly hasAccessToAllLanguages = this.#hasAccessToAllLanguages.asObservable(); + + constructor(host: UmbControllerHost) { + super(host, UMB_CURRENT_USER_CONTEXT.toString()); + } + + setLanguages(languages: Array) { + this.#languages.setValue(languages); + } + + setHasAccessToAllLanguages(value: boolean) { + this.#hasAccessToAllLanguages.setValue(value); + } +} + +@customElement('umb-test-block-language-access-host') +class UmbTestBlockLanguageAccessHostElement extends UmbControllerHostElementMixin(HTMLElement) { + workspaceContext!: UmbBlockWorkspaceContextStub; + blockManagerContext!: UmbBlockManagerContextStub; + currentUserContext!: UmbCurrentUserContextStub; + + override connectedCallback() { + super.connectedCallback(); + this.workspaceContext = new UmbBlockWorkspaceContextStub(this); + this.blockManagerContext = new UmbBlockManagerContextStub(this); + this.currentUserContext = new UmbCurrentUserContextStub(this); + } +} + +const enUS = UmbVariantId.Create({ culture: 'en-US', segment: null }); +const daDK = UmbVariantId.Create({ culture: 'da-DK', segment: null }); + +async function flushMicrotasks() { + // Two ticks: one for context-consumer resolution, one for the inner observe to fire. + await Promise.resolve(); + await Promise.resolve(); + await new Promise((r) => setTimeout(r, 0)); +} + +describe('UmbBlockLanguageAccessWorkspaceController', () => { + let host: UmbTestBlockLanguageAccessHostElement; + + beforeEach(async () => { + host = await fixture(html``); + }); + + afterEach(() => { + host.remove(); + }); + + function expectReadOnly(variantId: UmbVariantId) { + expect(host.workspaceContext.readOnlyGuard.getIsPermittedForVariant(variantId), 'workspace.readOnlyGuard').to.be + .true; + expect(host.workspaceContext.content.readOnlyGuard.getIsPermittedForVariant(variantId), 'content.readOnlyGuard').to + .be.true; + expect(host.workspaceContext.settings.readOnlyGuard.getIsPermittedForVariant(variantId), 'settings.readOnlyGuard') + .to.be.true; + } + + function expectEditable(variantId: UmbVariantId) { + expect(host.workspaceContext.readOnlyGuard.getIsPermittedForVariant(variantId), 'workspace.readOnlyGuard').to.be + .false; + expect(host.workspaceContext.content.readOnlyGuard.getIsPermittedForVariant(variantId), 'content.readOnlyGuard').to + .be.false; + expect(host.workspaceContext.settings.readOnlyGuard.getIsPermittedForVariant(variantId), 'settings.readOnlyGuard') + .to.be.false; + } + + describe('Invariant block — block manager state', () => { + it('is read-only when the block manager is read-only', async () => { + host.workspaceContext.setVariantId(UmbVariantId.CreateInvariant()); + host.blockManagerContext.setPermitted(true); + + new UmbBlockLanguageAccessWorkspaceController(host as unknown as UmbControllerHost); + await flushMicrotasks(); + + expectReadOnly(UmbVariantId.CreateInvariant()); + }); + + it('is editable when the block manager is not read-only', async () => { + host.workspaceContext.setVariantId(UmbVariantId.CreateInvariant()); + host.blockManagerContext.setPermitted(false); + + new UmbBlockLanguageAccessWorkspaceController(host as unknown as UmbControllerHost); + await flushMicrotasks(); + + expectEditable(UmbVariantId.CreateInvariant()); + }); + + it('flips to editable when the block manager flips from read-only to not read-only', async () => { + host.workspaceContext.setVariantId(UmbVariantId.CreateInvariant()); + host.blockManagerContext.setPermitted(true); + + new UmbBlockLanguageAccessWorkspaceController(host as unknown as UmbControllerHost); + await flushMicrotasks(); + expectReadOnly(UmbVariantId.CreateInvariant()); + + host.blockManagerContext.setPermitted(false); + await flushMicrotasks(); + expectEditable(UmbVariantId.CreateInvariant()); + }); + + it('flips to read-only when the block manager flips from not read-only to read-only', async () => { + host.workspaceContext.setVariantId(UmbVariantId.CreateInvariant()); + host.blockManagerContext.setPermitted(false); + + new UmbBlockLanguageAccessWorkspaceController(host as unknown as UmbControllerHost); + await flushMicrotasks(); + expectEditable(UmbVariantId.CreateInvariant()); + + host.blockManagerContext.setPermitted(true); + await flushMicrotasks(); + expectReadOnly(UmbVariantId.CreateInvariant()); + }); + }); + + describe('Variant block — language access', () => { + it('is editable when the user has access to all languages', async () => { + host.workspaceContext.setVariantId(enUS); + host.currentUserContext.setHasAccessToAllLanguages(true); + host.currentUserContext.setLanguages([]); + + new UmbBlockLanguageAccessWorkspaceController(host as unknown as UmbControllerHost); + await flushMicrotasks(); + + expectEditable(enUS); + }); + + it('is editable when the culture is in the user allowed languages', async () => { + host.workspaceContext.setVariantId(enUS); + host.currentUserContext.setHasAccessToAllLanguages(false); + host.currentUserContext.setLanguages(['en-US']); + + new UmbBlockLanguageAccessWorkspaceController(host as unknown as UmbControllerHost); + await flushMicrotasks(); + + expectEditable(enUS); + }); + + it('is read-only when the culture is not in the user allowed languages', async () => { + host.workspaceContext.setVariantId(enUS); + host.currentUserContext.setHasAccessToAllLanguages(false); + host.currentUserContext.setLanguages(['da-DK']); + + new UmbBlockLanguageAccessWorkspaceController(host as unknown as UmbControllerHost); + await flushMicrotasks(); + + expectReadOnly(enUS); + }); + + it('is read-only when the user has neither global access nor a matching language', async () => { + host.workspaceContext.setVariantId(enUS); + host.currentUserContext.setHasAccessToAllLanguages(false); + host.currentUserContext.setLanguages([]); + + new UmbBlockLanguageAccessWorkspaceController(host as unknown as UmbControllerHost); + await flushMicrotasks(); + + expectReadOnly(enUS); + }); + }); + + describe('Transitions', () => { + it('updates correctly when the variantId switches culture (en-US → da-DK)', async () => { + host.workspaceContext.setVariantId(enUS); + host.currentUserContext.setHasAccessToAllLanguages(false); + host.currentUserContext.setLanguages(['da-DK']); + + new UmbBlockLanguageAccessWorkspaceController(host as unknown as UmbControllerHost); + await flushMicrotasks(); + expectReadOnly(enUS); + + host.workspaceContext.setVariantId(daDK); + await flushMicrotasks(); + expectEditable(daDK); + }); + + it('drops the block-manager read-only state when switching from invariant to variant', async () => { + host.workspaceContext.setVariantId(UmbVariantId.CreateInvariant()); + host.blockManagerContext.setPermitted(true); + host.currentUserContext.setHasAccessToAllLanguages(true); + host.currentUserContext.setLanguages([]); + + new UmbBlockLanguageAccessWorkspaceController(host as unknown as UmbControllerHost); + await flushMicrotasks(); + expectReadOnly(UmbVariantId.CreateInvariant()); + + host.workspaceContext.setVariantId(enUS); + await flushMicrotasks(); + expectEditable(enUS); + }); + + it('stays correct after multiple invariant ↔ variant transitions', async () => { + host.workspaceContext.setVariantId(UmbVariantId.CreateInvariant()); + host.blockManagerContext.setPermitted(true); + host.currentUserContext.setHasAccessToAllLanguages(true); + host.currentUserContext.setLanguages([]); + + new UmbBlockLanguageAccessWorkspaceController(host as unknown as UmbControllerHost); + await flushMicrotasks(); + expectReadOnly(UmbVariantId.CreateInvariant()); + + host.workspaceContext.setVariantId(enUS); + await flushMicrotasks(); + expectEditable(enUS); + + host.workspaceContext.setVariantId(UmbVariantId.CreateInvariant()); + await flushMicrotasks(); + expectReadOnly(UmbVariantId.CreateInvariant()); + + host.workspaceContext.setVariantId(enUS); + await flushMicrotasks(); + expectEditable(enUS); + + host.workspaceContext.setVariantId(UmbVariantId.CreateInvariant()); + await flushMicrotasks(); + expectReadOnly(UmbVariantId.CreateInvariant()); + }); + }); +}); diff --git a/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-workspace-language-access.controller.ts b/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-workspace-language-access.controller.ts new file mode 100644 index 000000000000..1ef2325e83f4 --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-workspace-language-access.controller.ts @@ -0,0 +1,164 @@ +import { UMB_BLOCK_MANAGER_CONTEXT } from '../context/block-manager.context-token.js'; +import { UMB_BLOCK_WORKSPACE_CONTEXT } from './block-workspace.context-token.js'; +import type { UmbControllerHost } from '@umbraco-cms/backoffice/controller-api'; +import { UmbControllerBase } from '@umbraco-cms/backoffice/class-api'; +import { UMB_CURRENT_USER_CONTEXT } from '@umbraco-cms/backoffice/current-user'; +import type { UmbVariantId } from '@umbraco-cms/backoffice/variant'; +import type { UmbContextConsumerController } from '@umbraco-cms/backoffice/context-api'; + +const IDENTIFIER_PREFIX = 'UMB_LANGUAGE_PERMISSION_'; + +/** + * Configures the read-only state of a Block Workspace based on the parent Block Manager + * and the current user's language access. + * + * - For invariant blocks, the workspace inherits the read-only state of the parent Block Manager(Host Property). + * - For variant blocks (with a culture), the workspace is editable only when the current user + * has access to that culture (either via `hasAccessToAllLanguages` or an entry in their + * allowed languages). + */ +export class UmbBlockLanguageAccessWorkspaceController extends UmbControllerBase { + #workspaceContext?: typeof UMB_BLOCK_WORKSPACE_CONTEXT.TYPE; + #variantId?: UmbVariantId; + #currentUserAllowedLanguages?: Array; + #currentUserHasAccessToAllLanguages?: boolean; + #consumeBlockManager?: UmbContextConsumerController; + #appliedLanguageUnique?: string; + + constructor(host: UmbControllerHost) { + super(host); + + this.consumeContext(UMB_BLOCK_WORKSPACE_CONTEXT, (instance) => { + this.#workspaceContext = instance; + this.#workspaceContext?.readOnlyGuard.fallbackToNotPermitted(); + this.#workspaceContext?.content.readOnlyGuard.fallbackToNotPermitted(); + this.#workspaceContext?.settings.readOnlyGuard.fallbackToNotPermitted(); + + this.observe( + instance?.variantId, + (variantId) => { + this.#variantId = variantId; + + this.#observeBlockManager(variantId); + + this.#checkForLanguageAccess(); + }, + 'observeBlockVariantId', + ); + }); + + this.consumeContext(UMB_CURRENT_USER_CONTEXT, (context) => { + this.observe( + context?.languages, + (languages) => { + this.#currentUserAllowedLanguages = languages; + this.#checkForLanguageAccess(); + }, + 'observeCurrentUserLanguages', + ); + + this.observe( + context?.hasAccessToAllLanguages, + (hasAccessToAllLanguages) => { + this.#currentUserHasAccessToAllLanguages = hasAccessToAllLanguages; + this.#checkForLanguageAccess(); + }, + 'observeCurrentUserHasAccessToAllLanguages', + ); + }); + } + + #observeBlockManager(variantId?: UmbVariantId) { + const unique = 'UMB_BLOCK_MANAGER_CONTEXT'; + if (variantId?.isCultureInvariant()) { + /** + * If the Block Workspace is invariant, the readOnly state from the Block Manager should apply to the invariant fields(all) of this Workspace: [NL] + */ + // Destroy any prior consumer before reassigning, so a re-emit of an invariant + // variantId does not leak the previous context consumer. [NL] + this.#consumeBlockManager?.destroy(); + this.#consumeBlockManager = this.consumeContext(UMB_BLOCK_MANAGER_CONTEXT, (manager) => { + this.observe( + manager?.readOnlyState.permitted, + (isReadOnly) => { + if (isReadOnly === undefined) return; + + if (isReadOnly) { + const rule = { + unique, + permitted: true, + }; + + this.#workspaceContext?.readOnlyGuard.addRule(rule); + this.#workspaceContext?.content.readOnlyGuard.addRule(rule); + this.#workspaceContext?.settings.readOnlyGuard.addRule(rule); + } else { + this.#workspaceContext?.readOnlyGuard.removeRule(unique); + this.#workspaceContext?.content.readOnlyGuard.removeRule(unique); + this.#workspaceContext?.settings.readOnlyGuard.removeRule(unique); + } + }, + 'observeManagerReadOnly', + ); + }); + } else { + this.#workspaceContext?.readOnlyGuard.removeRule(unique); + this.#workspaceContext?.content.readOnlyGuard.removeRule(unique); + this.#workspaceContext?.settings.readOnlyGuard.removeRule(unique); + this.#consumeBlockManager?.destroy(); + this.#consumeBlockManager = undefined; + this.removeUmbControllerByAlias('observeManagerReadOnly'); + } + } + + #checkForLanguageAccess() { + if ( + !this.#workspaceContext || + this.#currentUserHasAccessToAllLanguages == undefined || + this.#currentUserAllowedLanguages == undefined + ) { + return; + } + + const culture = this.#variantId?.culture ?? undefined; + + // If the block is invariant/segment-only, or the user has access to all languages, + // there is no language-based restriction to apply. + const allowed = + !culture || this.#currentUserHasAccessToAllLanguages === true + ? true + : (this.#currentUserAllowedLanguages?.includes(culture) ?? false); + + // Always remove the previously applied rule (tracked by the actual unique key, + // not just the current culture). Without this, switching the workspace's culture + // from A → B leaves a stale UMB_LANGUAGE_PERMISSION_ rule lingering in the + // guard manager — `findRule()` is variant-scoped so it stays harmless, but + // `getRules()` accumulates one entry per visited culture over the workspace's + // lifetime. [NL] + if (this.#appliedLanguageUnique) { + this.#workspaceContext.readOnlyGuard.removeRule(this.#appliedLanguageUnique); + this.#workspaceContext.content.readOnlyGuard.removeRule(this.#appliedLanguageUnique); + this.#workspaceContext.settings.readOnlyGuard.removeRule(this.#appliedLanguageUnique); + this.#appliedLanguageUnique = undefined; + } + + if (allowed || !culture || !this.#variantId) return; + + const variantId = this.#variantId; + const unique = IDENTIFIER_PREFIX + culture; + const rule = { + unique, + variantId, + // `permitted: true` on a read-only guard means "to be read-only" + // — i.e. not editable. Combined with `fallbackToPermitted()` (default = read-only). + permitted: true, + }; + + this.#workspaceContext.readOnlyGuard.addRule(rule); + this.#workspaceContext.content.readOnlyGuard.addRule(rule); + this.#workspaceContext.settings.readOnlyGuard.addRule(rule); + this.#appliedLanguageUnique = unique; + } +} + +export { UmbBlockLanguageAccessWorkspaceController as api }; diff --git a/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-workspace.context.ts b/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-workspace.context.ts index 986f85db20c2..2eedd7f268b4 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-workspace.context.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/block-workspace.context.ts @@ -4,6 +4,7 @@ import { UmbBlockWorkspaceEditorElement } from './block-workspace-editor.element import { UmbBlockElementManager } from './block-element-manager.js'; import type { UmbBlockWorkspaceOriginData } from './block-workspace.modal-token.js'; import { UMB_BLOCK_WORKSPACE_VIEW_CONTENT, UMB_BLOCK_WORKSPACE_VIEW_SETTINGS } from './constants.js'; +import { UmbBlockLanguageAccessWorkspaceController } from './block-workspace-language-access.controller.js'; import { UmbSubmittableWorkspaceContextBase, type UmbRoutableWorkspaceContext, @@ -25,6 +26,7 @@ import { decodeFilePath, UmbReadOnlyVariantGuardManager } from '@umbraco-cms/bac import { UmbVariantId } from '@umbraco-cms/backoffice/variant'; import type { UUIModalSidebarSize } from '@umbraco-cms/backoffice/external/uui'; import { UmbUfmVirtualRenderController } from '@umbraco-cms/backoffice/ufm'; +import { UMB_IS_TRASHED_ENTITY_CONTEXT } from '@umbraco-cms/backoffice/recycle-bin'; export type UmbBlockWorkspaceElementManagerNames = 'content' | 'settings'; @@ -84,6 +86,8 @@ export class UmbBlockWorkspaceContext { + this.observe( + context?.isTrashed, + (isTrashed) => { + const trashed = isTrashed === true; + const unique = 'UMB_PREVENT_EDIT_TRASHED_ITEM'; + if (trashed) { + const rule = { + unique, + permitted: true, + }; + this.readOnlyGuard.addRule(rule); + this.content.readOnlyGuard.addRule(rule); + this.settings.readOnlyGuard.addRule(rule); + } else { + this.readOnlyGuard.removeRule(unique); + this.content.readOnlyGuard.removeRule(unique); + this.settings.readOnlyGuard.removeRule(unique); + } + }, + 'observeIsTrashed', + ); + }).skipHost(); + this.observe( this.variantId, (variantId) => { @@ -199,30 +227,6 @@ export class UmbBlockWorkspaceContext { - const unique = 'UMB_BLOCK_MANAGER_CONTEXT'; - - if (isReadOnly) { - const rule = { - unique, - variantId: this.#variantId.getValue(), - }; - - this.readOnlyGuard?.addRule(rule); - this.content.propertyWriteGuard.addRule({ unique, permitted: false }); - this.settings.propertyWriteGuard.addRule({ unique, permitted: false }); - } else { - this.readOnlyGuard?.removeRule(unique); - this.content.propertyWriteGuard.removeRule(unique); - this.settings.propertyWriteGuard.removeRule(unique); - } - }, - 'observeIsReadOnly', - ); - this.observe( this.content.contentTypeId, (contentTypeId) => { diff --git a/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/manifests.ts b/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/manifests.ts index bacea6a1ed2e..109eb30c15ba 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/manifests.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/block/block/workspace/manifests.ts @@ -1,3 +1,4 @@ +import { UMB_BLOCK_WORKSPACE_CONTEXT } from './block-workspace.context-token.js'; import { UMB_BLOCK_WORKSPACE_ALIAS, UMB_BLOCK_WORKSPACE_VIEW_CONTENT, @@ -16,6 +17,7 @@ export const manifests: Array = [ label: '#general_create', look: 'primary', color: 'positive', + workspaceContextToken: UMB_BLOCK_WORKSPACE_CONTEXT, }, conditions: [ { @@ -42,6 +44,7 @@ export const manifests: Array = [ label: '#general_update', look: 'primary', color: 'positive', + workspaceContextToken: UMB_BLOCK_WORKSPACE_CONTEXT, }, conditions: [ { diff --git a/src/Umbraco.Web.UI.Client/src/packages/code-editor/umbraco-package.ts b/src/Umbraco.Web.UI.Client/src/packages/code-editor/umbraco-package.ts index 6dffa1cf5411..de986145db81 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/code-editor/umbraco-package.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/code-editor/umbraco-package.ts @@ -1,3 +1,4 @@ +export const name = 'Umbraco.CodeEditor'; export const extensions = [ { name: 'Umbraco Code Editor Bundle', diff --git a/src/Umbraco.Web.UI.Client/src/packages/content/content/global-components/content-workspace-property.element.ts b/src/Umbraco.Web.UI.Client/src/packages/content/content/global-components/content-workspace-property.element.ts index 6325e0035dc4..23d277259fe6 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/content/content/global-components/content-workspace-property.element.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/content/content/global-components/content-workspace-property.element.ts @@ -60,7 +60,7 @@ export class UmbContentWorkspacePropertyElement extends UmbLitElement { // The Content Workspace Context is used to retrieve the property type we like to observe. // This gives us the configuration from the property type as part of the data type. - this.consumeContext(UMB_CONTENT_WORKSPACE_CONTEXT, async (workspaceContext) => { + this.consumeContext(UMB_CONTENT_WORKSPACE_CONTEXT, (workspaceContext) => { this._workspaceContext = workspaceContext; this.#observePropertyType(); }); diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/components/body-layout/body-layout.element.ts b/src/Umbraco.Web.UI.Client/src/packages/core/components/body-layout/body-layout.element.ts index a31aa8482649..670ab9bcd178 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/components/body-layout/body-layout.element.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/components/body-layout/body-layout.element.ts @@ -92,7 +92,9 @@ export class UmbBodyLayoutElement extends LitElement { this._navigationSlotHasChildren ? '' : 'none'}"> - ${this.headline ? html`

${this.headline}

` : nothing} + ${this.headline + ? html`

${this.headline}

` + : nothing} void; + #onChange: ((permitted: boolean) => void) | undefined; constructor(host: UmbControllerHost, args: { config: ConditionConfigType; onChange: (permitted: boolean) => void }) { super(host); @@ -25,8 +25,9 @@ export class UmbConditionBase im constructor(host: UmbControllerHost, args: UmbConditionControllerArguments) { super(host, args); + const offset = parseInt(this.config.offset); + if (isNaN(offset) || offset <= 0) { + throw new Error(`Offset must be a positive number (offset: ${this.config.offset})`); + } this.#timer = setTimeout(() => { this.permitted = true; - }, parseInt(this.config.offset)); + }, offset); } override destroy() { diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/extension-registry/conditions/switch.condition.ts b/src/Umbraco.Web.UI.Client/src/packages/core/extension-registry/conditions/switch.condition.ts index 454ca4bdaf10..15b693a32cd3 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/extension-registry/conditions/switch.condition.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/extension-registry/conditions/switch.condition.ts @@ -9,26 +9,32 @@ import type { export class UmbSwitchCondition extends UmbConditionBase implements UmbExtensionCondition { #timer?: ReturnType; + #frequency: number; constructor(host: UmbControllerHost, args: UmbConditionControllerArguments) { super(host, args); - this.startApprove(); + const frequency = parseInt(this.config.frequency); + if (isNaN(frequency) || frequency <= 0) { + throw new Error(`Frequency must be a positive number (frequency: ${this.config.frequency})`); + } + this.#frequency = frequency; + this.#startApprove(); } - startApprove() { + #startApprove() { clearTimeout(this.#timer); this.#timer = setTimeout(() => { this.permitted = true; - this.startDisapprove(); - }, parseInt(this.config.frequency)); + this.#startDisapprove(); + }, this.#frequency); } - startDisapprove() { + #startDisapprove() { clearTimeout(this.#timer); this.#timer = setTimeout(() => { this.permitted = false; - this.startApprove(); - }, parseInt(this.config.frequency)); + this.#startApprove(); + }, this.#frequency); } override destroy() { diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/property/property-guard-manager/property-guard.manager.test.ts b/src/Umbraco.Web.UI.Client/src/packages/core/property/property-guard-manager/property-guard.manager.test.ts index 51579107383c..8f5a7adea40d 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/property/property-guard-manager/property-guard.manager.test.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/property/property-guard-manager/property-guard.manager.test.ts @@ -141,4 +141,51 @@ describe('UmbPropertyGuardManager', () => { .unsubscribe(); }); }); + + describe('Fallback', () => { + it('isPermittedForProperty reacts to late fallback updates when no rules match', () => { + const emitted: boolean[] = []; + const subscription = manager.isPermittedForProperty(propA).subscribe((value) => emitted.push(value)); + + manager.fallbackToPermitted(); + manager.fallbackToNotPermitted(); + + subscription.unsubscribe(); + + expect(emitted).to.deep.equal([false, true, false]); + }); + + it('isPermittedForProperty is stable when a matching rule determines the result', () => { + manager.addRule(rulePropB); + const emitted: boolean[] = []; + const subscription = manager.isPermittedForProperty(propB).subscribe((value) => emitted.push(value)); + + manager.fallbackToPermitted(); + manager.fallbackToNotPermitted(); + + subscription.unsubscribe(); + + expect(emitted).to.deep.equal([true]); + }); + + it('isPermittedForProperty uses late fallback updates for properties without a matching rule', () => { + manager.addRule(rulePropB); + const emitted: boolean[] = []; + const subscription = manager.isPermittedForProperty(propA).subscribe((value) => emitted.push(value)); + + manager.fallbackToPermitted(); + + subscription.unsubscribe(); + + expect(emitted).to.deep.equal([false, true]); + }); + + it('getIsPermittedForProperty reflects the current fallback when no rules match', () => { + expect(manager.getIsPermittedForProperty(propA)).to.equal(false); + manager.fallbackToPermitted(); + expect(manager.getIsPermittedForProperty(propA)).to.equal(true); + manager.fallbackToNotPermitted(); + expect(manager.getIsPermittedForProperty(propA)).to.equal(false); + }); + }); }); diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/property/property-guard-manager/property-guard.manager.ts b/src/Umbraco.Web.UI.Client/src/packages/core/property/property-guard-manager/property-guard.manager.ts index 3cdf4e44338c..3c8f30f0621b 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/property/property-guard-manager/property-guard.manager.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/property/property-guard-manager/property-guard.manager.ts @@ -1,4 +1,4 @@ -import type { Observable } from '@umbraco-cms/backoffice/observable-api'; +import { mergeObservables, type Observable } from '@umbraco-cms/backoffice/observable-api'; import type { UmbReferenceByUnique } from '@umbraco-cms/backoffice/models'; import { UmbGuardManagerBase, type UmbGuardRule } from '@umbraco-cms/backoffice/utils'; @@ -29,7 +29,9 @@ export class UmbPropertyGuardManager extends UmbGuardManagerBase { - return this._rules.asObservablePart((rules) => this.#resolvePermission(rules, propertyType)); + return mergeObservables([this.rules, this._fallback], ([states, fallback]) => { + return this.#resolvePermission(states, propertyType) ?? fallback; + }); } /** @@ -39,16 +41,16 @@ export class UmbPropertyGuardManager extends UmbGuardManagerBase x.permitted === false).some((rule) => findRule(rule, propertyType))) { return false; } if (rules.filter((x) => x.permitted === true).some((rule) => findRule(rule, propertyType))) { return true; } - return this._fallback; + return undefined; } } diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/property/property-guard-manager/variant-property-guard.manager.test.ts b/src/Umbraco.Web.UI.Client/src/packages/core/property/property-guard-manager/variant-property-guard.manager.test.ts index a0f04cc985b6..56298003ab11 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/property/property-guard-manager/variant-property-guard.manager.test.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/property/property-guard-manager/variant-property-guard.manager.test.ts @@ -242,4 +242,43 @@ describe('UmbVariantPropertyGuardManager', () => { .unsubscribe(); }); }); + + describe('Fallback', () => { + it('isPermittedForVariantAndProperty reacts to late fallback updates when no rules match', () => { + const emitted: boolean[] = []; + const subscription = manager + .isPermittedForVariantAndProperty(englishVariant, propB, invariantVariant) + .subscribe((value) => emitted.push(value)); + + manager.fallbackToPermitted(); + manager.fallbackToNotPermitted(); + + subscription.unsubscribe(); + + expect(emitted).to.deep.equal([false, true, false]); + }); + + it('isPermittedForVariantAndProperty is stable when a matching rule determines the result', () => { + manager.addRule(statePropBEn); + const emitted: boolean[] = []; + const subscription = manager + .isPermittedForVariantAndProperty(englishVariant, propB, invariantVariant) + .subscribe((value) => emitted.push(value)); + + manager.fallbackToPermitted(); + manager.fallbackToNotPermitted(); + + subscription.unsubscribe(); + + expect(emitted).to.deep.equal([true]); + }); + + it('getIsPermittedForVariantAndProperty reflects the current fallback when no rules match', () => { + expect(manager.getIsPermittedForVariantAndProperty(englishVariant, propB, invariantVariant)).to.equal(false); + manager.fallbackToPermitted(); + expect(manager.getIsPermittedForVariantAndProperty(englishVariant, propB, invariantVariant)).to.equal(true); + manager.fallbackToNotPermitted(); + expect(manager.getIsPermittedForVariantAndProperty(englishVariant, propB, invariantVariant)).to.equal(false); + }); + }); }); diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/property/property-guard-manager/variant-property-guard.manager.ts b/src/Umbraco.Web.UI.Client/src/packages/core/property/property-guard-manager/variant-property-guard.manager.ts index 654be950e039..07383c6d1db7 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/property/property-guard-manager/variant-property-guard.manager.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/property/property-guard-manager/variant-property-guard.manager.ts @@ -1,6 +1,6 @@ import type { UmbPropertyGuardRule } from './property-guard.manager.js'; import type { UmbVariantId } from '@umbraco-cms/backoffice/variant'; -import type { Observable } from '@umbraco-cms/backoffice/observable-api'; +import { mergeObservables, type Observable } from '@umbraco-cms/backoffice/observable-api'; import type { UmbReferenceByUnique } from '@umbraco-cms/backoffice/models'; import { UmbGuardManagerBase } from '@umbraco-cms/backoffice/utils'; @@ -61,9 +61,9 @@ export class UmbVariantPropertyGuardManager extends UmbGuardManagerBase { - return this._rules.asObservablePart((rules) => - this.#resolvePermission(rules, propertyVariantId, propertyType, datasetVariantId), - ); + return mergeObservables([this.rules, this._fallback], ([rules, fallback]) => { + return this.#resolvePermission(rules, propertyVariantId, propertyType, datasetVariantId) ?? fallback; + }); } /** @@ -79,7 +79,10 @@ export class UmbVariantPropertyGuardManager extends UmbGuardManagerBase implements UmbExtensionCondition @@ -15,9 +16,13 @@ export class UmbEntityIsTrashedCondition super(host, args); this.consumeContext(UMB_IS_TRASHED_ENTITY_CONTEXT, (context) => { - this.observe(context?.isTrashed, (isTrashed) => { - this.permitted = isTrashed === true; - }); + this.observe( + context?.isTrashed, + (isTrashed) => { + this.permitted = isTrashed === true; + }, + s, + ); }); } } diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/repository/data-mapper/mapping/data-mapping-resolver.ts b/src/Umbraco.Web.UI.Client/src/packages/core/repository/data-mapper/mapping/data-mapping-resolver.ts index c0816ee9b899..6bb21f204028 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/repository/data-mapper/mapping/data-mapping-resolver.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/repository/data-mapper/mapping/data-mapping-resolver.ts @@ -50,6 +50,7 @@ export class UmbDataSourceDataMappingResolver extends UmbControllerBase { } // Pick the manifest with the highest priority + // TODO: We can remove this sorting, it is implemented in the extension registry. // TODO: This should have been handled in the extension registry, but until then we do it here: [NL] return supportedManifests.sort((a: ManifestBase, b: ManifestBase): number => (b.weight || 0) - (a.weight || 0))[0]; } diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/guard.manager.base.test.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/guard.manager.base.test.ts index c306b8b8e2fe..df5e7fe0e5b2 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/guard.manager.base.test.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/guard.manager.base.test.ts @@ -6,10 +6,15 @@ import { UmbControllerHostElementMixin } from '@umbraco-cms/backoffice/controlle @customElement('test-my-controller-host') class UmbTestControllerHostElement extends UmbControllerHostElementMixin(HTMLElement) {} -class UmbTestGuardManager extends UmbGuardManagerBase {} +class UmbTestGuardManager extends UmbGuardManagerBase { + public readonly fallback = this._fallback; + public getFallback() { + return this._getFallback(); + } +} describe('UmbPermissionGuardManager', () => { - let manager: UmbGuardManagerBase; + let manager: UmbTestGuardManager; const rule1: UmbGuardIncomingRuleBase = { unique: '1', message: 'Rule 1', permitted: true }; const rule2: UmbGuardIncomingRuleBase = { unique: '2', message: 'Rule 2', permitted: true }; const ruleFalse: UmbGuardIncomingRuleBase = { unique: '-1', message: 'Rule -1', permitted: false }; @@ -134,4 +139,47 @@ describe('UmbPermissionGuardManager', () => { .unsubscribe(); }); }); + + describe('Fallback', () => { + it('defaults to not permitted', () => { + expect(manager.getFallback()).to.equal(false); + }); + + it('fallbackToPermitted updates the current value', () => { + manager.fallbackToPermitted(); + expect(manager.getFallback()).to.equal(true); + }); + + it('fallbackToNotPermitted updates the current value', () => { + manager.fallbackToPermitted(); + manager.fallbackToNotPermitted(); + expect(manager.getFallback()).to.equal(false); + }); + + it('emits late fallback updates to existing subscribers', () => { + const emitted: boolean[] = []; + const subscription = manager.fallback.subscribe((value) => emitted.push(value)); + + manager.fallbackToPermitted(); + manager.fallbackToNotPermitted(); + manager.fallbackToPermitted(); + + subscription.unsubscribe(); + + expect(emitted).to.deep.equal([false, true, false, true]); + }); + + it('does not emit when the fallback value does not change', () => { + const emitted: boolean[] = []; + const subscription = manager.fallback.subscribe((value) => emitted.push(value)); + + manager.fallbackToNotPermitted(); + manager.fallbackToPermitted(); + manager.fallbackToPermitted(); + + subscription.unsubscribe(); + + expect(emitted).to.deep.equal([false, true]); + }); + }); }); diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/guard.manager.base.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/guard.manager.base.ts index ff5bef8a4474..e2e871475bb3 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/guard.manager.base.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/guard.manager.base.ts @@ -1,6 +1,6 @@ import type { UmbPartialSome } from '../type/index.js'; import { UmbControllerBase } from '@umbraco-cms/backoffice/class-api'; -import { UmbArrayState } from '@umbraco-cms/backoffice/observable-api'; +import { UmbArrayState, UmbBooleanState } from '@umbraco-cms/backoffice/observable-api'; export interface UmbGuardIncomingRuleBase { unique?: string | symbol; @@ -22,28 +22,32 @@ export abstract class UmbGuardManagerBase< public readonly rules = this._rules.asObservable(); public readonly hasRules = this._rules.asObservablePart((x) => x.length > 0); - protected _fallback = false; + #fallback = new UmbBooleanState(false); + protected _fallback = this.#fallback.asObservable(); + protected _getFallback() { + return this.#fallback.getValue(); + } public fallbackToNotPermitted() { - this._fallback = false; + this.#fallback.setValue(false); } public fallbackToPermitted() { - this._fallback = true; + this.#fallback.setValue(true); } /** * Add a new rule * @param {RuleType} rule */ - addRule(rule: IncomingRuleType) { + addRule(rule: IncomingRuleType): RuleType['unique'] { const newRule = { ...rule } as unknown as RuleType; newRule.unique ??= Symbol(); if (newRule.permitted === undefined) { newRule.permitted = true; } this._rules.appendOne(newRule); - return rule.unique; + return newRule.unique; } /** diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/readonly-guard.manager.test.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/readonly-guard.manager.test.ts new file mode 100644 index 000000000000..fcec59f8796b --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/readonly-guard.manager.test.ts @@ -0,0 +1,94 @@ +import { expect } from '@open-wc/testing'; +import { customElement } from '@umbraco-cms/backoffice/external/lit'; +import { UmbControllerHostElementMixin } from '@umbraco-cms/backoffice/controller-api'; +import { UmbReadOnlyGuardManager } from './readonly-guard.manager.js'; +import type { UmbGuardRule } from './guard.manager.base.js'; + +@customElement('test-my-controller-host') +class UmbTestControllerHostElement extends UmbControllerHostElementMixin(HTMLElement) {} + +describe('UmbReadOnlyGuardManager', () => { + let manager: UmbReadOnlyGuardManager; + const rulePositive = { unique: '1', message: 'Rule 1', permitted: true }; + const ruleNegative = { unique: '-1', message: 'Rule -1', permitted: false }; + + beforeEach(() => { + const hostElement = new UmbTestControllerHostElement(); + manager = new UmbReadOnlyGuardManager(hostElement); + }); + + describe('Rule based outcomes', () => { + it('is not permitted when there are no rules and fallback defaults to not permitted', (done) => { + manager.permitted + .subscribe((value) => { + expect(value).to.be.false; + done(); + }) + .unsubscribe(); + }); + + it('is permitted by a single positive rule', (done) => { + manager.addRule(rulePositive); + + manager.permitted + .subscribe((value) => { + expect(value).to.be.true; + done(); + }) + .unsubscribe(); + }); + + it('a negative rule wins over a positive rule', (done) => { + manager.addRules([rulePositive, ruleNegative]); + + manager.permitted + .subscribe((value) => { + expect(value).to.be.false; + done(); + }) + .unsubscribe(); + }); + }); + + describe('Fallback', () => { + it('permitted reacts to late fallback updates when no rules match', () => { + const emitted: boolean[] = []; + const subscription = manager.permitted.subscribe((value) => emitted.push(value)); + + manager.fallbackToPermitted(); + manager.fallbackToNotPermitted(); + + subscription.unsubscribe(); + + expect(emitted).to.deep.equal([false, true, false]); + }); + + it('permitted is stable when a matching rule determines the result', () => { + manager.addRule(rulePositive); + const emitted: boolean[] = []; + const subscription = manager.permitted.subscribe((value) => emitted.push(value)); + + manager.fallbackToPermitted(); + manager.fallbackToNotPermitted(); + + subscription.unsubscribe(); + + expect(emitted).to.deep.equal([true]); + }); + + it('getPermitted reflects the current fallback when no rules match', () => { + expect(manager.getPermitted()).to.equal(false); + manager.fallbackToPermitted(); + expect(manager.getPermitted()).to.equal(true); + manager.fallbackToNotPermitted(); + expect(manager.getPermitted()).to.equal(false); + }); + + it('getPermitted is unaffected by the fallback when a rule determines the result', () => { + manager.addRule(rulePositive); + expect(manager.getPermitted()).to.equal(true); + manager.fallbackToNotPermitted(); + expect(manager.getPermitted()).to.equal(true); + }); + }); +}); diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/readonly-guard.manager.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/readonly-guard.manager.ts index dc7f130e9a0f..1b22bb5f9448 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/readonly-guard.manager.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/readonly-guard.manager.ts @@ -1,16 +1,23 @@ +import { mergeObservables } from '@umbraco-cms/backoffice/observable-api'; import { UmbGuardManagerBase, type UmbGuardRule } from './guard.manager.base.js'; // TODO: Check the need for this one. export class UmbReadOnlyGuardManager extends UmbGuardManagerBase { - public readonly permitted = this._rules.asObservablePart((rules) => { - return this.#resolvePermission(rules); - }); + public readonly permitted = mergeObservables( + [ + this._rules.asObservablePart((rules) => { + return this.#resolvePermission(rules); + }), + this._fallback, + ], + ([permitted, fallback]) => permitted ?? fallback, + ); getPermitted(): boolean { - return this.#resolvePermission(this.getRules()); + return this.#resolvePermission(this.getRules()) ?? this._getFallback(); } - #resolvePermission(rules: Array): boolean { + #resolvePermission(rules: Array): boolean | undefined { if (rules.filter((x) => x.permitted === false).length > 0) { return false; } @@ -18,6 +25,6 @@ export class UmbReadOnlyGuardManager extends UmbG return true; } - return this._fallback; + return undefined; } } diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/readonly-variant-guard.manager.test.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/readonly-variant-guard.manager.test.ts index 50a76a7790ce..54e309792329 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/readonly-variant-guard.manager.test.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/readonly-variant-guard.manager.test.ts @@ -1,6 +1,7 @@ import { expect } from '@open-wc/testing'; import { customElement } from '@umbraco-cms/backoffice/external/lit'; import { UmbControllerHostElementMixin } from '@umbraco-cms/backoffice/controller-api'; +import { UmbBasicState } from '@umbraco-cms/backoffice/observable-api'; import { UmbReadOnlyVariantGuardManager } from './readonly-variant-guard.manager.js'; import { UmbVariantId } from '../../variant/variant-id.class.js'; @@ -144,4 +145,73 @@ describe('UmbReadOnlyVariantGuardManager', () => { .unsubscribe(); }); }); + + describe('Fallback', () => { + it('isPermittedForVariant reacts to late fallback updates when no rules match', () => { + const emitted: boolean[] = []; + const subscription = manager + .isPermittedForVariant(englishVariant) + .subscribe((value) => emitted.push(value)); + + manager.fallbackToPermitted(); + manager.fallbackToNotPermitted(); + manager.fallbackToPermitted(); + + subscription.unsubscribe(); + + expect(emitted).to.deep.equal([false, true, false, true]); + }); + + it('isPermittedForVariant is stable when a matching rule determines the result', () => { + manager.addRule(ruleEn); + const emitted: boolean[] = []; + const subscription = manager + .isPermittedForVariant(englishVariant) + .subscribe((value) => emitted.push(value)); + + manager.fallbackToPermitted(); + manager.fallbackToNotPermitted(); + + subscription.unsubscribe(); + + expect(emitted).to.deep.equal([true]); + }); + + it('isPermittedForVariant uses late fallback updates for variants without a matching rule', () => { + manager.addRule(ruleEn); + const emitted: boolean[] = []; + const subscription = manager + .isPermittedForVariant(invariantVariant) + .subscribe((value) => emitted.push(value)); + + manager.fallbackToPermitted(); + + subscription.unsubscribe(); + + expect(emitted).to.deep.equal([false, true]); + }); + + it('getIsPermittedForVariant reflects the current fallback when no rules match', () => { + expect(manager.getIsPermittedForVariant(englishVariant)).to.equal(false); + manager.fallbackToPermitted(); + expect(manager.getIsPermittedForVariant(englishVariant)).to.equal(true); + manager.fallbackToNotPermitted(); + expect(manager.getIsPermittedForVariant(englishVariant)).to.equal(false); + }); + + it('isPermittedForObservableVariant reacts to late fallback updates when no rules match', () => { + const variantIdState = new UmbBasicState(englishVariant); + const emitted: Array = []; + const subscription = manager + .isPermittedForObservableVariant(variantIdState.asObservable()) + .subscribe((value) => emitted.push(value)); + + manager.fallbackToPermitted(); + manager.fallbackToNotPermitted(); + + subscription.unsubscribe(); + + expect(emitted).to.deep.equal([false, true, false]); + }); + }); }); diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/readonly-variant-guard.manager.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/readonly-variant-guard.manager.ts index ed0c85bb0e94..5beaf648ae70 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/readonly-variant-guard.manager.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/utils/guard-manager/readonly-variant-guard.manager.ts @@ -16,7 +16,6 @@ function findRule(rule: UmbVariantGuardRule, variantId: UmbVariantId) { return rule.variantId?.compare(variantId) || rule.variantId === undefined; } -// TODO: Check the need for this one. /** * Read only guard manager for variant rules. * @export @@ -31,23 +30,45 @@ export class UmbReadOnlyVariantGuardManager extends UmbReadOnlyGuardManager { - return this._rules.asObservablePart((states) => { - return this.#resolvePermission(states, variantId); - }); + return mergeObservables( + [ + this._rules.asObservablePart((rules) => { + return this.#resolvePermission(rules, variantId); + }), + this._fallback, + ], + ([permitted, fallback]) => permitted ?? fallback, + ); } /** * @param {Observable} variantId - * @returns {Observable} - Observable that emits true if the variantId is permitted to read, false otherwise + * @returns {Observable} - Observable that emits true if the variantId is permitted to read, false otherwise * @memberof UmbReadOnlyVariantGuardManager */ - isPermittedForObservableVariant(variantId: Observable): Observable { - return mergeObservables([this.rules, variantId], ([states, variantId]) => { + isPermittedForObservableVariant(variantId: Observable): Observable { + return mergeObservables([this.rules, variantId, this._fallback], ([rules, variantId, fallback]) => { if (!variantId) { - // Or should we know about the fallback state here? [NL] - return false; + return undefined; + } + return this.#resolvePermission(rules, variantId) ?? fallback; + }); + } + + /** + * Observe the permission for multiple given variantIds + * @param {Observable} variantIds - Observable emitting the variantIds to evaluate + * @returns {Observable<{ variantId: UmbVariantId; permitted: boolean }[]>} - Observable that emits an array of objects with a permitted boolean and the variantId + * @memberof UmbReadOnlyVariantGuardManager + */ + isPermittedForObservableVariants( + variantIds: Observable, + ): Observable<{ variantId: UmbVariantId; permitted: boolean }[]> { + return mergeObservables([this.rules, variantIds, this._fallback], ([rules, variantIds, fallback]) => { + if (!variantIds || variantIds.length === 0) { + return []; } - return this.#resolvePermission(states, variantId); + return variantIds.map((id) => ({ variantId: id, permitted: this.#resolvePermission(rules, id) ?? fallback })); }); } @@ -58,16 +79,16 @@ export class UmbReadOnlyVariantGuardManager extends UmbReadOnlyGuardManager x.permitted === false).some((rule) => findRule(rule, variantId))) { return false; } if (rules.filter((x) => x.permitted === true).some((rule) => findRule(rule, variantId))) { return true; } - return this._fallback; + return undefined; } } diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/variant/variant-id.class.test.ts b/src/Umbraco.Web.UI.Client/src/packages/core/variant/variant-id.class.test.ts new file mode 100644 index 000000000000..24af08ab2363 --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/packages/core/variant/variant-id.class.test.ts @@ -0,0 +1,364 @@ +import { expect } from '@open-wc/testing'; +import { UMB_INVARIANT_CULTURE, UmbVariantId } from './variant-id.class.js'; + +describe('UmbVariantId', () => { + describe('constructor', () => { + it('defaults culture and segment to null when called with no arguments', () => { + const id = new UmbVariantId(); + expect(id.culture).to.equal(null); + expect(id.segment).to.equal(null); + }); + + it('normalizes the "invariant" culture literal to null', () => { + const id = new UmbVariantId(UMB_INVARIANT_CULTURE, 'xmas'); + expect(id.culture).to.equal(null); + expect(id.segment).to.equal('xmas'); + }); + + it('keeps non-invariant cultures and segments as given', () => { + const id = new UmbVariantId('en', 'xmas'); + expect(id.culture).to.equal('en'); + expect(id.segment).to.equal('xmas'); + }); + + it('coerces undefined inputs to null', () => { + const id = new UmbVariantId(undefined, undefined); + expect(id.culture).to.equal(null); + expect(id.segment).to.equal(null); + }); + }); + + describe('INVARIANT', () => { + it('is the same reference returned by CreateInvariant()', () => { + expect(UmbVariantId.CreateInvariant()).to.equal(UmbVariantId.INVARIANT); + }); + + it('compares true against a freshly created invariant', () => { + expect(UmbVariantId.INVARIANT.compare(UmbVariantId.CreateInvariant())).to.be.true; + }); + + it('has null culture and null segment', () => { + expect(UmbVariantId.INVARIANT.culture).to.equal(null); + expect(UmbVariantId.INVARIANT.segment).to.equal(null); + }); + + it('is frozen', () => { + expect(Object.isFrozen(UmbVariantId.INVARIANT)).to.be.true; + }); + }); + + describe('Create', () => { + it('creates a variant from a full culture/segment object', () => { + const id = UmbVariantId.Create({ culture: 'en', segment: 'xmas' }); + expect(id.culture).to.equal('en'); + expect(id.segment).to.equal('xmas'); + }); + + it('returns a frozen instance', () => { + const id = UmbVariantId.Create({ culture: 'en', segment: null }); + expect(Object.isFrozen(id)).to.be.true; + }); + + it('returns a new instance on each call', () => { + const a = UmbVariantId.Create({ culture: 'en', segment: null }); + const b = UmbVariantId.Create({ culture: 'en', segment: null }); + expect(a).to.not.equal(b); + expect(a.equal(b)).to.be.true; + }); + }); + + describe('CreateFromPartial', () => { + it('fills missing culture and segment with null', () => { + const id = UmbVariantId.CreateFromPartial({}); + expect(id.culture).to.equal(null); + expect(id.segment).to.equal(null); + }); + + it('keeps provided values', () => { + const id = UmbVariantId.CreateFromPartial({ culture: 'da' }); + expect(id.culture).to.equal('da'); + expect(id.segment).to.equal(null); + }); + + it('returns a frozen instance', () => { + const id = UmbVariantId.CreateFromPartial({ culture: 'da' }); + expect(Object.isFrozen(id)).to.be.true; + }); + }); + + describe('FromString', () => { + it('parses a culture-only string', () => { + const id = UmbVariantId.FromString('en'); + expect(id.culture).to.equal('en'); + expect(id.segment).to.equal(null); + }); + + it('parses a culture+segment string', () => { + const id = UmbVariantId.FromString('en_xmas'); + expect(id.culture).to.equal('en'); + expect(id.segment).to.equal('xmas'); + }); + + it('treats the "invariant" culture literal as null culture', () => { + const id = UmbVariantId.FromString('invariant'); + expect(id.culture).to.equal(null); + expect(id.segment).to.equal(null); + }); + + it('keeps segment when culture is the invariant literal', () => { + const id = UmbVariantId.FromString('invariant_xmas'); + expect(id.culture).to.equal(null); + expect(id.segment).to.equal('xmas'); + }); + + it('treats an empty segment after the separator as null', () => { + const id = UmbVariantId.FromString('en_'); + expect(id.culture).to.equal('en'); + expect(id.segment).to.equal(null); + }); + + it('splits only on the first underscore', () => { + const id = UmbVariantId.FromString('en_seg_ment'); + expect(id.culture).to.equal('en'); + expect(id.segment).to.equal('seg_ment'); + }); + + it('returns a frozen instance', () => { + const id = UmbVariantId.FromString('en_xmas'); + expect(Object.isFrozen(id)).to.be.true; + }); + + it('round-trips through toString()', () => { + const cases = ['en', 'en_xmas', 'invariant', 'invariant_xmas']; + cases.forEach((input) => { + expect(UmbVariantId.FromString(input).toString()).to.equal(input); + }); + }); + }); + + describe('compare', () => { + it('returns true for equal culture and segment objects', () => { + const id = new UmbVariantId('en', 'xmas'); + expect(id.compare({ culture: 'en', segment: 'xmas' })).to.be.true; + }); + + it('returns false when culture differs', () => { + const id = new UmbVariantId('en', null); + expect(id.compare({ culture: 'da', segment: null })).to.be.false; + }); + + it('returns false when segment differs', () => { + const id = new UmbVariantId('en', 'xmas'); + expect(id.compare({ culture: 'en', segment: null })).to.be.false; + }); + + it('treats the "invariant" culture literal as null culture when comparing', () => { + const id = new UmbVariantId(null, null); + expect(id.compare({ culture: UMB_INVARIANT_CULTURE, segment: null })).to.be.true; + }); + }); + + describe('equal', () => { + it('returns true for two variants with matching culture and segment', () => { + const a = new UmbVariantId('en', 'xmas'); + const b = new UmbVariantId('en', 'xmas'); + expect(a.equal(b)).to.be.true; + }); + + it('returns false when culture differs', () => { + const a = new UmbVariantId('en', null); + const b = new UmbVariantId('da', null); + expect(a.equal(b)).to.be.false; + }); + + it('returns false when segment differs', () => { + const a = new UmbVariantId('en', 'xmas'); + const b = new UmbVariantId('en', null); + expect(a.equal(b)).to.be.false; + }); + }); + + describe('toString', () => { + it('renders "invariant" when culture is null', () => { + expect(new UmbVariantId(null, null).toString()).to.equal('invariant'); + }); + + it('renders only the culture when segment is null', () => { + expect(new UmbVariantId('en', null).toString()).to.equal('en'); + }); + + it('appends the segment with an underscore when set', () => { + expect(new UmbVariantId('en', 'xmas').toString()).to.equal('en_xmas'); + }); + + it('renders "invariant_segment" for culture-invariant but segmented variants', () => { + expect(new UmbVariantId(null, 'xmas').toString()).to.equal('invariant_xmas'); + }); + }); + + describe('toCultureString', () => { + it('returns the culture when set', () => { + expect(new UmbVariantId('en', null).toCultureString()).to.equal('en'); + }); + + it('returns "invariant" when culture is null', () => { + expect(new UmbVariantId(null, 'xmas').toCultureString()).to.equal(UMB_INVARIANT_CULTURE); + }); + }); + + describe('toSegmentString', () => { + it('returns the segment when set', () => { + expect(new UmbVariantId('en', 'xmas').toSegmentString()).to.equal('xmas'); + }); + + it('returns an empty string when segment is null', () => { + expect(new UmbVariantId('en', null).toSegmentString()).to.equal(''); + }); + }); + + describe('isCultureInvariant', () => { + it('is true when culture is null', () => { + expect(new UmbVariantId(null, 'xmas').isCultureInvariant()).to.be.true; + }); + + it('is false when culture is set', () => { + expect(new UmbVariantId('en', null).isCultureInvariant()).to.be.false; + }); + }); + + describe('isSegmentInvariant', () => { + it('is true when segment is null', () => { + expect(new UmbVariantId('en', null).isSegmentInvariant()).to.be.true; + }); + + it('is false when segment is set', () => { + expect(new UmbVariantId('en', 'xmas').isSegmentInvariant()).to.be.false; + }); + }); + + describe('isInvariant', () => { + it('is true only when both culture and segment are null', () => { + expect(new UmbVariantId(null, null).isInvariant()).to.be.true; + }); + + it('is false when culture is set', () => { + expect(new UmbVariantId('en', null).isInvariant()).to.be.false; + }); + + it('is false when segment is set', () => { + expect(new UmbVariantId(null, 'xmas').isInvariant()).to.be.false; + }); + }); + + describe('clone', () => { + it('returns a new instance with the same culture and segment', () => { + const source = new UmbVariantId('en', 'xmas'); + const copy = source.clone(); + expect(copy).to.not.equal(source); + expect(copy.equal(source)).to.be.true; + }); + }); + + describe('toObject', () => { + it('returns a plain object with culture and segment', () => { + const id = new UmbVariantId('en', 'xmas'); + expect(id.toObject()).to.deep.equal({ culture: 'en', segment: 'xmas' }); + }); + + it('returns null values when invariant', () => { + const id = new UmbVariantId(null, null); + expect(id.toObject()).to.deep.equal({ culture: null, segment: null }); + }); + }); + + describe('toSegmentInvariant', () => { + it('returns a new variant with the segment cleared', () => { + const id = new UmbVariantId('en', 'xmas').toSegmentInvariant(); + expect(id.culture).to.equal('en'); + expect(id.segment).to.equal(null); + }); + + it('returns a frozen instance', () => { + expect(Object.isFrozen(new UmbVariantId('en', 'xmas').toSegmentInvariant())).to.be.true; + }); + }); + + describe('toCultureInvariant', () => { + it('returns a new variant with the culture cleared', () => { + const id = new UmbVariantId('en', 'xmas').toCultureInvariant(); + expect(id.culture).to.equal(null); + expect(id.segment).to.equal('xmas'); + }); + + it('returns a frozen instance', () => { + expect(Object.isFrozen(new UmbVariantId('en', 'xmas').toCultureInvariant())).to.be.true; + }); + }); + + describe('toCulture', () => { + it('returns a new variant with the specified culture', () => { + const id = new UmbVariantId('en', 'xmas').toCulture('da'); + expect(id.culture).to.equal('da'); + expect(id.segment).to.equal('xmas'); + }); + + it('accepts null to clear the culture', () => { + const id = new UmbVariantId('en', 'xmas').toCulture(null); + expect(id.culture).to.equal(null); + expect(id.segment).to.equal('xmas'); + }); + + it('returns a frozen instance', () => { + expect(Object.isFrozen(new UmbVariantId('en', null).toCulture('da'))).to.be.true; + }); + }); + + describe('toSegment', () => { + it('returns a new variant with the specified segment', () => { + const id = new UmbVariantId('en', null).toSegment('xmas'); + expect(id.culture).to.equal('en'); + expect(id.segment).to.equal('xmas'); + }); + + it('accepts null to clear the segment', () => { + const id = new UmbVariantId('en', 'xmas').toSegment(null); + expect(id.culture).to.equal('en'); + expect(id.segment).to.equal(null); + }); + + it('returns a frozen instance', () => { + expect(Object.isFrozen(new UmbVariantId('en', null).toSegment('xmas'))).to.be.true; + }); + }); + + describe('toVariant', () => { + const source = new UmbVariantId('en', 'xmas'); + + it('keeps culture and segment when both vary flags are true', () => { + const id = source.toVariant(true, true); + expect(id.culture).to.equal('en'); + expect(id.segment).to.equal('xmas'); + }); + + it('clears the segment when varyBySegment is false', () => { + const id = source.toVariant(true, false); + expect(id.culture).to.equal('en'); + expect(id.segment).to.equal(null); + }); + + it('clears the culture when varyByCulture is false', () => { + const id = source.toVariant(false, true); + expect(id.culture).to.equal(null); + expect(id.segment).to.equal('xmas'); + }); + + it('returns an invariant when both vary flags are false or omitted', () => { + expect(source.toVariant(false, false).isInvariant()).to.be.true; + expect(source.toVariant().isInvariant()).to.be.true; + }); + + it('returns a frozen instance', () => { + expect(Object.isFrozen(source.toVariant(true, true))).to.be.true; + }); + }); +}); diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/variant/variant-id.class.ts b/src/Umbraco.Web.UI.Client/src/packages/core/variant/variant-id.class.ts index d24fd8dda99f..5a8086c82dc8 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/variant/variant-id.class.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/variant/variant-id.class.ts @@ -7,6 +7,11 @@ export const UMB_INVARIANT_CULTURE = 'invariant'; * The identifier is not specific for ContentType Variants, but is used for many type of identification of a culture and a segment. One case is any property of a ContentType can be resolved into a VariantId depending on their structural settings such as Vary by Culture and Vary by Segmentation. */ export class UmbVariantId { + /** + * A frozen instance of the UmbVariantId class representing the invariant variant, meaning it has no culture and no segment. + */ + public static readonly INVARIANT = Object.freeze(new UmbVariantId(null, null)); + public static Create(variantData: UmbObjectWithVariantProperties): UmbVariantId { return Object.freeze(new UmbVariantId(variantData.culture, variantData.segment)); } @@ -16,7 +21,7 @@ export class UmbVariantId { } public static CreateInvariant(): UmbVariantId { - return Object.freeze(new UmbVariantId(null, null)); + return UmbVariantId.INVARIANT; } public static FromString(str: string): UmbVariantId { diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/workspace/components/workspace-action/common/submit/submit.action.ts b/src/Umbraco.Web.UI.Client/src/packages/core/workspace/components/workspace-action/common/submit/submit.action.ts index 1b92d77816c4..4c8af26ab877 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/workspace/components/workspace-action/common/submit/submit.action.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/workspace/components/workspace-action/common/submit/submit.action.ts @@ -9,7 +9,7 @@ export class UmbSubmitWorkspaceAction< ArgsMetaType extends MetaWorkspaceAction = MetaWorkspaceAction, WorkspaceContextType extends UmbSubmittableWorkspaceContext = UmbSubmittableWorkspaceContext, > extends UmbWorkspaceActionBase { - protected _retrieveWorkspaceContext: Promise; + protected _retrieveWorkspaceContext?: Promise; protected _workspaceContext?: WorkspaceContextType; constructor(host: UmbControllerHost, args: UmbSubmitWorkspaceActionArgs) { @@ -23,13 +23,18 @@ export class UmbSubmitWorkspaceAction< this.#observeUnique(); this._gotWorkspaceContext(); }, - ).asPromise(); + ) + .asPromise() + .catch(() => { + return undefined; + }); } #observeUnique() { this.observe( this._workspaceContext?.unique, (unique) => { + if (!this._workspaceContext) return; // We can't save if we don't have a unique if (unique === undefined) { this.disable(); @@ -48,6 +53,6 @@ export class UmbSubmitWorkspaceAction< override async execute() { await this._retrieveWorkspaceContext; - return await this._workspaceContext!.requestSubmit(); + return await this._workspaceContext?.requestSubmit(); } } diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/workspace/components/workspace-split-view/workspace-split-view-variant-selector.element.ts b/src/Umbraco.Web.UI.Client/src/packages/core/workspace/components/workspace-split-view/workspace-split-view-variant-selector.element.ts index b6716c34ee7e..358e53263140 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/workspace/components/workspace-split-view/workspace-split-view-variant-selector.element.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/workspace/components/workspace-split-view/workspace-split-view-variant-selector.element.ts @@ -12,12 +12,12 @@ import type { UUIInputElement, UUIPopoverContainerElement } from '@umbraco-cms/b import type { DocumentVariantStateModel } from '@umbraco-cms/backoffice/external/backend-api'; import { UMB_HINT_CONTEXT } from '@umbraco-cms/backoffice/hint'; import type { UmbHint, UmbVariantHint } from '@umbraco-cms/backoffice/hint'; -import { observeMultiple } from '@umbraco-cms/backoffice/observable-api'; +import { createObservablePart, observeMultiple } from '@umbraco-cms/backoffice/observable-api'; @customElement('umb-workspace-split-view-variant-selector') export class UmbWorkspaceSplitViewVariantSelectorElement< - VariantOptionModelType extends - UmbEntityVariantOptionModel = UmbEntityVariantOptionModel, + VariantOptionModelType extends UmbEntityVariantOptionModel = + UmbEntityVariantOptionModel, > extends UmbLitElement { @query('#popover') private _popoverElement?: UUIPopoverContainerElement; @@ -85,8 +85,8 @@ export class UmbWorkspaceSplitViewVariantSelectorElement< this.#observeVariants(workspaceContext); this.#observeActiveVariants(workspaceContext); + this.#observeReadOnlyCultures(workspaceContext); this.#observeCurrentVariant(); - this.#observeReadOnlyGuardRules(workspaceContext); this.observe( workspaceContext?.variesBySegment, @@ -141,7 +141,6 @@ export class UmbWorkspaceSplitViewVariantSelectorElement< (variantOptions) => { this._variantOptions = ((variantOptions ?? []) as VariantOptionModelType[]).sort(this._variantSorter); this._cultureVariantOptions = this._variantOptions.filter((variant) => variant.segment === null); - this.#setReadOnlyCultures(workspaceContext); }, '_observeVariantOptions', ); @@ -208,14 +207,6 @@ export class UmbWorkspaceSplitViewVariantSelectorElement< ); } - #observeReadOnlyGuardRules(workspaceContext?: UmbVariantDatasetWorkspaceContext) { - this.observe( - workspaceContext?.readOnlyGuard.rules, - () => this.#setReadOnlyCultures(workspaceContext), - 'umbObserveReadOnlyGuardRules', - ); - } - #handleInput(event: UUIInputEvent) { if (event instanceof UUIInputEvent) { const target = event.composedPath()[0] as UUIInputElement; @@ -262,13 +253,22 @@ export class UmbWorkspaceSplitViewVariantSelectorElement< return this._variantOptions.length > 1; } - #setReadOnlyCultures(workspaceContext?: UmbVariantDatasetWorkspaceContext) { + #observeReadOnlyCultures(workspaceContext?: UmbVariantDatasetWorkspaceContext) { if (workspaceContext) { - this._readOnlyCultures = this._variantOptions - .filter((variant) => workspaceContext.readOnlyGuard.getIsPermittedForVariant(UmbVariantId.Create(variant))) - .map((variant) => variant.culture); + this.observe( + workspaceContext.readOnlyGuard.isPermittedForObservableVariants( + createObservablePart(workspaceContext.variantOptions, (options) => + options.map((option) => UmbVariantId.Create(option)), + ), + ), + (permitted: { variantId: UmbVariantId; permitted: boolean }[]) => { + this._readOnlyCultures = permitted.filter((p) => p.permitted === true).map((p) => p.variantId.culture); + }, + '_observeReadOnlyCultures', + ); } else { this._readOnlyCultures = []; + this.removeUmbControllerByAlias('_observeReadOnlyCultures'); } } diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/workspace/namable/name-write-guard.manager.test.ts b/src/Umbraco.Web.UI.Client/src/packages/core/workspace/namable/name-write-guard.manager.test.ts new file mode 100644 index 000000000000..3c8185437f3c --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/packages/core/workspace/namable/name-write-guard.manager.test.ts @@ -0,0 +1,93 @@ +import { expect } from '@open-wc/testing'; +import { customElement } from '@umbraco-cms/backoffice/external/lit'; +import { UmbControllerHostElementMixin } from '@umbraco-cms/backoffice/controller-api'; +import { UmbNameWriteGuardManager } from './name-write-guard.manager.js'; + +@customElement('test-my-controller-host') +class UmbTestControllerHostElement extends UmbControllerHostElementMixin(HTMLElement) {} + +describe('UmbNameWriteGuardManager', () => { + let manager: UmbNameWriteGuardManager; + const rulePositive = { unique: '1', message: 'Rule 1', permitted: true }; + const ruleNegative = { unique: '-1', message: 'Rule -1', permitted: false }; + + beforeEach(() => { + const hostElement = new UmbTestControllerHostElement(); + manager = new UmbNameWriteGuardManager(hostElement); + }); + + describe('Rule based outcomes', () => { + it('is not permitted when there are no rules and the fallback is not permitted', (done) => { + manager + .isPermittedForName() + .subscribe((value) => { + expect(value).to.be.false; + done(); + }) + .unsubscribe(); + }); + + it('is permitted by a single positive rule', (done) => { + manager.addRule(rulePositive); + + manager + .isPermittedForName() + .subscribe((value) => { + expect(value).to.be.true; + done(); + }) + .unsubscribe(); + }); + + it('is not permitted by a single negative rule', (done) => { + manager.addRule(ruleNegative); + + manager + .isPermittedForName() + .subscribe((value) => { + expect(value).to.be.false; + done(); + }) + .unsubscribe(); + }); + + it('a negative rule wins over a positive rule', (done) => { + manager.addRules([rulePositive, ruleNegative]); + + manager + .isPermittedForName() + .subscribe((value) => { + expect(value).to.be.false; + done(); + }) + .unsubscribe(); + }); + }); + + describe('Fallback', () => { + it('isPermittedForName reacts to late fallback updates when no rules match', () => { + const emitted: boolean[] = []; + const subscription = manager.isPermittedForName().subscribe((value) => emitted.push(value)); + + manager.fallbackToPermitted(); + manager.fallbackToNotPermitted(); + + subscription.unsubscribe(); + + expect(emitted).to.deep.equal([false, true, false]); + }); + + it('isPermittedForName is stable when a matching rule determines the result', () => { + manager.addRule(rulePositive); + const emitted: boolean[] = []; + const subscription = manager.isPermittedForName().subscribe((value) => emitted.push(value)); + + manager.fallbackToPermitted(); + manager.fallbackToNotPermitted(); + + subscription.unsubscribe(); + + expect(emitted).to.deep.equal([true]); + }); + }); +}); diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/workspace/namable/name-write-guard.manager.ts b/src/Umbraco.Web.UI.Client/src/packages/core/workspace/namable/name-write-guard.manager.ts index 18da553e95b0..35b56fd1ea1e 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/workspace/namable/name-write-guard.manager.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/workspace/namable/name-write-guard.manager.ts @@ -1,12 +1,15 @@ import type { Observable } from '@umbraco-cms/backoffice/external/rxjs'; +import { mergeObservables } from '@umbraco-cms/backoffice/observable-api'; import { UmbGuardManagerBase, type UmbGuardRule } from '@umbraco-cms/backoffice/utils'; export class UmbNameWriteGuardManager extends UmbGuardManagerBase { public isPermittedForName(): Observable { - return this._rules.asObservablePart((rules) => this.#resolvePermission(rules)); + return mergeObservables([this.rules, this._fallback], ([rules, fallback]) => { + return this.#resolvePermission(rules) ?? fallback; + }); } - #resolvePermission(rules: Array): boolean { + #resolvePermission(rules: Array): boolean | undefined { if (rules.some((rule) => rule.permitted === false)) { return false; } @@ -15,6 +18,6 @@ export class UmbNameWriteGuardManager extends UmbGuardManagerBase { return true; } - return this._fallback; + return undefined; } } diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/workspace/workspace.element.ts b/src/Umbraco.Web.UI.Client/src/packages/core/workspace/workspace.element.ts index 51821b577202..8dffb0c2e3e2 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/workspace/workspace.element.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/workspace/workspace.element.ts @@ -1,5 +1,5 @@ import type { ManifestWorkspace } from './extensions/types.js'; -import { customElement, property, type PropertyValueMap, state, html } from '@umbraco-cms/backoffice/external/lit'; +import { customElement, property, state, html } from '@umbraco-cms/backoffice/external/lit'; import { UmbLitElement } from '@umbraco-cms/backoffice/lit-element'; import { UmbExtensionsApiInitializer, @@ -31,15 +31,11 @@ export class UmbWorkspaceElement extends UmbLitElement { this.#createController(value); } - protected override firstUpdated(_changedProperties: PropertyValueMap | Map): void { - super.firstUpdated(_changedProperties); - this.setAttribute(UMB_MARK_ATTRIBUTE_NAME, 'workspace'); - } - #createController(entityType: string) { if (this.#extensionsController) { this.#extensionsController.destroy(); } + this.setAttribute(UMB_MARK_ATTRIBUTE_NAME, 'workspace:' + entityType); this.#extensionsController = new UmbExtensionsElementAndApiInitializer( this, umbExtensionsRegistry, diff --git a/src/Umbraco.Web.UI.Client/src/packages/documents/document-types/property-type/manifests.ts b/src/Umbraco.Web.UI.Client/src/packages/documents/document-types/property-type/manifests.ts index e09a5cb90296..d773ce3b73fa 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/documents/document-types/property-type/manifests.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/documents/document-types/property-type/manifests.ts @@ -1,4 +1,5 @@ import { UMB_DOCUMENT_TYPE_PROPERTY_TYPE_ENTITY_TYPE } from './entity.js'; +import { UmbDocumentTypePropertyTypeReferenceResponseManagementApiDataMapping } from './document-type-property-type-reference-response.management-api.mapping.js'; import { UMB_MANAGEMENT_API_DATA_SOURCE_ALIAS } from '@umbraco-cms/backoffice/repository'; export const manifests: Array = [ @@ -6,7 +7,7 @@ export const manifests: Array = [ type: 'dataSourceDataMapping', alias: 'Umb.DataSourceDataMapping.ManagementApi.DocumentTypePropertyTypeReferenceResponse', name: 'Document Type Property Type Reference Response Management Api Data Mapping', - api: () => import('./document-type-property-type-reference-response.management-api.mapping.js'), + api: UmbDocumentTypePropertyTypeReferenceResponseManagementApiDataMapping, forDataSource: UMB_MANAGEMENT_API_DATA_SOURCE_ALIAS, forDataModel: 'DocumentTypePropertyTypeReferenceResponseModel', }, diff --git a/src/Umbraco.Web.UI.Client/src/packages/documents/documents/reference/repository/manifests.ts b/src/Umbraco.Web.UI.Client/src/packages/documents/documents/reference/repository/manifests.ts index d7d09175b248..86718c0031ae 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/documents/documents/reference/repository/manifests.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/documents/documents/reference/repository/manifests.ts @@ -1,4 +1,5 @@ import { UMB_DOCUMENT_REFERENCE_REPOSITORY_ALIAS } from './constants.js'; +import { UmbDocumentReferenceResponseManagementApiDataMapping } from './document-reference-response.management-api.mapping.js'; import { UMB_MANAGEMENT_API_DATA_SOURCE_ALIAS } from '@umbraco-cms/backoffice/repository'; export const manifests: Array = [ @@ -12,7 +13,7 @@ export const manifests: Array = [ type: 'dataSourceDataMapping', alias: 'Umb.DataSourceDataMapping.ManagementApi.DocumentReferenceResponse', name: 'Document Reference Response Management Api Data Mapping', - api: () => import('./document-reference-response.management-api.mapping.js'), + api: UmbDocumentReferenceResponseManagementApiDataMapping, forDataSource: UMB_MANAGEMENT_API_DATA_SOURCE_ALIAS, forDataModel: 'DocumentReferenceResponseModel', }, diff --git a/src/Umbraco.Web.UI.Client/src/packages/documents/documents/user-permissions/document-property-value/conditions/document-property-value-user-permission.condition.ts b/src/Umbraco.Web.UI.Client/src/packages/documents/documents/user-permissions/document-property-value/conditions/document-property-value-user-permission.condition.ts index e3dd9a7bdc27..58b958873f85 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/documents/documents/user-permissions/document-property-value/conditions/document-property-value-user-permission.condition.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/documents/documents/user-permissions/document-property-value/conditions/document-property-value-user-permission.condition.ts @@ -23,6 +23,8 @@ export class UmbDocumentPropertyValueUserPermissionCondition this.observe( context?.currentUser, (currentUser) => { + if (!currentUser) return; + this.#documentPropertyValuePermissions = currentUser?.permissions?.filter(isDocumentPropertyValueUserPermission) || []; this.#fallbackPermissions = currentUser?.fallbackPermissions || []; diff --git a/src/Umbraco.Web.UI.Client/src/packages/documents/documents/user-permissions/document-property-value/data/manifests.ts b/src/Umbraco.Web.UI.Client/src/packages/documents/documents/user-permissions/document-property-value/data/manifests.ts index afde9143747a..327108841e3e 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/documents/documents/user-permissions/document-property-value/data/manifests.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/documents/documents/user-permissions/document-property-value/data/manifests.ts @@ -1,13 +1,15 @@ import { UMB_DOCUMENT_PROPERTY_VALUE_USER_PERMISSION_TYPE } from '../user-permission.js'; import type { UmbExtensionManifestKind } from '@umbraco-cms/backoffice/extension-registry'; import { UMB_MANAGEMENT_API_DATA_SOURCE_ALIAS } from '@umbraco-cms/backoffice/repository'; +import { UmbDocumentPropertyValueUserPermissionFromManagementApiDataMapping } from './from-server.management-api.mapping.js'; +import { UmbDocumentPropertyValueUserPermissionToManagementApiDataMapping } from './to-server.management-api.mapping.js'; export const manifests: Array = [ { type: 'dataSourceDataMapping', alias: 'Umb.DataSourceDataMapping.ManagementApi.To.DocumentPropertyValuePermissionPresentationModel', name: 'Document Property Value Permission To Management Api Data Mapping', - api: () => import('./to-server.management-api.mapping.js'), + api: UmbDocumentPropertyValueUserPermissionToManagementApiDataMapping, forDataSource: UMB_MANAGEMENT_API_DATA_SOURCE_ALIAS, forDataModel: UMB_DOCUMENT_PROPERTY_VALUE_USER_PERMISSION_TYPE, }, @@ -15,7 +17,7 @@ export const manifests: Array = type: 'dataSourceDataMapping', alias: 'Umb.DataSourceDataMapping.ManagementApi.From.DocumentPropertyValuePermissionPresentationModel', name: 'Document Property Value Permission From Management Api Data Mapping', - api: () => import('./from-server.management-api.mapping.js'), + api: UmbDocumentPropertyValueUserPermissionFromManagementApiDataMapping, forDataSource: UMB_MANAGEMENT_API_DATA_SOURCE_ALIAS, forDataModel: 'DocumentPropertyValuePermissionPresentationModel', }, diff --git a/src/Umbraco.Web.UI.Client/src/packages/documents/documents/user-permissions/document-property-value/workspace-context/document-block-property-value-user-permission.workspace-context.ts b/src/Umbraco.Web.UI.Client/src/packages/documents/documents/user-permissions/document-property-value/workspace-context/document-block-property-value-user-permission.workspace-context.ts index e91b5f85074d..44da99dcda3c 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/documents/documents/user-permissions/document-property-value/workspace-context/document-block-property-value-user-permission.workspace-context.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/documents/documents/user-permissions/document-property-value/workspace-context/document-block-property-value-user-permission.workspace-context.ts @@ -14,11 +14,9 @@ export class UmbDocumentBlockPropertyValueUserPermissionWorkspaceContext extends this.#blockWorkspaceContext = context; // We only want to apply the permission logic if the block is in a document - // TODO: revisit this when getContext supports passContextAliasMatches - const contentWorkspaceContext = await this.consumeContext(UMB_CONTENT_WORKSPACE_CONTEXT, () => {}) - .passContextAliasMatches() - .asPromise() - .catch(() => undefined); + const contentWorkspaceContext = await this.getContext(UMB_CONTENT_WORKSPACE_CONTEXT, { + passContextAliasMatches: true, + }).catch(() => undefined); if (contentWorkspaceContext?.getEntityType() === UMB_DOCUMENT_ENTITY_TYPE) { this.#observeDocumentBlockProperties(); diff --git a/src/Umbraco.Web.UI.Client/src/packages/documents/documents/workspace/actions/save.action.ts b/src/Umbraco.Web.UI.Client/src/packages/documents/documents/workspace/actions/save.action.ts index bae7c347a3d3..3f06cd577a34 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/documents/documents/workspace/actions/save.action.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/documents/documents/workspace/actions/save.action.ts @@ -25,7 +25,8 @@ export class UmbDocumentSaveWorkspaceAction async hasAdditionalOptions() { await this._retrieveWorkspaceContext; - const variantOptions = await this.observe(this._workspaceContext!.variantOptions) + if (!this._workspaceContext) return false; + const variantOptions = await this.observe(this._workspaceContext.variantOptions) .asPromise() .catch(() => undefined); const cultureVariantOptions = variantOptions?.filter((option) => option.culture); diff --git a/src/Umbraco.Web.UI.Client/src/packages/documents/umbraco-package.ts b/src/Umbraco.Web.UI.Client/src/packages/documents/umbraco-package.ts index aa0f70813386..7d1dd853351f 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/documents/umbraco-package.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/documents/umbraco-package.ts @@ -1,9 +1,4 @@ +import { manifests } from './manifests.js'; + export const name = 'Umbraco.Core.DocumentManagement'; -export const extensions = [ - { - name: 'Document Management Bundle', - alias: 'Umb.Bundle.DocumentManagement', - type: 'bundle', - js: () => import('./manifests.js'), - }, -]; +export const extensions = manifests; diff --git a/src/Umbraco.Web.UI.Client/src/packages/media/media-types/property-type/manifests.ts b/src/Umbraco.Web.UI.Client/src/packages/media/media-types/property-type/manifests.ts index 4a83872ffe37..6124be6fc4c4 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/media/media-types/property-type/manifests.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/media/media-types/property-type/manifests.ts @@ -1,4 +1,5 @@ import { UMB_MEDIA_TYPE_PROPERTY_TYPE_ENTITY_TYPE } from './entity.js'; +import { UmbMediaTypePropertyTypeReferenceResponseManagementApiDataMapping } from './media-type-property-type-reference-response.management-api.mapping.js'; import { UMB_MANAGEMENT_API_DATA_SOURCE_ALIAS } from '@umbraco-cms/backoffice/repository'; export const manifests: Array = [ @@ -6,7 +7,7 @@ export const manifests: Array = [ type: 'dataSourceDataMapping', alias: 'Umb.DataSourceDataMapping.ManagementApi.MediaTypePropertyTypeReferenceResponse', name: 'Media Type Property Type Reference Response Management Api Data Mapping', - api: () => import('./media-type-property-type-reference-response.management-api.mapping.js'), + api: UmbMediaTypePropertyTypeReferenceResponseManagementApiDataMapping, forDataSource: UMB_MANAGEMENT_API_DATA_SOURCE_ALIAS, forDataModel: 'MediaTypePropertyTypeReferenceResponseModel', }, diff --git a/src/Umbraco.Web.UI.Client/src/packages/media/media/reference/repository/manifests.ts b/src/Umbraco.Web.UI.Client/src/packages/media/media/reference/repository/manifests.ts index 9b64e888df80..f73cb4af0520 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/media/media/reference/repository/manifests.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/media/media/reference/repository/manifests.ts @@ -1,4 +1,5 @@ import { UMB_MEDIA_REFERENCE_REPOSITORY_ALIAS } from './constants.js'; +import { UmbMediaReferenceResponseManagementApiDataMapping } from './media-reference-response.management-api.mapping.js'; import { UMB_MANAGEMENT_API_DATA_SOURCE_ALIAS } from '@umbraco-cms/backoffice/repository'; export const manifests: Array = [ @@ -12,7 +13,7 @@ export const manifests: Array = [ type: 'dataSourceDataMapping', alias: 'Umb.DataSourceDataMapping.ManagementApi.MediaReferenceResponse', name: 'Media Reference Response Management Api Data Mapping', - api: () => import('./media-reference-response.management-api.mapping.js'), + api: UmbMediaReferenceResponseManagementApiDataMapping, forDataSource: UMB_MANAGEMENT_API_DATA_SOURCE_ALIAS, forDataModel: 'MediaReferenceResponseModel', }, diff --git a/src/Umbraco.Web.UI.Client/src/packages/members/member-type/property-type/manifests.ts b/src/Umbraco.Web.UI.Client/src/packages/members/member-type/property-type/manifests.ts index 7b50fb5c2db1..cccbacb19ce6 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/members/member-type/property-type/manifests.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/members/member-type/property-type/manifests.ts @@ -1,4 +1,5 @@ import { UMB_MEMBER_TYPE_PROPERTY_TYPE_ENTITY_TYPE } from './entity.js'; +import { UmbMemberTypePropertyTypeReferenceResponseManagementApiDataMapping } from './member-type-property-type-reference-response.management-api.mapping.js'; import { UMB_MANAGEMENT_API_DATA_SOURCE_ALIAS } from '@umbraco-cms/backoffice/repository'; export const manifests: Array = [ @@ -6,7 +7,7 @@ export const manifests: Array = [ type: 'dataSourceDataMapping', alias: 'Umb.DataSourceDataMapping.ManagementApi.MemberTypePropertyTypeReferenceResponse', name: 'Member Type Property Type Reference Response Management Api Data Mapping', - api: () => import('./member-type-property-type-reference-response.management-api.mapping.js'), + api: UmbMemberTypePropertyTypeReferenceResponseManagementApiDataMapping, forDataSource: UMB_MANAGEMENT_API_DATA_SOURCE_ALIAS, forDataModel: 'MemberTypePropertyTypeReferenceResponseModel', }, diff --git a/src/Umbraco.Web.UI.Client/src/packages/members/member/reference/repository/manifests.ts b/src/Umbraco.Web.UI.Client/src/packages/members/member/reference/repository/manifests.ts index 745fc200fc85..9b05c9fdae01 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/members/member/reference/repository/manifests.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/members/member/reference/repository/manifests.ts @@ -1,4 +1,5 @@ import { UMB_MEMBER_REFERENCE_REPOSITORY_ALIAS } from './constants.js'; +import { UmbMemberReferenceResponseManagementApiDataMapping } from './member-reference-response.management-api.mapping.js'; import { UMB_MANAGEMENT_API_DATA_SOURCE_ALIAS } from '@umbraco-cms/backoffice/repository'; export const manifests: Array = [ @@ -12,7 +13,7 @@ export const manifests: Array = [ type: 'dataSourceDataMapping', alias: 'Umb.DataSourceDataMapping.ManagementApi.MemberReferenceResponse', name: 'Member Reference Response Management Api Data Mapping', - api: () => import('./member-reference-response.management-api.mapping.js'), + api: UmbMemberReferenceResponseManagementApiDataMapping, forDataSource: UMB_MANAGEMENT_API_DATA_SOURCE_ALIAS, forDataModel: 'MemberReferenceResponseModel', }, diff --git a/src/Umbraco.Web.UI.Client/src/packages/rte/components/rte-base.element.ts b/src/Umbraco.Web.UI.Client/src/packages/rte/components/rte-base.element.ts index f395ddf30a68..7be0149093ce 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/rte/components/rte-base.element.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/rte/components/rte-base.element.ts @@ -103,7 +103,18 @@ export abstract class UmbPropertyEditorUiRteElementBase * @default false */ @property({ type: Boolean, reflect: true }) - readonly = false; + public get readonly() { + return this.#readonly; + } + public set readonly(value) { + this.#readonly = value; + if (this.#readonly) { + this.#managerContext.readOnlyState.fallbackToPermitted(); + } else { + this.#managerContext.readOnlyState.fallbackToNotPermitted(); + } + } + #readonly = false; @property({ type: Boolean }) mandatory?: boolean; diff --git a/src/Umbraco.Web.UI.Client/src/packages/ufm/umbraco-package.ts b/src/Umbraco.Web.UI.Client/src/packages/ufm/umbraco-package.ts index bd67768d439e..5ad0eebda555 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/ufm/umbraco-package.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/ufm/umbraco-package.ts @@ -1,3 +1,4 @@ +export const name = 'Umbraco.FlavoredMarkdown'; export const extensions = [ { name: 'Umbraco Flavored Markdown Bundle', diff --git a/src/Umbraco.Web.UI.Client/src/packages/user/current-user/current-user.context.ts b/src/Umbraco.Web.UI.Client/src/packages/user/current-user/current-user.context.ts index d160525d8d49..902a49d697f8 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/user/current-user/current-user.context.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/user/current-user/current-user.context.ts @@ -4,7 +4,6 @@ import { UMB_CURRENT_USER_CONTEXT } from './current-user.context.token.js'; import { UmbContextBase } from '@umbraco-cms/backoffice/class-api'; import type { UmbControllerHost } from '@umbraco-cms/backoffice/controller-api'; import { filter, firstValueFrom } from '@umbraco-cms/backoffice/external/rxjs'; -import { UMB_AUTH_CONTEXT } from '@umbraco-cms/backoffice/auth'; import { UmbObjectState } from '@umbraco-cms/backoffice/observable-api'; import { umbLocalizationRegistry } from '@umbraco-cms/backoffice/localization'; import type { UmbReferenceByUnique } from '@umbraco-cms/backoffice/models'; @@ -30,33 +29,41 @@ export class UmbCurrentUserContext extends UmbContextBase { readonly unique = this.#currentUser.asObservablePart((user) => user?.unique); readonly userName = this.#currentUser.asObservablePart((user) => user?.userName); - #authContext?: typeof UMB_AUTH_CONTEXT.TYPE; #currentUserRepository = new UmbCurrentUserRepository(this); constructor(host: UmbControllerHost) { super(host, UMB_CURRENT_USER_CONTEXT); - this.consumeContext(UMB_AUTH_CONTEXT, (instance) => { - this.#authContext = instance; - this.#observeIsAuthorized(); - }); - this.observe(this.languageIsoCode, (currentLanguageIsoCode) => { if (!currentLanguageIsoCode) return; umbLocalizationRegistry.loadLanguage(currentLanguageIsoCode); }); } + #loadPromise?: Promise; /** - * Loads the current user + * Loads the current user. Concurrent callers share the same in-flight promise, + * so awaiting `load()` always waits for `#currentUser` to be populated. + * @returns {Promise} Resolves once the current user observable has emitted. */ - async load() { + public async load(): Promise { + if (!this.#loadPromise) { + this.#loadPromise = this.#doLoad(); + } + return this.#loadPromise; + } + + async #doLoad(): Promise { const { asObservable } = await this.#currentUserRepository.requestCurrentUser(); if (asObservable) { - await this.observe(asObservable(), (currentUser) => { - this.#currentUser?.setValue(currentUser); - }) + await this.observe( + asObservable(), + (currentUser) => { + this.#currentUser?.setValue(currentUser); + }, + 'observeUser', + ) .asPromise() // Ignore the error, we can assume that the flow was stopped (asPromise failed), but it does not mean that the consumption was not successful. .catch(() => undefined); @@ -217,15 +224,6 @@ export class UmbCurrentUserContext extends UmbContextBase { getUserName(): string | undefined { return this.#currentUser.getValue()?.userName; } - - #observeIsAuthorized() { - if (!this.#authContext) return; - this.observe(this.#authContext.isAuthorized, (isAuthorized) => { - if (isAuthorized) { - this.load(); - } - }); - } } export default UmbCurrentUserContext; diff --git a/src/Umbraco.Web.UI.Login/src/controllers/slim-backoffice-initializer.ts b/src/Umbraco.Web.UI.Login/src/controllers/slim-backoffice-initializer.ts index 3c7c3335ee61..bdb2f3f3e092 100644 --- a/src/Umbraco.Web.UI.Login/src/controllers/slim-backoffice-initializer.ts +++ b/src/Umbraco.Web.UI.Login/src/controllers/slim-backoffice-initializer.ts @@ -49,7 +49,6 @@ export class UmbSlimBackofficeController extends UmbControllerBase { console.error(`Failed to register public extensions for the slim backoffice.`, error); }); - const initializer = new UmbAppEntryPointExtensionInitializer(host, umbExtensionsRegistry); - await firstValueFrom(initializer.loaded); + new UmbAppEntryPointExtensionInitializer(host, umbExtensionsRegistry); } } diff --git a/tests/Umbraco.Tests.AcceptanceTest/lib/helpers/ContentUiHelper.ts b/tests/Umbraco.Tests.AcceptanceTest/lib/helpers/ContentUiHelper.ts index 095b47707c56..468fa944f496 100644 --- a/tests/Umbraco.Tests.AcceptanceTest/lib/helpers/ContentUiHelper.ts +++ b/tests/Umbraco.Tests.AcceptanceTest/lib/helpers/ContentUiHelper.ts @@ -186,6 +186,7 @@ export class ContentUiHelper extends UiBaseLocators { private readonly linkPickerTargetToggle: Locator; private readonly confirmToResetBtn: Locator; private readonly saveModal: Locator; + private readonly blockModal: Locator private readonly expandSegmentBtn: Locator; private readonly saveAndPreviewBtn: Locator; private readonly manualLinkRemoveBtn: Locator; @@ -267,6 +268,7 @@ export class ContentUiHelper extends UiBaseLocators { this.hostnameComboBox = this.hostNameItem.locator('[label="Culture"]').locator('uui-combobox-list-option'); this.saveModal = page.locator('umb-document-save-modal'); this.saveModalBtn = this.saveModal.getByLabel('Save', {exact: true}); + this.blockModal = page.getByTestId('workspace:block'); this.resetFocalPointBtn = page.getByLabel('Reset focal point'); this.addNewHostnameBtn = page.locator('umb-property-layout[label="Hostnames"]').locator('[label="Add new hostname"]'); // List View @@ -1233,8 +1235,8 @@ export class ContentUiHelper extends UiBaseLocators { } async clickCreateInModal(headline: string, options?: {waitForClose?: 'target' | 'any'}) { - const modalLocator = this.page.locator('[headline="' + headline + '"]'); - await this.click(modalLocator.getByLabel('Create')); + const modalLocator = this.blockModal.filter({has: this.page.getByTestId('layout-headline').filter({hasText: headline}),}); + await this.click(modalLocator.getByTestId('workspace-action:Umb.WorkspaceAction.Block.SubmitCreate')); if (options?.waitForClose === 'target') { await this.waitForHidden(modalLocator); @@ -1532,7 +1534,7 @@ export class ContentUiHelper extends UiBaseLocators { } async doesBlockEditorModalContainEditorSize(editorSize: string, elementName: string) { - await this.isVisible(this.backofficeModalContainer.locator(`[size="${editorSize}"]`).locator(`[headline="Add ${elementName}"]`)); + await this.isVisible(this.backofficeModalContainer.locator(`[size="${editorSize}"]`).getByTestId(`block-workspace:Add ${elementName}`)); } async doesBlockEditorModalContainInline(richTextEditorAlias: string, elementName: string) { @@ -1953,7 +1955,7 @@ export class ContentUiHelper extends UiBaseLocators { async isMemberGroupSelected(memberGroupName: string) { return await this.isVisible(this.page.locator('umb-input-member-group uui-ref-node[name="' + memberGroupName + '"]')); } - + async clickRemoveProtectionButton() { await this.click(this.container.getByLabel('Remove protection')); } diff --git a/tests/Umbraco.Tests.AcceptanceTest/lib/helpers/PartialViewUiHelper.ts b/tests/Umbraco.Tests.AcceptanceTest/lib/helpers/PartialViewUiHelper.ts index 90c6df16629f..d94f091cdcb2 100644 --- a/tests/Umbraco.Tests.AcceptanceTest/lib/helpers/PartialViewUiHelper.ts +++ b/tests/Umbraco.Tests.AcceptanceTest/lib/helpers/PartialViewUiHelper.ts @@ -16,7 +16,7 @@ export class PartialViewUiHelper extends UiBaseLocators { this.newEmptyPartialViewBtn = this.partialViewCreateModal.locator('umb-ref-item', {hasText: 'Empty partial view'}); this.newPartialViewFromSnippetBtn = this.partialViewCreateModal.locator('umb-ref-item', {hasText: 'Partial view from snippet'}); this.partialViewTree = page.locator('umb-tree[alias="Umb.Tree.PartialView"]'); - this.partialViewUiLoader = page.locator('uui-loader'); + this.partialViewUiLoader = page.getByTestId('workspace:partial-view').locator('uui-loader'); this.newFolderThreeDots = this.partialViewCreateModal.locator('umb-ref-item', {hasText: 'Folder'}); } diff --git a/tests/Umbraco.Tests.AcceptanceTest/lib/helpers/UiBaseLocators.ts b/tests/Umbraco.Tests.AcceptanceTest/lib/helpers/UiBaseLocators.ts index b44a39f52430..44134408f2f3 100644 --- a/tests/Umbraco.Tests.AcceptanceTest/lib/helpers/UiBaseLocators.ts +++ b/tests/Umbraco.Tests.AcceptanceTest/lib/helpers/UiBaseLocators.ts @@ -597,8 +597,10 @@ export class UiBaseLocators extends BasePage { // Editor this.monacoEditor = page.locator(".monaco-editor"); - // Loader - this.uiLoader = page.locator("uui-loader"); + // Loader (excludes the global app-level loader at #loader) + this.uiLoader = page.locator( + "uui-loader:not([data-mark='app-router-loader'])", + ); // Block this.blockTypeCard = page.locator("uui-card-block-type"); @@ -1590,15 +1592,17 @@ export class UiBaseLocators extends BasePage { } async selectMediaWithName(mediaName: string) { - const mediaLocator = this.mediaCardItems.filter({ hasText: mediaName }); + const mediaLocator = this.mediaCardItems.filter({hasText: mediaName}); await this.waitForVisible(mediaLocator); - await this.click(mediaLocator.locator("#select-checkbox"), { force: true }); + await this.hover(mediaLocator); + await this.click(mediaLocator.locator("#select-checkbox"), {force: true}); } async selectMediaWithTestId(mediaKey: string) { const mediaLocator = this.page.getByTestId("media:" + mediaKey); await this.waitForVisible(mediaLocator); - await this.click(mediaLocator.locator("#select-checkbox"), { force: true }); + await this.hover(mediaLocator); + await this.click(mediaLocator.locator("#select-checkbox"), {force: true}); } async clickMediaPickerModalSubmitButton() { diff --git a/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/BlockGrid/NestedVariantBlockInvariantWrapper.spec.ts b/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/BlockGrid/NestedVariantBlockInvariantWrapper.spec.ts index 947065703603..1b6d1a7d23f9 100644 --- a/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/BlockGrid/NestedVariantBlockInvariantWrapper.spec.ts +++ b/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/BlockGrid/NestedVariantBlockInvariantWrapper.spec.ts @@ -67,8 +67,8 @@ test('variant block values are readable in UI after page reload', {tag: '@smoke' await umbracoUi.content.clickBlockElementWithName(variantBlockName); await umbracoUi.content.enterPropertyValue(variantPropertyName, englishVariantText); await umbracoUi.content.enterPropertyValue(invariantPropertyName, invariantText); - await umbracoUi.content.clickCreateInModal('Add ' + variantBlockName); - await umbracoUi.content.clickCreateInModal('Add ' + wrapperBlockName, {waitForClose: 'target'}); + await umbracoUi.content.clickCreateInModal(variantBlockName); + await umbracoUi.content.clickCreateInModal(wrapperBlockName, {waitForClose: 'target'}); await umbracoUi.content.clickSaveButtonAndWaitForContentToBeUpdated(); await umbracoUi.reloadPage(); diff --git a/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/BlockGrid/SecondLevelBlockProperties.spec.ts b/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/BlockGrid/SecondLevelBlockProperties.spec.ts index 6afc6a5fcfd4..0bf5c75cc9d3 100644 --- a/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/BlockGrid/SecondLevelBlockProperties.spec.ts +++ b/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/BlockGrid/SecondLevelBlockProperties.spec.ts @@ -95,8 +95,8 @@ test('can publish a block grid editor with a block list editor', async ({umbraco await umbracoUi.content.clickAddBlockWithNameButton(textStringElementTypeName); await umbracoUi.content.clickBlockCardWithName(textStringElementTypeName, true); await umbracoUi.content.enterTextstring(textStringValue); - await umbracoUi.content.clickCreateInModal('Add ' + textStringElementTypeName); - await umbracoUi.content.clickCreateInModal('Add ' + blockListElementTypeName); + await umbracoUi.content.clickCreateInModal(textStringElementTypeName); + await umbracoUi.content.clickCreateInModal(blockListElementTypeName); await umbracoUi.content.clickSaveAndPublishButtonAndWaitForContentToBePublished(); // Assert @@ -138,8 +138,8 @@ test('can publish a block grid editor with a block grid editor', async ({umbraco await umbracoUi.content.clickAddBlockWithNameButton(textStringElementTypeName); await umbracoUi.content.clickBlockCardWithName(textStringElementTypeName, true); await umbracoUi.content.enterTextstring(textStringValue); - await umbracoUi.content.clickCreateInModal('Add ' + textStringElementTypeName); - await umbracoUi.content.clickCreateInModal('Add ' + blockGridElementTypeName); + await umbracoUi.content.clickCreateInModal(textStringElementTypeName); + await umbracoUi.content.clickCreateInModal(blockGridElementTypeName); await umbracoUi.content.clickSaveAndPublishButtonAndWaitForContentToBePublished(); // Assert diff --git a/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/BlockList/NestedVariantBlockInvariantWrapper.spec.ts b/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/BlockList/NestedVariantBlockInvariantWrapper.spec.ts index 557633c0c85e..05309c354cda 100644 --- a/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/BlockList/NestedVariantBlockInvariantWrapper.spec.ts +++ b/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/BlockList/NestedVariantBlockInvariantWrapper.spec.ts @@ -67,8 +67,8 @@ test('variant block values are readable in UI after page reload', {tag: '@smoke' await umbracoUi.content.clickBlockElementWithName(variantBlockName); await umbracoUi.content.enterPropertyValue(variantPropertyName, englishVariantText); await umbracoUi.content.enterPropertyValue(invariantPropertyName, invariantText); - await umbracoUi.content.clickCreateInModal('Add ' + variantBlockName); - await umbracoUi.content.clickCreateInModal('Add ' + wrapperBlockName, {waitForClose: 'target'}); + await umbracoUi.content.clickCreateInModal(variantBlockName); + await umbracoUi.content.clickCreateInModal(wrapperBlockName, {waitForClose: 'target'}); await umbracoUi.content.clickSaveButtonAndWaitForContentToBeUpdated(); await umbracoUi.reloadPage(); diff --git a/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/BlockList/SecondLevelBlockProperties.spec.ts b/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/BlockList/SecondLevelBlockProperties.spec.ts index 1275eb8118fe..9be53da51d8c 100644 --- a/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/BlockList/SecondLevelBlockProperties.spec.ts +++ b/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/BlockList/SecondLevelBlockProperties.spec.ts @@ -95,8 +95,8 @@ test('can publish a block list editor with a block grid editor', async ({umbraco await umbracoUi.content.clickAddBlockWithNameButton(textStringElementTypeName); await umbracoUi.content.clickBlockCardWithName(textStringElementTypeName, true); await umbracoUi.content.enterTextstring(textStringValue); - await umbracoUi.content.clickCreateInModal('Add ' + textStringElementTypeName); - await umbracoUi.content.clickCreateInModal('Add ' + blockGridElementTypeName); + await umbracoUi.content.clickCreateInModal(textStringElementTypeName); + await umbracoUi.content.clickCreateInModal(blockGridElementTypeName); await umbracoUi.content.clickSaveAndPublishButtonAndWaitForContentToBePublished(); // Assert @@ -138,8 +138,8 @@ test('can publish a block list editor with a block list editor', async ({umbraco await umbracoUi.content.clickAddBlockWithNameButton(textStringElementTypeName); await umbracoUi.content.clickBlockCardWithName(textStringElementTypeName, true); await umbracoUi.content.enterTextstring(textStringValue); - await umbracoUi.content.clickCreateInModal('Add ' + textStringElementTypeName); - await umbracoUi.content.clickCreateInModal('Add ' + blockListElementTypeName); + await umbracoUi.content.clickCreateInModal(textStringElementTypeName); + await umbracoUi.content.clickCreateInModal(blockListElementTypeName); await umbracoUi.content.clickSaveAndPublishButtonAndWaitForContentToBePublished(); // Assert diff --git a/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/RichTextEditor/NestedVariantBlockInvariantWrapper.spec.ts b/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/RichTextEditor/NestedVariantBlockInvariantWrapper.spec.ts index 7bda227b548d..f35118784f16 100644 --- a/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/RichTextEditor/NestedVariantBlockInvariantWrapper.spec.ts +++ b/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/RichTextEditor/NestedVariantBlockInvariantWrapper.spec.ts @@ -67,8 +67,8 @@ test('variant block values are readable in UI after page reload', {tag: '@smoke' await umbracoUi.content.clickBlockCardWithName(variantBlockName, true); await umbracoUi.content.enterPropertyValue(variantPropertyName, englishVariantText); await umbracoUi.content.enterPropertyValue(invariantPropertyName, invariantText); - await umbracoUi.content.clickCreateInModal('Add ' + variantBlockName); - await umbracoUi.content.clickCreateInModal('Add ' + wrapperBlockName, {waitForClose: 'target'}); + await umbracoUi.content.clickCreateInModal(variantBlockName); + await umbracoUi.content.clickCreateInModal(wrapperBlockName, {waitForClose: 'target'}); await umbracoUi.content.clickSaveButtonAndWaitForContentToBeUpdated(); await umbracoUi.reloadPage(); diff --git a/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/RichTextEditor/SecondLevelBlockProperties.spec.ts b/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/RichTextEditor/SecondLevelBlockProperties.spec.ts index 6a9c86944b94..5655c2afa118 100644 --- a/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/RichTextEditor/SecondLevelBlockProperties.spec.ts +++ b/tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/Content/RichTextEditor/SecondLevelBlockProperties.spec.ts @@ -102,8 +102,8 @@ test('can publish a rich text editor with a block grid editor', async ({umbracoA await umbracoUi.content.clickAddBlockWithNameButton(textStringElementTypeName); await umbracoUi.content.clickBlockCardWithName(textStringElementTypeName, true); await umbracoUi.content.enterTextstring(textStringValue); - await umbracoUi.content.clickCreateInModal('Add ' + textStringElementTypeName); - await umbracoUi.content.clickCreateInModal('Add ' + blockGridElementTypeName); + await umbracoUi.content.clickCreateInModal(textStringElementTypeName); + await umbracoUi.content.clickCreateInModal(blockGridElementTypeName); await umbracoUi.content.clickSaveAndPublishButtonAndWaitForContentToBePublished(); // Assert @@ -144,8 +144,8 @@ test('can publish a rich text editor with a block list editor', async ({umbracoA await umbracoUi.content.clickAddBlockWithNameButton(textStringElementTypeName); await umbracoUi.content.clickBlockCardWithName(textStringElementTypeName, true); await umbracoUi.content.enterTextstring(textStringValue); - await umbracoUi.content.clickCreateInModal('Add ' + textStringElementTypeName); - await umbracoUi.content.clickCreateInModal('Add ' + blockListElementTypeName); + await umbracoUi.content.clickCreateInModal(textStringElementTypeName); + await umbracoUi.content.clickCreateInModal(blockListElementTypeName); await umbracoUi.content.clickSaveAndPublishButtonAndWaitForContentToBePublished(); // Assert