Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto-set a torrent's parameters based on metadata #20502

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tgregerson
Copy link
Contributor

Add a system for configuring a set of rules that modify torrent parameters based on the torrent's metadata. Closes #5779.

Each Rule consists of a Condition and a Modifier. The Condition acts as a predicate applied to the metadata (e.g., trackers or files in the torrent). The Modifer performs some mutation on the torrent, such as setting its category or adding a tag. A given torrent may match multiple Rules, and Modifications are applied in the same order as the Rule definitions.

Rules are specified in a torrent_param_rules.json file placed in the config directory. If you can't access the attached file, a copy is available at https://pastebin.com/aMsE5ght. If this PR is accepted, further documentation can be added to the Wiki.

Rules are only applied to newly added torrents on initially receiving their metadata. Updates to an existing torrent's metadata, such as merging trackers, do not cause the Rules to be re-applied.

The following Conditions are included in this PR:

  • Logical AND of subconditions
  • Logical OR of subconditions
  • Logical NOT of a subcondition
  • Regex match of any file in the torrent
  • Regex match of any tracker URL attached to the torrent

The following Modifiers are included in this PR:

  • Set the category
  • Add a tag
  • Set the save path

The system is designed to be modular, so it should be possible to add more Conditions and Modifiers in later PRs without significant effort. I tried to limit this PR to some of the more commonly requested ones to keep it from getting too large.


Implementation notes:

The primary complicating factor for this feature is that some portions of metadata (e.g. file paths) are not guaranteed to be immediately available. For example, when using a magnet link, they are asynchronously downloaded. The download may not complete until after the add torrent GUI has been displayed or after the torrent has been added to the session. This requires handling three scenarios:

  • The metadata is immediately available or is downloaded prior to showing the add torrent dialog (if enabled). This is the simplest case, as all parameters are updated prior to displaying any related GUI elements.
  • The user has enabled the add torrent dialog, and the metadata is downloaded while the dialog is still active. In this case the modifications need to be applied and the add torrent dialog must be updated to reflect the modifications.
  • The metadata arrives after the torrent has been added, either because the add torrent dialog was disabled or the download finished after the user accepted the dialog. In this case the modifications are applied to the Torrent object, which in turn triggers update of the main UI (torrent list, categories, etc.)

@glassez glassez self-assigned this Mar 4, 2024
@glassez glassez added the Core label Mar 4, 2024
Copy link
Member

@glassez glassez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very preliminary comments. I haven't reviewed it completely.

src/base/bittorrent/torrentparamrules.cpp Outdated Show resolved Hide resolved
src/base/bittorrent/torrentparamrules.cpp Outdated Show resolved Hide resolved
src/base/bittorrent/torrentparamrules.cpp Outdated Show resolved Hide resolved
src/base/bittorrent/torrentparamrules.cpp Outdated Show resolved Hide resolved
src/base/bittorrent/torrentparamrules.cpp Outdated Show resolved Hide resolved
src/base/bittorrent/torrentparamrules.cpp Outdated Show resolved Hide resolved
src/base/bittorrent/torrentparamrules.cpp Outdated Show resolved Hide resolved
src/base/bittorrent/torrentparamrules.cpp Outdated Show resolved Hide resolved
@glassez
Copy link
Member

glassez commented Mar 4, 2024

Rules are specified in a torrent_param_rules.json file placed in the config directory.

The preliminary comment about the file format concerns the naming style of "predefined" entities, such as parameter names and values. Could you change them so that they use the same style, either snake_case or CamelCase? (I would prefer CamelCase.)

@tgregerson tgregerson force-pushed the master branch 2 times, most recently from 6b54657 to 9ed0866 Compare March 4, 2024 23:30
@tgregerson
Copy link
Contributor Author

I updated the sample rules config at https://pastebin.com/aMsE5ght to use CamelCase style for JSON keys.

@tgregerson tgregerson force-pushed the master branch 2 times, most recently from 2241c4a to a3b6b96 Compare March 5, 2024 23:27
Adds a system for configuring a set of rules that modify torrent
parameters based on the torrent's metadata. Rules are specified
via a JSON file loaded on startup.

Closes qbittorrent#5779.
@tgregerson
Copy link
Contributor Author

Any further comments @glassez ?

@glassez
Copy link
Member

glassez commented Mar 30, 2024

Any further comments @glassez ?

I need more time to think about it in detail, but right now I have other tasks as a priority.

@trim21
Copy link
Contributor

trim21 commented Apr 23, 2024

Rules are specified in a torrent_param_rules.json file placed in the config directory.

The preliminary comment about the file format concerns the naming style of "predefined" entities, such as parameter names and values. Could you change them so that they use the same style, either snake_case or CamelCase? (I would prefer CamelCase.)

but our API is in snake_case, would be better to use same case

@glassez
Copy link
Member

glassez commented May 13, 2024

Any further comments @glassez ?

