Issue 7094 - Encapsulate management of subscription filters

lib/subscriptionClasses.js

Index: lib/subscriptionClasses.js
===================================================================
--- a/lib/subscriptionClasses.js
+++ b/lib/subscriptionClasses.js
@@ -31,19 +31,25 @@
  *
  * @param {string} url    download location of the subscription
  * @param {string} [title]  title of the filter subscription
  * @constructor
  */
 function Subscription(url, title)
 {
   this.url = url;
-  this.filters = [];
+
+  this._filterText = [];
+  this._filters = [];
+
+  this._filterTextSet = new Set();
+
   if (title)
     this._title = title;
+
   Subscription.knownSubscriptions.set(url, this);
 }
 exports.Subscription = Subscription;
 
 Subscription.prototype =
 {
   /**
    * Download location of the subscription
@@ -53,20 +59,36 @@
 
   /**
    * Type of the subscription
    * @type {?string}
    */
   type: null,
 
   /**
-   * Filters contained in the filter subscription
-   * @type {Filter[]}
+   * Filter text contained in the filter subscription.
+   * @type {Array.<string>}
+   * @private
    */
-  filters: null,
+  _filterText: null,
+
+  /**
+   * {@link Filter} objects corresponding to the subscription's filter text.
+   * @type {Array.<Filter>}
+   * @private
+   */
+  _filters: null,
+
+  /**
+   * Set of filter text contained in the filter subscription, used for faster
+   * lookup.
+   * @type {Set.<string>}
+   * @private
+   */
+  _filterTextSet: null,
 
   _title: null,
   _fixedTitle: false,
   _disabled: false,
 
   /**
    * Title of the filter subscription
    * @type {string}
@@ -120,16 +142,134 @@
       let oldValue = this._disabled;
       this._disabled = value;
       filterNotifier.emit("subscription.disabled", this, value, oldValue);
     }
     return this._disabled;
   },
 
   /**
+   * The number of filters in the subscription.
+   * @type {number}
+   */
+  get filterCount()
+  {
+    return this._filters.length;
+  },
+
+  /**
+   * Yields the text for each filter in the subscription.
+   * @yields {string}
+   */
+  *filterText()
+  {
+    yield* this._filterText;
+  },
+
+  /**
+   * Yields the {@link Filter} object for each filter in the subscription.
+   * @yields {Filter}
+   */
+  *filters()
+  {
+    yield* this._filters;
+  },
+
+  /**
+   * Returns the {@link Filter} object at the given 0-based index.
+   * @param {number} index
+   * @returns {?Filter}
+   */
+  filterAt(index)
+  {
+    return this._filters[index] || null;
+  },
+
+  /**
+   * Returns the 0-based index of the given filter.
+   * @param {Filter} filter
+   * @param {number} [fromIndex] The index from which to start the search.
+   * @return {number}
+   */
+  searchFilter(filter, fromIndex = 0)
+  {
+    return this._filterText.indexOf(filter.text, fromIndex);

[Manish Jethani, 2018/11/17 21:41:09]

Note: We look up a filter always by the text, not by the filter object, which
can be temporary and so the references can keep changing.

+  },
+
+  /**
+   * Checks whether the subscription contains the given filter.
+   * @param {Filter} filter
+   * @return {boolean}
+   */
+  hasFilter(filter)

[Manish Jethani, 2018/11/17 21:41:09]

We will use this in this patch: https://codereview.adblockplus.org/29935568/

+  {
+    return this._filterTextSet.has(filter.text);
+  },
+
+  /**
+   * Removes all filters from the subscription.
+   */
+  clearFilters()
+  {
+    this._filterText = [];
+    this._filters = [];
+
+    this._filterTextSet.clear();
+  },
+
+  /**
+   * Adds a filter to the subscription.
+   * @param {Filter} filter
+   */
+  addFilter(filter)
+  {
+    this._filterText.push(filter.text);
+    this._filters.push(filter);
+
+    this._filterTextSet.add(filter.text);
+  },
+
+  /**
+   * Inserts a filter into the subscription.
+   * @param {Filter} filter
+   * @param {number} index The index at which to insert the filter.
+   */
+  insertFilterAt(filter, index)
+  {
+    this._filterText.splice(index, 0, filter.text);
+    this._filters.splice(index, 0, filter);
+
+    this._filterTextSet.add(filter.text);
+  },
+
+  /**
+   * Deletes a filter from the subscription.
+   * @param {number} index The index at which to delete the filter.
+   */
+  deleteFilterAt(index)
+  {
+    let items = this._filterText.splice(index, 1);
+
+    this._filters.splice(index, 1);
+
+    // Performance note: A subscription can contain the same filter multiple
+    // times. We can only delete the text from the set if all occurrences of
+    // the filter have been removed. This makes deletion an expensive
+    // operation. It is only needed when a user-defined filter is removed,
+    // which is relatively rare. In comparison, filters must be looked up in
+    // subscriptions a lot more often; therefore, it makes sense to maintain
+    // the set at this cost. An alternative here would be to use a map instead
+    // of a set, with the value of each entry being the reference count, but
+    // this would make addition more expensive thus slowing down the loading of
+    // the initial subscriptions.
+    if (items.length > 0 && this._filterText.indexOf(items[0]) == -1)
+      this._filterTextSet.delete(items[0]);
+  },
+
+  /**
    * Serializes the subscription for writing out on disk.
    * @yields {string}
    */
   *serialize()
   {
     let {url, type, _title, _fixedTitle, _disabled} = this;
 
     yield "[Subscription]";
@@ -142,22 +282,22 @@
     if (_fixedTitle)
       yield "fixedTitle=true";
     if (_disabled)
       yield "disabled=true";
   },
 
   *serializeFilters()
   {
-    let {filters} = this;
+    let {_filterText} = this;
 
     yield "[Subscription filters]";
 
-    for (let filter of filters)
-      yield filter.text.replace(/\[/g, "\\[");
+    for (let text of _filterText)
+      yield text.replace(/\[/g, "\\[");
   },
 
   toString()
   {
     return [...this.serialize()].join("\n");
   }
 };
 
@@ -328,17 +468,17 @@
  * Creates a new user-defined filter group and adds the given filter to it.
  * This group will act as the default group for this filter type.
  * @param {Filter} filter
  * @return {SpecialSubscription}
  */
 SpecialSubscription.createForFilter = function(filter)
 {
   let subscription = SpecialSubscription.create();
-  subscription.filters.push(filter);
+  subscription.addFilter(filter);
   for (let [type, class_] of SpecialSubscription.defaultsMap)
   {
     if (filter instanceof class_)
       subscription.defaults = [type];
   }
   if (!subscription.defaults)
     subscription.defaults = ["blocking"];
   return subscription;