feat(filters): RED fetch uploader from API

[alekc]

This refactors the AdditionalSizeCheck to a more generic AdditionalDetailsCheck and adds the extraction of the uploader for Red indexer.

partly extracted from #1249

[zze0s]

Thanks for the PR, this will be useful!

However, I do think this is too generic in it's current form. Previously we had a specific Size check function AdditionalSizeCheck whos only job was to fetch torrent size via API or default back to download the .torrent and parse the metainfo.

In it's current form this function will be triggered under the following conditions:

• Uploader is not announced and Match/Except uploader has value

In the case where uploader is not announced and the user has filled Match Uploader they will get a rejection, so it likely will not happen too much, but if they instead have Except Uploader that will lead to the AdditionalDetailsCheck triggering for each announce and it will try to download the .torrent but it will not find the correct info.

Some suggestions for something better:

• Split the external fetch code is one option. We can hardcode it for RED/OPS for now
• Keep the old if release.Size == 0 { check and another guard for uploader
• Maybe add a new case "redacted", "ops": that takes care of only RED and OPS that adds the uploader and size, and do something about the default case so it only handles size.

The more robust solution would be to add additional info on the indexer definition about what the api can return but that is a lot more work.

[alekc]

In it's current form this function will be triggered under the following conditions:

Uploader is not announced and Match/Except uploader has value

Not exactly. It will trigger in following cases:

• Filter by size is required but we are lacking such info
• A filter by MatchUploader is required but we are lacking such info (they would not get a rejection right away)
• A filter by ExceptUploader is required but we are lacking such info

In those cases it will try to hit the api and get some further info.

Once we have hopefully obtained some additional info I simply rerun all the checks.

I missed the torrent download, that bit should be executed only if we want to filter by size and we do not have information from the API.

I will rework the code to deal with the default case & file downloading.

What we can also do, is to extend the new checkUploader (with generics?) and to expand it to all filter values (if we want to filter by xxx but it's not set, then download additional info from the API.

func (f *Filter) checkUploader(r *Release, uploaders string, wantedResult bool) bool {
	if uploaders != "" && r.Uploader == "" {
		r.AdditionalDetailsCheckRequired = true
		return false
	}
	return contains(r.Uploader, uploaders) == wantedResult
}

However this would require a change in some type of data converting it to the pointer (similar to how AWS / Kubernetes deals with it, a default False doesn't mean that it's not seat but if a *bool == false, then it's set)

[zze0s]

In it's current form this function will be triggered under the following conditions:

Uploader is not announced and Match/Except uploader has value

Not exactly. It will trigger in following cases:

* Filter by size is required but we are lacking such info

* A filter by MatchUploader is required but we are lacking such info (they would not get a rejection right away)

* A filter by ExceptUploader is required but we are lacking such info

In those cases it will try to hit the api and get some further info.

I had more cases in there but was copy pasting stuff so they went missing. So yeah it will trigger for more cases which isn't ideal.

Once we have hopefully obtained some additional info I simply rerun all the checks.

I missed the torrent download, that bit should be executed only if we want to filter by size and we do not have information from the API.

I will rework the code to deal with the default case & file downloading.

What we can also do, is to extend the new checkUploader (with generics?) and to expand it to all filter values (if we want to filter by xxx but it's not set, then download additional info from the API.

func (f *Filter) checkUploader(r *Release, uploaders string, wantedResult bool) bool {
	if uploaders != "" && r.Uploader == "" {
		r.AdditionalDetailsCheckRequired = true
		return false
	}
	return contains(r.Uploader, uploaders) == wantedResult
}

However this would require a change in some type of data converting it to the pointer (similar to how AWS / Kubernetes deals with it, a default False doesn't mean that it's not seat but if a *bool == false, then it's set)

What would that change look like?

Also could you try not to force push, it's hard to keep track of changes between commits.

[alekc]

I had more cases in there but was copy pasting stuff so they went missing. So yeah it will trigger for more cases which isn't ideal.

Why it's not ideal? It only will be triggered once, and only if there is a will from the uploader to filter by something which is not available from the announce out of box.

As for the function, I've tried to flesh out something with generic but went into a rabbit hole, I will try to do a POC once this merged/approved

[zze0s]

Currently it will trigger when there's Size or Uploader check in the filter, and for indexers that does not announce size nor uploader.

As an example, the default case would trigger for Milkie to try and get uploader from the torrent file, and other indexers.

That's how I read it, maybe I'm missing something critical with your changes.

[alekc]

Hmm, it maintains exactly the same behaviour as in the current master.
So, lets say we want to filter by the uploader for the indexer Milkie

It will correctly flag that it needs some more information because it lacks uploaders, so it will go inside additionalDetailCheck

on https://github.com/alekc/autobrr/blob/10822ccf66bae99a9e5adf58a7f252c79ac79126/internal/filter/service.go#L441 It will see that it's not in the list of trackers who has api access so will fall into the default use case

on https://github.com/alekc/autobrr/blob/10822ccf66bae99a9e5adf58a7f252c79ac79126/internal/filter/service.go#L463 it will check if there is a requirement to filter by either Min/Max, if there is it will attempt to download torrent file, otherwise it will run checks again and continue

[zze0s]

There's a a few gotchas with the codebase and probably missing context as to why things are done a certain way and so on.

Current master only handles size for that extra check, not other info such as uploader. So if the filters decide to set additionalDetailCheck = true for uploader missing in the filter check

Then it will hit the default case if size is not announced for example with Milkie, even if it's trying to only match uploader.

I haven't checked other indexers but I'm pretty sure it will be the same other others.