I need more time to think about it in detail, but right now I have other tasks as a priority.

Well, I thought about it in terms of entire application architecture, precisely in terms of the coexistence of various aspects that are supposed to affect the "add torrent parameters". I will immediately make a reservation that qBittorrent does not fully meet below architecture internally, but it behaves accordingly from a user side point. But by introducing this new subsystem into it, we still have to adjust some of the existing logic so that they all continue to behave appropriately.

"Add torrent parameters" processing is defined as follows:

  1. Any parameter can either be set explicitly or not.
  2. The parameters pass through a chain of handlers that can specify one or another parameter.
  3. The parameter can be set by some handler ONLY if it was not set by any of the previous handlers.
  4. The parameters that are not explicitly set take the default value at the last stage of handling.

In this case, one of the main points is 3. That is, when you want to set a parameter, you must check whether it was explicitly set before.

The only problem is that currently the "Add new torrent dialog" is designed in such a way that it always explicitly sets the parameters, even if it just passes the default values, which means that it needs to be redesigned so that this new subsystem works as expected in the case when we are dealing with magnet links added using "Add new torrent dialog" (i.e. if user didn't change save path it should be passed as "not set" in order to be assigned by subsequent handler).

@tgregerson
If some aspect of the required changes is causing you difficulties, I can help with its implementation.

@tgregerson
Copy link
Contributor Author

tgregerson commented May 13, 2024

Any further comments @glassez ?

I need more time to think about it in detail, but right now I have other tasks as a priority.

Well, I thought about it in terms of entire application architecture, precisely in terms of the coexistence of various aspects that are supposed to affect the "add torrent parameters". I will immediately make a reservation that qBittorrent does not fully meet below architecture internally, but it behaves accordingly from a user side point. But by introducing this new subsystem into it, we still have to adjust some of the existing logic so that they all continue to behave appropriately.

"Add torrent parameters" processing is defined as follows:

1. Any parameter can either be set explicitly or not.

2. The parameters pass through a chain of handlers that can specify one or another parameter.

3. The parameter can be set by some handler ONLY if it was not set by any of the previous handlers.

4. The parameters that are not explicitly set take the default value at the last stage of handling.

In this case, one of the main points is 3. That is, when you want to set a parameter, you must check whether it was explicitly set before.

The only problem is that currently the "Add new torrent dialog" is designed in such a way that it always explicitly sets the parameters, even if it just passes the default values, which means that it needs to be redesigned so that this new subsystem works as expected in the case when we are dealing with magnet links added using "Add new torrent dialog" (i.e. if user didn't change save path it should be passed as "not set" in order to be assigned by subsequent handler).

@tgregerson If some aspect of the required changes is causing you difficulties, I can help with its implementation.

Regarding 3., what do you want to do about non-scalar parameters, such as tags (a set)?.

Should a handler only be allowed to insert tags if the tagset is empty? Or should it always be allowed?

Personally I would be inclined to go with the latter. I can imagine situations where that would be useful. For example, a user may want to specify some tags when adding a torrent via RSS or command line, but may also want additional tags added based on metadata.

@glassez
Copy link
Member

glassez commented May 14, 2024

Should a handler only be allowed to insert tags if the tagset is empty? Or should it always be allowed?

Personally I would be inclined to go with the latter. I can imagine situations where that would be useful.

(I forgot to mention them...)
I believe the handler should be allowed to add tags, but not delete previously added ones.

@glassez
Copy link
Member

glassez commented May 14, 2024

The only problem is that currently the "Add new torrent dialog" is designed in such a way that it always explicitly sets the parameters, even if it just passes the default values, which means that it needs to be redesigned so that this new subsystem works as expected in the case when we are dealing with magnet links added using "Add new torrent dialog" (i.e. if user didn't change save path it should be passed as "not set" in order to be assigned by subsequent handler).

It seems that not only the above problem is related to the magnet links.
Currently, the final resolution of "add torrent parameters" for all sources (both for torrent files and for magnet links) is performed immediately before adding the torrent to the session. However, due to the fact that this new handler needs the metadata to be received before it can do its job, we have to postpone final parameter resolution (some of them) until "metadata received" event.

@glassez
Copy link
Member

glassez commented May 14, 2024

@tgregerson
Considering the above, I think it is acceptable to leave the current behavior (at least until we receive feedback from users of this feature), i.e., to agree that this feature takes precedence in assigning parameters, so it will overwrite previously set parameters.

@tgregerson
Copy link
Contributor Author

@tgregerson Considering the above, I think it is acceptable to leave the current behavior (at least until we receive feedback from users of this feature), i.e., to agree that this feature takes precedence in assigning parameters, so it will overwrite previously set parameters.

Sounds good to me. I don't have particularly strong feelings one way or other on the precedence thing; I went with the current approach because it seemed to require fewer changes to the codebase. I'm happy to revisit it in the future based on user feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatic Category Assignment
3 participants