address review feedback

project-chip · Oct 19, 2023 · 23ee451 · 23ee451
1 parent 1b899a9
commit 23ee451
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 28 deletions.
diff --git a/packages/matter-node.js/src/storage/StorageBackendDisk.ts b/packages/matter-node.js/src/storage/StorageBackendDisk.ts
@@ -28,42 +28,43 @@ export class StorageBackendDisk implements Storage {
         this.localStorage.clear();
     }
 
-    buildStorageKey(contexts: string[], key: string): string {
+    getContextBaseKey(contexts: string[], allowEmptyContext = false): string {
         const contextKey = contexts.join(".");
         if (
-            !key.length ||
-            !contextKey.length ||
+            (!contextKey.length && !allowEmptyContext) ||
             contextKey.includes("..") ||
             contextKey.startsWith(".") ||
             contextKey.endsWith(".")
         )
-            throw new StorageError("Context must not be an empty string!");
+            throw new StorageError("Context must not be an empty and not contain dots.");
+        return contextKey;
+    }
+
+    buildStorageKey(contexts: string[], key: string): string {
+        if (!key.length) {
+            throw new StorageError("Key must not be an empty string.");
+        }
+        const contextKey = this.getContextBaseKey(contexts);
         return `${contextKey}.${key}`;
     }
 
     get<T extends SupportedStorageTypes>(contexts: string[], key: string): T | undefined {
-        if (!contexts.length || !key.length) throw new StorageError("Context and key must not be empty strings!");
         const value = this.localStorage.getItem(this.buildStorageKey(contexts, key));
         if (value === null) return undefined;
         return fromJson(value) as T;
     }
 
     set<T extends SupportedStorageTypes>(contexts: string[], key: string, value: T): void {
-        if (!contexts.length || !key.length) throw new StorageError("Context and key must not be empty strings!");
         this.localStorage.setItem(this.buildStorageKey(contexts, key), toJson(value));
     }
 
     delete(contexts: string[], key: string): void {
-        if (!contexts.length || !key.length) throw new StorageError("Context and key must not be empty strings!");
         this.localStorage.removeItem(this.buildStorageKey(contexts, key));
     }
 
     /** Returns all keys of a storage context without keys of sub-contexts */
     keys(contexts: string[]): string[] {
-        if (!contexts.length) throw new StorageError("Context must not be an empty string!");
-        const contextKey = contexts.join(".");
-        if (contextKey.includes("..") || contextKey.startsWith(".") || contextKey.endsWith("."))
-            throw new StorageError("Context must not contain dots!");
+        const contextKey = this.getContextBaseKey(contexts);
         const keys = [];
         const contextKeyStart = `${contextKey}.`;
         for (const key of Object.keys(this.localStorage)) {
@@ -75,9 +76,7 @@ export class StorageBackendDisk implements Storage {
     }
 
     clearAll(contexts: string[]): void {
-        const contextKey = contexts.join(".");
-        if (contextKey.includes("..") || contextKey.startsWith(".") || contextKey.endsWith("."))
-            throw new StorageError("Context must not contain dots!");
+        const contextKey = this.getContextBaseKey(contexts, true);
         const startContextKey = contextKey.length ? `${contextKey}.` : "";
         for (const key of Object.keys(this.localStorage)) {
             if (key.startsWith(startContextKey)) {

diff --git a/packages/matter-node.js/test/storage/StorageBackendDiskTest.ts b/packages/matter-node.js/test/storage/StorageBackendDiskTest.ts
@@ -103,7 +103,7 @@ describe("Storage node-localstorage", () => {
                 storage.set([""], "key", "value");
             },
             {
-                message: "Context must not be an empty string!",
+                message: "Context must not be an empty and not contain dots.",
             },
         );
     });
@@ -115,7 +115,7 @@ describe("Storage node-localstorage", () => {
                 storage.set(["context"], "", "value");
             },
             {
-                message: "Context and key must not be empty strings!",
+                message: "Key must not be an empty string.",
             },
         );
     });
@@ -127,7 +127,7 @@ describe("Storage node-localstorage", () => {
                 storage.get([""], "key");
             },
             {
-                message: "Context must not be an empty string!",
+                message: "Context must not be an empty and not contain dots.",
             },
         );
     });
