refactor: review SharedState contructor signature

collective-soundworks · Oct 4, 2024 · 7d27345 · 7d27345
1 parent a15e807
commit 7d27345
Show file tree

Hide file tree

Showing 5 changed files with 116 additions and 70 deletions.
diff --git a/misc/assets/custom.css b/misc/assets/custom.css
@@ -117,7 +117,7 @@ h4.name {
 }
 
 .type-signature {
-  color: #eeefea;
+  color: #9e9f9a;
 }
 
 .type-signature:last-child {

diff --git a/src/common/BaseStateManager.js b/src/common/BaseStateManager.js
@@ -95,21 +95,33 @@ class BaseStateManager {
     // ---------------------------------------------
     // CREATE
     // ---------------------------------------------
-    this[kStateManagerClient].transport.addListener(CREATE_RESPONSE, (reqId, stateId, remoteId, className, classDescription, initValues) => {
-      // cache class description to save some bandwidth
-      // @note: when we make the class dynamic, we will need some mecanism to
-      // invalidate the cached description
-      if (!this.#cachedClasses.has(className)) {
-        this.#cachedClasses.set(className, classDescription);
-      }
-
-      classDescription = this.#cachedClasses.get(className);
-
-      const state = new SharedState(stateId, remoteId, className, classDescription, this[kStateManagerClient], true, this, initValues, null);
+    this[kStateManagerClient].transport.addListener(
+      CREATE_RESPONSE,
+      (reqId, stateId, remoteId, className, classDescription, initValues) => {
+        // cache class description to save some bandwidth
+        // @note: when we make the class dynamic, we will need some mecanism to
+        // invalidate the cached description
+        if (!this.#cachedClasses.has(className)) {
+          this.#cachedClasses.set(className, classDescription);
+        }
 
-      this.#statesById.set(state.id, state);
-      this.#promiseStore.resolve(reqId, state);
-    });
+        classDescription = this.#cachedClasses.get(className);
+
+        const state = new SharedState({
+          manager: this,
+          className,
+          classDescription,
+          stateId,
+          remoteId,
+          isOwner: true,
+          initValues,
+          filter: null, // owner cannot filter paramters
+        });
+
+        this.#statesById.set(state.id, state);
+        this.#promiseStore.resolve(reqId, state);
+      }
+    );
 
     this[kStateManagerClient].transport.addListener(CREATE_ERROR, (reqId, msg) => {
       this.#promiseStore.reject(reqId, msg);
@@ -118,21 +130,33 @@ class BaseStateManager {
     // ---------------------------------------------
     // ATTACH (when creator, is attached by default)
     // ---------------------------------------------
-    this[kStateManagerClient].transport.addListener(ATTACH_RESPONSE, (reqId, stateId, remoteId, className, classDescription, currentValues, filter) => {
-      // cache class description to save some bandwidth
-      // @note: when we make the class dynamic, we will need some mecanism to
-      // invalidate the cached description
-      if (!this.#cachedClasses.has(className)) {
-        this.#cachedClasses.set(className, classDescription);
-      }
-
-      classDescription = this.#cachedClasses.get(className);
-
-      const state = new SharedState(stateId, remoteId, className, classDescription, this[kStateManagerClient], false, this, currentValues, filter);
+    this[kStateManagerClient].transport.addListener(
+      ATTACH_RESPONSE,
+      (reqId, stateId, remoteId, className, classDescription, currentValues, filter) => {
+        // cache class description to save some bandwidth
+        // @note: when we make the class dynamic, we will need some mecanism to
+        // invalidate the cached description
+        if (!this.#cachedClasses.has(className)) {
+          this.#cachedClasses.set(className, classDescription);
+        }
 
-      this.#statesById.set(state.id, state);
-      this.#promiseStore.resolve(reqId, state);
-    });
+        classDescription = this.#cachedClasses.get(className);
+
+        const state = new SharedState({
+          manager: this,
+          className,
+          classDescription,
+          stateId,
+          remoteId,
+          isOwner: false,
+          initValues: currentValues,
+          filter,
+        });
+
+        this.#statesById.set(state.id, state);
+        this.#promiseStore.resolve(reqId, state);
+      }
+    );
 
     this[kStateManagerClient].transport.addListener(ATTACH_ERROR, (reqId, msg) => {
       this.#promiseStore.reject(reqId, msg);
@@ -180,16 +204,19 @@ class BaseStateManager {
       this.#promiseStore.reject(reqId, msg);
     });
 
-    this[kStateManagerClient].transport.addListener(OBSERVE_NOTIFICATION, (className, stateId, nodeId) => {
-      this.#observeListeners.forEach(observeInfos => {
-        const [observedClassName, callback, options] = observeInfos;
-        const filter = this.#filterObserve(observedClassName, className, nodeId, options);
+    this[kStateManagerClient].transport.addListener(
+      OBSERVE_NOTIFICATION,
+      (className, stateId, nodeId) => {
+        this.#observeListeners.forEach(observeInfos => {
+          const [observedClassName, callback, options] = observeInfos;
+          const filter = this.#filterObserve(observedClassName, className, nodeId, options);
 
-        if (!filter) {
-          callback(className, stateId, nodeId);
-        }
-      });
-    });
+          if (!filter) {
+            callback(className, stateId, nodeId);
+          }
+        });
+      }
+    );
 
     // ---------------------------------------------
     // Clear cache when a shared state class is deleted
@@ -201,14 +228,17 @@ class BaseStateManager {
     // ---------------------------------------------
     // Get class description
     // ---------------------------------------------
-    this[kStateManagerClient].transport.addListener(GET_CLASS_DESCRIPTION_RESPONSE, (reqId, className, classDescription) => {
-      if (!this.#cachedClasses.has(className)) {
-        this.#cachedClasses.set(className, classDescription);
+    this[kStateManagerClient].transport.addListener(
+      GET_CLASS_DESCRIPTION_RESPONSE,
+      (reqId, className, classDescription) => {
+        if (!this.#cachedClasses.has(className)) {
+          this.#cachedClasses.set(className, classDescription);
+        }
+        // return a full class description
+        const parameterBag = new ParameterBag(classDescription);
+        this.#promiseStore.resolve(reqId, parameterBag.getDescription());
       }
-      // return a full class description
-      const parameterBag = new ParameterBag(classDescription);
-      this.#promiseStore.resolve(reqId, parameterBag.getDescription());
-    });
+    );
 
     this[kStateManagerClient].transport.addListener(GET_CLASS_DESCRIPTION_ERROR, (reqId, msg) => {
       this.#promiseStore.reject(reqId, msg);

diff --git a/src/common/SharedState.js b/src/common/SharedState.js
@@ -17,6 +17,7 @@ import {
 } from './constants.js';
 
 import {
+  kStateManagerClient,
   kStateManagerDeleteState,
 } from './BaseStateManager.js';
 
@@ -112,13 +113,22 @@ class SharedState {
   #onDetachCallbacks = new Set();
   #onDeleteCallbacks = new Set();
 
-  constructor(id, remoteId, className, classDescription, client, isOwner, manager, initValues, filter) {
-    this.#id = id;
-    this.#remoteId = remoteId;
+  constructor({
+    stateId,
+    remoteId,
+    className,
+    classDescription,
+    isOwner,
+    manager,
+    initValues,
+    filter,
+  }) {
+    this.#manager = manager;
+    this.#client = manager[kStateManagerClient];
     this.#className = className;
+    this.#id = stateId;
+    this.#remoteId = remoteId;
     this.#isOwner = isOwner; // may be any node
-    this.#client = client;
-    this.#manager = manager;
     this.#filter = filter;
 
     try {

diff --git a/src/server/ServerStateManager.js b/src/server/ServerStateManager.js
@@ -176,7 +176,7 @@ class ServerStateManager extends BaseStateManager {
             const classDescription = this.#classes.get(className);
             const stateId = generateStateId.next().value;
             const remoteId = generateRemoteId.next().value;
-            const state = new SharedStatePrivate(stateId, className, classDescription, this, initValues);
+            const state = new SharedStatePrivate(this, className, classDescription, stateId, initValues);
 
             // attach client to the state as owner
             const isOwner = true;
@@ -190,7 +190,12 @@ class ServerStateManager extends BaseStateManager {
 
             client.transport.emit(
               CREATE_RESPONSE,
-              reqId, stateId, remoteId, className, classDescriptionOption, currentValues,
+              reqId,
+              stateId,
+              remoteId,
+              className,
+              classDescriptionOption,
+              currentValues,
             );
 
             const isObservable = this.#isObservableState(state);
@@ -202,14 +207,10 @@ class ServerStateManager extends BaseStateManager {
             }
           } catch (err) {
             const msg = `[stateManager] Cannot create state "${className}", ${err.message}`;
-            console.error(msg);
-
             client.transport.emit(CREATE_ERROR, reqId, msg);
           }
         } else {
           const msg = `[stateManager] Cannot create state "${className}", class is not defined`;
-          console.error(msg);
-
           client.transport.emit(CREATE_ERROR, reqId, msg);
         }
       },
@@ -257,8 +258,6 @@ class ServerStateManager extends BaseStateManager {
 
               if (!isValid) {
                 const msg = `[stateManager] Cannot attach, invalid filter (${filter.join(', ')}) for class "${className}"`;
-                console.error(msg);
-
                 return client.transport.emit(ATTACH_ERROR, reqId, msg);
               }
             }
@@ -267,19 +266,21 @@ class ServerStateManager extends BaseStateManager {
 
             client.transport.emit(
               ATTACH_RESPONSE,
-              reqId, state.id, remoteId, className, classDescriptionOption, currentValues, filter,
+              reqId,
+              state.id,
+              remoteId,
+              className,
+              classDescriptionOption,
+              currentValues,
+              filter,
             );
 
           } else {
             const msg = `[stateManager] Cannot attach, no existing state for class "${className}" with stateId: "${stateId}"`;
-            console.error(msg);
-
             client.transport.emit(ATTACH_ERROR, reqId, msg);
           }
         } else {
           const msg = `[stateManager] Cannot attach, class "${className}" does not exists`;
-          console.error(msg);
-
           client.transport.emit(ATTACH_ERROR, reqId, msg);
         }
       },
@@ -290,22 +291,22 @@ class ServerStateManager extends BaseStateManager {
     // ---------------------------------------------
     client.transport.addListener(OBSERVE_REQUEST, (reqId, observedClassName) => {
       if (observedClassName === null || this.#classes.has(observedClassName)) {
-        const statesInfos = [];
+        const list = [];
 
         this.#sharedStatePrivateById.forEach(state => {
           const isObservable = this.#isObservableState(state);
 
           if (isObservable) {
             const { className, id, creatorId } = state;
-            statesInfos.push([className, id, creatorId]);
+            list.push([className, id, creatorId]);
           }
         });
 
         // add client to observers first because if some synchronous server side
         // callback throws, the client would never be added to the list
         this.#observers.add(client);
 
-        client.transport.emit(OBSERVE_RESPONSE, reqId, ...statesInfos);
+        client.transport.emit(OBSERVE_RESPONSE, reqId, ...list);
       } else {
         const msg = `[stateManager] Cannot observe class "${observedClassName}", class does not exists`;
         client.transport.emit(OBSERVE_ERROR, reqId, msg);
@@ -322,7 +323,12 @@ class ServerStateManager extends BaseStateManager {
     client.transport.addListener(GET_CLASS_DESCRIPTION_REQUEST, (reqId, className) => {
       if (this.#classes.has(className)) {
         const classDescription = this.#classes.get(className);
-        client.transport.emit(GET_CLASS_DESCRIPTION_RESPONSE, reqId, className, classDescription);
+        client.transport.emit(
+          GET_CLASS_DESCRIPTION_RESPONSE,
+          reqId,
+          className,
+          classDescription
+        );
       } else {
         const msg = `[stateManager] Cannot get class "${className}", class does not exists`;
         client.transport.emit(GET_CLASS_DESCRIPTION_ERROR, reqId, msg);

diff --git a/src/server/SharedStatePrivate.js b/src/server/SharedStatePrivate.js
@@ -53,10 +53,10 @@ class SharedStatePrivate {
   #creatorRemoteId = null;
   #attachedClients = new Map();
 
-  constructor(id, className, classDefinition, manager, initValues = {}) {
-    this.#id = id;
-    this.#className = className;
+  constructor(manager, className, classDefinition, id, initValues = {}) {
     this.#manager = manager;
+    this.#className = className;
+    this.#id = id;
     this.#parameters = new ParameterBag(classDefinition, initValues);
   }