Extract utility functions for JSON parsing

11 files changed, 143 insertions, 80 deletions

diff --git a/src/helpers/url-helpers.test.ts b/src/helpers/url-helpers.test.ts
index e6036893e..5af3025e9 100644
--- a/src/helpers/url-helpers.test.ts
+++ b/src/helpers/url-helpers.test.ts
@@ -3,7 +3,7 @@ import * as url_helpers from './url-helpers'
 describe('url_helpers', () => {
   describe('isValidExternalURL', () => {
     describe('with string', () => {
-      it('returns false for empty string ', () => {
+      it('returns false for empty string', () => {
         const result = url_helpers.isValidExternalURL('');
         expect(result).toBe(false);
       });
diff --git a/src/internal-server/app/Controllers/Http/RecipeController.js b/src/internal-server/app/Controllers/Http/RecipeController.js
index e88a89a0b..eac34aa5c 100644
--- a/src/internal-server/app/Controllers/Http/RecipeController.js
+++ b/src/internal-server/app/Controllers/Http/RecipeController.js
@@ -6,6 +6,7 @@ const Env = use('Env');
 const fetch = require('node-fetch');
 const debug = require('../../../../preload-safe-debug')('Ferdium:internalServer:RecipeController');
 const { LIVE_FERDIUM_API } = require('../../../../config');
+const { convertToJSON } = require('../../../../jsUtils');
 const { API_VERSION } = require('../../../../environment-remote');
 
 const RECIPES_URL = `${LIVE_FERDIUM_API}/${API_VERSION}/recipes`;
@@ -14,13 +15,13 @@ class RecipeController {
   // List official and custom recipes
   async list({ response }) {
     const recipesUrlFetch = await fetch(RECIPES_URL);
-    const officialRecipes = JSON.parse(await recipesUrlFetch.text());
+    const officialRecipes = convertToJSON(await recipesUrlFetch.text());
     const allRecipes = await Recipe.all();
     const customRecipesArray = allRecipes.rows;
     const customRecipes = customRecipesArray.map(recipe => ({
       id: recipe.recipeId,
       name: recipe.name,
-      ...JSON.parse(recipe.data),
+      ...convertToJSON(recipe.data),
     }));
 
     const recipes = [...officialRecipes, ...customRecipes];
@@ -53,7 +54,7 @@ class RecipeController {
       results = dbResults.map(recipe => ({
         id: recipe.recipeId,
         name: recipe.name,
-        ...JSON.parse(recipe.data),
+        ...convertToJSON(recipe.data),
       }));
     } else {
       let remoteResults = [];
@@ -62,7 +63,7 @@ class RecipeController {
         const recipesUrlFetch = await fetch(
           `${RECIPES_URL}/search?needle=${encodeURIComponent(needle)}`,
         );
-        remoteResults = JSON.parse(await recipesUrlFetch.text());
+        remoteResults = convertToJSON(await recipesUrlFetch.text());
       }
 
       debug('remoteResults:', remoteResults);
@@ -74,7 +75,7 @@ class RecipeController {
       const localResults = localResultsArray.map(recipe => ({
         id: recipe.recipeId,
         name: recipe.name,
-        ...JSON.parse(recipe.data),
+        ...convertToJSON(recipe.data),
       }));
 
       debug('localResults:', localResults);
@@ -96,7 +97,7 @@ class RecipeController {
     response,
   }) {
     const recipesUrlFetch = await fetch(`${RECIPES_URL}/popular`);
-    const featuredRecipes = JSON.parse(await recipesUrlFetch.text());
+    const featuredRecipes = convertToJSON(await recipesUrlFetch.text());
     return response.send(featuredRecipes);
   }
 
diff --git a/src/internal-server/app/Controllers/Http/ServiceController.js b/src/internal-server/app/Controllers/Http/ServiceController.js
index f2f6e7028..81c03e5ff 100644
--- a/src/internal-server/app/Controllers/Http/ServiceController.js
+++ b/src/internal-server/app/Controllers/Http/ServiceController.js
@@ -4,6 +4,7 @@ const Env = use('Env');
 
 const { v4: uuid } = require('uuid');
 const { LOCAL_HOSTNAME, DEFAULT_SERVICE_ORDER } = require('../../../../config');
+const { convertToJSON } = require('../../../../jsUtils');
 const { API_VERSION } = require('../../../../environment-remote');
 const moveIcon = require('../../ImageHelper');
 
@@ -72,10 +73,7 @@ class ServiceController {
     const services = allServices.rows;
     // Convert to array with all data Franz wants
     const servicesArray = services.map(service => {
-      const settings =
-        typeof service.settings === 'string'
-          ? JSON.parse(service.settings)
-          : service.settings;
+      const settings = convertToJSON(service.settings);
 
       return {
         customRecipe: false,
@@ -88,7 +86,7 @@ class ServiceController {
         order: DEFAULT_SERVICE_ORDER,
         spellcheckerLanguage: '',
         workspaces: [],
-        ...JSON.parse(service.settings),
+        ...settings,
         iconUrl: settings.iconId
           ? `http://${hostname}:${port}/${API_VERSION}/icon/${settings.iconId}`
           : null,
@@ -108,10 +106,7 @@ class ServiceController {
       const { id } = params;
       const serviceQuery = await Service.query().where('serviceId', id).fetch();
       const service = serviceQuery.rows[0];
-      const settings =
-        typeof service.settings === 'string'
-          ? JSON.parse(service.settings)
-          : service.settings;
+      const settings = convertToJSON(service.settings);
 
       const icon = request.file('icon', {
         types: ['image'],
@@ -160,9 +155,7 @@ class ServiceController {
     const serviceData = serviceQuery.rows[0];
 
     const settings = {
-      ...(typeof serviceData.settings === 'string'
-        ? JSON.parse(serviceData.settings)
-        : serviceData.settings),
+      ...convertToJSON(serviceData.settings),
       ...data,
     };
 
@@ -205,7 +198,7 @@ class ServiceController {
       const serviceData = serviceQuery.rows[0];
 
       const settings = {
-        ...JSON.parse(serviceData.settings),
+        ...convertToJSON(serviceData.settings),
         order: data[service],
       };
 
@@ -222,10 +215,7 @@ class ServiceController {
     const services = allServices.rows;
     // Convert to array with all data Franz wants
     const servicesArray = services.map(service => {
-      const settings =
-        typeof service.settings === 'string'
-          ? JSON.parse(service.settings)
-          : service.settings;
+      const settings = convertToJSON(service.settings);
 
       return {
         customRecipe: false,
@@ -238,7 +228,7 @@ class ServiceController {
         order: DEFAULT_SERVICE_ORDER,
         spellcheckerLanguage: '',
         workspaces: [],
-        ...JSON.parse(service.settings),
+        ...settings,
         iconUrl: settings.iconId
           ? `http://${hostname}:${port}/${API_VERSION}/icon/${settings.iconId}`
           : null,
diff --git a/src/internal-server/app/Controllers/Http/UserController.js b/src/internal-server/app/Controllers/Http/UserController.js
index 6bd4f85a7..fb1a19db6 100644
--- a/src/internal-server/app/Controllers/Http/UserController.js
+++ b/src/internal-server/app/Controllers/Http/UserController.js
@@ -9,6 +9,7 @@ const fetch = require('node-fetch');
 const { v4: uuid } = require('uuid');
 const crypto = require('crypto');
 const { DEFAULT_APP_SETTINGS } = require('../../../../config');
+const { convertToJSON } = require('../../../../jsUtils');
 const { API_VERSION } = require('../../../../environment-remote');
 const { default: userAgent } = require('../../../../helpers/userAgent-helpers');
 
@@ -87,10 +88,7 @@ class UserController {
   async me({ response }) {
     const user = await User.find(1);
 
-    const settings =
-      typeof user.settings === 'string'
-        ? JSON.parse(user.settings)
-        : user.settings;
+    const settings = convertToJSON(user.settings);
 
     return response.send({
       ...DEFAULT_USER_DATA,
@@ -101,10 +99,7 @@ class UserController {
   async updateMe({ request, response }) {
     const user = await User.find(1);
 
-    let settings = user.settings || {};
-    if (typeof settings === 'string') {
-      settings = JSON.parse(settings);
-    }
+    const settings = convertToJSON(user.settings || {});
 
     const newSettings = {
       ...settings,
@@ -304,12 +299,8 @@ class UserController {
         .rows.length > 0
     );
 
-    if (
-      workspace.services &&
-      typeof workspace.services === 'string' &&
-      workspace.services.length > 0
-    ) {
-      workspace.services = JSON.parse(workspace.services);
+    if (workspace.services) {
+      workspace.services = convertToJSON(workspace.services);
     }
     const services =
       workspace.services && typeof workspace.services === 'object'
@@ -317,12 +308,8 @@ class UserController {
             oldServiceId => serviceIdTranslation[oldServiceId],
           )
         : [];
-    if (
-      workspace.data &&
-      typeof workspace.data === 'string' &&
-      workspace.data.length > 0
-    ) {
-      workspace.data = JSON.parse(workspace.data);
+    if (workspace.data) {
+      workspace.data = convertToJSON(workspace.data);
     }
 
     await Workspace.create({
@@ -347,12 +334,8 @@ class UserController {
     // store the old serviceId as the key for future lookup
     serviceIdTranslation[service.serviceId] = newServiceId;
 
-    if (
-      service.settings &&
-      typeof service.settings === 'string' &&
-      service.settings.length > 0
-    ) {
-      service.settings = JSON.parse(service.settings);
+    if (service.settings) {
+      service.settings = convertToJSON(service.settings);
     }
 
     await Service.create({
diff --git a/src/internal-server/app/Controllers/Http/WorkspaceController.js b/src/internal-server/app/Controllers/Http/WorkspaceController.js
index ee5731caf..352589567 100644
--- a/src/internal-server/app/Controllers/Http/WorkspaceController.js
+++ b/src/internal-server/app/Controllers/Http/WorkspaceController.js
@@ -2,6 +2,7 @@ const Workspace = use('App/Models/Workspace');
 const { validateAll } = use('Validator');
 
 const { v4: uuid } = require('uuid');
+const { convertToJSON } = require('../../../../jsUtils');
 
 class WorkspaceController {
   // Create a new workspace for user
@@ -130,10 +131,7 @@ class WorkspaceController {
         id: workspace.workspaceId,
         name: workspace.name,
         order: workspace.order,
-        services:
-          typeof workspace.services === 'string'
-            ? JSON.parse(workspace.services)
-            : workspace.services,
+        services: convertToJSON(workspace.services),
         userId: 1,
       }));
     }
diff --git a/src/internal-server/start/migrate.js b/src/internal-server/start/migrate.js
index 403f7cb86..0af33a977 100644
--- a/src/internal-server/start/migrate.js
+++ b/src/internal-server/start/migrate.js
@@ -1,3 +1,4 @@
+const { convertToJSON } = require('../../jsUtils');
 const { ferdiumVersion } = require('../../environment-remote');
 
 /**
@@ -26,10 +27,7 @@ module.exports = async () => {
     // Create new user
     await Database.raw('INSERT INTO  "users" ("id") VALUES (\'1\');');
   } else {
-    settings =
-      typeof user.settings === 'string'
-        ? JSON.parse(user.settings)
-        : user.settings;
+    settings = convertToJSON(user.settings);
   }
 
   if (
diff --git a/src/jsUtils.test.ts b/src/jsUtils.test.ts
new file mode 100644
index 000000000..651caee5f
--- /dev/null
+++ b/src/jsUtils.test.ts
@@ -0,0 +1,96 @@
+import * as jsUtils from './jsUtils'
+
+describe('jsUtils', () => {
+  describe('ifUndefinedString', () => {
+    it('returns the default value for undefined input', () => {
+      const result = jsUtils.ifUndefinedString(undefined, 'abc');
+      expect(result).toEqual('abc');
+    });
+
+    it('returns the default value for null input', () => {
+      const result = jsUtils.ifUndefinedString(null, 'abc');
+      expect(result).toEqual('abc');
+    });
+
+    it('returns the non-default input value for regular string input', () => {
+      const result = jsUtils.ifUndefinedString('some random string', 'abc');
+      expect(result).toEqual('some random string');
+    });
+  });
+
+  describe('ifUndefinedBoolean', () => {
+    it('returns the default value for undefined input', () => {
+      const result = jsUtils.ifUndefinedBoolean(undefined, false);
+      expect(result).toEqual(false);
+    });
+
+    it('returns the default value for null input', () => {
+      const result = jsUtils.ifUndefinedBoolean(null, true);
+      expect(result).toEqual(true);
+    });
+
+    it('returns the non-default input value for regular boolean input', () => {
+      const result = jsUtils.ifUndefinedBoolean(true, false);
+      expect(result).toEqual(true);
+    });
+  });
+
+  describe('ifUndefinedNumber', () => {
+    it('returns the default value for undefined input', () => {
+      const result = jsUtils.ifUndefinedNumber(undefined, 123);
+      expect(result).toEqual(123);
+    });
+
+    it('returns the default value for null input', () => {
+      const result = jsUtils.ifUndefinedNumber(null, 234);
+      expect(result).toEqual(234);
+    });
+
+    it('returns the non-default input value for regular Number input', () => {
+      const result = jsUtils.ifUndefinedNumber(1234, 5678);
+      expect(result).toEqual(1234);
+    });
+  });
+
+  describe('convertToJSON', () => {
+    it('returns undefined for undefined input', () => {
+      const result = jsUtils.convertToJSON(undefined);
+      expect(result).toEqual(undefined);
+    });
+
+    it('returns null for null input', () => {
+      const result = jsUtils.convertToJSON(null);
+      expect(result).toEqual(null);
+    });
+
+    it('returns the object for the object input', () => {
+      const result = jsUtils.convertToJSON(['a', 'b']);
+      expect(result).toEqual(['a', 'b']);
+    });
+
+    it('returns the parsed JSON for the string input', () => {
+      const result1 = jsUtils.convertToJSON('{"a":"b","c":"d"}');
+      expect(result1).toEqual({a: 'b', c: 'd'});
+
+      const result2 = jsUtils.convertToJSON('[{"a":"b"},{"c":"d"}]');
+      expect(result2).toEqual([{a: 'b'}, {c: 'd'}]);
+    });
+  });
+
+  describe('cleanseJSObject', () => {
+    xit('throws error for undefined input', () => {
+      const result = jsUtils.cleanseJSObject(undefined);
+      expect(result).toThrow();
+    });
+
+    xit('throws error for null input', () => {
+      const result = jsUtils.cleanseJSObject(null);
+      expect(result).toThrow();
+    });
+
+    it('returns cloned object for valid input', () => {
+      const result = jsUtils.cleanseJSObject([{a: 'b'}, {c: 'd'}]);
+      expect(result).toEqual([{a: 'b'}, {c: 'd'}]);
+    });
+  });
+});
diff --git a/src/jsUtils.ts b/src/jsUtils.ts
index d7ea4eb40..250d595eb 100644
--- a/src/jsUtils.ts
+++ b/src/jsUtils.ts
@@ -1,14 +1,9 @@
-export const ifUndefinedString = (
-  source: string | undefined | null,
-  defaultValue: string,
-): string => (source !== undefined && source !== null ? source : defaultValue);
-export const ifUndefinedBoolean = (
-  source: boolean | undefined | null,
-  defaultValue: boolean,
-): boolean =>
-  Boolean(source !== undefined && source !== null ? source : defaultValue);
-export const ifUndefinedNumber = (
-  source: number | undefined | null,
-  defaultValue: number,
-): number =>
-  Number(source !== undefined && source !== null ? source : defaultValue);
+export const ifUndefinedString = (source: string | undefined | null, defaultValue: string): string => (source !== undefined && source !== null ? source : defaultValue);
+
+export const ifUndefinedBoolean = (source: boolean | undefined | null, defaultValue: boolean): boolean => Boolean(source !== undefined && source !== null ? source : defaultValue);
+
+export const ifUndefinedNumber = (source: number | undefined | null, defaultValue: number): number => Number(source !== undefined && source !== null ? source : defaultValue);
+
+export const convertToJSON = (data: string | any | undefined | null) => data && typeof data === 'string' && data.length > 0 ? JSON.parse(data) : data
+
+export const cleanseJSObject = (data: any | undefined | null) => JSON.parse(JSON.stringify(data))
diff --git a/src/stores/AppStore.js b/src/stores/AppStore.js
index 76956fdc7..a9c92fe87 100644
--- a/src/stores/AppStore.js
+++ b/src/stores/AppStore.js
@@ -17,6 +17,7 @@ import { readJsonSync } from 'fs-extra';
 import Store from './lib/Store';
 import Request from './lib/Request';
 import { CHECK_INTERVAL, DEFAULT_APP_SETTINGS } from '../config';
+import { cleanseJSObject } from '../jsUtils';
 import { isMac, isWindows, electronVersion, osRelease } from '../environment';
 import { ferdiumVersion, userDataPath, ferdiumLocale } from '../environment-remote';
 import { generatedTranslations } from '../i18n/translations';
@@ -265,7 +266,7 @@ export default class AppStore extends Store {
   }
 
   @computed get debugInfo() {
-    const settings = JSON.parse(JSON.stringify(this.stores.settings.app));
+    const settings = cleanseJSObject(this.stores.settings.app);
     settings.lockedPassword = '******';
 
     return {
diff --git a/src/stores/ServicesStore.js b/src/stores/ServicesStore.js
index c8042e9de..3f551eddb 100644
--- a/src/stores/ServicesStore.js
+++ b/src/stores/ServicesStore.js
@@ -16,6 +16,7 @@ import {
 } from '../helpers/recipe-helpers';
 import { workspaceStore } from '../features/workspaces';
 import { DEFAULT_SERVICE_SETTINGS, KEEP_WS_LOADED_USID } from '../config';
+import { cleanseJSObject } from '../jsUtils';
 import { SPELLCHECKER_LOCALES } from '../i18n/languages';
 import { ferdiumVersion } from '../environment-remote';
 
@@ -861,7 +862,7 @@ export default class ServicesStore extends Store {
     const service = this.one(serviceId);
 
     // Make sure the args are clean, otherwise ElectronJS can't transmit them
-    const cleanArgs = JSON.parse(JSON.stringify(args));
+    const cleanArgs = cleanseJSObject(args);
 
     if (service.webview) {
       service.webview.send(channel, cleanArgs);
@@ -1273,9 +1274,7 @@ export default class ServicesStore extends Store {
 
     if (service.webview) {
       // We need to completely clone the object, otherwise Electron won't be able to send the object via IPC
-      const shareWithWebview = JSON.parse(
-        JSON.stringify(service.shareWithWebview),
-      );
+      const shareWithWebview = cleanseJSObject(service.shareWithWebview);
 
       debug('Initialize recipe', service.recipe.id, service.name);
       service.webview.send(
diff --git a/src/webview/lib/Userscript.ts b/src/webview/lib/Userscript.ts
index f02a8f135..b0e4fb805 100644
--- a/src/webview/lib/Userscript.ts
+++ b/src/webview/lib/Userscript.ts
@@ -1,3 +1,5 @@
+import { cleanseJSObject, convertToJSON } from '../../jsUtils';
+
 type Recipe = {
   setBadge: (direct: number, indirect: number) => void;
   setDialogTitle: (title: string) => void;
@@ -34,7 +36,7 @@ export default class Userscript {
   internal_setSettings(settings: any) {
     // This is needed to get a clean JS object from the settings itself to provide better accessibility
     // Otherwise this will be a mobX instance
-    this.settings = JSON.parse(JSON.stringify(settings));
+    this.settings = cleanseJSObject(settings);
   }
 
   /**
@@ -101,7 +103,7 @@ export default class Userscript {
     const ferdiumUserKey = window.localStorage.getItem(`ferdium-user-${key}`);
 
     if (ferdiumUserKey) {
-      return JSON.parse(ferdiumUserKey);
+      return convertToJSON(ferdiumUserKey);
     }
   }
 }