Skip to content

Commit

Permalink
Make it harder to misuse Clusters. (#12914)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@bzbarsky-apple @pull
bzbarsky-apple authored and pull[bot] committed Oct 27, 2023
1 parent 59f3e86 commit 6749028
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 54 deletions.
12 changes: 7 additions & 5 deletions src/app/zap-templates/common/ClusterTestGeneration.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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* '));
Expand All @@ -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);
}
Expand Down Expand Up @@ -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;
Expand Down
54 changes: 34 additions & 20 deletions src/app/zap-templates/common/ClustersHelper.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
//
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -468,44 +480,48 @@ 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;
}

//
// Helpers: Get all clusters/commands/responses/attributes.
//
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);
}

//
Expand All @@ -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));
Expand Down Expand Up @@ -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;
18 changes: 9 additions & 9 deletions src/app/zap-templates/common/simulated-clusters/SimulatedClusters.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
Expand Down
42 changes: 22 additions & 20 deletions src/app/zap-templates/templates/chip/helper.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand All @@ -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
Expand All @@ -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 });

Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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));
}

/**
Expand All @@ -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));
}

/**
Expand All @@ -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);
}
Expand Down

0 comments on commit 6749028

Please sign in to comment.