Fix Provisional Cluster Warning Bugs (#1523)

* fix bugs in provisional cluster warning

Author: ethanzhouyc

src-electron/db/zap-schema.sql

@@ -2997,12 +2997,10 @@ BEGIN
 END;
 
 /*
- SQL Trigger that adds a warning to the notification table when enabling a provisional cluster. 
- Triggers are needed for both AFTER INSERT and AFTER UPDATE,
- as initial enabling inserts a row into ENDPOINT_TYPE_CLUSTER, while subsequent enabling updates the table.
+SQL UPDATE Trigger that adds a warning to the notification table when re-enabling a provisional cluster.
 */
 CREATE TRIGGER
-  UPDATE_TRIGGER_ENABLE_PROVISIONAL_CLUSTER_WARNING
+  UPDATE_TRIGGER_PROVISIONAL_CLUSTER_WARNING
 AFTER
   UPDATE ON ENDPOINT_TYPE_CLUSTER
 WHEN
@@ -3127,95 +3125,153 @@ BEGIN
     );
 END;
 
+/*
+SQL INSERT Trigger that adds a warning to the notification table when first-time enabling a provisional cluster.
+*/
 CREATE TRIGGER
-  INSERT_TRIGGER_ENABLE_PROVISIONAL_CLUSTER_WARNING
+  INSERT_TRIGGER_PROVISIONAL_CLUSTER_WARNING_PER_CLUSTER
 AFTER
   INSERT ON ENDPOINT_TYPE_CLUSTER
 WHEN
-  (
+  new.ENABLED = 1
+  AND EXISTS (
     SELECT
-      COUNT()
+      1
     FROM
-      ENDPOINT_TYPE_CLUSTER
-    INNER JOIN
       CLUSTER
-    ON
-      ENDPOINT_TYPE_CLUSTER.CLUSTER_REF = CLUSTER.CLUSTER_ID
     WHERE
-      ENDPOINT_TYPE_CLUSTER.CLUSTER_REF = new.CLUSTER_REF
+      CLUSTER.CLUSTER_ID = new.CLUSTER_REF
     AND
-      ENDPOINT_TYPE_CLUSTER.ENDPOINT_TYPE_REF = new.ENDPOINT_TYPE_REF
-    AND
-      ENDPOINT_TYPE_CLUSTER.SIDE = new.SIDE
-    AND
-      new.ENABLED = 1
-    AND
-      CLUSTER.API_MATURITY = "provisional"     
-  ) > 0
+      CLUSTER.API_MATURITY = "provisional"
+  )
+  AND EXISTS (
+    SELECT
+      1
+    FROM
+      SESSION_PARTITION
+    INNER JOIN
+      ENDPOINT_TYPE
+    ON
+      SESSION_PARTITION.SESSION_PARTITION_ID = ENDPOINT_TYPE.SESSION_PARTITION_REF
+    WHERE
+      ENDPOINT_TYPE.ENDPOINT_TYPE_ID = new.ENDPOINT_TYPE_REF
+  )
 BEGIN
   INSERT INTO
