Issue 7016 - Convert serialization functions into generators

lib/subscriptionClasses.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 107 matching lines @@
     if (value != this._disabled)
     {
       let oldValue = this._disabled;
       this._disabled = value;
       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();
 
 /**
@@ skipping 107 matching lines @@
       }
     }
 
     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]
 ]);
 
 /**
@@ skipping 85 matching lines @@
       this._lastDownload = value;
       filterNotifier.emit("subscription.lastDownload", this, value, oldValue);
     }
     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
  * @augments RegularSubscription
  */
 function ExternalSubscription(url, title)
 {
   RegularSubscription.call(this, url, title);
 }
 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"
     );
   }
 });
 
 /**
  * Class for filter subscriptions updated externally (by other extension)
  * @param {string} url  see {@link Subscription Subscription()}
@@ skipping 104 matching lines @@
   /**
    * Number indicating how often the object was downloaded.
    * @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;
   }
 });