Provide interface to access contents of subscriptions.json

[greiner]

Background

We're fetching, parsing and interpreting the contents of subscriptions.json (previously subscriptions.xml) in various places throughout our code (adblockpluschrome/subscriptionInit.js, adblockplusui/js/desktop-options.js, adblockplusui/mobile-options.js) and with ​ui#9, we'd do that again.

While we could share that code across adblockplusui or even adblockpluschrome, this seems to be something that's relevant for anyone who has Core as a dependency. Also, discouraging non-Core code from directly accessing subscriptions.json would allow Core to assert more control over how it's being used.

Further information

Currently implemented at:

• ​adblockpluschrome/subscriptionInit.js
• ​adblockplusui/js/desktop-options.js
• ​adblockplusui/mobile-options.js

What to change

Expose API for accessing information contained within subscriptions.json from outside Core

Introduce a new *recommendations() generator function in a new file lib/recommendations.js that yields Recommendation objects based on data/subscriptions.json.

Also add a test/recommendations.js.

Hints for testers

Please see the relevant dependency update ticket for adblockpluschrome (Platform).

[sebastian]

Also note that with manifest V3 we no longer have access to DOMParser in the background page but are limited to the APIs available in Service Workers (plus extension APIs). So it might be a good time to change the format of subscription.xml to JSON.

[mjethani]

@kzar thanks for pointing out about DOMParser

Would it be enough for the three use cases to simply provide a subscriptions.json instead?

Also I am wondering if we should move the file to the data directory in adblockpluscore now.

[hfiguiere]

This involve changing the sitescripts to generate JSON instead of XML from the subscriptionlist (an issue should be filed). Moving it to the data directory would make sense.

[sebastian]

Replying to mjethani:

@kzar thanks for pointing out about DOMParser

That was me. ;)

Also I am wondering if we should move the file to the data directory in adblockpluscore now.

Agreed.

Replying to hfiguiere:

This involve changing the sitescripts to generate JSON instead of XML from the subscriptionlist (an issue should be filed). Moving it to the data directory would make sense.

Please correct me if I'm wrong, but IIRC that automation has been inactive for years. Either way, we should replace it with an npm script within adblockpluscore, and if necessary automate it through GitLab CI (it supports periodical jobs).

[hfiguiere]

Replying to sebastian:

Please correct me if I'm wrong, but IIRC that automation has been inactive for years. Either way, we should replace it with an npm script within adblockpluscore, and if necessary automate it through GitLab CI (it supports periodical jobs).

I have been running these scripts each and every time I updated the .xml file.
But yes we can replace this with something less obscure.

[mjethani]

Let's open a separate ticket for the work in the Automation module for generating a subscriptions.json in the data directory in adblockpluscore. Let's keep this one for any further work in adblockpluscore for providing an API over the subscriptions.json.

There are two options: adblockpluschrome and adblockplusui use the subscriptions.json file directly by require()'ing it, or adblockpluscore provides an API (e.g. subscriptions.query({prefixes: ["es"]})).

[mjethani]

This ticket is available to work on now (cc: @jsonesen).

[mjethani]

Alternatively we might add a filter() function for iterables (#6434) and then the recommendations() function could just yield all recommendations, which can then be filtered like so:

for (let recommendation of filter(
  recommendations(),
  ({type, languages}) => ["ads", "circumvention"].includes(type) &&
                         languages.includes(Utils.appLocale.substring(0, 2))
))
{
  ...
}

This way we don't have to implement any filtering logic here.

[greiner]

Maybe it's just me and feel free to ignore but there appears to be quite a bit of redundancy in let recommendations = require("recommendations").recommendations(…);?

Presumably, as part of a different module, that generator name would be more fitting (e.g. require("data").recommendations()). Otherwise a name such as require("recommendations").query() could also be an option, in particular to describe what to do with the recommendations.

[sebastian]

I may have another suggestion. How about exposing metadata from subscriptions.json as properties on the Subscription object, and then add a new function to the subscriptionClasses module that returns an array/generator of all subscription objects for the filter list URLs given in subscriptions.json?

[mjethani]

Replying to greiner:

Maybe it's just me and feel free to ignore but there appears to be quite a bit of redundancy in let recommendations = require("recommendations").recommendations(…);?

I was thinking you would import it like this:

let {recommendations} = require("recommendations");

This is similar to how various other modules are imported.

[mjethani]

Replying to sebastian:

I may have another suggestion. How about exposing metadata from subscriptions.json as properties on the Subscription object, and then add a new function to the subscriptionClasses module that returns an array/generator of all subscription objects for the filter list URLs given in subscriptions.json?

That would be too much coupling and actually we are trying to go in the other direction of using more and more plain objects. In the end I see both lib/filterClasses.js and lib/subscriptionClasses.js going away.

[abpbot]

A commit referencing this issue has landed:
​Issue 7360 - Add recommendations module

[Ross]

Done. Subscription behaviour still works as expected.

ABP 0.9.15.2340
Microsoft Edge 44.17763.1.0 / Windows 10 1809

ABP 3.5.2.2340
Chrome 49.0.2623.75 / Windows 10 1809
Chrome 75.0.3770.142 / Windows 10 1809
Opera 36.0.2130.65 / Windows 10 1809
Opera 62.0.3331.72 / Windows 10 1809
Firefox 51.0 / Windows 10 1809
Firefox 68.0 / Windows 10 1809
Firefox Mobile 68.0 / Android 7.2.2