-    SESSION_NOTICE(SESSION_REF, NOTICE_TYPE, NOTICE_MESSAGE, NOTICE_SEVERITY, DISPLAY, SEEN)
-  VALUES
-    (
-      (
-        SELECT
-          SESSION_PARTITION.SESSION_REF
-        FROM
-          ENDPOINT_TYPE_CLUSTER
-        INNER JOIN
-          ENDPOINT_TYPE
-        ON
-          ENDPOINT_TYPE.ENDPOINT_TYPE_ID = ENDPOINT_TYPE_CLUSTER.ENDPOINT_TYPE_REF
-        INNER JOIN
-          SESSION_PARTITION
-        ON
-          SESSION_PARTITION.SESSION_PARTITION_ID = ENDPOINT_TYPE.SESSION_PARTITION_REF
-        WHERE
-          ENDPOINT_TYPE_CLUSTER.ENDPOINT_TYPE_CLUSTER_ID = new.ENDPOINT_TYPE_CLUSTER_ID
-      ),
-      "WARNING",
-      "On endpoint "
-      ||
-      (
-        SELECT
-          ENDPOINT.ENDPOINT_IDENTIFIER
-        FROM
-          ENDPOINT
-        INNER JOIN
-          ENDPOINT_TYPE
-        ON
-          ENDPOINT.ENDPOINT_TYPE_REF = ENDPOINT_TYPE.ENDPOINT_TYPE_ID
-        INNER JOIN
-          ENDPOINT_TYPE_CLUSTER
-        ON
-          ENDPOINT_TYPE_CLUSTER.ENDPOINT_TYPE_REF = ENDPOINT_TYPE.ENDPOINT_TYPE_ID
-        WHERE
-          ENDPOINT_TYPE_CLUSTER.ENDPOINT_TYPE_CLUSTER_ID = new.ENDPOINT_TYPE_CLUSTER_ID
-      )
-      ||
-      ", support for cluster: "
-      ||
-      (
-        SELECT
-          CLUSTER.NAME || " " || new.SIDE
-        FROM
-          CLUSTER
-        WHERE
-          CLUSTER.CLUSTER_ID = new.CLUSTER_REF
-      )
-      ||
-      " is provisional.",
-      1,
-      1,
-      0
-    );
+    SESSION_NOTICE (SESSION_REF, NOTICE_TYPE, NOTICE_MESSAGE, NOTICE_SEVERITY, DISPLAY, SEEN)
+  SELECT 
+    SP.SESSION_REF,
+    "WARNING",
+    "On endpoint " || E.ENDPOINT_IDENTIFIER || ", support for cluster: " || C.NAME || " " || new.SIDE || " is provisional.",
+    1,
+    1,
+    0
+  FROM
+    ENDPOINT E
+  INNER JOIN
+    ENDPOINT_TYPE ET
+  ON
+    E.ENDPOINT_TYPE_REF = ET.ENDPOINT_TYPE_ID
+  INNER JOIN
+    SESSION_PARTITION SP
+  ON
+    ET.SESSION_PARTITION_REF = SP.SESSION_PARTITION_ID
+  INNER JOIN
+    CLUSTER C
+  ON
+    C.CLUSTER_ID = new.CLUSTER_REF
+  WHERE
+    ET.ENDPOINT_TYPE_ID = new.ENDPOINT_TYPE_REF;
+END;
+
+/*
+SQL INSERT Trigger that adds warnings for provisional clusters to the notification table when creating an endpoint.
+The triggered is placed on ENDPOINT table since ENDPOINT data is required in the warning and is loaded after ENDPOINT_TYPE and ENDPOINT_TYPE_CLUSTER.
+*/
+CREATE TRIGGER
+  INSERT_TRIGGER_PROVISIONAL_CLUSTER_WARNING_PER_ENDPOINT
+AFTER
+  INSERT ON ENDPOINT
+BEGIN
+  INSERT INTO
+    SESSION_NOTICE (SESSION_REF, NOTICE_TYPE, NOTICE_MESSAGE, NOTICE_SEVERITY, DISPLAY, SEEN)
+  SELECT
+    SP.SESSION_REF,
+    "WARNING",
+    "On endpoint " || E.ENDPOINT_IDENTIFIER || ", support for cluster: " || C.NAME || " " || ETC.SIDE || " is provisional.",
+    1,
+    1,
+    0
+  FROM
+    ENDPOINT E
+  INNER JOIN
+    ENDPOINT_TYPE ET
+  ON
+    E.ENDPOINT_TYPE_REF = ET.ENDPOINT_TYPE_ID
+  INNER JOIN
+    SESSION_PARTITION SP
+  ON
+    SP.SESSION_PARTITION_ID = ET.SESSION_PARTITION_REF
+  INNER JOIN
+    ENDPOINT_TYPE_CLUSTER ETC
+  ON
+    ETC.ENDPOINT_TYPE_REF = ET.ENDPOINT_TYPE_ID
+  INNER JOIN
+    CLUSTER C
+  ON
+    ETC.CLUSTER_REF = C.CLUSTER_ID
+  WHERE
+    E.ENDPOINT_ID = NEW.ENDPOINT_ID
+  AND
+    ETC.ENABLED = 1
+  AND
+    C.API_MATURITY = "provisional";
+END;
+
+/*
+ SQL DELETE Trigger that removes provisional cluster warnings from the notification table when deleting an endpoint.
+*/
+CREATE TRIGGER
+  DELETE_TRIGGER_PROVISIONAL_CLUSTER_WARNING_REMOVAL
+AFTER
+  DELETE ON ENDPOINT
+BEGIN
+  DELETE FROM
+    SESSION_NOTICE
+  WHERE
+    SESSION_REF = (
+      SELECT
+        SESSION_REF
+      FROM
+        SESSION_PARTITION
+      WHERE
+        SESSION_PARTITION.SESSION_PARTITION_ID = (
+          SELECT
+            SESSION_PARTITION_REF
+          FROM
+            ENDPOINT_TYPE
+          WHERE
+            ENDPOINT_TYPE.ENDPOINT_TYPE_ID = old.ENDPOINT_TYPE_REF
+        )
+    )
+    AND
+      NOTICE_TYPE="WARNING"
+    AND
+  NOTICE_MESSAGE LIKE 
+  (
+    "On endpoint " 
+    ||
+    old.ENDPOINT_IDENTIFIER
+    ||
+    ", support for cluster: % is provisional."
+  );
 END;
 
 /*
- SQL Trigger that removes a warning from the notification table when disabling a provisional cluster. 
+ SQL UPDATE Trigger that removes a warning from the notification table when disabling a provisional cluster. 
 */
 CREATE TRIGGER
-  UPDATE_TRIGGER_DISABLE_PROVISIONAL_CLUSTER_WARNING_REMOVAL
+  UPDATE_TRIGGER_PROVISIONAL_CLUSTER_WARNING_REMOVAL
 AFTER
   UPDATE ON ENDPOINT_TYPE_CLUSTER
 WHEN

src-electron/importexport/import-json.js

@@ -1766,9 +1766,7 @@ async function importEndpointTypes(
       }
     }
 
