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 109 matching lines @@
       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
    * 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);
+    yield "[Subscription]";
+    let {url, type, _title, _fixedTitle, _disabled} = this;

[Manish Jethani, 2018/10/14 20:05:37]

Let's place this line first before any yield statements, and ideally I would
also leave a line after it.

[Jon Sonesen, 2018/10/21 06:18:23]

Done.

[Manish Jethani, 2018/10/21 13:08:58]

Sorry if I wasn't clear, it should be the first line:

  let {url, type, _title, _fixedTitle, _disabled} = this;

  yield "[Subscription]";
  yield "url=" + url;

+    yield "url=" + url;
+
     if (this.type)

[Manish Jethani, 2018/10/14 20:05:37]

Similarly, we should remove the `this.` prefix here.

[Jon Sonesen, 2018/10/21 06:18:23]

Done.

[Manish Jethani, 2018/10/21 13:08:58]

What about `this.type` and `this._title`?

-      buffer.push("type=" + this.type);
+      yield "type=" + type;
     if (this._title)
-      buffer.push("title=" + this._title);
-    if (this._fixedTitle)
-      buffer.push("fixedTitle=true");
-    if (this._disabled)
-      buffer.push("disabled=true");
+      yield "title=" + _title;
+    if (_fixedTitle)
+      yield "fixedTitle=true";
+    if (_disabled)
+      yield "disabled=true";
   },
 
-  serializeFilters(buffer)
+  *serializeFilters()
   {
     for (let filter of this.filters)
-      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(
+      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);
+    yield* RegularSubscription.prototype.serialize.call(this);
+
+    let {downloadStatus, lastSuccess, lastCheck, expires,

[Manish Jethani, 2018/10/14 20:05:36]

Let's move this line to the top of the function body. We should take a snapshot
of the state before we ever yield anything.

[Jon Sonesen, 2018/10/21 06:18:24]

Done.

+         softExpiration, errors, version, requiredVersion,
+         downloadCount} = 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;
   }
 });