From 133773c03c6a4babd616552cc01d47356f2981f6 Mon Sep 17 00:00:00 2001
From: Oliver Hamlet <oliver.hamlet@gmail.com>
Date: Sat, 19 Dec 2015 23:34:38 +0000
Subject: [PATCH] Refactor loot.query into separate module

Also reimplement it so that it uses rest parameters, so no more
JSON.stringify is needed externally to prepare more complex queries.
---
 src/gui/html/index.html             |  1 +
 src/gui/html/js/events.js           | 83 +++++------------------------
 src/gui/html/js/helpers.js          | 23 +-------
 src/gui/html/js/query.js            | 36 +++++++++++++
 src/tests/gui/html/js/test.html     |  2 +
 src/tests/gui/html/js/test_query.js | 39 ++++++++++++++
 6 files changed, 91 insertions(+), 93 deletions(-)
 create mode 100644 src/gui/html/js/query.js
 create mode 100644 src/tests/gui/html/js/test_query.js

diff --git a/src/gui/html/index.html b/src/gui/html/index.html
index 277e7c9f..756a45f7 100644
--- a/src/gui/html/index.html
+++ b/src/gui/html/index.html
@@ -321,6 +321,7 @@
     <script src="js/helpers.js"></script>
     <script src="js/l10n.js"></script>
     <script src="js/plugin.js"></script>
+    <script src="js/query.js"></script>
     <script src="js/translator.js"></script>
     <script src="js/init.js"></script>
 </body>