-    await setAndPrintProvisionalClusterWarnings(
-      db,
-      sessionId,
+    await printProvisionalClusterWarnings(
       provisionalClusterWarnings,
       provisionalClusterWarningTitle,
       specMessageIndent,
@@ -2266,36 +2264,21 @@ function generateProvisionalClusterWarnings(clusters, endpointId) {
 }
 
 /**
- * Set provisional cluster warnings in the notification table,
- * and print the concatenated warning message to the console.
+ * Print the concatenated provisional cluster warning message to the console.
  *
- * @param {*} db
- * @param {*} sessionId
  * @param {*} warnings
  * @param {*} provisionalClusterWarningTitle
  * @param {*} specMessageIndent
  * @param {*} dottedLine
  */
-async function setAndPrintProvisionalClusterWarnings(
-  db,
-  sessionId,
+async function printProvisionalClusterWarnings(
   warnings,
   provisionalClusterWarningTitle,
   specMessageIndent,
   dottedLine
 ) {
   let warningMessage = ''
   if (warnings.length > 0) {
-    for (let warning of warnings) {
-      await querySessionNotice.setNotification(
-        db,
-        'WARNING',
-        warning,
-        sessionId,
-        1,
-        0
-      )
-    }
     warningMessage = dottedLine.concat(
       provisionalClusterWarningTitle,
       specMessageIndent,

test/provisional-cluster.test.js

@@ -37,6 +37,7 @@ let sid
 let pkgId
 let eptTypeId
 let clusterId
+let eptId
 
 beforeAll(async () => {
   env.setDevelopmentEnv()
@@ -61,7 +62,7 @@ beforeAll(async () => {
 afterAll(() => dbApi.closeDatabase(db), testUtil.timeout.short())
 
 test(
-  'Initialize a provisional cluster',
+  'Initialize a provisional cluster and test triggers for enabling an endpoint',
   async () => {
     // insert a cluster with provisional apiMaturity
     let provisionalClusterCode = 0x1111
@@ -110,7 +111,21 @@ test(
     )
     expect(eptTypeId).not.toBeNull()
 
-    await queryEndpoint.insertEndpoint(db, sid, 0, eptTypeId, null, null)
+    /* insert an endpoint should trigger a warning for the provisional cluster Scenes enabled
+       according to requirement of On/Off Light Device Type */
+    eptId = await queryEndpoint.insertEndpoint(
+      db,
+      sid,
+      0,
+      eptTypeId,
+      null,
+      null
+    )
+    let notifications = await querySessionNotification.getNotification(db, sid)
+    expect(notifications.length).toBe(1)
+    expect(notifications[0].message).toBe(
+      'On endpoint 0, support for cluster: Scenes server is provisional.'
+    )
   },
   testUtil.timeout.long()
 )
@@ -138,7 +153,8 @@ test(
       false
     )
     let notifications = await querySessionNotification.getNotification(db, sid)
-    expect(notifications.length).toBe(0)
+    expect(listContainsClientWarning(notifications)).toBe(false)
+    expect(listContainsServerWarning(notifications)).toBe(false)
 
     // test update trigger on enable a cluster
     // update provisional client cluster to enabled should trigger a warning
@@ -150,11 +166,12 @@ test(
       true
     )
     notifications = await querySessionNotification.getNotification(db, sid)
-    expect(notifications.length).toBe(1)
     expect(listContainsClientWarning(notifications)).toBe(true)
+    expect(listContainsServerWarning(notifications)).toBe(false)
 
     // test insert trigger
     // insert an enabled provisional server cluster should trigger a warning
+    // 3 notifications should be present: Scenes server, Provisional client, Provisional server
     await queryConfig.insertOrReplaceClusterState(
       db,
       eptTypeId,
@@ -163,7 +180,7 @@ test(
       true
     )
     notifications = await querySessionNotification.getNotification(db, sid)
-    expect(notifications.length).toBe(2)
+    expect(notifications.length).toBe(3)
     expect(listContainsClientWarning(notifications)).toBe(true)
     expect(listContainsServerWarning(notifications)).toBe(true)
 
@@ -177,8 +194,8 @@ test(
       false
     )
     notifications = await querySessionNotification.getNotification(db, sid)
-    expect(notifications.length).toBe(1)
     expect(listContainsClientWarning(notifications)).toBe(true)
+    expect(listContainsServerWarning(notifications)).toBe(false)
 
     // disable the provisional client cluster should remove the client warning
     await queryConfig.insertOrReplaceClusterState(
@@ -189,6 +206,14 @@ test(
       false
     )
     notifications = await querySessionNotification.getNotification(db, sid)
+    expect(listContainsClientWarning(notifications)).toBe(false)
+    expect(listContainsServerWarning(notifications)).toBe(false)
+
+    expect(notifications.length).toBe(1)
+
+    // test delete trigger: delete the endpoint should remove the remaining warning
+    await queryEndpoint.deleteEndpoint(db, eptId)
+    notifications = await querySessionNotification.getNotification(db, sid)
     expect(notifications.length).toBe(0)
   },
   testUtil.timeout.medium()