Merge pull request #1106 from remotestorage/refactorings

Refactorings

Author: Sebastian Kippe

src/dropbox.js

@@ -46,6 +46,7 @@ const isFolder = util.isFolder;
 const cleanPath = util.cleanPath;
 const shouldBeTreatedAsBinary = util.shouldBeTreatedAsBinary;
 const readBinaryData = util.readBinaryData;
+const getJSONFromLocalStorage = util.getJSONFromLocalStorage;
 
 /**
  * Map a local path to a path in Dropbox.
@@ -180,16 +181,11 @@ var Dropbox = function (rs) {
   hasLocalStorage = util.localStorageAvailable();
 
   if (hasLocalStorage){
-    var settings;
-    try {
-      settings = JSON.parse(localStorage.getItem(SETTINGS_KEY));
-    } catch(e) { /* ok to ignore, probably no data in localStorage */ }
+    const settings = getJSONFromLocalStorage(SETTINGS_KEY);
     if (settings) {
       this.configure(settings);
     }
-    try {
-      this._itemRefs = JSON.parse(localStorage.getItem(SETTINGS_KEY+':shares')) || {};
-    } catch(e) { /* ok to ignore, no shares in localStorage */ }
+    this._itemRefs = getJSONFromLocalStorage(`${SETTINGS_KEY}:shares`) || {};
   }
   if (this.connected) {
     setTimeout(this._emit.bind(this), 0, 'connected');

src/googledrive.js

@@ -32,6 +32,7 @@ const isFolder = util.isFolder;
 const cleanPath = util.cleanPath;
 const shouldBeTreatedAsBinary = util.shouldBeTreatedAsBinary;
 const readBinaryData = util.readBinaryData;
+const getJSONFromLocalStorage = util.getJSONFromLocalStorage;
 
 let hasLocalStorage;
 
@@ -92,6 +93,18 @@ function googleDrivePath (path) {
   return cleanPath(`${PATH_PREFIX}/${path}`);
 }
 
+/**
+ * Remove surrounding quotes from a string.
+ *
+ * @param {string} string - string with surrounding quotes
+ * @returns {string} string without surrounding quotes
+ *
+ * @private
+ */
+function removeQuotes (string) {
+  return string.replace(/^["'](.*)["']$/, "$1");
+}
+
 /**
  * Internal cache object for storing Google file IDs.
  *
@@ -129,12 +142,7 @@ const GoogleDrive = function (remoteStorage, clientId) {
   hasLocalStorage = util.localStorageAvailable();
 
   if (hasLocalStorage){
-    let settings;
-    try {
-      settings = JSON.parse(localStorage.getItem(SETTINGS_KEY));
-    } catch(e) {
-      // no settings stored
-    }
+    const settings = getJSONFromLocalStorage(SETTINGS_KEY);
     if (settings) {
       this.configure(settings);
     }
@@ -258,7 +266,7 @@ GoogleDrive.prototype = {
     function putDone(response) {
       if (response.status >= 200 && response.status < 300) {
         const meta = JSON.parse(response.responseText);
-        const etagWithoutQuotes = meta.etag.substring(1, meta.etag.length-1);
+        const etagWithoutQuotes = removeQuotes(meta.etag);
         return Promise.resolve({statusCode: 200, contentType: meta.mimeType, revision: etagWithoutQuotes});
       } else if (response.status === 412) {
         return Promise.resolve({statusCode: 412, revision: 'conflict'});
@@ -302,7 +310,7 @@ GoogleDrive.prototype = {
       return this._getMeta(id).then((meta) => {
         let etagWithoutQuotes;
         if ((typeof meta === 'object') && (typeof meta.etag === 'string')) {
-          etagWithoutQuotes = meta.etag.substring(1, meta.etag.length-1);
+          etagWithoutQuotes = removeQuotes(meta.etag);
         }
         if (options && options.ifMatch && (options.ifMatch !== etagWithoutQuotes)) {
           return {statusCode: 412, revision: etagWithoutQuotes};
@@ -430,14 +438,13 @@ GoogleDrive.prototype = {
       return this._getMeta(id).then((meta) => {
         let etagWithoutQuotes;
         if (typeof(meta) === 'object' && typeof(meta.etag) === 'string') {
-          etagWithoutQuotes = meta.etag.substring(1, meta.etag.length-1);
+          etagWithoutQuotes = removeQuotes(meta.etag);
         }
 
         if (options && options.ifNoneMatch && (etagWithoutQuotes === options.ifNoneMatch)) {
           return Promise.resolve({statusCode: 304});
         }
 
-        const options2 = {};
         if (!meta.downloadUrl) {
           if (meta.exportLinks && meta.exportLinks['text/html']) {
             // Documents that were generated inside GoogleDocs have no
@@ -450,7 +457,7 @@ GoogleDrive.prototype = {
           }
         }
 
-        return this._request('GET', meta.downloadUrl, options2).then((response) => {
+        return this._request('GET', meta.downloadUrl, {}).then((response) => {
           let body = response.response;
           if (meta.mimeType.match(/^application\/json/)) {
             try {
@@ -514,7 +521,7 @@ GoogleDrive.prototype = {
 
         itemsMap = {};
         for (const item of data.items) {
-          etagWithoutQuotes = item.etag.substring(1, item.etag.length-1);
+          etagWithoutQuotes = removeQuotes(item.etag);
           if (item.mimeType === GD_DIR_MIME_TYPE) {
             this._fileIdCache.set(path + item.title + '/', item.id);
             itemsMap[item.title + '/'] = {

src/remotestorage.js

@@ -12,6 +12,7 @@ const log = require('./log');
 const Features = require('./features');
 const globalContext = util.getGlobalContext();
 const eventHandling = require('./eventhandling');
+const getJSONFromLocalStorage = util.getJSONFromLocalStorage;
 
 var hasLocalStorage;
 
@@ -90,11 +91,7 @@ var RemoteStorage = function (cfg) {
   hasLocalStorage = util.localStorageAvailable();
 
   if (hasLocalStorage) {
-    try {
-      this.apiKeys = JSON.parse(localStorage.getItem('remotestorage:api-keys')) || {};
-    } catch(exc) {
-      // ignored
-    }
+    this.apiKeys = getJSONFromLocalStorage('remotestorage:api-keys') || {};
     this.setBackend(localStorage.getItem('remotestorage:backend') || 'remotestorage');
   }
 

src/util.js

@@ -205,6 +205,23 @@ var util = {
     }
   },
 
+  /**
+   * Extract and parse JSON data from localStorage.
+   *
+   * @param {string} key - localStorage key
+   *
+   * @returns {object} parsed object or undefined
+   */
+  getJSONFromLocalStorage (key) {
+    const context = util.getGlobalContext();
+
+    try {
+      return JSON.parse(context.localStorage.getItem(key));
+    } catch(e) {
+      // no JSON stored
+    }
+  },
+
   /**
    * Decide if data should be treated as binary based on the content
    * and content-type.

src/wireclient.js

@@ -75,6 +75,7 @@ if (typeof(ArrayBufferView) === 'function') {
 const isFolder = util.isFolder;
 const cleanPath = util.cleanPath;
 const shouldBeTreatedAsBinary = util.shouldBeTreatedAsBinary;
+const getJSONFromLocalStorage = util.getJSONFromLocalStorage;
 
 function addQuotes(str) {
   if (typeof(str) !== 'string') {
@@ -167,12 +168,7 @@ var WireClient = function WireClient(rs) {
   eventHandling(this, 'connected', 'not-connected');
 
   if (hasLocalStorage) {
-    var settings;
-    try {
-      settings = JSON.parse(localStorage[SETTINGS_KEY]);
-    } catch(e) {
-      // no settings stored
-    }
+    const settings = getJSONFromLocalStorage(SETTINGS_KEY);
     if (settings) {
       setTimeout(function () {
         this.configure(settings);

test/unit/util-suite.js

@@ -151,6 +151,34 @@ define(['./src/util'], function (util) {
 
           test.assert(util.localStorageAvailable(), false);
         }
+      },
+
+      {
+        desc: "getJSONFromLocalStorage returns the object for the given key",
+        run: function(env, test) {
+
+          global.localStorage = {
+            getItem: function() {
+              return '{ "foo": "bar" }';
+            }
+          };
+
+          test.assert(util.getJSONFromLocalStorage('somekey'), { foo: 'bar' });
+        }
+      },
+
+      {
+        desc: "getJSONFromLocalStorage returns null when there is no object for the given key",
+        run: function(env, test) {
+
+          global.localStorage = {
+            getItem: function() {
+              return null;
+            }
+          };
+
+          test.assert(util.getJSONFromLocalStorage('somekey'), null);
+        }
       }
 
     ]