Issue 7016 - Convert serialization functions into generators

lib/subscriptionClasses.js

Index: lib/subscriptionClasses.js
===================================================================
--- a/lib/subscriptionClasses.js
+++ b/lib/subscriptionClasses.js
@@ -122,43 +122,44 @@
       filterNotifier.emit("subscription.disabled", this, value, oldValue);
     }
     return this._disabled;
   },
 
   /**
    * Serializes the subscription to an array of strings for writing

[Manish Jethani, 2018/10/21 21:51:43]

We could remove the "to an array of strings" part from this comment and keep the
rest of it.

[Jon Sonesen, 2018/10/22 19:27:47]

Acknowledged.

    * out on the disk.
-   * @param {string[]} buffer  buffer to push the serialization results into
+   * @yields {string}
    */
-  serialize(buffer)
+  *serialize()
   {
-    buffer.push("[Subscription]");
-    buffer.push("url=" + this.url);
-    if (this.type)
-      buffer.push("type=" + this.type);
-    if (this._title)
-      buffer.push("title=" + this._title);
-    if (this._fixedTitle)
-      buffer.push("fixedTitle=true");
-    if (this._disabled)
-      buffer.push("disabled=true");
+    let {url, type, _title, _fixedTitle, _disabled} = this;
+
+    yield "[Subscription]";
+    yield "url=" + url;
+
+    if (type)
+      yield "type=" + type;
+    if (_title)
+      yield "title=" + _title;
+    if (_fixedTitle)
+      yield "fixedTitle=true";
+    if (_disabled)
+      yield "disabled=true";
   },
 
-  serializeFilters(buffer)
+  *serializeFilters()
   {
     for (let filter of this.filters)

[Jon Sonesen, 2018/10/22 19:32:40]

I noticed this method goes against our pattern elsewhere. Not sure if it makes
sense here since it can have an empty section when writing to disk, but also
this will ensure uniform behavior for serialization.

[Manish Jethani, 2018/10/22 20:18:01]

Just for the sake of consistency, I wouldn't mind if we added the following line
to the start of the function:

  let {filters} = this;

Even though it does nothing, just for consistency.

[Jon Sonesen, 2018/10/23 02:04:24]

Done.

-      buffer.push(filter.text.replace(/\[/g, "\\["));
+      yield filter.text.replace(/\[/g, "\\[");
   },
 
   toString()
   {
-    let buffer = [];
-    this.serialize(buffer);
-    return buffer.join("\n");
+    return [...this.serialize()].join("\n");
   }
 };
 
 /**
  * Cache for known filter subscriptions, maps URL to subscription objects.
  * @type {Map.<string,Subscription>}
  */
 Subscription.knownSubscriptions = new Map();
@@ -276,29 +277,31 @@
 
     return false;
   },
 
   /**
    * See Subscription.serialize()
    * @inheritdoc
    */
-  serialize(buffer)
+  *serialize()
   {
-    Subscription.prototype.serialize.call(this, buffer);
-    if (this.defaults && this.defaults.length)
+    let {defaults, _lastDownload} = this;
+
+    yield* Subscription.prototype.serialize.call(this);
+
+    if (defaults && defaults.length)
     {
-      buffer.push("defaults=" +
-        this.defaults.filter(
-          type => SpecialSubscription.defaultsMap.has(type)
-        ).join(" ")
-      );
+      yield "defaults=" +
+            defaults.filter(
+              type => SpecialSubscription.defaultsMap.has(type)
+            ).join(" ");
     }
-    if (this._lastDownload)
-      buffer.push("lastDownload=" + this._lastDownload);
+    if (_lastDownload)
+      yield "lastDownload=" + _lastDownload;
   }
 });
 
 SpecialSubscription.defaultsMap = new Map([
   ["whitelist", WhitelistFilter],
   ["blocking", BlockingFilter],
   ["elemhide", ElemHideBase]
 ]);
@@ -394,23 +397,26 @@
     }
     return this._lastDownload;
   },
 
   /**
    * See Subscription.serialize()
    * @inheritdoc
    */
-  serialize(buffer)
+  *serialize()
   {
-    Subscription.prototype.serialize.call(this, buffer);
-    if (this._homepage)
-      buffer.push("homepage=" + this._homepage);
-    if (this._lastDownload)
-      buffer.push("lastDownload=" + this._lastDownload);
+    let {_homepage, _lastDownload} = this;
+
+    yield* Subscription.prototype.serialize.call(this);
+
+    if (_homepage)
+      yield "homepage=" + _homepage;
+    if (_lastDownload)
+      yield "lastDownload=" + _lastDownload;
   }
 });
 
 /**
  * Class for filter subscriptions updated externally (by other extension)
  * @param {string} url    see {@link Subscription Subscription()}
  * @param {string} [title]  see {@link Subscription Subscription()}
  * @constructor
@@ -422,17 +428,17 @@
 }
 exports.ExternalSubscription = ExternalSubscription;
 
 ExternalSubscription.prototype = extend(RegularSubscription, {
   /**
    * See Subscription.serialize()
    * @inheritdoc
    */
-  serialize(buffer)
+  *serialize() // eslint-disable-line require-yield
   {
     throw new Error(
       "Unexpected call, external subscriptions should not be serialized"
     );
   }
 });
 
 /**
@@ -547,31 +553,36 @@
    * @type {number}
    */
   downloadCount: 0,
 
   /**
    * See Subscription.serialize()
    * @inheritdoc
    */
-  serialize(buffer)
+  *serialize()
   {
-    RegularSubscription.prototype.serialize.call(this, buffer);
-    if (this.downloadStatus)
-      buffer.push("downloadStatus=" + this.downloadStatus);
-    if (this.lastSuccess)
-      buffer.push("lastSuccess=" + this.lastSuccess);
-    if (this.lastCheck)
-      buffer.push("lastCheck=" + this.lastCheck);
-    if (this.expires)
-      buffer.push("expires=" + this.expires);
-    if (this.softExpiration)
-      buffer.push("softExpiration=" + this.softExpiration);
-    if (this.errors)
-      buffer.push("errors=" + this.errors);
-    if (this.version)
-      buffer.push("version=" + this.version);
-    if (this.requiredVersion)
-      buffer.push("requiredVersion=" + this.requiredVersion);
-    if (this.downloadCount)
-      buffer.push("downloadCount=" + this.downloadCount);
+    let {downloadStatus, lastSuccess, lastCheck, expires,
+         softExpiration, errors, version, requiredVersion,
+         downloadCount} = this;
+
+    yield* RegularSubscription.prototype.serialize.call(this);
+
+    if (downloadStatus)
+      yield "downloadStatus=" + downloadStatus;
+    if (lastSuccess)
+      yield "lastSuccess=" + lastSuccess;
+    if (lastCheck)
+      yield "lastCheck=" + lastCheck;
+    if (expires)
+      yield "expires=" + expires;
+    if (softExpiration)
+      yield "softExpiration=" + softExpiration;
+    if (errors)
+      yield "errors=" + errors;
+    if (version)
+      yield "version=" + version;
+    if (requiredVersion)
+      yield "requiredVersion=" + requiredVersion;
+    if (downloadCount)
+      yield "downloadCount=" + downloadCount;
   }
 });