[WIP] Redesign RSS subsystem

[glassez]

@sledgehammer999 I started this job that has long been required to do.

The RSS subsystem has a "horrible" design, which prevents to add new features and optimize code acceptable way. Some old feature-requests are still not satisfied because of its fundamental problems.
So I want to fix this. My work focused mainly on structural changes and internal code optimization of its base part, not the addition of some new features. I plan to make more lightweight "flat" model that will enable you to conveniently utilize it not only via GUI, but in a different way too (via WebUI, for example). Of course this will lead to changes in other code to fit to the new model.

However, I see a lot of pending RSS-related PRs (from @magao and @mgziminsky). Our changes have a lot of conflict, because I replace the existing design, and they are changing it. So I ask to resolve this proactively. I offer three options for your (@sledgehammer999) decision:

1. You reject my work and I stop doing it.
2. You freeze the code from other changes (if it not so necessary), until I'm done and you merge it.
3. You have to merge the other PR as needed, and I continue my work, but I base on some legacy code, and when I'm done, we're trying to apply missing changes.

[glassez]

@qbittorrent/qbittorrent-frequent-contributors, I want to know your opinion about this too.

[mgziminsky]

I'd be happy to help contribute to a newly designed RSS system should that be the decision.

Personally, I don't think the overall design of the current system is really that bad. I do think the implementation of it is awful though. I've been working on incrementally rewriting what I believe to be the worst parts, and I expect the result to be something that is much more maintainable and flexible.

[magao]

I'd be happy for @glassez and @mgziminsky to work out a new design/implementation for RSS. I don't really have the time to contribute to a large task and would be happy to look at reimplementing my PRs on top of any such redesign should they still be necessary. I've been going with the approach of trying to fit into the existing design so it's not too much of a commitment of my time.

I would request that you look at my PRs for an indication of the problems I've found with the current implementation and workflows I want to be able to follow - #6177, #6178 and #6187. BTW I'm not at all happy with the approach I took for #6178 - I think it should be using QThread, but I couldn't work out how.

I should review #6474 but can't get to it this weekend - maybe @glassez
should discuss with @mgziminsky on how/whether he should proceed on it.

[sledgehammer999]

Unless I am interpreting wrong what @mgziminsky and @magao have said, I want to wait for your RSS redesign without merging their PRs. And then those 2 users (if still interested and active) can work on the new code for their features.

One minor exception is bugfixing to current code, without adding new features.

@glassez I know that your primary goal is to transfer current functionality to your new design. But if it isn't too much hassle, take a look at what both those 2 users are trying to fix. Maybe it will be easier for you to fix along the way of redesigning. (this is not a mandatory step though).

[magao]

@sledgehammer999 You've interpreted me correctly.

One thing I would suggest with a redesign is to separate the rules from being directly associated with RSS. Instead I think rules should be applied to any torrent that is added in any way e.g. if manually adding a torrent then the rules should be applied to whatever information we have available (e.g. filename, maybe info from DHT for magnet links). The rules wouldn't prevent adding the torrent, but would apply save directories, labels, etc from matching rules.

[glassez]

Instead I think rules should be applied to any torrent that is added in any way e.g. if manually adding a torrent then the rules should be applied to whatever information we have available (e.g. filename, maybe info from DHT for magnet links). The rules wouldn't prevent adding the torrent, but would apply save directories, labels, etc from matching rules.

Hmm... you've got far-reaching plans. I can't imagine the usefulness of this, but I'm only basic user and I do not need many advanced features.
At least I'm going to separate basic rss code and autodownloader code.

[magao]

No problem - just thought that since that is in my eventual plans, I'd bring it up now so you didn't design something that was likely to preclude it.

[glassez]

@sledgehammer999, @magao, @mgziminsky, a few suggestions for you:

1. One of current behaviors is to forget empty rss folders on restart (IIRC). Is it expected behavior? Redesign is the point to change it if needed.
2. Now we store RSS feeds in application settings (IMO, very bad in any case) and feed articles in separate .ini file (using QSettings). Since one of my work goals is allowing alternative RSS management (via WebUI, command line or just editing RSS configure file) I propose storing RSS feeds in separate human-readable/writable JSON (IMO, better) or XML file. Also we should store articles cache in per-feed JSON/XML files in some qBittorrent temp folder (e.g. cache). Additionally I want to perform file operations in separate thread.

Waiting your opinion on things above...

[glassez]

Feed config file example:

{
    "folder1": {
        "subfolder1": {
            "http://some-feed-url1": "Feed name (Alias)",
            "http://some-feed-url2": ""
        },
        "subfolder2": {},
        "http://some-feed-url3": "",
        "http://some-feed-url4": "Feed name (Alias)"
    },
    "folder2": {},
    "folder3": {}
}

[magao]

@glassez I don't know if it was originally intentional that empty folders be discarded. I can see arguments for and against.

I definitely think an alternative format for the feed definitions would be preferable. One thing I would suggest would be rather than just a mapping from URL: Alias, you should have a URL: Map to allow for additional attributes. In particular I'm thinking of #1991 and #5738 (which are essentially duplicates). So something like:

{
    "folder1": {
        "subfolder1": {
            "http://some-feed-url1": {
                "alias": "Alias 1",
                "enabled": true,
            },
            "http://some-feed-url2": ""
        },
        "subfolder2": {},
        "http://some-feed-url3": "",
        "http://some-feed-url4": {
            "alias": "Alias 4",
            "enabled": false,
        }
    },
    "folder2": {},
    "folder3": {}
}

