Issue 5052 - Allow importing and exporting filterStorage data without file access

Issue 5052 - Allow importing and exporting filterStorage data without file access

[kzar, 2017-03-30 12:58:48]

https://codereview.adblockplus.org/29398705/diff/29398706/lib/filterStorage.js
File lib/filterStorage.js (right):

https://codereview.adblockplus.org/29398705/diff/29398706/lib/filterStorage.j...
lib/filterStorage.js:417: parser.process(line);
I wonder why we parser.process the final null?

https://codereview.adblockplus.org/29398705/diff/29398706/lib/filterStorage.j...
lib/filterStorage.js:515: let [, part1, part2] = /^(.*)(\.\w+)$/.exec(
Well I just learned something, I didn't realise values could be dropped with
destructuring like that.

https://codereview.adblockplus.org/29398705/diff/29398706/lib/filterStorage.j...
lib/filterStorage.js:543: // Do not persist external subscriptions
Nit: I guess we could avoid doing this filtering until after the header lines
have been consumed. Probably doesn't matter in practice.

[Wladimir Palant, 2017-03-30 13:02:06]

https://codereview.adblockplus.org/29398705/diff/29398706/lib/filterStorage.js
File lib/filterStorage.js (right):

https://codereview.adblockplus.org/29398705/diff/29398706/lib/filterStorage.j...
lib/filterStorage.js:417: parser.process(line);
On 2017/03/30 12:58:48, kzar wrote:
> I wonder why we parser.process the final null?

The parser uses it to flush out whatever object was "in progress" right now.

https://codereview.adblockplus.org/29398705/diff/29398706/lib/filterStorage.j...
lib/filterStorage.js:543: // Do not persist external subscriptions
On 2017/03/30 12:58:48, kzar wrote:
> Nit: I guess we could avoid doing this filtering until after the header lines
> have been consumed. Probably doesn't matter in practice.

Better not. This is creating a copy of the list for another reason as well - if
the list changes in the meantime we won't save bogus data. And consumption of
generator data is async.

[kzar, 2017-03-30 13:10:54]

LGTM

https://codereview.adblockplus.org/29398705/diff/29398706/lib/filterStorage.js
File lib/filterStorage.js (right):

https://codereview.adblockplus.org/29398705/diff/29398706/lib/filterStorage.j...
lib/filterStorage.js:417: parser.process(line);
On 2017/03/30 13:02:06, Wladimir Palant wrote:
> On 2017/03/30 12:58:48, kzar wrote:
> > I wonder why we parser.process the final null?
> 
> The parser uses it to flush out whatever object was "in progress" right now.

Acknowledged.

https://codereview.adblockplus.org/29398705/diff/29398706/lib/filterStorage.j...
lib/filterStorage.js:543: // Do not persist external subscriptions
On 2017/03/30 13:02:06, Wladimir Palant wrote:
> On 2017/03/30 12:58:48, kzar wrote:
> > Nit: I guess we could avoid doing this filtering until after the header
lines
> > have been consumed. Probably doesn't matter in practice.
> 
> Better not. This is creating a copy of the list for another reason as well -
if
> the list changes in the meantime we won't save bogus data. And consumption of
> generator data is async.

Ah OK, makes sense.