Changeset: Migrate to the new attribute API

Author: rhansen

CHANGELOG.md

@@ -8,6 +8,18 @@
   (low-level API) and `ep_etherpad-lite/static/js/AttributeMap` (high-level
   API).
 
+### Compatibility changes
+
+#### For plugin authors
+
+* Changes to the `src/static/js/Changeset.js` library:
+  * The following attribute processing functions are deprecated (use the new
+    attribute APIs instead):
+    * `attribsAttributeValue()`
+    * `eachAttribNumber()`
+    * `makeAttribsString()`
+    * `opAttributeValue()`
+
 # 1.8.15
 
 ### Security fixes

doc/api/hooks_server-side.md

@@ -665,14 +665,15 @@ Context properties:
 Example:
 
 ```javascript
+const AttributeMap = require('ep_etherpad-lite/static/js/AttributeMap');
 const Changeset = require('ep_etherpad-lite/static/js/Changeset');
 
 exports.getLineHTMLForExport = async (hookName, context) => {
   if (!context.attribLine) return;
   const opIter = Changeset.opIterator(context.attribLine);
   if (!opIter.hasNext()) return;
   const op = opIter.next();
-  const heading = Changeset.opAttributeValue(op, 'heading', apool);
+  const heading = AttributeMap.fromString(op.attribs, context.apool).get('heading');
   if (!heading) return;
   context.lineContent = `<${heading}>${context.lineContent}</${heading}>`;
 };

src/node/handler/PadMessageHandler.js

@@ -19,6 +19,7 @@
  * limitations under the License.
  */
 
+const AttributeMap = require('../../static/js/AttributeMap');
 const padManager = require('../db/PadManager');
 const Changeset = require('../../static/js/Changeset');
 const ChatMessage = require('../../static/js/ChatMessage');
@@ -32,7 +33,6 @@ const plugins = require('../../static/js/pluginfw/plugin_defs.js');
 const log4js = require('log4js');
 const messageLogger = log4js.getLogger('message');
 const accessLogger = log4js.getLogger('access');
-const _ = require('underscore');
 const hooks = require('../../static/js/pluginfw/hooks.js');
 const stats = require('../stats');
 const assert = require('assert').strict;
@@ -585,14 +585,6 @@ const handleUserChanges = async (socket, message) => {
     // Verify that the changeset has valid syntax and is in canonical form
     Changeset.checkRep(changeset);
 
-    // Verify that the attribute indexes used in the changeset are all
-    // defined in the accompanying attribute pool.
-    Changeset.eachAttribNumber(changeset, (n) => {
-      if (!wireApool.getAttrib(n)) {
-        throw new Error(`Attribute pool is missing attribute ${n} for changeset ${changeset}`);
-      }
-    });
-
     // Validate all added 'author' attribs to be the same value as the current user
     const iterator = Changeset.opIterator(Changeset.unpack(changeset).ops);
     let op;
@@ -605,19 +597,14 @@ const handleUserChanges = async (socket, message) => {
       // - can have attribs, but they are discarded and don't show up in the attribs -
       // but do show up in the pool
 
-      op.attribs.split('*').forEach((attr) => {
-        if (!attr) return;
-
-        attr = wireApool.getAttrib(Changeset.parseNum(attr));
-        if (!attr) return;
-
-        // the empty author is used in the clearAuthorship functionality so this
-        // should be the only exception
-        if ('author' === attr[0] && (attr[1] !== thisSession.author && attr[1] !== '')) {
-          throw new Error(`Author ${thisSession.author} tried to submit changes as author ` +
-                          `${attr[1]} in changeset ${changeset}`);
-        }
-      });
+      // Besides verifying the author attribute, this serves a second purpose:
+      // AttributeMap.fromString() ensures that all attribute numbers are valid (it will throw if
+      // an attribute number isn't in the pool).
+      const opAuthorId = AttributeMap.fromString(op.attribs, wireApool).get('author');
+      if (opAuthorId && opAuthorId !== thisSession.author) {
+        throw new Error(`Author ${thisSession.author} tried to submit changes as author ` +
+                        `${opAuthorId} in changeset ${changeset}`);
+      }
     }
 
     // ex. adoptChangesetAttribs
@@ -758,11 +745,8 @@ const _correctMarkersInPad = (atext, apool) => {
   let offset = 0;
   while (iter.hasNext()) {
     const op = iter.next();
-
-    const hasMarker = _.find(
-        AttributeManager.lineAttributes,
-        (attribute) => Changeset.opAttributeValue(op, attribute, apool)) !== undefined;
-
+    const attribs = AttributeMap.fromString(op.attribs, apool);
+    const hasMarker = AttributeManager.lineAttributes.some((a) => attribs.has(a));
     if (hasMarker) {
       for (let i = 0; i < op.chars; i++) {
         if (offset > 0 && text.charAt(offset - 1) !== '\n') {

src/node/utils/ExportHelper.js

@@ -19,6 +19,7 @@
  * limitations under the License.
  */
 
+const AttributeMap = require('../../static/js/AttributeMap');
 const Changeset = require('../../static/js/Changeset');
 
 exports.getPadPlainText = (pad, revNum) => {
@@ -54,7 +55,8 @@ exports._analyzeLine = (text, aline, apool) => {
     const opIter = Changeset.opIterator(aline);
     if (opIter.hasNext()) {
       const op = opIter.next();
-      let listType = Changeset.opAttributeValue(op, 'list', apool);
+      const attribs = AttributeMap.fromString(op.attribs, apool);
+      let listType = attribs.get('list');
       if (listType) {
         lineMarker = 1;
         listType = /([a-z]+)([0-9]+)/.exec(listType);
@@ -63,7 +65,7 @@ exports._analyzeLine = (text, aline, apool) => {
           line.listLevel = Number(listType[2]);
         }
       }
-      const start = Changeset.opAttributeValue(op, 'start', apool);
+      const start = attribs.get('start');
       if (start) {
         line.start = start;
       }

src/node/utils/ExportHtml.js

@@ -16,6 +16,7 @@
  */
 
 const Changeset = require('../../static/js/Changeset');
+const attributes = require('../../static/js/attributes');
 const padManager = require('../db/PadManager');
 const _ = require('underscore');
 const Security = require('../../static/js/security');
@@ -206,11 +207,11 @@ const getHTMLFromAtext = async (pad, atext, authorColors) => {
         const usedAttribs = [];
 
         // mark all attribs as used
-        Changeset.eachAttribNumber(o.attribs, (a) => {
+        for (const a of attributes.decodeAttribString(o.attribs)) {
           if (a in anumMap) {
             usedAttribs.push(anumMap[a]); // i = 0 => bold, etc.
           }
-        });
+        }
         let outermostTag = -1;
         // find the outer most open tag that is no longer used
         for (let i = openTags.length - 1; i >= 0; i--) {

src/node/utils/ExportTxt.js

@@ -20,6 +20,7 @@
  */
 
 const Changeset = require('../../static/js/Changeset');
+const attributes = require('../../static/js/attributes');
 const padManager = require('../db/PadManager');
 const _analyzeLine = require('./ExportHelper')._analyzeLine;
 
@@ -82,7 +83,7 @@ const getTXTFromAtext = (pad, atext, authorColors) => {
         const o = iter.next();
         let propChanged = false;
 
-        Changeset.eachAttribNumber(o.attribs, (a) => {
+        for (const a of attributes.decodeAttribString(o.attribs)) {
           if (a in anumMap) {
             const i = anumMap[a]; // i = 0 => bold, etc.
 
@@ -93,7 +94,7 @@ const getTXTFromAtext = (pad, atext, authorColors) => {
               propVals[i] = STAY;
             }
           }
-        });
+        }
 
         for (let i = 0; i < propVals.length; i++) {
           if (propVals[i] === true) {

src/node/utils/padDiff.js

@@ -1,5 +1,8 @@
 'use strict';
+
+const AttributeMap = require('../../static/js/AttributeMap');
 const Changeset = require('../../static/js/Changeset');
+const attributes = require('../../static/js/attributes');
 const exportHtml = require('./ExportHtml');
 
 function PadDiff(pad, fromRev, toRev) {
@@ -54,17 +57,11 @@ PadDiff.prototype._isClearAuthorship = function (changeset) {
     return false;
   }
 
-  const attributes = [];
-  Changeset.eachAttribNumber(changeset, (attrNum) => {
-    attributes.push(attrNum);
-  });
-
-  // check that this changeset uses only one attribute
-  if (attributes.length !== 1) {
-    return false;
-  }
+  const [appliedAttribute, anotherAttribute] =
+      attributes.attribsFromString(clearOperator.attribs, this._pad.pool);
 
-  const appliedAttribute = this._pad.pool.getAttrib(attributes[0]);
+  // Check that the operation has exactly one attribute.
+  if (appliedAttribute == null || anotherAttribute != null) return false;
 
   // check if the applied attribute is an anonymous author attribute
   if (appliedAttribute[0] !== 'author' || appliedAttribute[1] !== '') {
@@ -376,27 +373,19 @@ PadDiff.prototype._createDeletionChangeset = function (cs, startAText, apool) {
       // If the text this operator applies to is only a star,
       // than this is a false positive and should be ignored
       if (csOp.attribs && textBank !== '*') {
-        const deletedAttrib = apool.putAttrib(['removed', true]);
-        let authorAttrib = apool.putAttrib(['author', '']);
-        const attribs = [];
-        Changeset.eachAttribNumber(csOp.attribs, (n) => {
-          const attrib = apool.getAttrib(n);
-          attribs.push(attrib);
-          if (attrib[0] === 'author') authorAttrib = n;
-        });
+        const attribs = AttributeMap.fromString(csOp.attribs, apool);
         const undoBackToAttribs = cachedStrFunc((oldAttribsStr) => {
-          const backAttribs = [];
+          const oldAttribs = AttributeMap.fromString(oldAttribsStr, apool);
+          const backAttribs = new AttributeMap(apool)
+              .set('author', '')
+              .set('removed', 'true');
           for (const [key, value] of attribs) {
-            const oldValue = Changeset.attribsAttributeValue(oldAttribsStr, key, apool);
-            if (oldValue !== value) backAttribs.push([key, oldValue])
+            const oldValue = oldAttribs.get(key);
+            if (oldValue !== value) backAttribs.set(key, oldValue);
           }
-
-          return Changeset.makeAttribsString('=', backAttribs, apool);
+          return backAttribs.toString();
         });
 
-        const oldAttribsAddition =
-            `*${Changeset.numToString(deletedAttrib)}*${Changeset.numToString(authorAttrib)}`;
-
         let textLeftToProcess = textBank;
 
         while (textLeftToProcess.length > 0) {
@@ -429,7 +418,7 @@ PadDiff.prototype._createDeletionChangeset = function (cs, startAText, apool) {
             let textBankIndex = 0;
             consumeAttribRuns(lengthToProcess, (len, attribs, endsLine) => {
               // get the old attributes back
-              const oldAttribs = (undoBackToAttribs(attribs) || '') + oldAttribsAddition;
+              const oldAttribs = undoBackToAttribs(attribs);
 
               builder.insert(processText.substr(textBankIndex, len), oldAttribs);
               textBankIndex += len;

src/static/js/AttributeManager.js

@@ -1,7 +1,9 @@
 'use strict';
 
+const AttributeMap = require('./AttributeMap');
 const Changeset = require('./Changeset');
 const ChangesetUtils = require('./ChangesetUtils');
+const attributes = require('./attributes');
 const _ = require('./underscore');
 
 const lineMarkerAttribute = 'lmkr';
@@ -150,7 +152,7 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
     if (!aline) return '';
     const opIter = Changeset.opIterator(aline);
     if (!opIter.hasNext()) return '';
-    return Changeset.opAttributeValue(opIter.next(), attributeName, this.rep.apool) || '';
+    return AttributeMap.fromString(opIter.next().attribs, this.rep.apool).get(attributeName) || '';
   },
 
   /*
@@ -164,10 +166,7 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
     const opIter = Changeset.opIterator(aline);
     if (!opIter.hasNext()) return [];
     const op = opIter.next();
-    if (!op.attribs) return [];
-    const attributes = [];
-    Changeset.eachAttribNumber(op.attribs, (n) => attributes.push(this.rep.apool.getAttrib(n)));
-    return attributes;
+    return [...attributes.attribsFromString(op.attribs, this.rep.apool)];
   },
 
   /*
@@ -191,9 +190,7 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
       }
     }
 
-    const withIt = Changeset.makeAttribsString('+', [
-      [attributeName, 'true'],
-    ], rep.apool);
+    const withIt = new AttributeMap(rep.apool).set(attributeName, 'true').toString();
     const withItRegex = new RegExp(`${withIt.replace(/\*/g, '\\*')}(\\*|$)`);
     const hasIt = (attribs) => withItRegex.test(attribs);
 
@@ -274,10 +271,7 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
       currentOperation = opIter.next();
       currentPointer += currentOperation.chars;
       if (currentPointer <= column) continue;
-      const attributes = [];
-      Changeset.eachAttribNumber(
-          currentOperation.attribs, (n) => attributes.push(this.rep.apool.getAttrib(n)));
-      return attributes;
+      return [...attributes.attribsFromString(currentOperation.attribs, this.rep.apool)];
     }
     return [];
   },