From 8aeffd0ec4d405a928750858a46da6c6a03e9bff Mon Sep 17 00:00:00 2001 From: dhchandw Date: Fri, 25 Oct 2024 10:35:00 -0400 Subject: [PATCH] Handling Cluster Extension loading when custom XML is reloaded: * Making sure cluster extension is not duplicated when custom XML is reloaded * Added a MANUFACTURER_CODE_DERIVED column since NULL!=NULL in SQL * Added SIDE to the UNIQUE constraint in attributes * Added logic to catch duplicates during loading before insertion * Added relevant tests --- docs/api.md | 19 ++++ src-electron/db/query-loader.js | 100 +++++++++++++++++- src-electron/db/zap-schema.sql | 12 ++- test/custom-matter-xml.test.js | 8 +- .../custom-cluster/matter-bad-custom.xml | 2 + test/test-util.js | 6 +- zcl-builtin/silabs/demo.xml | 4 +- 7 files changed, 138 insertions(+), 13 deletions(-) diff --git a/docs/api.md b/docs/api.md index 03ae696c24..c21557c392 100644 --- a/docs/api.md +++ b/docs/api.md @@ -3673,6 +3673,7 @@ This module provides queries for ZCL loading * [~commandMap(clusterId, packageId, commands)](#module_DB API_ zcl loading queries..commandMap) ⇒ * [~fieldMap(eventId, packageId, fields)](#module_DB API_ zcl loading queries..fieldMap) ⇒ * [~argMap(cmdId, packageId, args)](#module_DB API_ zcl loading queries..argMap) ⇒ + * [~filterDuplicates(db, packageId, data, keys, elementName)](#module_DB API_ zcl loading queries..filterDuplicates) ⇒ Array * [~insertAttributeAccessData(db, packageId, accessData)](#module_DB API_ zcl loading queries..insertAttributeAccessData) ⇒ * [~insertCommandAccessData(db, packageId, accessData)](#module_DB API_ zcl loading queries..insertCommandAccessData) ⇒ * [~insertEventAccessData(db, packageId, accessData)](#module_DB API_ zcl loading queries..insertEventAccessData) ⇒ @@ -3785,6 +3786,24 @@ Transforms the array of command args in a certain format and returns it. | packageId | \* | | args | \* | + + +### DB API: zcl loading queries~filterDuplicates(db, packageId, data, keys, elementName) ⇒ Array +Filters out duplicates in an array of objects based on specified keys and logs a warning for each duplicate found. +This function is used to filter out duplicates in command, attribute, and event data before inserting into the database. +Treats `null` and `0` as equivalent. + +**Kind**: inner method of [DB API: zcl loading queries](#module_DB API_ zcl loading queries) +**Returns**: Array - - Array of unique objects (duplicates removed). + +| Param | Type | Description | +| --- | --- | --- | +| db | \* | | +| packageId | \* | | +| data | Array | Array of objects. | +| keys | Array | Array of keys to compare for duplicates (e.g., ['code', 'manufacturerCode']). | +| elementName | \* | | + ### DB API: zcl loading queries~insertAttributeAccessData(db, packageId, accessData) ⇒ diff --git a/src-electron/db/query-loader.js b/src-electron/db/query-loader.js index 680ce6c04b..cbdf24b220 100644 --- a/src-electron/db/query-loader.js +++ b/src-electron/db/query-loader.js @@ -138,7 +138,7 @@ INSERT INTO COMMAND_ARG ( // Attribute table needs to be unique based on: // UNIQUE("CLUSTER_REF", "PACKAGE_REF", "CODE", "MANUFACTURER_CODE") const INSERT_ATTRIBUTE_QUERY = ` -INSERT OR REPLACE INTO ATTRIBUTE ( +INSERT INTO ATTRIBUTE ( CLUSTER_REF, PACKAGE_REF, CODE, @@ -360,6 +360,50 @@ function argMap(cmdId, packageId, args) { packageId ]) } +/** + * Filters out duplicates in an array of objects based on specified keys and logs a warning for each duplicate found. + * This function is used to filter out duplicates in command, attribute, and event data before inserting into the database. + * Treats `null` and `0` as equivalent. + * + * @param {*} db + * @param {*} packageId + * @param {Array} data - Array of objects. + * @param {Array} keys - Array of keys to compare for duplicates (e.g., ['code', 'manufacturerCode']). + * @param {*} elementName + * @returns {Array} - Array of unique objects (duplicates removed). + */ +function filterDuplicates(db, packageId, data, keys, elementName) { + let seen = new Map() + let uniqueItems = [] + + data.forEach((item, index) => { + let anyKeysPresent = keys.some((key) => key in item) + + if (!anyKeysPresent) { + // If all keys are missing, treat this item as unique + uniqueItems.push(item) + } else { + let uniqueKey = keys + .map((key) => (item[key] === null || item[key] === 0 ? 0 : item[key])) + .join('|') + + if (seen.has(uniqueKey)) { + // Log a warning with the duplicate information + queryNotification.setNotification( + db, + 'ERROR', + `Duplicate ${elementName} found: ${JSON.stringify(item)}`, + packageId + ) + } else { + seen.set(uniqueKey, true) + uniqueItems.push(item) + } + } + }) + + return uniqueItems +} /** * access data is array of objects, containing id/op/role/modifier. @@ -655,7 +699,7 @@ async function insertClusterExtensions(db, packageId, knownPackages, data) { )}) AND CODE = ?`, data.map((cluster) => [cluster.code]) ) - .then((rows) => { + .then(async (rows) => { let commands = { data: [], args: [], @@ -678,17 +722,38 @@ async function insertClusterExtensions(db, packageId, knownPackages, data) { // NOTE: This code must stay in sync with insertClusters if ('commands' in data[i]) { let cmds = data[i].commands + cmds = filterDuplicates( + db, + packageId, + cmds, + ['code', 'manufacturerCode', 'source'], + 'command' + ) // Removes any duplicates based of db unique constraint and logs package notification (avoids SQL error) commands.data.push(...commandMap(lastId, packageId, cmds)) commands.args.push(...cmds.map((command) => command.args)) commands.access.push(...cmds.map((command) => command.access)) } if ('attributes' in data[i]) { let atts = data[i].attributes + atts = filterDuplicates( + db, + packageId, + atts, + ['code', 'manufacturerCode', 'side'], + 'attribute' + ) // Removes any duplicates based of db unique constraint and logs package notification (avoids SQL error) attributes.data.push(...attributeMap(lastId, packageId, atts)) attributes.access.push(...atts.map((at) => at.access)) } if ('events' in data[i]) { let evs = data[i].events + evs = filterDuplicates( + db, + packageId, + evs, + ['code', 'manufacturerCode'], + 'event' + ) // Removes any duplicates based of db unique constraint and logs package notification (avoids SQL error) events.data.push(...eventMap(lastId, packageId, evs)) events.fields.push(...evs.map((event) => event.fields)) events.access.push(...evs.map((event) => event.access)) @@ -720,7 +785,15 @@ async function insertClusterExtensions(db, packageId, knownPackages, data) { let pCommand = insertCommands(db, packageId, commands) let pAttribute = insertAttributes(db, packageId, attributes) let pEvent = insertEvents(db, packageId, events) - return Promise.all([pCommand, pAttribute, pEvent]) + return Promise.all([pCommand, pAttribute, pEvent]).catch((err) => { + if (err.includes('SQLITE_CONSTRAINT') && err.includes('UNIQUE')) { + env.logDebug( + `CRC match for file with package id ${packageId}, skipping parsing.` + ) + } else { + throw err + } + }) }) } @@ -783,17 +856,38 @@ async function insertClusters(db, packageId, data) { // NOTE: This code must stay in sync with insertClusterExtensions if ('commands' in data[i]) { let cmds = data[i].commands + cmds = filterDuplicates( + db, + packageId, + cmds, + ['code', 'manufacturerCode', 'source'], + 'command' + ) // Removes any duplicates based of db unique constraint and logs package notification (avoids SQL error) commands.data.push(...commandMap(lastId, packageId, cmds)) commands.args.push(...cmds.map((command) => command.args)) commands.access.push(...cmds.map((command) => command.access)) } if ('attributes' in data[i]) { let atts = data[i].attributes + atts = filterDuplicates( + db, + packageId, + atts, + ['code', 'manufacturerCode', 'side'], + 'attribute' + ) // Removes any duplicates based of db unique constraint and logs package notification (avoids SQL error) attributes.data.push(...attributeMap(lastId, packageId, atts)) attributes.access.push(...atts.map((at) => at.access)) } if ('events' in data[i]) { let evs = data[i].events + evs = filterDuplicates( + db, + packageId, + evs, + ['code', 'manufacturerCode'], + 'event' + ) // Removes any duplicates based of db unique constraint and logs package notification (avoids SQL error) events.data.push(...eventMap(lastId, packageId, evs)) events.fields.push(...evs.map((event) => event.fields)) events.access.push(...evs.map((event) => event.access)) diff --git a/src-electron/db/zap-schema.sql b/src-electron/db/zap-schema.sql index 797ecf02bb..b1fb5e8352 100644 --- a/src-electron/db/zap-schema.sql +++ b/src-electron/db/zap-schema.sql @@ -156,10 +156,11 @@ CREATE TABLE IF NOT EXISTS "CLUSTER" ( "INTRODUCED_IN_REF" integer, "REMOVED_IN_REF" integer, "API_MATURITY" text, + "MANUFACTURER_CODE_DERIVED" AS (COALESCE(MANUFACTURER_CODE, 0)), foreign key (INTRODUCED_IN_REF) references SPEC(SPEC_ID) ON DELETE CASCADE ON UPDATE CASCADE, foreign key (REMOVED_IN_REF) references SPEC(SPEC_ID) ON DELETE CASCADE ON UPDATE CASCADE, foreign key (PACKAGE_REF) references PACKAGE(PACKAGE_ID) ON DELETE CASCADE ON UPDATE CASCADE - UNIQUE(PACKAGE_REF, CODE, MANUFACTURER_CODE) + UNIQUE(PACKAGE_REF, CODE, MANUFACTURER_CODE_DERIVED) ); /* COMMAND table contains commands contained inside a cluster. @@ -183,12 +184,13 @@ CREATE TABLE IF NOT EXISTS "COMMAND" ( "RESPONSE_REF" integer, "IS_DEFAULT_RESPONSE_ENABLED" integer, "IS_LARGE_MESSAGE" integer, + "MANUFACTURER_CODE_DERIVED" AS (COALESCE(MANUFACTURER_CODE, 0)), foreign key (INTRODUCED_IN_REF) references SPEC(SPEC_ID) ON DELETE CASCADE ON UPDATE CASCADE, foreign key (REMOVED_IN_REF) references SPEC(SPEC_ID) ON DELETE CASCADE ON UPDATE CASCADE, foreign key (CLUSTER_REF) references CLUSTER(CLUSTER_ID) ON DELETE CASCADE ON UPDATE CASCADE, foreign key (PACKAGE_REF) references PACKAGE(PACKAGE_ID) ON DELETE CASCADE ON UPDATE CASCADE, foreign key (RESPONSE_REF) references COMMAND(COMMAND_ID) ON DELETE CASCADE ON UPDATE CASCADE - UNIQUE(CLUSTER_REF, PACKAGE_REF, CODE, MANUFACTURER_CODE, SOURCE) + UNIQUE(CLUSTER_REF, PACKAGE_REF, CODE, MANUFACTURER_CODE_DERIVED, SOURCE) ); /* COMMAND_ARG table contains arguments for a command. @@ -233,11 +235,12 @@ CREATE TABLE IF NOT EXISTS "EVENT" ( "PRIORITY" text, "INTRODUCED_IN_REF" integer, "REMOVED_IN_REF" integer, + "MANUFACTURER_CODE_DERIVED" AS (COALESCE(MANUFACTURER_CODE, 0)), foreign key (CLUSTER_REF) references CLUSTER(CLUSTER_ID) ON DELETE CASCADE ON UPDATE CASCADE, foreign key (PACKAGE_REF) references PACKAGE(PACKAGE_ID) ON DELETE CASCADE ON UPDATE CASCADE, foreign key (INTRODUCED_IN_REF) references SPEC(SPEC_ID) ON DELETE CASCADE ON UPDATE CASCADE, foreign key (REMOVED_IN_REF) references SPEC(SPEC_ID) ON DELETE CASCADE ON UPDATE CASCADE - UNIQUE(CLUSTER_REF, PACKAGE_REF, CODE, MANUFACTURER_CODE) + UNIQUE(CLUSTER_REF, PACKAGE_REF, CODE, MANUFACTURER_CODE_DERIVED) ); /* EVENT_FIELD table contains events for a given cluster. @@ -294,11 +297,12 @@ CREATE TABLE IF NOT EXISTS "ATTRIBUTE" ( "API_MATURITY" text, "IS_CHANGE_OMITTED" integer, "PERSISTENCE" text, + "MANUFACTURER_CODE_DERIVED" AS (COALESCE(MANUFACTURER_CODE, 0)), foreign key (INTRODUCED_IN_REF) references SPEC(SPEC_ID) ON DELETE CASCADE ON UPDATE CASCADE, foreign key (REMOVED_IN_REF) references SPEC(SPEC_ID) ON DELETE CASCADE ON UPDATE CASCADE, foreign key (CLUSTER_REF) references CLUSTER(CLUSTER_ID) ON DELETE CASCADE ON UPDATE CASCADE, foreign key (PACKAGE_REF) references PACKAGE(PACKAGE_ID) ON DELETE CASCADE ON UPDATE CASCADE - UNIQUE("CLUSTER_REF", "PACKAGE_REF", "CODE", "MANUFACTURER_CODE") + UNIQUE("CLUSTER_REF", "PACKAGE_REF", "CODE", "MANUFACTURER_CODE_DERIVED", "SIDE") ); /* diff --git a/test/custom-matter-xml.test.js b/test/custom-matter-xml.test.js index c8e540bcce..69ba1d3955 100644 --- a/test/custom-matter-xml.test.js +++ b/test/custom-matter-xml.test.js @@ -448,7 +448,13 @@ test( ) expect( packageNotif.some((notif) => notif.message.includes('type contradiction')) - ).toBeTruthy() // checks if the correct warning is thrown + ).toBeTruthy() // checks if the correct type contradiction warning is thrown + + expect( + packageNotif.some((notif) => + notif.message.includes('Duplicate attribute found') + ) + ).toBeTruthy() // checks if the correct duplicate attribute error is thrown let sessionNotif = await querySessionNotification.getNotification(db, sid) expect( diff --git a/test/resource/custom-cluster/matter-bad-custom.xml b/test/resource/custom-cluster/matter-bad-custom.xml index 9c05d06d58..dda988cf8b 100644 --- a/test/resource/custom-cluster/matter-bad-custom.xml +++ b/test/resource/custom-cluster/matter-bad-custom.xml @@ -1,3 +1,4 @@ + @@ -11,6 +12,7 @@ // intentional undefined type errors Sample Mfg Specific Attribute 6 + Sample Mfg Specific Attribute 6 Duplicate Sample Mfg Specific Attribute 8 Client command that turns the device on with a transition given diff --git a/test/test-util.js b/test/test-util.js index d6157c3deb..7aff4c5e92 100644 --- a/test/test-util.js +++ b/test/test-util.js @@ -170,12 +170,12 @@ exports.testMatterCustomZap = exports.totalClusterCount = 111 exports.totalDomainCount = 20 -exports.totalCommandArgsCount = 1786 -exports.totalCommandCount = 632 +exports.totalCommandArgsCount = 1785 +exports.totalCommandCount = 631 exports.totalEventFieldCount = 3 exports.totalEventCount = 1 exports.totalAttributeCount = 3438 -exports.totalClusterCommandCount = 609 +exports.totalClusterCommandCount = 608 exports.totalServerAttributeCount = 2962 exports.totalSpecCount = 24 exports.totalEnumCount = 211 diff --git a/zcl-builtin/silabs/demo.xml b/zcl-builtin/silabs/demo.xml index a983b5fe55..272661e718 100644 --- a/zcl-builtin/silabs/demo.xml +++ b/zcl-builtin/silabs/demo.xml @@ -15,10 +15,10 @@ Send a hello command to the server - + Example test event