Skip to content

Store WebUI search tabs between app restarts and across sessions#23784

Open
Piccirello wants to merge 3 commits intoqbittorrent:masterfrom
Piccirello:tom/search-improvements
Open

Store WebUI search tabs between app restarts and across sessions#23784
Piccirello wants to merge 3 commits intoqbittorrent:masterfrom
Piccirello:tom/search-improvements

Conversation

@Piccirello
Copy link
Member

This PR makes two major improvements to WebAPI/WebUI searches:

  • Searches and their results are now persisted across qBittorrent restarts. This mimics the same behavior available in the GUI.
  • All searches are now available to and shared among all WebAPI sessions. This makes sense given that qBittorrent's WebAPI is single user.

@Piccirello Piccirello requested a review from a team January 26, 2026 02:32
@Piccirello Piccirello added WebUI WebUI-related issues/changes Search engine Issues related to the search engine/search plugins functionality WebAPI WebAPI-related issues/changes labels Jan 26, 2026
@Piccirello Piccirello force-pushed the tom/search-improvements branch from ec7600e to 3230ad2 Compare January 26, 2026 02:33
@glassez
Copy link
Member

glassez commented Jan 26, 2026

  • This makes sense given that qBittorrent's WebAPI is single user.

But multi user access has been requested several times.
Is it generally rejected, so it won't be implemented for some time?

@Piccirello
Copy link
Member Author

  • This makes sense given that qBittorrent's WebAPI is single user.

But multi user access has been requested several times. Is it generally rejected, so it won't be implemented for some time?

It's been requested but I don't think anyone plans to build it. I personally think qBittorrent should remain single user.

@glassez
Copy link
Member

glassez commented Jan 26, 2026

It's been requested but I don't think anyone plans to build it.

Well, why is that? Given that web sessions are initially implemented as isolated, this is not so difficult to do in a simplified form (which is sufficient for most users requesting this). In addition, it would allow access through a restricted account, which, for example, would not have access to settings.

But if you prefer to nip this opportunity in the bud and you get the approval of the other members, well, so be it.

@Piccirello
Copy link
Member Author

Piccirello commented Jan 26, 2026

This isn't the first time this has come up on one of my PRs. The approach we've always taken is that we shouldn't prevent adding new features just because we "might" add multi user support some day. This PR also doesn't prevent us from adding multi user support, it would just need to be refactored a bit.

Well, why is that?

If I had to guess, it's because none of the maintainers use qBittorrent in a multi user setup.

@glassez
Copy link
Member

glassez commented Jan 26, 2026

If I had to guess, it's because none of the maintainers use qBittorrent in a multi user setup.

If qBittorrent only had what maintainers use, then it wouldn't have quite a lot of its current features.

@glassez
Copy link
Member

glassez commented Jan 27, 2026

@Piccirello
As for the PR itself.
Since you want to provide sharing search related data between web sessions, it would be easier to just have a single instance of the SearchController that lives outside of any WebSession (like AuthController does). Creating a separate instance of the controller per session served only to restrict access to the data of another session, so it no longer makes sense for SearchController.

@Piccirello
Copy link
Member Author

I can make that change. However, in the past whenever I've wanted to share data across session instances, the recommendation has been to instantiate the data structure in WebApplication and pass it via the controller's constructor. I think this approach has the benefit that it follows the same pattern as all other controllers (except AuthController, which is unique). Won't using a different approach for SearchController be confusing? There's also no guarantee that all future SearchController data will be shared across clients.

@glassez
Copy link
Member

glassez commented Jan 27, 2026

I can make that change. However, in the past whenever I've wanted to share data across session instances, the recommendation has been to instantiate the data structure in WebApplication and pass it via the controller's constructor. I think this approach has the benefit that it follows the same pattern as all other controllers (except AuthController, which is unique).

It's not about the pattern (in the sense of simply choosing one of several equivalent ways). The design was based on the fact that sessions are supposed to be isolated from each other so all the controllers was instantiated inside the sessions by default. AuthController is an exception because (it is obvious that) "authentication" lives outside of sessions.
If we abandon the idea of multi-user access, then we can change it so that the controllers have one instance by default, with the exception of those that still require data isolation, such as SyncController (since it is unlikely that the synchronized state is supposed to be shared between sessions even in single user architecture).

Won't using a different approach for SearchController be confusing?

As I said above, this is not a different approach. This is a unified approach in which the controller instance lives in the area corresponding to its nature.

@glassez
Copy link
Member

glassez commented Jan 28, 2026

There's also no guarantee that all future SearchController data will be shared across clients.

Okay, that might make sense. Let's try to review it, since you've already done it this way. Although the first thought was "why make so many changes if you could just make the SearchController instance live outside of sessions?".

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.

I would prefer to review SearchJobManager implementation after my current comments are addressed.

@Piccirello Piccirello force-pushed the tom/search-improvements branch 2 times, most recently from 58cc4a7 to cec6f96 Compare February 8, 2026 17:48
@Piccirello Piccirello requested a review from glassez February 8, 2026 17:49
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.

Yet another portion of review from me.
I'll try to get back to it ASAP.

@Piccirello Piccirello force-pushed the tom/search-improvements branch 2 times, most recently from a36d513 to 21f9c03 Compare February 11, 2026 06:47
@Piccirello
Copy link
Member Author

Latest round of comments were very helpful, and all feedback has been addressed.

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.

Still unfinished review...

@Piccirello Piccirello force-pushed the tom/search-improvements branch from 21f9c03 to 29075cb Compare February 16, 2026 01:02
Rename preference methods and keys for clarity and reuse between GUI and WebAPI.
This allows users to access their search jobs across different web sessions. Switching to a different browser/device will now show the same search jobs.
@Piccirello Piccirello force-pushed the tom/search-improvements branch from 29075cb to 30f09e2 Compare February 16, 2026 01:35
This was inspired by the GUI version implemented in e644a91.
@Piccirello Piccirello force-pushed the tom/search-improvements branch from 30f09e2 to bf47beb Compare February 23, 2026 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Search engine Issues related to the search engine/search plugins functionality WebAPI WebAPI-related issues/changes WebUI WebUI-related issues/changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants