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

Side by Side Diff: lib/filterStorage.js

Issue 29886685: Issue 6856 - Remove FilterStorage.moveSubscription (Closed) Base URL: https://hg.adblockplus.org/adblockpluscore
Patch Set: Address PS4 Comment Created Oct. 1, 2018, 2:31 a.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View unified diff | Download patch
« no previous file with comments | « lib/filterListener.js ('k') | lib/synchronizer.js » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 /* 1 /*
2 * This file is part of Adblock Plus <https://adblockplus.org/>, 2 * This file is part of Adblock Plus <https://adblockplus.org/>,
3 * Copyright (C) 2006-present eyeo GmbH 3 * Copyright (C) 2006-present eyeo GmbH
4 * 4 *
5 * Adblock Plus is free software: you can redistribute it and/or modify 5 * Adblock Plus is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as 6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation. 7 * published by the Free Software Foundation.
8 * 8 *
9 * Adblock Plus is distributed in the hope that it will be useful, 9 * Adblock Plus is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of 10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
(...skipping 62 matching lines...) Expand 10 before | Expand all | Expand 10 after
73 */ 73 */
74 firstRun: false, 74 firstRun: false,
75 75
76 /** 76 /**
77 * Map of properties listed in the filter storage file before the sections 77 * Map of properties listed in the filter storage file before the sections
78 * start. Right now this should be only the format version. 78 * start. Right now this should be only the format version.
79 */ 79 */
80 fileProperties: Object.create(null), 80 fileProperties: Object.create(null),
81 81
82 /** 82 /**
83 * List of filter subscriptions containing all filters 83 * Yields subscriptions containing all filters
84 * @type {Subscription[]} 84 * @yields {Subscription}
85 */ 85 */
86 subscriptions: [], 86 *subscriptions()
87 {
88 yield* this.knownSubscriptions.values();
89 },
90
91 /**
92 * Number of known subscriptions.
93 * @type {number}
94 */
95 get subscriptionCount()
96 {
97 return this.knownSubscriptions.size;
98 },
87 99
88 /** 100 /**
89 * Map of subscriptions already on the list, by their URL/identifier 101 * Map of subscriptions already on the list, by their URL/identifier
90 * @type {Map.<string,Subscription>} 102 * @type {Map.<string,Subscription>}
91 */ 103 */
92 knownSubscriptions: new Map(), 104 knownSubscriptions: new Map(),
93 105
94 /** 106 /**
95 * Finds the filter group that a filter should be added to by default. Will 107 * Finds the filter group that a filter should be added to by default. Will
96 * return null if this group doesn't exist yet. 108 * return null if this group doesn't exist yet.
97 * @param {Filter} filter 109 * @param {Filter} filter
98 * @return {?SpecialSubscription} 110 * @return {?SpecialSubscription}
99 */ 111 */
100 getGroupForFilter(filter) 112 getGroupForFilter(filter)
101 { 113 {
102 let generalSubscription = null; 114 let generalSubscription = null;
103 for (let subscription of FilterStorage.subscriptions) 115 for (let subscription of FilterStorage.subscriptions())
Manish Jethani 2018/10/01 10:40:45 Since this is within the FilterStorage object, may
Jon Sonesen 2018/10/01 15:40:57 Yeah, I think you are right. I had the same though
Manish Jethani 2018/10/01 16:12:52 I'm not sure, but it shouldn't hurt if we just avo
Jon Sonesen 2018/10/02 16:23:48 Done.
104 { 116 {
105 if (subscription instanceof SpecialSubscription && !subscription.disabled) 117 if (subscription instanceof SpecialSubscription && !subscription.disabled)
106 { 118 {
107 // Always prefer specialized subscriptions 119 // Always prefer specialized subscriptions
108 if (subscription.isDefaultFor(filter)) 120 if (subscription.isDefaultFor(filter))
109 return subscription; 121 return subscription;
110 122
111 // If this is a general subscription - store it as fallback 123 // If this is a general subscription - store it as fallback
112 if (!generalSubscription && 124 if (!generalSubscription &&
113 (!subscription.defaults || !subscription.defaults.length)) 125 (!subscription.defaults || !subscription.defaults.length))
114 { 126 {
115 generalSubscription = subscription; 127 generalSubscription = subscription;
116 } 128 }
117 } 129 }
118 } 130 }
119 return generalSubscription; 131 return generalSubscription;
120 }, 132 },
121 133
122 /** 134 /**
123 * Adds a filter subscription to the list 135 * Adds a filter subscription to the list
124 * @param {Subscription} subscription filter subscription to be added 136 * @param {Subscription} subscription filter subscription to be added
125 */ 137 */
126 addSubscription(subscription) 138 addSubscription(subscription)
127 { 139 {
128 if (FilterStorage.knownSubscriptions.has(subscription.url)) 140 if (FilterStorage.knownSubscriptions.has(subscription.url))
129 return; 141 return;
130 142
131 FilterStorage.subscriptions.push(subscription);
132 FilterStorage.knownSubscriptions.set(subscription.url, subscription); 143 FilterStorage.knownSubscriptions.set(subscription.url, subscription);
133 addSubscriptionFilters(subscription); 144 addSubscriptionFilters(subscription);
134 145
135 filterNotifier.emit("subscription.added", subscription); 146 filterNotifier.emit("subscription.added", subscription);
136 }, 147 },
137 148
138 /** 149 /**
139 * Removes a filter subscription from the list 150 * Removes a filter subscription from the list
140 * @param {Subscription} subscription filter subscription to be removed 151 * @param {Subscription} subscription filter subscription to be removed
141 */ 152 */
142 removeSubscription(subscription) 153 removeSubscription(subscription)
143 { 154 {
144 for (let i = 0; i < FilterStorage.subscriptions.length; i++) 155 if (!FilterStorage.knownSubscriptions.has(subscription.url))
145 {
146 if (FilterStorage.subscriptions[i].url == subscription.url)
147 {
148 removeSubscriptionFilters(subscription);
149
150 FilterStorage.subscriptions.splice(i--, 1);
151 FilterStorage.knownSubscriptions.delete(subscription.url);
152
153 // This should be the last remaining reference to the Subscription
154 // object.
155 Subscription.knownSubscriptions.delete(subscription.url);
156
157 filterNotifier.emit("subscription.removed", subscription);
158 return;
159 }
160 }
161 },
162
163 /**
164 * Moves a subscription in the list to a new position.
165 * @param {Subscription} subscription filter subscription to be moved
166 * @param {Subscription} [insertBefore] filter subscription to insert before
167 * (if omitted the subscription will be put at the end of the list)
168 */
169 moveSubscription(subscription, insertBefore)
170 {
171 let currentPos = FilterStorage.subscriptions.indexOf(subscription);
172 if (currentPos < 0)
173 return; 156 return;
174 157
175 let newPos = -1; 158 removeSubscriptionFilters(subscription);
176 if (insertBefore)
177 newPos = FilterStorage.subscriptions.indexOf(insertBefore);
178 159
179 if (newPos < 0) 160 FilterStorage.knownSubscriptions.delete(subscription.url);
180 newPos = FilterStorage.subscriptions.length;
181 161
182 if (currentPos < newPos) 162 // This should be the last remaining reference to the Subscription
183 newPos--; 163 // object.
184 if (currentPos == newPos) 164 Subscription.knownSubscriptions.delete(subscription.url);
185 return;
186 165
187 FilterStorage.subscriptions.splice(currentPos, 1); 166 filterNotifier.emit("subscription.removed", subscription);
188 FilterStorage.subscriptions.splice(newPos, 0, subscription);
189 filterNotifier.emit("subscription.moved", subscription);
190 }, 167 },
191 168
192 /** 169 /**
193 * Replaces the list of filters in a subscription by a new list 170 * Replaces the list of filters in a subscription by a new list
194 * @param {Subscription} subscription filter subscription to be updated 171 * @param {Subscription} subscription filter subscription to be updated
195 * @param {Filter[]} filters new filter list 172 * @param {Filter[]} filters new filter list
196 */ 173 */
197 updateSubscriptionFilters(subscription, filters) 174 updateSubscriptionFilters(subscription, filters)
198 { 175 {
199 removeSubscriptionFilters(subscription); 176 removeSubscriptionFilters(subscription);
(...skipping 166 matching lines...) Expand 10 before | Expand all | Expand 10 after
366 return line => 343 return line =>
367 { 344 {
368 parser.process(line); 345 parser.process(line);
369 if (line === null) 346 if (line === null)
370 { 347 {
371 let knownSubscriptions = new Map(); 348 let knownSubscriptions = new Map();
372 for (let subscription of parser.subscriptions) 349 for (let subscription of parser.subscriptions)
373 knownSubscriptions.set(subscription.url, subscription); 350 knownSubscriptions.set(subscription.url, subscription);
374 351
375 this.fileProperties = parser.fileProperties; 352 this.fileProperties = parser.fileProperties;
376 this.subscriptions = parser.subscriptions;
Manish Jethani 2018/10/01 10:53:13 Note: An implication of this change is that a patt
Jon Sonesen 2018/10/01 15:40:57 Acknowledged.
377 this.knownSubscriptions = knownSubscriptions; 353 this.knownSubscriptions = knownSubscriptions;
378 Filter.knownFilters = parser.knownFilters; 354 Filter.knownFilters = parser.knownFilters;
379 Subscription.knownSubscriptions = parser.knownSubscriptions; 355 Subscription.knownSubscriptions = parser.knownSubscriptions;
380 356
381 if (!silent) 357 if (!silent)
382 filterNotifier.emit("load"); 358 filterNotifier.emit("load");
383 } 359 }
384 }; 360 };
385 }, 361 },
386 362
387 /** 363 /**
388 * Loads all subscriptions from the disk. 364 * Loads all subscriptions from the disk.
389 * @return {Promise} promise resolved or rejected when loading is complete 365 * @return {Promise} promise resolved or rejected when loading is complete
390 */ 366 */
391 loadFromDisk() 367 loadFromDisk()
392 { 368 {
393 let tryBackup = backupIndex => 369 let tryBackup = backupIndex =>
394 { 370 {
395 return this.restoreBackup(backupIndex, true).then(() => 371 return this.restoreBackup(backupIndex, true).then(() =>
396 { 372 {
397 if (this.subscriptions.length == 0) 373 if (this.subscriptionCount == 0)
Manish Jethani 2018/10/01 10:40:45 Here (and below) too I think it's probably better
Jon Sonesen 2018/10/01 15:40:57 Acknowledged.
Jon Sonesen 2018/10/02 16:23:48 Done.
398 return tryBackup(backupIndex + 1); 374 return tryBackup(backupIndex + 1);
399 }).catch(error => 375 }).catch(error =>
400 { 376 {
401 // Give up 377 // Give up
402 }); 378 });
403 }; 379 };
404 380
405 return IO.statFile(this.sourceFile).then(statData => 381 return IO.statFile(this.sourceFile).then(statData =>
406 { 382 {
407 if (!statData.exists) 383 if (!statData.exists)
408 { 384 {
409 this.firstRun = true; 385 this.firstRun = true;
410 return; 386 return;
411 } 387 }
412 388
413 let parser = this.importData(true); 389 let parser = this.importData(true);
414 return IO.readFromFile(this.sourceFile, parser).then(() => 390 return IO.readFromFile(this.sourceFile, parser).then(() =>
415 { 391 {
416 parser(null); 392 parser(null);
417 if (this.subscriptions.length == 0) 393 if (this.subscriptionCount == 0)
418 { 394 {
419 // No filter subscriptions in the file, this isn't right. 395 // No filter subscriptions in the file, this isn't right.
420 throw new Error("No data in the file"); 396 throw new Error("No data in the file");
421 } 397 }
422 }); 398 });
423 }).catch(error => 399 }).catch(error =>
424 { 400 {
425 Cu.reportError(error); 401 Cu.reportError(error);
426 return tryBackup(1); 402 return tryBackup(1);
427 }).then(() => 403 }).then(() =>
(...skipping 33 matching lines...) Expand 10 before | Expand all | Expand 10 after
461 return this.saveToDisk(); 437 return this.saveToDisk();
462 }); 438 });
463 }, 439 },
464 440
465 /** 441 /**
466 * Generator serializing filter data and yielding it line by line. 442 * Generator serializing filter data and yielding it line by line.
467 */ 443 */
468 *exportData() 444 *exportData()
469 { 445 {
470 // Do not persist external subscriptions 446 // Do not persist external subscriptions
471 let subscriptions = this.subscriptions.filter( 447 let subscriptions = [...this.subscriptions()].filter(
472 s => !(s instanceof ExternalSubscription) 448 s => !(s instanceof ExternalSubscription)
473 ); 449 );
474 450
475 yield "# Adblock Plus preferences"; 451 yield "# Adblock Plus preferences";
476 yield "version=" + formatVersion; 452 yield "version=" + formatVersion;
477 453
478 let saved = new Set(); 454 let saved = new Set();
479 let buf = []; 455 let buf = [];
480 456
481 // Save subscriptions 457 // Save subscriptions
(...skipping 189 matching lines...) Expand 10 before | Expand all | Expand 10 after
671 * @param {Subscription} subscription filter subscription to be removed 647 * @param {Subscription} subscription filter subscription to be removed
672 */ 648 */
673 function removeSubscriptionFilters(subscription) 649 function removeSubscriptionFilters(subscription)
674 { 650 {
675 if (!FilterStorage.knownSubscriptions.has(subscription.url)) 651 if (!FilterStorage.knownSubscriptions.has(subscription.url))
676 return; 652 return;
677 653
678 for (let filter of subscription.filters) 654 for (let filter of subscription.filters)
679 filter.removeSubscription(subscription); 655 filter.removeSubscription(subscription);
680 } 656 }
OLDNEW
« no previous file with comments | « lib/filterListener.js ('k') | lib/synchronizer.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld