From 0bdb966095272ffe103962a2c4484507e2c8dbb4 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Mon, 19 Apr 2021 19:07:28 -0230 Subject: [PATCH] Optional AllowedEvents/AllowedActions (#446) The restricted controller messenger now allows omitting allowedActions and allowedEvents. Technically it already allowed this, but it required setting the type of the allowed actions/events to an empty string. Now the type is `never` if no actions/events are used. --- src/BaseControllerV2.test.ts | 20 ++++---------- src/BaseControllerV2.ts | 8 +++--- src/ControllerMessenger.test.ts | 31 --------------------- src/ControllerMessenger.ts | 48 ++++++++++++++++++--------------- 4 files changed, 36 insertions(+), 71 deletions(-) diff --git a/src/BaseControllerV2.test.ts b/src/BaseControllerV2.test.ts index ec3439e1cc2..16a71c7dfe9 100644 --- a/src/BaseControllerV2.test.ts +++ b/src/BaseControllerV2.test.ts @@ -52,7 +52,6 @@ describe('BaseController', () => { >(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'CountController', - allowedActions: [], allowedEvents: ['CountController:stateChange'], }); const controller = new CountController({ @@ -72,7 +71,6 @@ describe('BaseController', () => { >(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'CountController', - allowedActions: [], allowedEvents: ['CountController:stateChange'], }); const controller = new CountController({ @@ -92,7 +90,6 @@ describe('BaseController', () => { >(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'CountController', - allowedActions: [], allowedEvents: ['CountController:stateChange'], }); const controller = new CountController({ @@ -116,7 +113,6 @@ describe('BaseController', () => { >(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'CountController', - allowedActions: [], allowedEvents: ['CountController:stateChange'], }); const controller = new CountController({ @@ -140,7 +136,6 @@ describe('BaseController', () => { >(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'CountController', - allowedActions: [], allowedEvents: ['CountController:stateChange'], }); const controller = new CountController({ @@ -164,7 +159,6 @@ describe('BaseController', () => { >(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'CountController', - allowedActions: [], allowedEvents: ['CountController:stateChange'], }); const controller = new CountController({ @@ -191,7 +185,6 @@ describe('BaseController', () => { >(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'CountController', - allowedActions: [], allowedEvents: ['CountController:stateChange'], }); const controller = new CountController({ @@ -228,7 +221,6 @@ describe('BaseController', () => { >(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'CountController', - allowedActions: [], allowedEvents: ['CountController:stateChange'], }); const controller = new CountController({ @@ -260,7 +252,6 @@ describe('BaseController', () => { >(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'CountController', - allowedActions: [], allowedEvents: ['CountController:stateChange'], }); const controller = new CountController({ @@ -287,7 +278,6 @@ describe('BaseController', () => { >(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'CountController', - allowedActions: [], allowedEvents: ['CountController:stateChange'], }); const controller = new CountController({ @@ -315,7 +305,6 @@ describe('BaseController', () => { >(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'CountController', - allowedActions: [], allowedEvents: ['CountController:stateChange'], }); new CountController({ @@ -340,7 +329,6 @@ describe('BaseController', () => { >(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'CountController', - allowedActions: [], allowedEvents: ['CountController:stateChange'], }); const controller = new CountController({ @@ -792,10 +780,12 @@ describe('getPersistentState', () => { VisitorControllerAction | VisitorOverflowControllerAction, VisitorControllerEvent | VisitorOverflowControllerEvent >(); - const visitorControllerMessenger = controllerMessenger.getRestricted({ + const visitorControllerMessenger = controllerMessenger.getRestricted< + 'VisitorController', + never, + never + >({ name: 'VisitorController', - allowedActions: [], - allowedEvents: [], }); const visitorController = new VisitorController( visitorControllerMessenger, diff --git a/src/BaseControllerV2.ts b/src/BaseControllerV2.ts index d803163a75d..7f532ac80b9 100644 --- a/src/BaseControllerV2.ts +++ b/src/BaseControllerV2.ts @@ -125,8 +125,8 @@ export class BaseController< N, any, StateChangeEvent, - string, - string + string | never, + string | never >; private name: N; @@ -153,8 +153,8 @@ export class BaseController< N, any, StateChangeEvent, - string, - string + string | never, + string | never >; metadata: StateMetadata; name: N; diff --git a/src/ControllerMessenger.test.ts b/src/ControllerMessenger.test.ts index b29f64755c3..4cd67d88e59 100644 --- a/src/ControllerMessenger.test.ts +++ b/src/ControllerMessenger.test.ts @@ -313,7 +313,6 @@ describe('RestrictedControllerMessenger', () => { const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'CountController', allowedActions: ['CountController:count'], - allowedEvents: [], }); let count = 0; @@ -339,7 +338,6 @@ describe('RestrictedControllerMessenger', () => { const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'MessageController', allowedActions: ['MessageController:reset', 'MessageController:concat'], - allowedEvents: [], }); let message = ''; @@ -374,7 +372,6 @@ describe('RestrictedControllerMessenger', () => { const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'CountController', allowedActions: ['CountController:increment'], - allowedEvents: [], }); let count = 0; @@ -398,7 +395,6 @@ describe('RestrictedControllerMessenger', () => { const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'MessageController', allowedActions: ['MessageController:message'], - allowedEvents: [], }); const messages: Record = {}; @@ -426,7 +422,6 @@ describe('RestrictedControllerMessenger', () => { const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'MathController', allowedActions: ['MathController:add'], - allowedEvents: [], }); restrictedControllerMessenger.registerActionHandler( @@ -450,7 +445,6 @@ describe('RestrictedControllerMessenger', () => { const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'PingController', allowedActions: ['PingController:ping'], - allowedEvents: [], }); restrictedControllerMessenger.registerActionHandler( @@ -472,7 +466,6 @@ describe('RestrictedControllerMessenger', () => { const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'PingController', allowedActions: ['PingController:ping'], - allowedEvents: [], }); expect(() => { @@ -486,7 +479,6 @@ describe('RestrictedControllerMessenger', () => { const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'PingController', allowedActions: ['PingController:ping'], - allowedEvents: [], }); expect(() => { @@ -519,7 +511,6 @@ describe('RestrictedControllerMessenger', () => { const controllerMessenger = new ControllerMessenger(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'MessageController', - allowedActions: [], allowedEvents: ['MessageController:message'], }); @@ -541,7 +532,6 @@ describe('RestrictedControllerMessenger', () => { const controllerMessenger = new ControllerMessenger(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'MessageController', - allowedActions: [], allowedEvents: ['MessageController:message', 'MessageController:ping'], }); @@ -570,7 +560,6 @@ describe('RestrictedControllerMessenger', () => { const controllerMessenger = new ControllerMessenger(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'PingController', - allowedActions: [], allowedEvents: ['PingController:ping'], }); @@ -590,7 +579,6 @@ describe('RestrictedControllerMessenger', () => { const controllerMessenger = new ControllerMessenger(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'MessageController', - allowedActions: [], allowedEvents: ['MessageController:message'], }); @@ -617,7 +605,6 @@ describe('RestrictedControllerMessenger', () => { const controllerMessenger = new ControllerMessenger(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'MessageController', - allowedActions: [], allowedEvents: ['MessageController:message'], }); @@ -644,7 +631,6 @@ describe('RestrictedControllerMessenger', () => { const controllerMessenger = new ControllerMessenger(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'MessageController', - allowedActions: [], allowedEvents: ['MessageController:message'], }); @@ -674,7 +660,6 @@ describe('RestrictedControllerMessenger', () => { const controllerMessenger = new ControllerMessenger(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'MessageController', - allowedActions: [], allowedEvents: ['MessageController:message'], }); @@ -700,7 +685,6 @@ describe('RestrictedControllerMessenger', () => { const controllerMessenger = new ControllerMessenger(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'MessageController', - allowedActions: [], allowedEvents: ['MessageController:message'], }); @@ -721,7 +705,6 @@ describe('RestrictedControllerMessenger', () => { const controllerMessenger = new ControllerMessenger(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'MessageController', - allowedActions: [], allowedEvents: ['MessageController:message'], }); @@ -748,7 +731,6 @@ describe('RestrictedControllerMessenger', () => { const controllerMessenger = new ControllerMessenger(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'MessageController', - allowedActions: [], allowedEvents: ['MessageController:message'], }); @@ -773,7 +755,6 @@ describe('RestrictedControllerMessenger', () => { const controllerMessenger = new ControllerMessenger(); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'MessageController', - allowedActions: [], allowedEvents: ['MessageController:message'], }); @@ -793,14 +774,11 @@ describe('RestrictedControllerMessenger', () => { const externalRestrictedControllerMessenger = controllerMessenger.getRestricted( { name: 'CountController', - allowedActions: [], - allowedEvents: [], }, ); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'OtherController', allowedActions: ['CountController:count'], - allowedEvents: [], }); let count = 0; @@ -824,13 +802,10 @@ describe('RestrictedControllerMessenger', () => { const externalRestrictedControllerMessenger = controllerMessenger.getRestricted( { name: 'MessageController', - allowedActions: [], - allowedEvents: [], }, ); const restrictedControllerMessenger = controllerMessenger.getRestricted({ name: 'OtherController', - allowedActions: [], allowedEvents: ['MessageController:message'], }); @@ -867,12 +842,9 @@ describe('RestrictedControllerMessenger', () => { const messageControllerMessenger = controllerMessenger.getRestricted({ name: 'MessageController', allowedActions: ['MessageController:reset', 'CountController:count'], - allowedEvents: [], }); const countControllerMessenger = controllerMessenger.getRestricted({ name: 'CountController', - allowedActions: [], - allowedEvents: [], }); let count = 0; @@ -916,13 +888,10 @@ describe('RestrictedControllerMessenger', () => { const messageControllerMessenger = controllerMessenger.getRestricted({ name: 'MessageController', - allowedActions: [], allowedEvents: ['MessageController:ping', 'CountController:update'], }); const countControllerMessenger = controllerMessenger.getRestricted({ name: 'CountController', - allowedActions: [], - allowedEvents: [], }); let pings = 0; diff --git a/src/ControllerMessenger.ts b/src/ControllerMessenger.ts index 041d419f9a0..570d2ee1a29 100644 --- a/src/ControllerMessenger.ts +++ b/src/ControllerMessenger.ts @@ -45,16 +45,16 @@ export class RestrictedControllerMessenger< N extends string, Action extends ActionConstraint, Event extends EventConstraint, - AllowedAction extends string, - AllowedEvent extends string + AllowedAction extends string | never, + AllowedEvent extends string | never > { private controllerMessenger: ControllerMessenger; private controllerName: N; - private allowedActions: AllowedAction[]; + private allowedActions: AllowedAction[] | null; - private allowedEvents: AllowedEvent[]; + private allowedEvents: AllowedEvent[] | null; /** * Constructs a restricted controller messenger @@ -82,13 +82,13 @@ export class RestrictedControllerMessenger< }: { controllerMessenger: ControllerMessenger; name: N; - allowedActions: AllowedAction[]; - allowedEvents: AllowedEvent[]; + allowedActions?: AllowedAction[]; + allowedEvents?: AllowedEvent[]; }) { this.controllerMessenger = controllerMessenger; this.controllerName = name; - this.allowedActions = allowedActions; - this.allowedEvents = allowedEvents; + this.allowedActions = allowedActions || null; + this.allowedEvents = allowedEvents || null; } /** @@ -148,12 +148,14 @@ export class RestrictedControllerMessenger< * registered action handler. * @throws Will throw when no handler has been registered for the given type. */ - call( + call( action: T, ...params: ExtractActionParameters ): ExtractActionResponse { - /* istanbul ignore if */ // Branch unreachable with valid types - if (!this.allowedActions.includes(action)) { + /* istanbul ignore next */ // Branches unreachable with valid types + if (this.allowedActions === null) { + throw new Error('No actions allowed'); + } else if (!this.allowedActions.includes(action)) { throw new Error(`Action missing from allow list: ${action}`); } return this.controllerMessenger.call(action, ...params); @@ -194,12 +196,14 @@ export class RestrictedControllerMessenger< * @param handler - The event handler. The type of the parameters for this event handler must * match the type of the payload for this event type. */ - subscribe( + subscribe( event: E, handler: ExtractEventHandler, ) { - /* istanbul ignore if */ // Branch unreachable with valid types - if (!this.allowedEvents.includes(event)) { + /* istanbul ignore next */ // Branches unreachable with valid types + if (this.allowedEvents === null) { + throw new Error('No events allowed'); + } else if (!this.allowedEvents.includes(event)) { throw new Error(`Event missing from allow list: ${event}`); } return this.controllerMessenger.subscribe(event, handler); @@ -216,12 +220,14 @@ export class RestrictedControllerMessenger< * @param handler - The event handler to unregister. * @throws Will throw when the given event handler is not registered for this event. */ - unsubscribe( + unsubscribe( event: E, handler: ExtractEventHandler, ) { - /* istanbul ignore if */ // Branch unreachable with valid types - if (!this.allowedEvents.includes(event)) { + /* istanbul ignore next */ // Branches unreachable with valid types + if (this.allowedEvents === null) { + throw new Error('No events allowed'); + } else if (!this.allowedEvents.includes(event)) { throw new Error(`Event missing from allow list: ${event}`); } return this.controllerMessenger.unsubscribe(event, handler); @@ -434,16 +440,16 @@ export class ControllerMessenger< */ getRestricted< N extends string, - AllowedAction extends string, - AllowedEvent extends string + AllowedAction extends string | never, + AllowedEvent extends string | never >({ name, allowedActions, allowedEvents, }: { name: N; - allowedActions: Extract[] | []; - allowedEvents: Extract[] | []; + allowedActions?: Extract[]; + allowedEvents?: Extract[]; }) { return new RestrictedControllerMessenger< N,