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
-   * out on the disk.
+   * Serializes the subscription for writing out on disk.
    * @yields {string}
    */
   *serialize()
   {
+    let {url, type, _title, _fixedTitle, _disabled} = this;
+
     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`?

+    if (type)
       yield "type=" + type;
-    if (this._title)
+    if (_title)
       yield "title=" + _title;
     if (_fixedTitle)
       yield "fixedTitle=true";
     if (_disabled)
       yield "disabled=true";
   },
 
   *serializeFilters()
   {
-    for (let filter of this.filters)
+    let {filters} = this;
+
+    yield "[Subscription filters]";
+
+    for (let filter of filters)
       yield filter.text.replace(/\[/g, "\\[");
   },
 
   toString()
   {
     return [...this.serialize()].join("\n");
   }
 };
 
 /**
@@ skipping 116 matching lines @@
     return false;
   },
 
   /**
    * See Subscription.serialize()
    * @inheritdoc
    */
   *serialize()
   {
     let {defaults, _lastDownload} = this;
+
     yield* Subscription.prototype.serialize.call(this);
-    if (defaults && defaults.length)
+
+    if (defaults)
     {
       yield "defaults=" +
-        defaults.filter(
-          type => SpecialSubscription.defaultsMap.has(type)
-        ).join(" ")
-      ;
+            defaults.filter(
+              type => SpecialSubscription.defaultsMap.has(type)
+            ).join(" ");
     }
     if (_lastDownload)
       yield "lastDownload=" + _lastDownload;
   }
 });
 
 SpecialSubscription.defaultsMap = new Map([
   ["whitelist", WhitelistFilter],
   ["blocking", BlockingFilter],
   ["elemhide", ElemHideBase]
@@ skipping 91 matching lines @@
     return this._lastDownload;
   },
 
   /**
    * See Subscription.serialize()
    * @inheritdoc
    */
   *serialize()
   {
     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()}
@@ skipping 132 matching lines @@
    * @type {number}
    */
   downloadCount: 0,
 
   /**
    * See Subscription.serialize()
    * @inheritdoc
    */
   *serialize()
   {
-    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;
+
+    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;
   }
 });