From 204b71d14391fc605fea5127e6a9c8472476b294 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 13 Dec 2021 05:05:36 -0500 Subject: [PATCH] Make it harder to misuse Clusters. (#12914) This ensures that we throw on any attempt to use Clusters without calling init on it, which can end up waiting forever on promises that never resolve. --- .../common/ClusterTestGeneration.js | 12 +++-- .../zap-templates/common/ClustersHelper.js | 54 ++++++++++++------- .../simulated-clusters/SimulatedClusters.js | 18 +++---- .../zap-templates/templates/chip/helper.js | 42 ++++++++------- 4 files changed, 72 insertions(+), 54 deletions(-) diff --git a/src/app/zap-templates/common/ClusterTestGeneration.js b/src/app/zap-templates/common/ClusterTestGeneration.js index cea49e602086f5..b92589239b2a05 100644 --- a/src/app/zap-templates/common/ClusterTestGeneration.js +++ b/src/app/zap-templates/common/ClusterTestGeneration.js @@ -27,7 +27,7 @@ const path = require('path'); const templateUtil = require(zapPath + 'dist/src-electron/generator/template-util.js') const { getClusters, getCommands, getAttributes, isTestOnlyCluster } = require('./simulated-clusters/SimulatedClusters.js'); -const { asBlocks, initClusters } = require('./ClustersHelper.js'); +const { asBlocks, ensureClusters } = require('./ClustersHelper.js'); const kIdentityName = 'identity'; const kClusterName = 'cluster'; @@ -365,7 +365,7 @@ function printErrorAndExit(context, msg) function assertCommandOrAttribute(context) { const clusterName = context.cluster; - return getClusters().then(clusters => { + return getClusters(context).then(clusters => { if (!clusters.find(cluster => cluster.name == clusterName)) { const names = clusters.map(item => item.name); printErrorAndExit(context, 'Missing cluster "' + clusterName + '" in: \n\t* ' + names.join('\n\t* ')); @@ -376,10 +376,10 @@ function assertCommandOrAttribute(context) if (context.isCommand) { filterName = context.command; - items = getCommands(clusterName); + items = getCommands(context, clusterName); } else if (context.isAttribute) { filterName = context.attribute; - items = getAttributes(clusterName); + items = getAttributes(context, clusterName); } else { printErrorAndExit(context, 'Unsupported command type: ', context); } @@ -435,12 +435,14 @@ function chip_tests_pics(options) async function chip_tests(list, options) { - initClusters.call(this); + // Set a global on our items so assertCommandOrAttribute can work. + let global = this.global; const items = Array.isArray(list) ? list : list.split(','); const names = items.map(name => name.trim()); let tests = names.map(item => parse(item)); tests = await Promise.all(tests.map(async function(test) { test.tests = await Promise.all(test.tests.map(async function(item) { + item.global = global; if (item.isCommand) { let command = await assertCommandOrAttribute(item); item.commandObject = command; diff --git a/src/app/zap-templates/common/ClustersHelper.js b/src/app/zap-templates/common/ClustersHelper.js index e1c337e4529b1e..13ac6f7e391b3e 100644 --- a/src/app/zap-templates/common/ClustersHelper.js +++ b/src/app/zap-templates/common/ClustersHelper.js @@ -30,6 +30,16 @@ const ListHelper = require('./ListHelper.js'); const StringHelper = require('./StringHelper.js'); const ChipTypesHelper = require('./ChipTypesHelper.js'); +// Helper for better error reporting. +function ensureState(condition, error) +{ + if (!condition) { + let err = new Error(error); + console.log(`${error}: ` + err.stack); + throw err; + } +} + // // Load Step 1 // @@ -431,13 +441,15 @@ const Clusters = { ready : new Deferred() }; -Clusters.init = function(context, packageId) { +Clusters.init = async function(context) { if (this.ready.running) { return this.ready; } this.ready.running = true; + let packageId = await templateUtil.ensureZclPackageId(context).catch(err => { console.log(err); throw err; }); + const loadTypes = [ loadAtomics.call(context, packageId), loadEnums.call(context, packageId), @@ -468,19 +480,17 @@ Clusters.init = function(context, packageId) { // function asBlocks(promise, options) { - const fn = pkgId => Clusters.init(this, pkgId).then(() => promise.then(data => templateUtil.collectBlocks(data, options, this))); - return templateUtil.ensureZclPackageId(this).then(fn).catch(err => { console.log(err); throw err; }); + return promise.then(data => templateUtil.collectBlocks(data, options, this)) } -function asPromise(promise) -{ - const fn = pkgId => Clusters.init(this, pkgId).then(() => promise); - return templateUtil.ensureZclPackageId(this).then(fn).catch(err => { console.log(err); throw err; }); -} -function initClusters() +function ensureClusters(context) { - const fn = pkgId => Clusters.init(this, pkgId); - templateUtil.ensureZclPackageId(this).then(fn).catch(err => { console.log(err); throw err; }); + // Kick off Clusters initialization. This is async, but that's fine: all the + // getters on Clusters wait on that initialziation to complete. + ensureState(context, "Don't have a context"); + + Clusters.init(context); + return Clusters; } // @@ -488,24 +498,30 @@ function initClusters() // const kResponseFilter = (isResponse, item) => isResponse == item.isResponse; +Clusters.ensureReady = function() +{ + ensureState(this.ready.running); + return this.ready; +} + Clusters.getClusters = function() { - return this.ready.then(() => this._clusters); + return this.ensureReady().then(() => this._clusters); } Clusters.getCommands = function() { - return this.ready.then(() => this._commands.filter(kResponseFilter.bind(null, false))); + return this.ensureReady().then(() => this._commands.filter(kResponseFilter.bind(null, false))); } Clusters.getResponses = function() { - return this.ready.then(() => this._commands.filter(kResponseFilter.bind(null, true))); + return this.ensureReady().then(() => this._commands.filter(kResponseFilter.bind(null, true))); } Clusters.getAttributes = function() { - return this.ready.then(() => this._attributes); + return this.ensureReady().then(() => this._attributes); } // @@ -525,7 +541,7 @@ Clusters.getResponsesByClusterName = function(name) Clusters.getAttributesByClusterName = function(name) { - return this.ready.then(() => { + return this.ensureReady().then(() => { const clusterId = this._clusters.find(kNameFilter.bind(null, name)).id; const filter = attribute => attribute.clusterId == clusterId; return this.getAttributes().then(items => items.filter(filter)); @@ -606,7 +622,5 @@ Clusters.getServerAttributes = function(name) // // Module exports // -exports.Clusters = Clusters; -exports.asBlocks = asBlocks; -exports.asPromise = asPromise; -exports.initClusters = initClusters +exports.asBlocks = asBlocks; +exports.ensureClusters = ensureClusters; diff --git a/src/app/zap-templates/common/simulated-clusters/SimulatedClusters.js b/src/app/zap-templates/common/simulated-clusters/SimulatedClusters.js index ee7af0c2454238..f43d4841343b8d 100644 --- a/src/app/zap-templates/common/simulated-clusters/SimulatedClusters.js +++ b/src/app/zap-templates/common/simulated-clusters/SimulatedClusters.js @@ -15,9 +15,9 @@ * limitations under the License. */ -const { Clusters } = require('../ClustersHelper.js'); -const { DelayCommands } = require('./TestDelayCommands.js'); -const { LogCommands } = require('./TestLogCommands.js'); +const { ensureClusters } = require('../ClustersHelper.js'); +const { DelayCommands } = require('./TestDelayCommands.js'); +const { LogCommands } = require('./TestLogCommands.js'); const SimulatedClusters = [ DelayCommands, @@ -29,21 +29,21 @@ function getSimulatedCluster(clusterName) return SimulatedClusters.find(cluster => cluster.name == clusterName); } -function getClusters() +function getClusters(context) { - return Clusters.getClusters().then(clusters => clusters.concat(SimulatedClusters).flat(1)); + return ensureClusters(context).getClusters().then(clusters => clusters.concat(SimulatedClusters).flat(1)); } -function getCommands(clusterName) +function getCommands(context, clusterName) { const cluster = getSimulatedCluster(clusterName); - return cluster ? Promise.resolve(cluster.commands) : Clusters.getClientCommands(clusterName); + return cluster ? Promise.resolve(cluster.commands) : ensureClusters(context).getClientCommands(clusterName); } -function getAttributes(clusterName) +function getAttributes(context, clusterName) { const cluster = getSimulatedCluster(clusterName); - return cluster ? Promise.resolve(cluster.attributes) : Clusters.getServerAttributes(clusterName); + return cluster ? Promise.resolve(cluster.attributes) : ensureClusters(context).getServerAttributes(clusterName); } function isTestOnlyCluster(clusterName) diff --git a/src/app/zap-templates/templates/chip/helper.js b/src/app/zap-templates/templates/chip/helper.js index 4ad4300907818b..915fe9ebab0d29 100644 --- a/src/app/zap-templates/templates/chip/helper.js +++ b/src/app/zap-templates/templates/chip/helper.js @@ -21,9 +21,9 @@ const templateUtil = require(zapPath + 'generator/template-util.js'); const zclHelper = require(zapPath + 'generator/helper-zcl.js'); const iteratorUtil = require(zapPath + 'util/iterator-util.js'); -const { Clusters, asBlocks, asPromise } = require('../../common/ClustersHelper.js'); -const StringHelper = require('../../common/StringHelper.js'); -const ChipTypesHelper = require('../../common/ChipTypesHelper.js'); +const { asBlocks, ensureClusters } = require('../../common/ClustersHelper.js'); +const StringHelper = require('../../common/StringHelper.js'); +const ChipTypesHelper = require('../../common/ChipTypesHelper.js'); function throwErrorIfUndefined(item, errorMsg, conditions) { @@ -80,13 +80,15 @@ function checkIsChipType(context, name) function getCommands(methodName) { const { clusterName, clusterSide } = checkIsInsideClusterBlock(this, methodName); - return clusterSide == 'client' ? Clusters.getClientCommands(clusterName) : Clusters.getServerCommands(clusterName); + return clusterSide == 'client' ? ensureClusters(this).getClientCommands(clusterName) + : ensureClusters(this).getServerCommands(clusterName); } function getResponses(methodName) { const { clusterName, clusterSide } = checkIsInsideClusterBlock(this, methodName); - return clusterSide == 'client' ? Clusters.getClientResponses(clusterName) : Clusters.getServerResponses(clusterName); + return clusterSide == 'client' ? ensureClusters(this).getClientResponses(clusterName) + : ensureClusters(this).getServerResponses(clusterName); } /** @@ -96,7 +98,7 @@ function getResponses(methodName) */ function chip_server_clusters(options) { - return asBlocks.call(this, Clusters.getServerClusters(), options); + return asBlocks.call(this, ensureClusters(this).getServerClusters(), options); } /** @@ -105,7 +107,7 @@ function chip_server_clusters(options) */ function chip_has_server_clusters(options) { - return asPromise.call(this, Clusters.getServerClusters().then(clusters => !!clusters.length)); + return ensureClusters(this).getServerClusters().then(clusters => !!clusters.length); } /** @@ -115,7 +117,7 @@ function chip_has_server_clusters(options) */ function chip_client_clusters(options) { - return asBlocks.call(this, Clusters.getClientClusters(), options); + return asBlocks.call(this, ensureClusters(this).getClientClusters(), options); } /** @@ -124,7 +126,7 @@ function chip_client_clusters(options) */ function chip_has_client_clusters(options) { - return asPromise.call(this, Clusters.getClientClusters().then(clusters => !!clusters.length)); + return ensureClusters(this).getClientClusters().then(clusters => !!clusters.length); } /** @@ -134,7 +136,7 @@ function chip_has_client_clusters(options) */ function chip_clusters(options) { - return asBlocks.call(this, Clusters.getClusters(), options); + return asBlocks.call(this, ensureClusters(this).getClusters(), options); } /** @@ -143,7 +145,7 @@ function chip_clusters(options) */ function chip_has_clusters(options) { - return asPromise.call(this, Clusters.getClusters().then(clusters => !!clusters.length)); + return ensureClusters(this).getClusters().then(clusters => !!clusters.length); } /** @@ -153,13 +155,13 @@ function chip_has_clusters(options) */ function chip_server_global_responses(options) { - return asBlocks.call(this, getServerGlobalAttributeResponses(), options); + return asBlocks.call(this, getServerGlobalAttributeResponses(this), options); } async function if_in_global_responses(options) { const attribute = this.response.arguments[0]; - const globalResponses = await getServerGlobalAttributeResponses(); + const globalResponses = await getServerGlobalAttributeResponses(this); const responseTypeExists = globalResponses.find( // Some fields of item/attribute here may be undefined. item => item.isList == attribute.isList && item.isStruct == attribute.isStruct && item.chipType == attribute.chipType @@ -175,7 +177,7 @@ async function if_in_global_responses(options) } } -function getServerGlobalAttributeResponses() +function getServerGlobalAttributeResponses(context) { const sorter = (a, b) => a.chipCallback.name.localeCompare(b.chipCallback.name, 'en', { numeric : true }); @@ -195,7 +197,7 @@ function getServerGlobalAttributeResponses() }; const filter = attributes => attributes.reduce(reducer, []).sort(sorter); - return Clusters.getAttributesByClusterSide('server').then(filter); + return ensureClusters(context).getAttributesByClusterSide('server').then(filter); } /** @@ -305,10 +307,10 @@ function chip_cluster_response_arguments(options) function chip_server_has_list_attributes(options) { const { clusterName } = checkIsInsideClusterBlock(this, 'chip_server_has_list_attributes'); - const attributes = Clusters.getServerAttributes(clusterName); + const attributes = ensureClusters(this).getServerAttributes(clusterName); const filter = attribute => attribute.isList; - return asPromise.call(this, attributes.then(items => items.find(filter))); + return attributes.then(items => items.find(filter)); } /** @@ -322,10 +324,10 @@ function chip_server_has_list_attributes(options) function chip_client_has_list_attributes(options) { const { clusterName } = checkIsInsideClusterBlock(this, 'chip_client_has_list_attributes'); - const attributes = Clusters.getClientAttributes(clusterName); + const attributes = ensureClusters(this).getClientAttributes(clusterName); const filter = attribute => attribute.isList; - return asPromise.call(this, attributes.then(items => items.find(filter))); + return attributes.then(items => items.find(filter)); } /** @@ -340,7 +342,7 @@ function chip_client_has_list_attributes(options) function chip_server_cluster_attributes(options) { const { clusterName } = checkIsInsideClusterBlock(this, 'chip_server_cluster_attributes'); - const attributes = Clusters.getServerAttributes(clusterName); + const attributes = ensureClusters(this).getServerAttributes(clusterName); return asBlocks.call(this, attributes, options); }