I think using JSON for the rules would also be good. Currently they're completely opaque.

BTW the feed articles were split out just recently in PR #6175 - previously they were stored with the rules. A file per feed sounds like a good idea to me (in a subfolder of the config directory).

[mgziminsky]

1. I think empty folders should be kept. It definitely surprised me the first time it happened to me. I thought one of my changes had broken something until I determined it was always like that.

2. I definitely think all the feed configuration should be stored together in a separate file from the main settings. I think JSON would be better than XML, and having a cache file per feed would be good. I don't know that doing the file IO on a separate thread would actually provide any real benefit though, except possibly for the articles, which should be fetched on a separate thread regardless.

I also like @magao's suggestion of having the other feed attributes as part of the same json, but in that case we will need a well defined way to disambiguate feeds, folders, and attributes.

[glassez]

A file per feed sounds like a good idea to me (in a subfolder of the config directory).

It's a bad idea storing application data (cached rss articles in this case) alongside with application configuration. So I insist on some more appropriate location for it.

I also like @magao's suggestion of having the other feed attributes as part of the same json, but in that case we will need a well defined way to disambiguate feeds, folders, and attributes.

Of course, we should use more complex format in this case. Or we can determine item type using some exclusive required attribute (e.g. we can make feed alias to be required and determine feed using it; we still can use shortcuts for feed in this case - if item (JSON) type is string or type is object and it has "alias" field then the item is feed, otherwise the item is folder).

{
    "folder1": {
        "subfolder1": {
            "http://some-feed-url1": "Feed name (Alias)",
            "http://some-feed-url2": ""
        },
        "subfolder2": {},
        "http://some-feed-url3": "",
        "http://some-feed-url4": {
            "alias": "Feed name (Alias)",
            "enabled": false
        }
    },
    "folder2": {},
    "folder3": {}
}

[glassez]

As we are going to store the rules in a human-readable format, do we still have legacy Import/Export features?

[mgziminsky]

I think it would be ideal if the rules were in json as well. At least for me, the majority of my rules follow the same basic template. So when I want to add multiple new ones, it would be great if i could just open up a text editor and copy/paste a few times rather than having to type the same thing into every one from the UI.

There is currently a rule import/export feature, which is convenient to have sometimes. The issue with the current version though, is that it only exports rules, not feeds, which significantly reduces its usefulness since rules are tied to feeds. I think it would be better if users could import/export their "RSS configuration" and selectively choose what to include, be it feeds, rules, settings, etc. That's more of a feature request though, and doesn't necessarily impact the design of the new system

[glassez]

I mean, if we already have everything we need in JSON format in the same folder, why we need to do something with this? The user can simply copy what they need. Another thing is the import from third-party applications which we do not yet have.
In General, I suggest to get rid of these Import/Export features in their current meaning. Later we can add something new if necessary.

[mgziminsky]

I agree, I don't think it's very important, especially given everything is stored in its own file, even now. If you want to drop it that's fine. I don't think an import/export feature really matters one way or the other to the RSS system's design.

[glassez]

Here's another problem that I faced and which needs to be solved.
Since the RSS core will be independent from the GUI (for access from WebUI; and in General, it is a good idea in itself), we are faced with the problem of concurrency, if we make components to be enable/disable during runtime. E.g. RSS GUI can be opened when RSS feature is enabled and then RSS feature can be disabled (from WebUI, for example).
We have several ways to solve this problem:

1. Fully implement the concurrent access (the most complex).
2. Allow enabling/disabling components when qBittorrent is restarted only.
3. Make permanent components, but allow enabling/disabling them processing features (i.e. we can add/edit/remove feeds/rules everytime, but feed loading and torrent autodownloading can be disabled).

When I talk about concurrency here, I don't mean multi-threading. We still can have event-based concurrency, since dialogs have its own event loops.

@evsh, I inviting you to join the discussion on this topic. Even if you have nothing about RSS, you can offer interesting solutions for common issues here.

[glassez]

We still can have event-based concurrency, since dialogs have its own event loops.

Even having only one event loop we still may have this issue.

[magao]

IMO we should allow configuration (feeds, URLs, etc) at all times, and simply enable/disable processing - your option 3.

[glassez]

IMO we should allow configuration (feeds, URLs, etc) at all times, and simply enable/disable processing - your option 3.

I thought about it. Although, this may be the easiest option to implement, but we must not forget those who do not use RSS feature at all. Why should they get an unnecessary application activity?

[glassez]

@sledgehammer999, most of the work is already finished, and I'm almost ready to publish the PR. But first I need to know your opinion about optional application components (see above). @ngosang, @Chocobo1, what are your opinions (if you have something about this issue)?

[glassez]

I thought about it. Although, this may be the easiest option to implement, but we must not forget those who do not use RSS feature at all. Why should they get an unnecessary application activity?

Although, if we look closely, the basic components of the RSS takes a little memory and do nothing if they are inactive. So option 3 is preferable, if no one would mind.

[glassez]

So option 3 is preferable, if no one would mind.

As I understand it, no one has opinions about it.
Waiting for a couple more days while I finish the rest, then I implement it for my taste.