@@ -139,7 +139,7 @@ describe("Storage node-localstorage", () => {
                 storage.get(["ok", ""], "key");
             },
             {
-                message: "Context must not be an empty string!",
+                message: "Context must not be an empty and not contain dots.",
             },
         );
     });
@@ -151,7 +151,7 @@ describe("Storage node-localstorage", () => {
                 storage.get(["context"], "");
             },
             {
-                message: "Context and key must not be empty strings!",
+                message: "Key must not be an empty string.",
             },
         );
     });

diff --git a/packages/matter.js/src/storage/StorageBackendMemory.ts b/packages/matter.js/src/storage/StorageBackendMemory.ts
@@ -13,7 +13,7 @@ export class StorageBackendMemory implements Storage {
     private createContextKey(contexts: string[]) {
         const key = contexts.join(".");
         if (!key.length || key.includes("..") || key.startsWith(".") || key.endsWith("."))
-            throw new StorageError("Context must not be an empty string!");
+            throw new StorageError("Context must not be an empty string.");
         return key;
     }
 
@@ -30,12 +30,12 @@ export class StorageBackendMemory implements Storage {
     }
 
     get<T extends SupportedStorageTypes>(contexts: string[], key: string): T | undefined {
-        if (!contexts.length || !key.length) throw new StorageError("Context and key must not be empty!");
+        if (!contexts.length || !key.length) throw new StorageError("Context and key must not be empty.");
         return this.store[this.createContextKey(contexts)]?.[key];
     }
 
     set<T extends SupportedStorageTypes>(contexts: string[], key: string, value: T): void {
-        if (!contexts.length || !key.length) throw new StorageError("Context and key must not be empty!");
+        if (!contexts.length || !key.length) throw new StorageError("Context and key must not be empty.");
         const contextKey = this.createContextKey(contexts);
         if (this.store[contextKey] === undefined) {
             this.store[contextKey] = {};
@@ -44,7 +44,7 @@ export class StorageBackendMemory implements Storage {
     }
 
     delete(contexts: string[], key: string): void {
-        if (!contexts.length || !key.length) throw new StorageError("Context and key must not be empty!");
+        if (!contexts.length || !key.length) throw new StorageError("Context and key must not be empty.");
         delete this.store[this.createContextKey(contexts)]?.[key];
     }
 

diff --git a/packages/matter.js/test/storage/StorageBackendMemoryTest.ts b/packages/matter.js/test/storage/StorageBackendMemoryTest.ts
@@ -72,34 +72,34 @@ describe("StorageBackendMemory", () => {
         expect(() => {
             const storage = new StorageBackendMemory();
             storage.set([], "key", "value");
-        }).throw(StorageError, "Context and key must not be empty!");
+        }).throw(StorageError, "Context and key must not be empty.");
     });
 
     it("Throws error when context is empty on set", () => {
         expect(() => {
             const storage = new StorageBackendMemory();
             storage.set([""], "key", "value");
-        }).throw(StorageError, "Context must not be an empty string!");
+        }).throw(StorageError, "Context must not be an empty string.");
     });
 
     it("Throws error when key is empty on set", () => {
         expect(() => {
             const storage = new StorageBackendMemory();
             storage.set(["context"], "", "value");
-        }).throw(StorageError, "Context and key must not be empty!");
+        }).throw(StorageError, "Context and key must not be empty.");
     });
 
     it("Throws error when context is empty on get with subcontext", () => {
         expect(() => {
             const storage = new StorageBackendMemory();
             storage.get(["ok", ""], "key");
-        }).throw(StorageError, "Context must not be an empty string!");
+        }).throw(StorageError, "Context must not be an empty string.");
     });
 
     it("Throws error when key is empty on get", () => {
         expect(() => {
             const storage = new StorageBackendMemory();
             storage.get(["context", "subcontext"], "");
-        }).throw(StorageError, "Context and key must not be empty!");
+        }).throw(StorageError, "Context and key must not be empty.");
     });
 });