Handling Cluster Extension loading when custom XML is reloaded (#1471)

* 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

JIRA ZAPP-1486

Author: dhchandw

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) ⇒ <code>Array</code>
     * [~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 | <code>\*</code> | 
 | args | <code>\*</code> | 
 
+<a name="module_DB API_ zcl loading queries..filterDuplicates"></a>
+
+### DB API: zcl loading queries~filterDuplicates(db, packageId, data, keys, elementName) ⇒ <code>Array</code>
+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 [<code>DB API: zcl loading queries</code>](#module_DB API_ zcl loading queries)  
+**Returns**: <code>Array</code> - - Array of unique objects (duplicates removed).  
+
+| Param | Type | Description |
+| --- | --- | --- |
+| db | <code>\*</code> |  |
+| packageId | <code>\*</code> |  |
+| data | <code>Array</code> | Array of objects. |
+| keys | <code>Array</code> | Array of keys to compare for duplicates (e.g., ['code', 'manufacturerCode']). |
+| elementName | <code>\*</code> |  |
+
 <a name="module_DB API_ zcl loading queries..insertAttributeAccessData"></a>
 
 ### DB API: zcl loading queries~insertAttributeAccessData(db, packageId, accessData) ⇒

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.
@@ -647,81 +691,101 @@ async function insertGlobals(db, packageId, data) {
  * @returns Promise of cluster extension insertion.
  */
 async function insertClusterExtensions(db, packageId, knownPackages, data) {
-  return dbApi
-    .dbMultiSelect(
-      db,
-      `SELECT CLUSTER_ID FROM CLUSTER WHERE PACKAGE_REF IN (${dbApi.toInClause(
-        knownPackages
-      )}) AND CODE = ?`,
-      data.map((cluster) => [cluster.code])
-    )
-    .then((rows) => {
-      let commands = {
-        data: [],
-        args: [],
-        access: []
-      }
-      let events = {
-        data: [],
-        fields: [],
-        access: []
+  let rows = await dbApi.dbMultiSelect(
+    db,
+    `SELECT CLUSTER_ID FROM CLUSTER WHERE PACKAGE_REF IN (${dbApi.toInClause(
+      knownPackages
+    )}) AND CODE = ?`,
+    data.map((cluster) => [cluster.code])
+  )
+
+  let commands = {
+    data: [],
+    args: [],
+    access: []
+  }
+  let events = {
+    data: [],
+    fields: [],
+    access: []
+  }
+  let attributes = {
+    data: [],
+    access: []
+  }
+
+  let i, lastId
+  for (i = 0; i < rows.length; i++) {
+    let row = rows[i]
+    if (row != null) {
+      lastId = row.CLUSTER_ID
+      // NOTE: This code must stay in sync with insertClusters
+      if ('commands' in data[i]) {
+        let cmds = filterDuplicates(
+          db,
+          packageId,
+          data[i].commands,
+          ['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))
       }
-      let attributes = {
-        data: [],
-        access: []
+      if ('attributes' in data[i]) {
+        let atts = filterDuplicates(
+          db,
+          packageId,
+          data[i].attributes,
+          ['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))
       }
-      let i, lastId
-      for (i = 0; i < rows.length; i++) {
-        let row = rows[i]
-        if (row != null) {
-          lastId = row.CLUSTER_ID
-          // NOTE: This code must stay in sync with insertClusters
-          if ('commands' in data[i]) {
-            let cmds = data[i].commands
-            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
-            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
-            events.data.push(...eventMap(lastId, packageId, evs))
-            events.fields.push(...evs.map((event) => event.fields))
-            events.access.push(...evs.map((event) => event.access))
-          }
-        } else {
-          // DANGER: We got here because we are adding a cluster extension for a
-          // cluster which is not defined. For eg:
-          // <clusterExtension code="0x0000">
-          // <attribute side="server" code="0x4000" define="SW_BUILD_ID"
-          //            type="CHAR_STRING" length="16" writable="false"
-          //            default="" optional="true"
-          //            introducedIn="zll-1.0-11-0037-10">sw build id</attribute>
-          // </clusterExtension>
-          // If a cluster with code 0x0000 does not exist then we run into this
-          // issue.
-          let message = `Attempting to insert cluster extension for a cluster which does not
-          exist. Check clusterExtension meta data in xml file.
-          Cluster Code: ${data[i].code}`
-          env.logWarning(message)
-          queryNotification.setNotification(
-            db,
-            'WARNING',
-            message,
-            packageId,
-            2
-          )
-        }
+      if ('events' in data[i]) {
+        let evs = filterDuplicates(
+          db,
+          packageId,
+          data[i].events,
+          ['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))
       }
-      let pCommand = insertCommands(db, packageId, commands)
-      let pAttribute = insertAttributes(db, packageId, attributes)
-      let pEvent = insertEvents(db, packageId, events)
-      return Promise.all([pCommand, pAttribute, pEvent])
-    })
+    } else {
+      // DANGER: We got here because we are adding a cluster extension for a
+      // cluster which is not defined. For eg:
+      // <clusterExtension code="0x0000">
+      // <attribute side="server" code="0x4000" define="SW_BUILD_ID"
+      //            type="CHAR_STRING" length="16" writable="false"
+      //            default="" optional="true"
+      //            introducedIn="zll-1.0-11-0037-10">sw build id</attribute>
+      // </clusterExtension>
+      // If a cluster with code 0x0000 does not exist then we run into this
+      // issue.
+      let message = `Attempting to insert cluster extension for a cluster which does not
+      exist. Check clusterExtension meta data in xml file.
+      Cluster Code: ${data[i].code}`
+      env.logWarning(message)
+      queryNotification.setNotification(db, 'WARNING', message, packageId, 2)
+    }
+  }
+
+  let pCommand = insertCommands(db, packageId, commands)
+  let pAttribute = insertAttributes(db, packageId, attributes)
+  let pEvent = insertEvents(db, packageId, events)
+  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 +847,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))

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")
 );
 
 /*

test/custom-matter-xml.test.js

@@ -448,7 +448,19 @@ 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
+
+    expect(
+      packageNotif.some((notif) =>
+        notif.message.includes('Duplicate command found')
+      )
+    ).toBeTruthy() // checks if the correct duplicate command error is thrown
 
     let sessionNotif = await querySessionNotification.getNotification(db, sid)
     expect(