fix: Protect against incomplete data coming from Spoolman (GH-43) (#44)

* fix: Enable actual typescript code verification (GH-43) Signed-off-by: Michał Dziekoński <[email protected]> * fix: Provide basic typings (global vars & own code) (GH-43) Signed-off-by: Michał Dziekoński <[email protected]> * fix: Fix typescript include (GH-43) Signed-off-by: Michał Dziekoński <[email protected]> * fix: Fix spool typings (GH-43) Signed-off-by: Michał Dziekoński <[email protected]> * fix: When rendering spools data, use safer, more error resistant field access method (GH-43) Signed-off-by: Michał Dziekoński <[email protected]> * fix: Return failure on empty get spools response (GH-43) Signed-off-by: Michał Dziekoński <[email protected]> * fix: Fix Spool.filament.vendor typing (GH-43) Signed-off-by: Michał Dziekoński <[email protected]> * fix: Add spool data display formatter (GH-43) Signed-off-by: Michał Dziekoński <[email protected]> * fix: Use safe and formatted spool data in confirmation modal (GH-43) Signed-off-by: Michał Dziekoński <[email protected]> * fix: Display invalid spool warning (GH-43) Signed-off-by: Michał Dziekoński <[email protected]> * fix: Use safe and formatted spool data in selector modal (GH-43) Signed-off-by: Michał Dziekoński <[email protected]> * fix: Do not allow to select invalid spools (GH-43) Signed-off-by: Michał Dziekoński <[email protected]> * fix: Use safe and formatted spool data in sidebar list (GH-43) Signed-off-by: Michał Dziekoński <[email protected]> * fix: Use safe and formatted spool data in selector modal, current selection (GH-43) Signed-off-by: Michał Dziekoński <[email protected]> * fix: Improve sidebar layout in edge cases (GH-43) Signed-off-by: Michał Dziekoński <[email protected]> --------- Signed-off-by: Michał Dziekoński <[email protected]>
mdziekon · May 3, 2024 · 871a69a · 871a69a
1 parent ef9de75
commit 871a69a
Show file tree

Hide file tree

Showing 14 changed files with 452 additions and 111 deletions.
diff --git a/octoprint_Spoolman/SpoolmanPlugin.py b/octoprint_Spoolman/SpoolmanPlugin.py
@@ -88,6 +88,7 @@ def get_assets(self):
                 "js/common/api.js",
                 "js/common/promiseCache.js",
                 "js/common/octoprint.js",
+                "js/common/formatting.js",
                 "js/api/getSpoolmanSpools.js",
                 "js/api/getCurrentJobRequirements.js",
                 "js/api/updateActiveSpool.js",

diff --git a/octoprint_Spoolman/static/js/Spoolman_api.js b/octoprint_Spoolman/static/js/Spoolman_api.js
@@ -1,20 +1,21 @@
-$(() => {
+const createApi = () => {
     // from setup.py plugin_identifier
     const PLUGIN_ID = "Spoolman";
 
     const cache = new PromiseCache();
     const apiClient = new APIClient(PLUGIN_ID, BASEURL);
 
-    const cacheGetSpoolmanSpoolsResult = cache.registerResource("getSpoolmanSpools", {
-        getter: async () => {
-            const request = await getSpoolmanSpools(apiClient);
+    const sharedGetSpoolmanSpools = async () => {
+        const request = await getSpoolmanSpools(apiClient);
 
-            if (!request.isSuccess) {
-                console.error("Request error", request.error);
-            }
+        if (!request.isSuccess) {
+            console.error("Request error", request.error);
+        }
 
-            return request;
-        },
+        return request;
+    };
+    const cacheGetSpoolmanSpoolsResult = cache.registerResource("getSpoolmanSpools", {
+        getter: sharedGetSpoolmanSpools,
     });
     const sharedGetCurrentJobRequirements = async () => {
         const request = await getCurrentJobRequirements(apiClient);
@@ -25,6 +26,9 @@ $(() => {
 
         return request;
     };
+    /**
+     * @param {Parameters<typeof updateActiveSpool>[1]} params
+     */
     const sharedUpdateActiveSpool = async ({ toolIdx, spoolId }) => {
         const request = await updateActiveSpool(apiClient, { toolIdx, spoolId });
 
@@ -39,11 +43,30 @@ $(() => {
         throw new Error('[Spoolman][api] Could not create cached "getSpoolmanSpools"');
     }
 
-    window.pluginSpoolmanApi = {
+    const pluginSpoolmanApiNamespace = {
         cache,
 
+        /** @type {typeof sharedGetSpoolmanSpools & { invalidate: CachedGetterFn['invalidate'] }} */
+        // @ts-ignore
         getSpoolmanSpools: cacheGetSpoolmanSpoolsResult.getter,
         getCurrentJobRequirements: sharedGetCurrentJobRequirements,
         updateActiveSpool: sharedUpdateActiveSpool,
     };
-});
+
+    /**
+     * @typedef {typeof pluginSpoolmanApiNamespace} PluginSpoolmanApiType
+     */
+
+    // @ts-ignore
+    window.pluginSpoolmanApi = pluginSpoolmanApiNamespace;
+
+    return {
+        pluginSpoolmanApiNamespace,
+    };
+};
+
+$(createApi);
+
+/**
+ * @typedef {ReturnType<typeof createApi>['pluginSpoolmanApiNamespace']} PluginSpoolmanApiType
+ */
diff --git a/octoprint_Spoolman/static/js/Spoolman_modal_confirmSpool.js b/octoprint_Spoolman/static/js/Spoolman_modal_confirmSpool.js
@@ -20,6 +20,8 @@ $(() => {
      * Events:
      * - onConfirm
      * - onHidden
+     *
+     * @param {*} params
      */
     function SpoolmanModalConfirmSpoolViewModel(params) {
         const self = this;
@@ -109,6 +111,8 @@ $(() => {
                 const spoolId = selectedSpoolIds[extruderIdx]?.spoolId();
 
                 const spoolData = spoolmanSpools.find((spool) => String(spool.id) === spoolId);
+                const safeSpool = spoolData && toSafeSpool(spoolData);
+                const spoolDisplayData = spoolData && toSpoolForDisplay(spoolData, { constants: self.constants });
                 const toolFilamentUsage = currentJobRequirements.tools[extruderIdx];
 
                 if (
@@ -118,6 +122,8 @@ $(() => {
                     return {
                         spoolId,
                         spoolData,
+                        spoolDisplayData,
+                        isSpoolValid: safeSpool?.isSpoolValid,
                         /** @type false */
                         isToolInUse: false,
                         isToolMissingSelection: undefined,
@@ -126,13 +132,20 @@ $(() => {
                 }
 
                 const isToolMissingSelection = !toolFilamentUsage.spoolId;
-                const isEnoughFilamentAvailable = isToolMissingSelection || !spoolData
-                    ? undefined
-                    : toolFilamentUsage.filamentWeight <= spoolData.remaining_weight;
+
+                const isEnoughFilamentAvailable = (
+                    !isToolMissingSelection &&
+                    safeSpool &&
+                    safeSpool.isSpoolValid
+                )
+                    ? toolFilamentUsage.filamentWeight <= safeSpool.spoolData.remaining_weight
+                    : undefined;
 
                 return {
                     spoolId,
-                    spoolData,
+                    spoolData: safeSpool && safeSpool.spoolData,
+                    spoolDisplayData,
+                    isSpoolValid: safeSpool?.isSpoolValid,
                     /** @type true */
                     isToolInUse: true,
                     isToolMissingSelection,
@@ -155,6 +168,16 @@ $(() => {
                         ? self.constants.filament_problems.NOT_ENOUGH_FILAMENT
                         : undefined
                 ),
+                (
+                    selectedSpools.some((spool) => {
+                        return (
+                            spool.isToolInUse &&
+                            spool.isSpoolValid === false
+                        );
+                    })
+                        ? self.constants.filament_problems.INVALID_SPOOL
+                        : undefined
+                ),
                 (
                     selectedSpools.some((spool) => spool.isToolMissingSelection === true)
                         ? self.constants.filament_problems.MISSING_SPOOL_SELECTION
@@ -204,6 +227,7 @@ $(() => {
             length_unit: 'mm',
 
             filament_problems: {
+                INVALID_SPOOL: 'INVALID_SPOOL',
                 NO_FILAMENT_USAGE_DATA: 'NO_FILAMENT_USAGE_DATA',
                 NOT_ENOUGH_FILAMENT: 'NOT_ENOUGH_FILAMENT',
                 MISSING_SPOOL_SELECTION: 'MISSING_SPOOL_SELECTION',

diff --git a/octoprint_Spoolman/static/js/Spoolman_modal_selectSpool.js b/octoprint_Spoolman/static/js/Spoolman_modal_selectSpool.js
@@ -69,9 +69,23 @@ $(() => {
                 return String(spool.id) === toolSpoolId;
             });
 
+            const spoolmanSafeSpools = spoolmanSpools.map((spool) => {
+                return {
+                    ...toSafeSpool(spool),
+                    displayData: toSpoolForDisplay(spool, { constants: self.constants }),
+                };
+            });
+
             self.templateData.toolCurrentSpoolId(toolSpoolId);
-            self.templateData.toolCurrentSpool(toolSpool);
-            self.templateData.tableItemsOnCurrentPage(spoolmanSpools);
+            self.templateData.toolCurrentSpool(
+                toolSpool
+                    ? {
+                        ...toSafeSpool(toolSpool),
+                        displayData: toSpoolForDisplay(toolSpool, { constants: self.constants }),
+                    }
+                    : undefined
+            );
+            self.templateData.tableItemsOnCurrentPage(spoolmanSafeSpools);
 
             self.templateData.spoolmanUrl(getPluginSettings().spoolmanUrl());
 

diff --git a/octoprint_Spoolman/static/js/Spoolman_sidebar.js b/octoprint_Spoolman/static/js/Spoolman_sidebar.js
@@ -84,6 +84,7 @@ $(() => {
                 return {
                     spoolId,
                     spoolData,
+                    spoolDisplayData: spoolData && toSpoolForDisplay(spoolData, { constants: self.constants }),
                 };
             });
 

diff --git a/octoprint_Spoolman/static/js/api/getSpoolmanSpools.js b/octoprint_Spoolman/static/js/api/getSpoolmanSpools.js
@@ -3,28 +3,29 @@
  * @property {Object} data
  * @property {Array<Spool>} data.spools
  *
+ * @typedef {Object} FilamentVendor
+ * @property {number} id
+ * @property {string} name
+ *
  * @typedef {Object} Spool
  * @property {number} id
  * @property {boolean} archived
  * @property {string} registered
  * @property {number} used_length
  * @property {number} used_weight
- * @property {number} remaining_length
- * @property {number} remaining_weight
- * @property {boolean} archived
+ * @property {number | undefined} remaining_length
+ * @property {number | undefined} remaining_weight
  * @property {Object} filament
  * @property {number} filament.id
+ * @property {string} filament.registered
  * @property {number} filament.diameter
  * @property {number} filament.density
- * @property {number} filament.weight
- * @property {number} filament.spool_weight
- * @property {string} filament.material
- * @property {string} filament.name
- * @property {string} filament.registered
- * @property {string} filament.color_hex
- * @property {Object} filament.vendor
- * @property {number} filament.vendor.id
- * @property {string} filament.vendor.name
+ * @property {number | undefined} filament.weight
+ * @property {number | undefined} filament.spool_weight
+ * @property {string | undefined} filament.material
+ * @property {string | undefined} filament.name
+ * @property {string | undefined} filament.color_hex
+ * @property {FilamentVendor | undefined} filament.vendor
  */
 
 /**
@@ -37,8 +38,44 @@ async function getSpoolmanSpools(apiClient) {
         return request;
     }
     if (!request.payload.response) {
-        return /** @type Success<{ response: undefined }> */ (request);
+        return /** @type Failure<undefined> */ ({
+            isSuccess: false,
+            error: undefined,
+        });
     }
 
     return /** @type Success<{ response: GetSpoolsResponse }> */ (request);
 }
+
+/**
+ * Checks whether spool has "complete-enough" data, so that the plugin
+ * can function properly.
+ *
+ * @type {(spool: Spool) => spool is Spool & { remaining_weight: number }}
+ */
+const isSpoolValid = (spool) => {
+    return (
+        spool.remaining_weight !== undefined
+    );
+};
+
+/**
+ * @param {Spool} spool
+ */
+const toSafeSpool = (spool) => {
+    if (isSpoolValid(spool)) {
+        return {
+            spoolId: spool.id,
+            /** @type true */
+            isSpoolValid: true,
+            spoolData: spool,
+        };
+    }
+
+    return {
+        spoolId: spool.id,
+        /** @type false */
+        isSpoolValid: false,
+        spoolData: spool,
+    };
+};
diff --git a/octoprint_Spoolman/static/js/common/api.js b/octoprint_Spoolman/static/js/common/api.js
@@ -76,6 +76,9 @@ const methodsWithBody = [
  * @param {string} baseUrl
  */
 function APIClient(pluginId, baseUrl) {
+    /**
+     * @param {string | Record<string, string>} data
+     */
     const _buildRequestQuery = function (data) {
         if (typeof (data) === 'string') {
             return data;
@@ -92,10 +95,20 @@ function APIClient(pluginId, baseUrl) {
             .join('&');
     };
 
+    /**
+     * @param {string} url
+     */
     const buildApiUrl = (url) => {
         return `${baseUrl}plugin/${pluginId}/${url}`;
     };
 
+    /**
+     * @param {{
+     *  method?: string;
+     *  headers?: Record<string, string>;
+     *  body?: string;
+     * }} options
+     */
     const buildFetchOptions = (options) => {
         const requestMethod = (options.method ?? "GET").toUpperCase();
 
@@ -120,7 +133,7 @@ function APIClient(pluginId, baseUrl) {
 
     /**
      * @param {string} url
-     * @param {Object} options
+     * @param {Parameters<typeof buildFetchOptions>[0]} options
      */
     const callApi = async (url, options) => {
         const endpointUrl = buildApiUrl(url);

diff --git a/octoprint_Spoolman/static/js/common/formatting.js b/octoprint_Spoolman/static/js/common/formatting.js
@@ -0,0 +1,65 @@
+/**
+ * @param {Spool} spool
+ * @param {{
+*  constants: Record<string, unknown>
+* }} params
+*/
+const toSpoolForDisplay = (spool, params) => {
+    return {
+        filament: {
+            color: {
+                cssProperty: (
+                    spool.filament.color_hex
+                        ? 'background-color'
+                        : 'background'
+                ),
+                cssValue: (
+                    spool.filament.color_hex
+                        ? `#${spool.filament.color_hex}`
+                        : 'linear-gradient(45deg, #000000 -25%, #ffffff)'
+                ),
+            },
+            name: (
+                spool.filament.name
+                    ? {
+                        displayValue: spool.filament.name,
+                    } : {
+                        displayValue: "Unnamed filament",
+                    }
+            ),
+            material: (
+                spool.filament.material
+                    ? {
+                        displayShort: spool.filament.material,
+                        displayValue: spool.filament.material,
+                    } : {
+                        displayShort: "Unknown",
+                        displayValue: "Unknown material",
+                    }
+            ),
+            vendor: {
+                name: (
+                    spool.filament.vendor?.name
+                        ? {
+                            displayValue: spool.filament.vendor.name,
+                        } : {
+                            displayValue: "Unknown filament vendor",
+                        }
+                ),
+            },
+        },
+        used_weight: {
+            displayValue: spool.used_weight.toFixed(1),
+        },
+        remaining_weight: (
+            spool.remaining_weight !== undefined
+                ? {
+                    isValid: true,
+                    displayValue: `${spool.remaining_weight.toFixed(1)}${params.constants['weight_unit']}`,
+                } : {
+                    isValid: false,
+                    displayValue: "Unavailable",
+                }
+        ),
+    };
+};