Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: lib/subscriptionClasses.js

Issue 29934588: Issue 7094 - Encapsulate management of subscription filters (Closed) Base URL: https://hg.adblockplus.org/adblockpluscore/
Patch Set: Pass old filters to disconnectSubscriptionFilters Created Nov. 17, 2018, 10:08 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « lib/iniParser.js ('k') | lib/synchronizer.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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);
+ },
+
+ /**
+ * Checks whether the subscription contains the given filter.
+ * @param {Filter} filter
+ * @return {boolean}
+ */
+ hasFilter(filter)
+ {
+ 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;
« no previous file with comments | « lib/iniParser.js ('k') | lib/synchronizer.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld