From da459888dc2a7e38b3f301551e14f043cecf6c49 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Sun, 6 Sep 2020 15:27:18 -0400
Subject: [PATCH] plugins: Move plugin definitions to avoid monkey patching

Also document the plugin data structures.
---
 src/node/handler/PadMessageHandler.js    |  2 +-
 src/node/hooks/express/adminplugins.js   |  3 +-
 src/node/hooks/express/static.js         |  2 +-
 src/node/hooks/i18n.js                   |  2 +-
 src/node/server.js                       |  1 -
 src/node/utils/Minify.js                 |  2 +-
 src/node/utils/tar.json                  |  1 +
 src/static/js/ace.js                     | 15 ++++++--
 src/static/js/pluginfw/client_plugins.js | 35 +++++++----------
 src/static/js/pluginfw/hooks.js          | 49 ++++++------------------
 src/static/js/pluginfw/plugin_defs.js    | 24 ++++++++++++
 src/static/js/pluginfw/plugins.js        | 29 +++++++-------
 src/static/js/pluginfw/shared.js         | 23 +++++++++++
 src/templates/pad.html                   |  6 +--
 src/templates/timeslider.html            |  2 -
 15 files changed, 104 insertions(+), 92 deletions(-)
 create mode 100644 src/static/js/pluginfw/plugin_defs.js

diff --git a/src/node/handler/PadMessageHandler.js b/src/node/handler/PadMessageHandler.js
index af5ba4d84..14540820f 100644
--- a/src/node/handler/PadMessageHandler.js
+++ b/src/node/handler/PadMessageHandler.js
@@ -27,7 +27,7 @@ var authorManager = require("../db/AuthorManager");
 var readOnlyManager = require("../db/ReadOnlyManager");
 var settings = require('../utils/Settings');
 var securityManager = require("../db/SecurityManager");
-var plugins = require("ep_etherpad-lite/static/js/pluginfw/plugins.js");
+var plugins = require("ep_etherpad-lite/static/js/pluginfw/plugin_defs.js");
 var log4js = require('log4js');
 var messageLogger = log4js.getLogger("message");
 var accessLogger = log4js.getLogger("access");
diff --git a/src/node/hooks/express/adminplugins.js b/src/node/hooks/express/adminplugins.js
index 983d29ea6..f6f184ed3 100644
--- a/src/node/hooks/express/adminplugins.js
+++ b/src/node/hooks/express/adminplugins.js
@@ -1,14 +1,13 @@
 var eejs = require('ep_etherpad-lite/node/eejs');
 var settings = require('ep_etherpad-lite/node/utils/Settings');
 var installer = require('ep_etherpad-lite/static/js/pluginfw/installer');
