Issue 4485 - Clear cache directory

Issue 4485 - Clear cache directory

[anton, 2016-09-30 06:17:16]

https://codereview.adblockplus.org/29355392/diff/29355393/src/org/adblockplus...
File src/org/adblockplus/sbrowser/contentblocker/engine/Engine.java (right):

https://codereview.adblockplus.org/29355392/diff/29355393/src/org/adblockplus...
src/org/adblockplus/sbrowser/contentblocker/engine/Engine.java:261: for (final
File file : getFilterCacheDir(this.serviceContext).listFiles())
listFiles() can return `null` is getFilterCacheDir() does not exist. If you're
sure the directory exists, then it's ok, Otherwise check for null.
BTW, if something like that is used anywhere else i'd prefer to move it to
`FileUtils` helper class or smth like that. Not too important though

[diegocarloslima, 2016-09-30 14:35:47]

https://codereview.adblockplus.org/29355392/diff/29355393/src/org/adblockplus...
File src/org/adblockplus/sbrowser/contentblocker/engine/Engine.java (right):

https://codereview.adblockplus.org/29355392/diff/29355393/src/org/adblockplus...
src/org/adblockplus/sbrowser/contentblocker/engine/Engine.java:261: for (final
File file : getFilterCacheDir(this.serviceContext).listFiles())
On 2016/09/30 06:17:15, anton wrote:
> listFiles() can return `null` is getFilterCacheDir() does not exist. If you're
> sure the directory exists, then it's ok, Otherwise check for null.
> BTW, if something like that is used anywhere else i'd prefer to move it to
> `FileUtils` helper class or smth like that. Not too important though

The directory should exists in order for the previous line to succeed: 'final
File filterFile = this.subscriptions.createAndWriteFile()', but it won't hurt to
add a null check here since the cache cleaning is sort of an optional step, so
it shouldn't ever raise an Exception.

About the FileUtils: There are some other IO logic across the project that can
be moved to a helper class too, but that refactoring should be done in a
separate issue.

[anton, 2016-10-12 06:53:05]

On 2016/09/30 14:35:47, diegocarloslima wrote:
>
https://codereview.adblockplus.org/29355392/diff/29355393/src/org/adblockplus...
> File src/org/adblockplus/sbrowser/contentblocker/engine/Engine.java (right):
> 
>
https://codereview.adblockplus.org/29355392/diff/29355393/src/org/adblockplus...
> src/org/adblockplus/sbrowser/contentblocker/engine/Engine.java:261: for (final
> File file : getFilterCacheDir(this.serviceContext).listFiles())
> On 2016/09/30 06:17:15, anton wrote:
> > listFiles() can return `null` is getFilterCacheDir() does not exist. If
you're
> > sure the directory exists, then it's ok, Otherwise check for null.
> > BTW, if something like that is used anywhere else i'd prefer to move it to
> > `FileUtils` helper class or smth like that. Not too important though
> 
> The directory should exists in order for the previous line to succeed: 'final
> File filterFile = this.subscriptions.createAndWriteFile()', but it won't hurt
to
> add a null check here since the cache cleaning is sort of an optional step, so
> it shouldn't ever raise an Exception.
> 
> About the FileUtils: There are some other IO logic across the project that can
> be moved to a helper class too, but that refactoring should be done in a
> separate issue.

LGTM

[Felix Dahlke, 2016-10-12 07:59:25]

LGTM