yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

update to life cycle of extension initializers

Browse Source
This commit is contained in:
Niels Lyngsø
2024-11-05 20:11:53 +01:00
parent 861c91c981
commit 4edf1bad7d
3 changed files with 103 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files

49
src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extension-initializer.controller.test.ts
View File

@@ -59,12 +59,12 @@ class UmbTestConditionAlwaysInvalid extends UmbControllerBase implements UmbExte
describe('UmbBaseExtensionController', () => {
describe('Manifest without conditions', () => {
//let hostElement: UmbControllerHostElement;
let hostElement: UmbControllerHostElement;
let extensionRegistry: UmbExtensionRegistry<ManifestWithDynamicConditions>;
let manifest: ManifestWithDynamicConditions;
beforeEach(async () => {
//hostElement = await fixture(html`<umb-test-controller-host></umb-test-controller-host>`);
hostElement = await fixture(html`<umb-test-controller-host></umb-test-controller-host>`);
extensionRegistry = new UmbExtensionRegistry();
manifest = {
type: 'section',
@@ -74,7 +74,7 @@ describe('UmbBaseExtensionController', () => {
extensionRegistry.register(manifest);
});
/*
it('permits when there is no conditions', (done) => {
const extensionController = new UmbTestExtensionController(
hostElement,
@@ -92,16 +92,15 @@ describe('UmbBaseExtensionController', () => {
},
);
});
*/
});
describe('Manifest with empty conditions', () => {
//let hostElement: UmbControllerHostElement;
let hostElement: UmbControllerHostElement;
let extensionRegistry: UmbExtensionRegistry<ManifestWithDynamicConditions>;
let manifest: ManifestWithDynamicConditions;
beforeEach(async () => {
//hostElement = await fixture(html`<umb-test-controller-host></umb-test-controller-host>`);
hostElement = await fixture(html`<umb-test-controller-host></umb-test-controller-host>`);
extensionRegistry = new UmbExtensionRegistry();
manifest = {
type: 'section',
@@ -113,7 +112,6 @@ describe('UmbBaseExtensionController', () => {
extensionRegistry.register(manifest);
});
/*
it('permits when there is empty conditions', (done) => {
const extensionController = new UmbTestExtensionController(
hostElement,
@@ -124,7 +122,7 @@ describe('UmbBaseExtensionController', () => {
if (extensionController.permitted) {
expect(extensionController?.manifest?.alias).to.eq('Umb.Test.Section.1');
// Also verifying that the promise gets resolved.
// Also verifying that the promise gets resolved. [NL]
extensionController.asPromise().then(() => {
done();
});
@@ -132,7 +130,6 @@ describe('UmbBaseExtensionController', () => {
},
);
});
*/
});
describe('Manifest with valid conditions', () => {
@@ -225,14 +222,14 @@ describe('UmbBaseExtensionController', () => {
if (isPermitted) {
count++;
if (count === 1) {
// First time render, there is no conditions.
// First time render, there is no conditions. [NL]
expect(extensionController.manifest?.weight).to.be.equal(2);
expect(extensionController.manifest?.conditions?.length).to.be.equal(1);
} else if (count === 2) {
// Second time render, there is conditions and weight is 22.
// Second time render, there is conditions and weight is 22. [NL]
expect(extensionController.manifest?.weight).to.be.equal(22);
expect(extensionController.manifest?.conditions?.length).to.be.equal(1);
// Check that the promise has been resolved for the first render to ensure timing is right.
// Check that the promise has been resolved for the first render to ensure timing is right. [NL]
expect(initialPromiseResolved).to.be.true;
done();
extensionController.destroy();
@@ -270,14 +267,14 @@ describe('UmbBaseExtensionController', () => {
if (isPermitted) {
count++;
if (count === 1) {
// First time render, there is no conditions.
// First time render, there is no conditions. [NL]
expect(extensionController.manifest?.weight).to.be.equal(3);
expect(extensionController.manifest?.conditions?.length).to.be.equal(0);
} else if (count === 2) {
// Second time render, there is conditions and weight is 33.
// Second time render, there is conditions and weight is 33. [NL]
expect(extensionController.manifest?.weight).to.be.equal(33);
expect(extensionController.manifest?.conditions?.length).to.be.equal(0);
// Check that the promise has been resolved for the first render to ensure timing is right.
// Check that the promise has been resolved for the first render to ensure timing is right. [NL]
expect(initialPromiseResolved).to.be.true;
done();
extensionController.destroy();
@@ -315,14 +312,14 @@ describe('UmbBaseExtensionController', () => {
if (isPermitted) {
count++;
if (count === 1) {
// First time render, there is no conditions.
// First time render, there is no conditions. [NL]
expect(extensionController.manifest?.weight).to.be.equal(4);
expect(extensionController.manifest?.conditions?.length).to.be.equal(0);
} else if (count === 2) {
// Second time render, there is conditions and weight is 33.
// Second time render, there is conditions and weight is updated. [NL]
expect(extensionController.manifest?.weight).to.be.equal(44);
expect(extensionController.manifest?.conditions?.length).to.be.equal(1);
// Check that the promise has been resolved for the first render to ensure timing is right.
// Check that the promise has been resolved for the first render to ensure timing is right. [NL]
expect(initialPromiseResolved).to.be.true;
done();
extensionController.destroy();
@@ -370,14 +367,14 @@ describe('UmbBaseExtensionController', () => {
if (isPermitted) {
count++;
if (count === 1) {
// First time render, there is no conditions.
// First time render, there is no conditions. [NL]
expect(extensionController.manifest?.weight).to.be.undefined;
expect(extensionController.manifest?.conditions?.length).to.be.equal(0);
} else if (count === 2) {
// Second time render, there is a matching kind and then weight is 123.
// Second time render, there is a matching kind and then weight is 123. [NL]
expect(extensionController.manifest?.weight).to.be.equal(123);
expect(extensionController.manifest?.conditions?.length).to.be.equal(0);
// Check that the promise has been resolved for the first render to ensure timing is right.
// Check that the promise has been resolved for the first render to ensure timing is right. [NL]
expect(initialPromiseResolved).to.be.true;
done();
extensionController.destroy();
@@ -531,7 +528,7 @@ describe('UmbBaseExtensionController', () => {
'Umb.Test.Section.1',
async () => {
count++;
// We want the controller callback to first fire when conditions are initialized.
// We want the controller callback to first fire when conditions are initialized. [NL]
expect(extensionController.manifest?.conditions?.length).to.be.equal(1);
expect(extensionController?.manifest?.alias).to.eq('Umb.Test.Section.1');
if (count === 1) {
@@ -596,27 +593,27 @@ describe('UmbBaseExtensionController', () => {
'Umb.Test.Section.1',
async (isPermitted) => {
count++;
// We want the controller callback to first fire when conditions are initialized.
// We want the controller callback to first fire when conditions are initialized. [NL]
expect(extensionController.manifest?.conditions?.length).to.be.equal(2);
expect(extensionController?.manifest?.alias).to.eq('Umb.Test.Section.1');
if (count === 1) {
expect(isPermitted).to.be.true;
expect(extensionController?.permitted).to.be.true;
// Hack to double check that its two conditions that make up the state:
// Hack to double check that its two conditions that make up the state: [NL]
expect(
extensionController.getUmbControllers((controller) => (controller as any).permitted).length,
).to.equal(2);
} else if (count === 2) {
expect(isPermitted).to.be.false;
expect(extensionController?.permitted).to.be.false;
// Hack to double check that its two conditions that make up the state, in this case its one, cause we already got the callback when one of the conditions changed. meaning in this split second one is still good:
// Hack to double check that its two conditions that make up the state, in this case its one, cause we already got the callback when one of the conditions changed. meaning in this split second one is still good: [NL]
expect(
extensionController.getUmbControllers((controller) => (controller as any).permitted).length,
).to.equal(1);
// Then we are done:
extensionController.destroy(); // End this test.
setTimeout(() => done(), 60); // Lets wait another round of the conditions approve/disapprove, just to see if the destroy stopped the conditions. (60ms, as that should be enough to test that another round does not happen.)
setTimeout(() => done(), 60); // Lets wait another round of the conditions approve/disapprove, just to see if the destroy stopped the conditions. (60ms, as that should be enough to test that another round does not happen.) [NL]
} else if (count === 5) {
expect(false).to.be.true; // This should not be called.
}

83
src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extension-initializer.controller.ts
View File

@@ -8,7 +8,10 @@ import type {
ManifestWithDynamicConditions,
UmbExtensionRegistry,
} from '@umbraco-cms/backoffice/extension-api';
import type { UmbObserverController } from '@umbraco-cms/backoffice/observable-api';
import { jsonStringComparison, type UmbObserverController } from '@umbraco-cms/backoffice/observable-api';
const observeConditionsCtrlAlias = Symbol();
const observeExtensionsCtrlAlias = Symbol();
/**
* This abstract Controller holds the core to manage a single Extension.
@@ -73,8 +76,6 @@ export abstract class UmbBaseExtensionInitializer<
this.#manifestObserver = this.observe(
this.#extensionRegistry.byAlias<ManifestType>(this.#alias),
(extensionManifest) => {
this.#clearPermittedState();
this.#manifest = extensionManifest;
if (extensionManifest) {
if (extensionManifest.overwrites) {
if (typeof extensionManifest.overwrites === 'string') {
@@ -83,13 +84,15 @@ export abstract class UmbBaseExtensionInitializer<
this.#overwrites = extensionManifest.overwrites;
}
}
this.#gotManifest();
this.#gotManifest(extensionManifest);
} else {
this.#manifest = undefined;
this.#clearPermittedState();
this.#overwrites = [];
this.#cleanConditions();
}
},
'_observeExtensionManifest',
observeExtensionsCtrlAlias,
);
}
@@ -107,19 +110,22 @@ export abstract class UmbBaseExtensionInitializer<
if (this.#conditionControllers === undefined || this.#conditionControllers.length === 0) return;
this.#conditionControllers.forEach((controller) => controller.destroy());
this.#conditionControllers = [];
this.removeUmbControllerByAlias('_observeConditions');
this.removeUmbControllerByAlias(observeConditionsCtrlAlias);
}
#gotManifest() {
if (!this.#manifest) return;
const conditionConfigs = this.#manifest.conditions ?? [];
#gotManifest(manifest: ManifestType) {
const conditionConfigs = manifest.conditions ?? [];
const oldManifest = this.#manifest;
this.#manifest = manifest;
// As conditionConfigs might have been configured as something else than an array, then we ignorer them.
/*
// As conditionConfigs might have been configured as something else than an array, then we ignorer them. [NL]
if (conditionConfigs.length === 0) {
this.#cleanConditions();
this.#onConditionsChangedCallback();
return;
}
*/
const conditionAliases = conditionConfigs
.map((condition) => condition.alias)
@@ -130,32 +136,39 @@ export abstract class UmbBaseExtensionInitializer<
this.#conditionControllers = this.#conditionControllers.filter((current) => {
const continueExistence = conditionConfigs.find((config) => config === current.config);
if (!continueExistence) {
// Destroy condition that is no longer needed.
// Destroy condition that is no longer needed. [NL]
current.destroy();
}
return continueExistence;
});
// Check if there was no change in conditions:
// First check if any got removed(old amount equal controllers after clean-up)
// && check if any new is about to be added(old equal new amount):
const noChangeInConditions =
oldAmountOfControllers === this.#conditionControllers.length &&
oldAmountOfControllers === conditionConfigs.length;
if (conditionConfigs.length > 0) {
// Observes the conditions and initialize as they come in.
// Observes the conditions and initialize as they come in. [NL]
this.observe(
this.#extensionRegistry.byTypeAndAliases('condition', conditionAliases),
this.#gotConditions,
'_observeConditions',
observeConditionsCtrlAlias,
);
} else {
this.removeUmbControllerByAlias('_observeConditions');
this.removeUmbControllerByAlias(observeConditionsCtrlAlias);
}
if (noChangeInConditions) {
// There was not change in the amount of conditions, but the manifest was changed, this means this.#isPermitted is set to undefined and this will always fire the callback:
// If permitted we want to fire an update because we got a new manifest. [NL]
if (this.#isPermitted) {
// Check if there was no change in conditions:
// First check if any got removed(old amount equal controllers after clean-up)
// && check if any new is about to be added(old equal new amount): [NL]
// The reason for this is because we will get an update via the code above if there is a change in conditions. But if not we will trigger it here [NL]
const noChangeInConditions =
oldAmountOfControllers === this.#conditionControllers.length &&
oldAmountOfControllers === conditionConfigs.length;
if (noChangeInConditions) {
if (jsonStringComparison(oldManifest, manifest) === false) {
// There was not change in the amount of conditions, but the manifest was changed, this means this.#isPermitted is set to undefined and this will always fire the callback: [NL]
this.#onPermissionChanged?.(this.#isPermitted, this as any);
}
}
} else {
this.#onConditionsChangedCallback();
}
}
@@ -189,9 +202,9 @@ export abstract class UmbBaseExtensionInitializer<
newConditionControllers
.filter((x) => x !== undefined)
.forEach((emerging) => {
// TODO: All of this could use a refactor at one point, when someone is fresh in their mind.
// Niels Notes: Current problem being that we are not aware about what is in the making, so we don't know if we end up creating the same condition multiple times.
// Because it took some time to create the conditions, it maybe have already gotten created by another cycle, so lets test again.
// TODO: All of this could use a refactor at one point, when someone is fresh in their mind. [NL]
// Niels Notes: Current problem being that we are not aware about what is in the making, so we don't know if we end up creating the same condition multiple times. [NL]
// Because it took some time to create the conditions, it maybe have already gotten created by another cycle, so lets test again. [NL]
const existing = this.#conditionControllers.find((existing) => existing.config === emerging?.config);
if (!existing) {
this.#conditionControllers.push(emerging!);
@@ -200,7 +213,7 @@ export abstract class UmbBaseExtensionInitializer<
}
});
// If a change to amount of condition controllers, this will make sure that when new conditions are added, the callback is fired, so the extensions can be re-evaluated, starting out as bad.
// If a change to amount of condition controllers, this will make sure that when new conditions are added, the callback is fired, so the extensions can be re-evaluated, starting out as bad. [NL]
if (oldLength !== this.#conditionControllers.length) {
this.#onConditionsChangedCallback();
}
@@ -230,14 +243,14 @@ export abstract class UmbBaseExtensionInitializer<
#conditionsAreInitialized() {
// Not good if we don't have a manifest.
// Only good if conditions of manifest is equal to the amount of condition controllers (one for each condition).
// 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
);
}
#onConditionsChangedCallback = async () => {
// 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.
// 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)
@@ -250,13 +263,13 @@ export abstract class UmbBaseExtensionInitializer<
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.
// 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();
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.
// 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;
}
@@ -264,11 +277,11 @@ export abstract class UmbBaseExtensionInitializer<
// Clean up:
await this._conditionsAreBad();
// Only continue if we are still negative, otherwise it means that something changed in the mean time.
// 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.
// 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 = false;
}
@@ -296,6 +309,7 @@ export abstract class UmbBaseExtensionInitializer<
}
*/
/*
public override hostDisconnected(): void {
super.hostDisconnected();
this._isConditionsPositive = false;
@@ -305,6 +319,7 @@ export abstract class UmbBaseExtensionInitializer<
this.#onPermissionChanged?.(false, this as unknown as SubClassType);
}
}
*/
#clearPermittedState() {
if (this.#isPermitted === true) {
@@ -318,7 +333,7 @@ export abstract class UmbBaseExtensionInitializer<
if (!this.#extensionRegistry) return;
this.#manifest = undefined;
this.#promiseResolvers = [];
this.#clearPermittedState(); // This fires the callback as not permitted, if it was permitted before.
this.#clearPermittedState(); // This fires the callback as not permitted, if it was permitted before. [NL]
this.#isPermitted = undefined;
this._isConditionsPositive = false;
this.#overwrites = [];

52
src/Umbraco.Web.UI.Client/src/libs/extension-api/controller/base-extensions-initializer.controller.test.ts
View File

@@ -13,6 +13,8 @@ import { customElement, html } from '@umbraco-cms/backoffice/external/lit';
class UmbTestControllerHostElement extends UmbControllerHostElementMixin(HTMLElement) {}
class UmbTestExtensionController extends UmbBaseExtensionInitializer {
testInsidesIsDestroyed?: boolean;
constructor(
host: UmbControllerHost,
extensionRegistry: UmbExtensionRegistry<ManifestWithDynamicConditions>,
@@ -28,7 +30,8 @@ class UmbTestExtensionController extends UmbBaseExtensionInitializer {
}
protected async _conditionsAreBad() {
// Destroy the element/class.
// Destroy the element/class. (or in the case of this test, then we just set a flag) [NL]
this.testInsidesIsDestroyed = true;
}
}
@@ -111,7 +114,7 @@ describe('UmbBaseExtensionsController', () => {
it('exposes both manifests', (done) => {
let count = 0;
const extensionController = new UmbTestExtensionsController(
const extensionsInitController = new UmbTestExtensionsController(
hostElement,
testExtensionRegistry,
'extension-type',
@@ -120,7 +123,7 @@ describe('UmbBaseExtensionsController', () => {
count++;
if (count === 1) {
expect(permitted.length).to.eq(2);
extensionController.destroy();
extensionsInitController.destroy();
} else if (count === 2) {
done();
}
@@ -138,7 +141,7 @@ describe('UmbBaseExtensionsController', () => {
testExtensionRegistry.register(manifestExtra);
let count = 0;
const extensionController = new UmbTestExtensionsController(
const extensionsInitController = new UmbTestExtensionsController(
hostElement,
testExtensionRegistry,
['extension-type', 'extension-type-extra'],
@@ -151,9 +154,9 @@ describe('UmbBaseExtensionsController', () => {
expect(permitted[1].alias).to.eq('Umb.Test.Extension.B');
expect(permitted[2].alias).to.eq('Umb.Test.Extension.Extra');
extensionController.destroy();
extensionsInitController.destroy();
} else if (count === 2) {
// Cause we destroyed there will be a last call to reset permitted controllers:
// Cause we destroyed there will be a last call to reset permitted controllers: [NL]
expect(permitted.length).to.eq(0);
done();
}
@@ -189,7 +192,7 @@ describe('UmbBaseExtensionsController', () => {
it('exposes just one manifests', (done) => {
let count = 0;
const extensionController = new UmbTestExtensionsController(
const extensionsInitController = new UmbTestExtensionsController(
hostElement,
testExtensionRegistry,
'extension-type',
@@ -197,17 +200,17 @@ describe('UmbBaseExtensionsController', () => {
(permitted) => {
count++;
if (count === 1) {
// Still just equal one, as the second one overwrites the first one.
// Still just equal one, as the second one overwrites the first one. [NL]
expect(permitted.length).to.eq(1);
expect(permitted[0].alias).to.eq('Umb.Test.Extension.B');
// lets remove the overwriting extension to see the original coming back.
// lets remove the overwriting extension to see the original coming back. [NL]
testExtensionRegistry.unregister('Umb.Test.Extension.B');
} else if (count === 2) {
expect(permitted.length).to.eq(1);
expect(permitted[0].alias).to.eq('Umb.Test.Extension.A');
done();
extensionController.destroy();
extensionsInitController.destroy();
}
},
);
@@ -255,7 +258,8 @@ describe('UmbBaseExtensionsController', () => {
it('exposes only the overwriting manifest', (done) => {
let count = 0;
const extensionController = new UmbTestExtensionsController(
let lastPermitted: PermittedControllerType<UmbTestExtensionController>[] = [];
const extensionsInitController = new UmbTestExtensionsController(
hostElement,
testExtensionRegistry,
'extension-type',
@@ -263,24 +267,30 @@ describe('UmbBaseExtensionsController', () => {
(permitted) => {
count++;
if (count === 1) {
// Still just equal one, as the second one overwrites the first one.
// Still just equal one, as the second one overwrites the first one. [NL]
expect(permitted.length).to.eq(1);
expect(permitted[0].alias).to.eq('Umb.Test.Extension.B');
// lets remove the overwriting extension to see the original coming back.
// lets remove the overwriting extension to see the original coming back. [NL]
testExtensionRegistry.unregister('Umb.Test.Extension.B');
} else if (count === 2) {
expect(permitted.length).to.eq(1);
expect(permitted[0].alias).to.eq('Umb.Test.Extension.A');
extensionController.destroy();
// CHecks that the controller that got overwritten is destroyed. [NL]
expect(lastPermitted[0].testInsidesIsDestroyed).to.be.true;
// Then continue the test and destroy the initializer. [NL]
extensionsInitController.destroy();
// And then checks that the controller is destroyed. [NL]
expect(permitted[0].testInsidesIsDestroyed).to.be.true;
} else if (count === 3) {
// Expect that destroy will only result in one last callback with no permitted controllers.
// Expect that destroy will only result in one last callback with no permitted controllers. [NL]
expect(permitted.length).to.eq(0);
Promise.resolve().then(() => done()); // This wrap is to enable the test to detect if more callbacks are fired.
Promise.resolve().then(() => done()); // This wrap is to enable the test to detect if more callbacks are fired. [NL]
} else if (count === 4) {
// This should not happen, we do only want one last callback when destroyed.
// This should not happen, we do only want one last callback when destroyed. [NL]
expect(false).to.eq(true);
}
lastPermitted = permitted;
},
);
});
@@ -308,7 +318,7 @@ describe('UmbBaseExtensionsController', () => {
],
};
// Register opposite order, to ensure B is there when A comes around. A fix to be able to test this. Cause a late registration of B would not cause a change that is test able.
// Register opposite order, to ensure B is there when A comes around. A fix to be able to test this. Cause a late registration of B would not cause a change that is test able. [NL]
testExtensionRegistry.register(manifestB);
testExtensionRegistry.register(manifestA);
testExtensionRegistry.register({
@@ -329,7 +339,7 @@ describe('UmbBaseExtensionsController', () => {
it('exposes only the original manifest', (done) => {
let count = 0;
const extensionController = new UmbTestExtensionsController(
const extensionsInitController = new UmbTestExtensionsController(
hostElement,
testExtensionRegistry,
'extension-type',
@@ -338,11 +348,11 @@ describe('UmbBaseExtensionsController', () => {
count++;
if (count === 1) {
// First callback gives just one. We need to make a feature to gather changes to only reply after a computation cycle if we like to avoid this.
// First callback gives just one. We need to make a feature to gather changes to only reply after a computation cycle if we like to avoid this. [NL]
expect(permitted.length).to.eq(1);
expect(permitted[0].alias).to.eq('Umb.Test.Extension.A');
done();
extensionController.destroy();
extensionsInitController.destroy();
}
},
);