-var plugins = require('ep_etherpad-lite/static/js/pluginfw/plugins');
+var plugins = require('ep_etherpad-lite/static/js/pluginfw/plugin_defs');
 var _ = require('underscore');
 var semver = require('semver');
 const UpdateCheck = require('ep_etherpad-lite/node/utils/UpdateCheck');
 
 exports.expressCreateServer = function(hook_name, args, cb) {
   args.app.get('/admin/plugins', function(req, res) {
-    var plugins = require("ep_etherpad-lite/static/js/pluginfw/plugins");
     var render_args = {
       plugins: plugins.plugins,
       search_results: {},
diff --git a/src/node/hooks/express/static.js b/src/node/hooks/express/static.js
index 4c17fbe3b..b8c6c9d52 100644
--- a/src/node/hooks/express/static.js
+++ b/src/node/hooks/express/static.js
@@ -1,5 +1,5 @@
 var minify = require('../../utils/Minify');
-var plugins = require("ep_etherpad-lite/static/js/pluginfw/plugins");
+var plugins = require("ep_etherpad-lite/static/js/pluginfw/plugin_defs");
 var CachingMiddleware = require('../../utils/caching_middleware');
 var settings = require("../../utils/Settings");
 var Yajsml = require('etherpad-yajsml');
diff --git a/src/node/hooks/i18n.js b/src/node/hooks/i18n.js
index 902ef3130..2265978bf 100644
--- a/src/node/hooks/i18n.js
+++ b/src/node/hooks/i18n.js
@@ -3,7 +3,7 @@ var languages = require('languages4translatewiki')
   , path = require('path')
   , _ = require('underscore')
   , npm = require('npm')
-  , plugins = require('ep_etherpad-lite/static/js/pluginfw/plugins.js').plugins
+  , plugins = require('ep_etherpad-lite/static/js/pluginfw/plugin_defs.js').plugins
   , semver = require('semver')
   , existsSync = require('../utils/path_exists')
   , settings = require('../utils/Settings')
diff --git a/src/node/server.js b/src/node/server.js
index be595173a..a1f62df4f 100755
--- a/src/node/server.js
+++ b/src/node/server.js
@@ -61,7 +61,6 @@ npm.load({}, function() {
   var db = require('./db/DB');
   var plugins = require("ep_etherpad-lite/static/js/pluginfw/plugins");
   var hooks = require("ep_etherpad-lite/static/js/pluginfw/hooks");
-  hooks.plugins = plugins;
 
   db.init()
     .then(plugins.update)
diff --git a/src/node/utils/Minify.js b/src/node/utils/Minify.js
index 3480ae762..a4194eb9e 100644
--- a/src/node/utils/Minify.js
+++ b/src/node/utils/Minify.js
@@ -26,7 +26,7 @@ var fs = require('fs');
 var StringDecoder = require('string_decoder').StringDecoder;
 var CleanCSS = require('clean-css');
 var path = require('path');
-var plugins = require("ep_etherpad-lite/static/js/pluginfw/plugins");
+var plugins = require("ep_etherpad-lite/static/js/pluginfw/plugin_defs");
 var RequireKernel = require('etherpad-require-kernel');
 var urlutil = require('url');
 var mime = require('mime-types')
diff --git a/src/node/utils/tar.json b/src/node/utils/tar.json
index efb346895..1c1102f46 100644
--- a/src/node/utils/tar.json
+++ b/src/node/utils/tar.json
@@ -72,6 +72,7 @@
   , "security.js"
   , "$security.js"
   , "pluginfw/client_plugins.js"
+  , "pluginfw/plugin_defs.js"
   , "pluginfw/shared.js"
   , "pluginfw/hooks.js"
   ]
diff --git a/src/static/js/ace.js b/src/static/js/ace.js
index 5dbf24545..17834e435 100644
--- a/src/static/js/ace.js
+++ b/src/static/js/ace.js
@@ -21,7 +21,6 @@
  */
 
 // requires: top
-// requires: plugins
 // requires: undefined
 
 var KERNEL_SOURCE = '../static/js/require-kernel.js';
@@ -31,6 +30,7 @@ Ace2Editor.registry = {
 };
 
 var hooks = require('./pluginfw/hooks');
+var pluginUtils = require('./pluginfw/shared');
 var _ = require('./underscore');
 
 function scriptTag(source) {
@@ -263,9 +263,7 @@ require.setRootURI("../javascripts/src");\n\
 require.setLibraryURI("../javascripts/lib");\n\
 require.setGlobalKeyPath("require");\n\
 \n\
-var hooks = require("ep_etherpad-lite/static/js/pluginfw/hooks");\n\
 var plugins = require("ep_etherpad-lite/static/js/pluginfw/client_plugins");\n\
-hooks.plugins = plugins;\n\
 plugins.adoptPluginsFromAncestorsOf(window);\n\
 \n\
 $ = jQuery = require("ep_etherpad-lite/static/js/rjquery").jQuery; // Expose jQuery #HACK\n\
@@ -337,7 +335,16 @@ window.onload = function () {\n\
 
       // bizarrely, in FF2, a file with no "external" dependencies won't finish loading properly
       // (throbs busy while typing)
-      outerHTML.push('<style type="text/css" title="dynamicsyntax"></style>', '<link rel="stylesheet" type="text/css" href="data:text/css,"/>', scriptTag(outerScript), '</head><body id="outerdocbody" class="outerdocbody ', hooks.clientPluginNames().join(' '),'"><div id="sidediv" class="sidediv"><!-- --></div><div id="linemetricsdiv">x</div></body></html>');
+      var pluginNames = pluginUtils.clientPluginNames();
+      outerHTML.push(
+          '<style type="text/css" title="dynamicsyntax"></style>',
+          '<link rel="stylesheet" type="text/css" href="data:text/css,"/>',
+          scriptTag(outerScript),
+          '</head>',
+          '<body id="outerdocbody" class="outerdocbody ', pluginNames.join(' '), '">',
+          '<div id="sidediv" class="sidediv"><!-- --></div>',
+          '<div id="linemetricsdiv">x</div>',
+          '</body></html>');
 
       var outerFrame = document.createElement("IFRAME");
       outerFrame.name = "ace_outer";
diff --git a/src/static/js/pluginfw/client_plugins.js b/src/static/js/pluginfw/client_plugins.js
index 126dc3f08..ab12eeb47 100644
--- a/src/static/js/pluginfw/client_plugins.js
+++ b/src/static/js/pluginfw/client_plugins.js
@@ -3,15 +3,12 @@ $ = jQuery = require("ep_etherpad-lite/static/js/rjquery").$;
 var _ = require("underscore");
 
 var pluginUtils = require('./shared');
+var defs = require('./plugin_defs');
 
-exports.loaded = false;
-exports.plugins = {};
-exports.parts = [];
-exports.hooks = {};
 exports.baseURL = '';
 
 exports.ensure = function (cb) {
-  if (!exports.loaded)
+  if (!defs.loaded)
     exports.update(cb);
   else
     cb();
@@ -24,10 +21,10 @@ exports.update = function (cb) {
   var callback = function () {setTimeout(cb, 0);};
   $.ajaxSetup({ cache: false });
   jQuery.getJSON(exports.baseURL + 'pluginfw/plugin-definitions.json').done(function(data) {
-    exports.plugins = data.plugins;
-    exports.parts = data.parts;
-    exports.hooks = pluginUtils.extractHooks(exports.parts, "client_hooks");
-    exports.loaded = true;
+    defs.plugins = data.plugins;
+    defs.parts = data.parts;
+    defs.hooks = pluginUtils.extractHooks(defs.parts, "client_hooks");
+    defs.loaded = true;
     callback();
   }).fail(function(e){
     console.error("Failed to load plugin-definitions: " + err);
@@ -35,16 +32,6 @@ exports.update = function (cb) {
   });
 };
 
-function adoptPlugins(plugins) {
-  var keys = [
-      'loaded', 'plugins', 'parts', 'hooks', 'baseURL', 'ensure', 'update'];
-
-  for (var i = 0, ii = keys.length; i < ii; i++) {
-    var key = keys[i];
-    exports[key] = plugins[key];
-  }
-}
-
 function adoptPluginsFromAncestorsOf(frame) {
   // Bind plugins with parent;
   var parentRequire = null;
@@ -59,12 +46,18 @@ function adoptPluginsFromAncestorsOf(frame) {
     // Silence (this can only be a XDomain issue).
   }
   if (parentRequire) {
+    var ancestorPluginDefs = parentRequire("ep_etherpad-lite/static/js/pluginfw/plugin_defs");
+    defs.hooks = ancestorPluginDefs.hooks;
+    defs.loaded = ancestorPluginDefs.loaded;
+    defs.parts = ancestorPluginDefs.parts;
+    defs.plugins = ancestorPluginDefs.plugins;
     var ancestorPlugins = parentRequire("ep_etherpad-lite/static/js/pluginfw/client_plugins");
-    exports.adoptPlugins(ancestorPlugins);
+    exports.baseURL = ancestorPlugins.baseURL;
+    exports.ensure = ancestorPlugins.ensure;
+    exports.update = ancestorPlugins.update;
   } else {
     throw new Error("Parent plugins could not be found.")
   }
 }
 
-exports.adoptPlugins = adoptPlugins;
 exports.adoptPluginsFromAncestorsOf = adoptPluginsFromAncestorsOf;
diff --git a/src/static/js/pluginfw/hooks.js b/src/static/js/pluginfw/hooks.js
index 7bc8dd2b7..3b6be4d9a 100644
--- a/src/static/js/pluginfw/hooks.js
+++ b/src/static/js/pluginfw/hooks.js
@@ -1,4 +1,5 @@
 var _ = require("underscore");
+var pluginDefs = require('./plugin_defs');
 
 // Maps the name of a server-side hook to a string explaining the deprecation
 // (e.g., 'use the foo hook instead').
@@ -92,20 +93,18 @@ exports.flatten = function (lst) {
 
 exports.callAll = function (hook_name, args) {
   if (!args) args = {};
-  if (exports.plugins){
-    if (exports.plugins.hooks[hook_name] === undefined) return [];
-    return _.flatten(_.map(exports.plugins.hooks[hook_name], function (hook) {
-      return hookCallWrapper(hook, hook_name, args);
-    }), true);
-  }
+  if (pluginDefs.hooks[hook_name] === undefined) return [];
+  return _.flatten(_.map(pluginDefs.hooks[hook_name], function(hook) {
+    return hookCallWrapper(hook, hook_name, args);
+  }), true);
 }
 
 async function aCallAll(hook_name, args, cb) {
   if (!args) args = {};
   if (!cb) cb = function () {};
-  if (exports.plugins.hooks[hook_name] === undefined) return cb(null, []);
+  if (pluginDefs.hooks[hook_name] === undefined) return cb(null, []);
 
-  var hooksPromises = exports.plugins.hooks[hook_name].map(async function(hook, index){
+  var hooksPromises = pluginDefs.hooks[hook_name].map(async function(hook, index) {
     return await hookCallWrapper(hook, hook_name, args, function (res) {
       return Promise.resolve(res);
     });
@@ -137,8 +136,8 @@ exports.aCallAll = function (hook_name, args, cb) {
 
 exports.callFirst = function (hook_name, args) {
   if (!args) args = {};
-  if (exports.plugins.hooks[hook_name] === undefined) return [];
-  return exports.syncMapFirst(exports.plugins.hooks[hook_name], function (hook) {
+  if (pluginDefs.hooks[hook_name] === undefined) return [];
+  return exports.syncMapFirst(pluginDefs.hooks[hook_name], function(hook) {
     return hookCallWrapper(hook, hook_name, args);
   });
 }
@@ -146,9 +145,9 @@ exports.callFirst = function (hook_name, args) {
 function aCallFirst(hook_name, args, cb) {
   if (!args) args = {};
   if (!cb) cb = function () {};
-  if (exports.plugins.hooks[hook_name] === undefined) return cb(null, []);
+  if (pluginDefs.hooks[hook_name] === undefined) return cb(null, []);
   exports.mapFirst(
-    exports.plugins.hooks[hook_name],
+    pluginDefs.hooks[hook_name],
     function (hook, cb) {
       hookCallWrapper(hook, hook_name, args, function (res) { cb(null, res); });
     },
@@ -180,29 +179,3 @@ exports.callAllStr = function(hook_name, args, sep, pre, post) {
   }
   return newCallhooks.join(sep || "");
 }
-
-/*
- * Returns an array containing the names of the installed client-side plugins
- *
- * If no client-side plugins are installed, returns an empty array.
- * Duplicate names are always discarded.
- *
- * A client-side plugin is a plugin that implements at least one client_hook
- *
- * EXAMPLE:
- *   No plugins:   []
- *   Some plugins: [ 'ep_adminpads', 'ep_add_buttons', 'ep_activepads' ]
- */
-exports.clientPluginNames = function() {
-  if (!(exports.plugins)) {
-    return [];
-  }
-
-  var client_plugin_names = _.uniq(
-    exports.plugins.parts
-      .filter(function(part) { return part.hasOwnProperty('client_hooks'); })
-      .map(function(part) { return 'plugin-' + part['plugin']; })
-  );
-
-  return client_plugin_names;
-}
diff --git a/src/static/js/pluginfw/plugin_defs.js b/src/static/js/pluginfw/plugin_defs.js
new file mode 100644
index 000000000..95bbcb95c
--- /dev/null
+++ b/src/static/js/pluginfw/plugin_defs.js
@@ -0,0 +1,24 @@
+// This module contains processed plugin definitions. The data structures in this file are set by
+// plugins.js (server) or client_plugins.js (client).
+
+// Maps a hook name to a list of hook objects. Each hook object has the following properties:
+//   * hook_name: Name of the hook.
+//   * hook_fn: Plugin-supplied hook function.
+//   * hook_fn_name: Name of the hook function, with the form <filename>:<functionName>.
+//   * part: The ep.json part object that declared the hook. See exports.plugins.
+exports.hooks = {};
+
+// Whether the plugins have been loaded.
+exports.loaded = false;
+
+// Topologically sorted list of parts from exports.plugins.
+exports.parts = [];
+
+// Maps the name of a plugin to the plugin's definition provided in ep.json. The ep.json object is
+// augmented with additional metadata:
+//   * parts: Each part from the ep.json object is augmented with the following properties:
+//       - plugin: The name of the plugin.
+//       - full_name: Equal to <plugin>/<name>.
+//   * package (server-side only): Object containing details about the plugin package (version,
+//     path).
+exports.plugins = {};
diff --git a/src/static/js/pluginfw/plugins.js b/src/static/js/pluginfw/plugins.js
index bb34e2107..0ea55b403 100644
--- a/src/static/js/pluginfw/plugins.js
+++ b/src/static/js/pluginfw/plugins.js
@@ -8,28 +8,25 @@ var _ = require("underscore");
 var settings = require('../../../node/utils/Settings');
 
 var pluginUtils = require('./shared');
+var defs = require('./plugin_defs');
 
 exports.prefix = 'ep_';
-exports.loaded = false;
-exports.plugins = {};
-exports.parts = [];
-exports.hooks = {};
 
 // @TODO RPB this appears to be unused
 exports.ensure = function (cb) {
-  if (!exports.loaded)
+  if (!defs.loaded)
     exports.update(cb);
   else
     cb();
 };
 
 exports.formatPlugins = function () {
-  return _.keys(exports.plugins).join(", ");
+  return _.keys(defs.plugins).join(", ");
 };
 
 exports.formatPluginsWithVersion = function () {
   var plugins = [];
-  _.forEach(exports.plugins, function(plugin){
+  _.forEach(defs.plugins, function(plugin) {
     if(plugin.package.name !== "ep_etherpad-lite"){
       var pluginStr = plugin.package.name + "@" + plugin.package.version;
       plugins.push(pluginStr);
@@ -39,12 +36,12 @@ exports.formatPluginsWithVersion = function () {
 };
 
 exports.formatParts = function () {
-  return _.map(exports.parts, function (part) { return part.full_name; }).join("\n");
+  return _.map(defs.parts, function(part) { return part.full_name; }).join('\n');
 };
 
 exports.formatHooks = function (hook_set_name) {
   var res = [];
-  var hooks = pluginUtils.extractHooks(exports.parts, hook_set_name || "hooks");
+  var hooks = pluginUtils.extractHooks(defs.parts, hook_set_name || 'hooks');
 
   _.chain(hooks).keys().forEach(function (hook_name) {
     _.forEach(hooks[hook_name], function (hook) {
@@ -60,8 +57,8 @@ exports.callInit = function () {
 
   var hooks = require("./hooks");
 
-  let p = Object.keys(exports.plugins).map(function (plugin_name) {
-    let plugin = exports.plugins[plugin_name];
+  let p = Object.keys(defs.plugins).map(function(plugin_name) {
+    let plugin = defs.plugins[plugin_name];
     let ep_init = path.normalize(path.join(plugin.package.path, ".ep_initialized"));
     return fsp_stat(ep_init).catch(async function() {
       await fsp_writeFile(ep_init, "done");
@@ -75,7 +72,7 @@ exports.callInit = function () {
 exports.pathNormalization = function (part, hook_fn_name, hook_name) {
   const parts = hook_fn_name.split(':');
   const functionName = (parts.length > 1) ? parts.pop() : hook_name;
-  const packageDir = path.dirname(exports.plugins[part.plugin].package.path);
+  const packageDir = path.dirname(defs.plugins[part.plugin].package.path);
   const fileName = path.normalize(path.join(packageDir, parts.join(':')));
   return fileName + ((functionName == null) ? '' : (':' + functionName));
 }
@@ -91,10 +88,10 @@ exports.update = async function () {
   });
 
   return Promise.all(p).then(function() {
-    exports.plugins = plugins;
-    exports.parts = sortParts(parts);
-    exports.hooks = pluginUtils.extractHooks(exports.parts, "hooks", exports.pathNormalization);
-    exports.loaded = true;
+    defs.plugins = plugins;
+    defs.parts = sortParts(parts);
+    defs.hooks = pluginUtils.extractHooks(defs.parts, 'hooks', exports.pathNormalization);
+    defs.loaded = true;
   }).then(exports.callInit);
 }
 
diff --git a/src/static/js/pluginfw/shared.js b/src/static/js/pluginfw/shared.js
index a9971e939..cf91a9005 100644
--- a/src/static/js/pluginfw/shared.js
+++ b/src/static/js/pluginfw/shared.js
@@ -1,4 +1,5 @@
 var _ = require("underscore");
+var defs = require('./plugin_defs');
 
 function loadFn(path, hookName) {
   var functionName
@@ -59,3 +60,25 @@ function extractHooks(parts, hook_set_name, normalizer) {
 };
 
 exports.extractHooks = extractHooks;
+
+/*
+ * Returns an array containing the names of the installed client-side plugins
+ *
+ * If no client-side plugins are installed, returns an empty array.
+ * Duplicate names are always discarded.
+ *
+ * A client-side plugin is a plugin that implements at least one client_hook
+ *
+ * EXAMPLE:
+ *   No plugins:   []
+ *   Some plugins: [ 'ep_adminpads', 'ep_add_buttons', 'ep_activepads' ]
+ */
+exports.clientPluginNames = function() {
+  var client_plugin_names = _.uniq(
+    defs.parts
+      .filter(function(part) { return part.hasOwnProperty('client_hooks'); })
+      .map(function(part) { return 'plugin-' + part['plugin']; })
+  );
+
+  return client_plugin_names;
+}
diff --git a/src/templates/pad.html b/src/templates/pad.html
index a23892d07..bd718c77d 100644
--- a/src/templates/pad.html
+++ b/src/templates/pad.html
@@ -1,13 +1,13 @@
 <%
   var settings = require("ep_etherpad-lite/node/utils/Settings")
-    , hooks = require('ep_etherpad-lite/static/js/pluginfw/hooks')
     , langs = require("ep_etherpad-lite/node/hooks/i18n").availableLangs
+    , pluginUtils = require('ep_etherpad-lite/static/js/pluginfw/shared')
     ;
 %>
 <!doctype html>
 
 <% e.begin_block("htmlHead"); %>
-<html class="<%=hooks.clientPluginNames().join(' '); %> <%=settings.skinVariants%>">
+<html class="<%=pluginUtils.clientPluginNames().join(' '); %> <%=settings.skinVariants%>">
 <% e.end_block(); %>
 
   <title><%=settings.title%></title>
@@ -488,8 +488,6 @@
 
         plugins.baseURL = baseURL;
         plugins.update(function () {
-          hooks.plugins = plugins;
-
           // Call documentReady hook
           $(function() {
             hooks.aCallAll('documentReady');
diff --git a/src/templates/timeslider.html b/src/templates/timeslider.html
index ec0c7af1b..82da26254 100644
--- a/src/templates/timeslider.html
+++ b/src/templates/timeslider.html
@@ -283,8 +283,6 @@
 
     plugins.update(function () {
       var hooks = require('ep_etherpad-lite/static/js/pluginfw/hooks');
-      hooks.plugins = plugins;
-
       var timeslider = require('ep_etherpad-lite/static/js/timeslider')
       timeslider.baseURL = baseURL;
       timeslider.init();