diff --git a/src/gui/html/js/events.js b/src/gui/html/js/events.js
index 87378a27..a064f59f 100644
--- a/src/gui/html/js/events.js
+++ b/src/gui/html/js/events.js
@@ -77,14 +77,7 @@ function onPluginIsDirtyChange(evt) {
   }
 }
 function saveFilterState(evt) {
-    var request = JSON.stringify({
-        name: 'saveFilterState',
-        args: [
-            evt.target.id,
-            evt.target.checked,
-        ]
-    });
-    loot.query(request).catch(processCefError);
+    loot.query('saveFilterState', evt.target.id, evt.target.checked).catch(processCefError);
 }
 function onToggleDisplayCSS(evt) {
     var attr = 'data-hide-' + evt.target.getAttribute('data-class');
@@ -118,13 +111,7 @@ function onChangeGame(evt) {
 
     /* Send off a CEF query with the folder name of the new game. */
     showProgress(loot.l10n.translate('Loading game data...'));
-    var request = JSON.stringify({
-        name: 'changeGame',
-        args: [
-            evt.currentTarget.getAttribute('value')
-        ]
-    });
-    loot.query(request).then(function(result){
+    loot.query('changeGame', evt.currentTarget.getAttribute('value')).then(function(result){
         /* Filters should be re-applied on game change, except the conflicts
            filter. Don't need to deactivate the others beforehand. Strictly not
            deactivating the conflicts filter either, just resetting it's value.
@@ -274,13 +261,7 @@ function onApplySort(evt) {
     loot.game.plugins.forEach(function(plugin){
         loadOrder.push(plugin.name);
     });
-    var request = JSON.stringify({
-        name: 'applySort',
-        args: [
-            loadOrder
-        ]
-    });
-    return loot.query(request).then(function(result){
+    return loot.query('applySort', loadOrder).then(function(result){
         /* Remove old load order storage. */
         delete loot.game.loadOrder;
         delete loot.game.oldLoadOrder;
@@ -399,15 +380,10 @@ function onCopyContent(evt) {
         }
     }
 
-    var request = JSON.stringify({
-        name: 'copyContent',
-        args: [{
-            messages: messages,
-            plugins: plugins
-        }]
-    });
-
-    loot.query(request).then(function(){
+    loot.query('copyContent', {
+        messages: messages,
+        plugins: plugins
+    }).then(function(){
         toast(loot.l10n.translate("LOOT's content has been copied to the clipboard."));
     }).catch(processCefError);
 }
@@ -422,14 +398,7 @@ function onCopyLoadOrder(evt) {
         }
     }
 
-    var request = JSON.stringify({
-        name: 'copyLoadOrder',
-        args: [
-            plugins
-        ]
-    });
-
-    loot.query(request).then(function(){
+    loot.query('copyLoadOrder', plugins).then(function(){
         toast(loot.l10n.translate("The load order has been copied to the clipboard."));
     }).catch(processCefError);
 }
@@ -469,13 +438,7 @@ function onCloseSettingsDialog(evt) {
         };
 
         /* Send the settings back to the C++ side. */
-        var request = JSON.stringify({
-            name: 'closeSettings',
-            args: [
-                settings
-            ]
-        });
-        loot.query(request).then(function(result){
+        loot.query('closeSettings', settings).then(function(result){
 
             try {
                 setInstalledGames(JSON.parse(result));
@@ -547,14 +510,7 @@ function onEditorClose(evt) {
            majority of the work to the C++ side of things. */
 
         var edits = evt.target.readFromEditor(evt.target.data);
-
-        var request = JSON.stringify({
-            name: 'editorClosed',
-            args: [
-                edits
-            ]
-        });
-        promise = loot.query(request).then(JSON.parse).then(function(result){
+        promise = loot.query('editorClosed', edits).then(JSON.parse).then(function(result){
             if (result) {
                 evt.target.data.priority = result.priority;
                 evt.target.data.isPriorityGlobal = result.isPriorityGlobal;
@@ -633,29 +589,14 @@ function onConflictsFilter(evt) {
     setFilteredUIData(evt);
 }
 function onCopyMetadata(evt) {
-    /* evt.detail is the name of the plugin. */
-    var request = JSON.stringify({
-        name: 'copyMetadata',
-        args: [
-            evt.target.getName(),
-        ]
-    });
-
-    loot.query(request).then(function(){
+    loot.query('copyMetadata', evt.target.getName()).then(function(){
         toast(loot.l10n.translate('The metadata for "%s" has been copied to the clipboard.', evt.target.getName()));
     }).catch(processCefError);
 }
 function onClearMetadata(evt) {
     showMessageDialog('', loot.l10n.translate('Are you sure you want to clear all existing user-added metadata from "%s"?', evt.target.getName()), loot.l10n.translate('Clear'), function(result){
         if (result) {
-            var request = JSON.stringify({
-                name: 'clearPluginMetadata',
-                args: [
-                    evt.target.getName()
-                ]
-            });
-
-            loot.query(request).then(JSON.parse).then(function(result){
+            loot.query('clearPluginMetadata', evt.target.getName()).then(JSON.parse).then(function(result){
                 if (result) {
                     /* Need to empty the UI-side user metadata. */
                     for (var i = 0; i < loot.game.plugins.length; ++i) {
diff --git a/src/gui/html/js/helpers.js b/src/gui/html/js/helpers.js
index bea6e87b..23bec811 100644
--- a/src/gui/html/js/helpers.js
+++ b/src/gui/html/js/helpers.js
@@ -77,16 +77,9 @@ function getConflictingPlugins(pluginName)  {
   }
 
   /* Now get conflicts for the plugin. */
-  const request = JSON.stringify({
-    name: 'getConflictingPlugins',
-    args: [
-      pluginName,
-    ],
-  });
-
   showProgress(loot.l10n.translate('Checking if plugins have been loaded...'));
 
-  return loot.query(request).then(JSON.parse).then((result) => {
+  return loot.query('getConflictingPlugins', pluginName).then(JSON.parse).then((result) => {
     if (result) {
       /* Filter everything but the plugin itself if there are no
          conflicts. */
@@ -214,17 +207,3 @@ function updateSettingsUI() {
     updateEnabledGames(loot.installedGames);
     updateSelectedGame(loot.game.folder);
 }
-/* Returns a cefQuery as a Promise. */
-var loot = loot || {};
-loot.query = function query(request) {
-    return new Promise(function(resolve, reject) {
-        window.cefQuery({
-            request: request,
-            persistent: false,
-            onSuccess: resolve,
-            onFailure: function(errorCode, errorMessage) {
-                reject(Error('Error code: ' + errorCode + '; ' + errorMessage))
-            }
-        });
-    });
-}
diff --git a/src/gui/html/js/query.js b/src/gui/html/js/query.js
new file mode 100644
index 00000000..8c5c7696
--- /dev/null
+++ b/src/gui/html/js/query.js
@@ -0,0 +1,36 @@
+'use strict';
+(function exportModule(root, factory) {
+  if (typeof define === 'function' && define.amd) {
+    // AMD. Register as an anonymous module.
+    define([], factory);
+  } else {
+    // Browser globals
+    root.loot = root.loot || {};
+    root.loot.query = factory();
+  }
+}(this, () => {
+  return (requestName, ...args) => {
+    if (!requestName) {
+      throw new Error('No request name passed');
+    }
+    let request;
+    if (args.length === 0) {
+      request = requestName;
+    } else {
+      request = JSON.stringify({
+        name: requestName,
+        args,
+      });
+    }
+    return new Promise((resolve, reject) => {
+      window.cefQuery({
+        request,
+        persistent: false,
+        onSuccess: resolve,
+        onFailure: (errorCode, errorMessage) => {
+          reject(new Error('Error code: ' + errorCode + '; ' + errorMessage));
+        },
+      });
+    });
+  };
+}));
diff --git a/src/tests/gui/html/js/test.html b/src/tests/gui/html/js/test.html
index 77e945f6..44de2886 100644
--- a/src/tests/gui/html/js/test.html
+++ b/src/tests/gui/html/js/test.html
@@ -14,10 +14,12 @@
 <script src="../../../../gui/html/js/filters.js"></script>
 <script src="../../../../gui/html/js/game.js"></script>
 <script src="../../../../gui/html/js/plugin.js"></script>
+<script src="../../../../gui/html/js/query.js"></script>
 <script src="../../../../gui/html/js/translator.js"></script>
 <script src="test_filters.js"></script>
 <script src="test_game.js"></script>
 <script src="test_plugin.js"></script>
+<script src="test_query.js"></script>
 <script src="test_translator.js"></script>
 <script>
   onload = function(){
diff --git a/src/tests/gui/html/js/test_query.js b/src/tests/gui/html/js/test_query.js
new file mode 100644
index 00000000..7c4d0193
--- /dev/null
+++ b/src/tests/gui/html/js/test_query.js
@@ -0,0 +1,39 @@
+'use strict';
+
+/* Mock the window.cefQuery method */
+window.cefQuery = (obj) => {
+  if (obj.request === 'fail') {
+    obj.onFailure(-1, obj.request);
+  } else {
+    obj.onSuccess(obj.request);
+  }
+};
+
+describe('query()', () => {
+  it('should throw if no arguments are passed', () => {
+    (() => { loot.query(); }).should.throw();
+  });
+
+  it('should return a promise', () => {
+    loot.query('test').should.be.a.Promise(); // eslint-disable-line new-cap
+  });
+
+  it('should succeed if a request name is passed', () => {
+    return loot.query('test').should.be.fulfilledWith('test');
+  });
+
+  it('should succeed if a request name and arguments are passed', () => {
+    return loot.query('test', 1, false, ['a']).should.be.fulfilledWith(JSON.stringify({
+      name: 'test',
+      args: [
+        1,
+        false,
+        ['a'],
+      ],
+    }));
+  });
+
+  it('should fail with an Error object when an error occurs', () => {
+    return loot.query('fail').should.be.rejectedWith(Error, { message: 'Error code: -1; fail' });
+  });
+});