Skip to content

WebUI: Store durable settings in client data API #23191

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+266 −88
Open

WebUI: Store durable settings in client data API #23191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/webui/www/private/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
<script defer src="scripts/monkeypatch.js?v=${CACHEID}"></script>
<script defer src="scripts/cache.js?v=${CACHEID}"></script>
<script defer src="scripts/localpreferences.js?v=${CACHEID}"></script>
<script defer src="scripts/client-data.js?v=${CACHEID}"></script>
<script defer src="scripts/color-scheme.js?v=${CACHEID}"></script>
<script defer src="scripts/mocha-init.js?locale=${LANG}&v=${CACHEID}"></script>
<script defer src="scripts/lib/clipboard-copy.js"></script>
Expand Down
9 changes: 3 additions & 6 deletions src/webui/www/private/scripts/addtorrent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ window.qBittorrent.AddTorrent ??= (() => {
let source = "";
let downloader = "";

const localPreferences = new window.qBittorrent.LocalPreferences.LocalPreferences();
const clientData = window.parent.qBittorrent.ClientData;

const getCategories = () => {
const defaultCategory = localPreferences.get("add_torrent_default_category", "");
const defaultCategory = clientData.get("add_torrent_default_category") ?? "";
const categorySelect = document.getElementById("categorySelect");
for (const name of window.parent.qBittorrent.Client.categoryMap.keys()) {
const option = document.createElement("option");
Expand Down Expand Up @@ -320,10 +320,7 @@ window.qBittorrent.AddTorrent ??= (() => {

if (document.getElementById("setDefaultCategory").checked) {
const category = document.getElementById("category").value.trim();
if (category.length === 0)
localPreferences.remove("add_torrent_default_category");
else
localPreferences.set("add_torrent_default_category", category);
clientData.set({ add_torrent_default_category: (category.length > 0) ? category : null }).catch(console.error);
}
};

Expand Down
129 changes: 129 additions & 0 deletions src/webui/www/private/scripts/client-data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
/*
* Bittorrent Client using Qt and libtorrent.
* Copyright (C) 2025 Thomas Piccirello <[email protected]>
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 2
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*
* In addition, as a special exception, the copyright holders give permission to
* link this program with the OpenSSL project's "OpenSSL" library (or with
* modified versions of it that use the same license as the "OpenSSL" library),
* and distribute the linked executables. You must obey the GNU General Public
* License in all respects for all of the code used other than "OpenSSL". If you
* modify file(s), you may extend this exception to your version of the file(s),
* but you are not obligated to do so. If you do not wish to do so, delete this
* exception statement from your version.
*/

"use strict";

window.qBittorrent ??= {};
window.qBittorrent.ClientData ??= (() => {
const exports = () => {
return new ClientData();
};

// this is exposed as a singleton
class ClientData {
/**
* @type Map<string, any>
*/
#cache = new Map();

#keyPrefix = "qbt_";

#addKeyPrefix(data) {
return Object.fromEntries(Object.entries(data).map(([key, value]) => ([`${this.#keyPrefix}${key}`, value])));
}

#removeKeyPrefix(data) {
return Object.fromEntries(Object.entries(data).map(([key, value]) => ([key.substring(this.#keyPrefix.length), value])));
}

/**
* @param {string[]} keys
* @returns {Record<string, any>}
*/
async #fetch(keys) {
keys = keys.map(key => `${this.#keyPrefix}${key}`);
return await fetch("api/v2/clientdata/load", {
method: "POST",
body: new URLSearchParams({
keys: JSON.stringify(keys)
})
})
.then(async (response) => {
if (!response.ok)
return;

const data = await response.json();
return this.#removeKeyPrefix(data);
});
}

/**
* @param {Record<string, any>} data
*/
async #set(data) {
data = this.#addKeyPrefix(data);
await fetch("api/v2/clientdata/store", {
method: "POST",
body: new URLSearchParams({
data: JSON.stringify(data)
})
})
.then((response) => {
if (!response.ok)
throw new Error("Failed to store client data");
});
}

/**
* @param {string} key
* @returns {any}
*/
get(key) {
return this.#cache.get(key);
}

/**
* @param {string[]} keys
* @returns {Record<string, any>}
*/
async fetch(keys = []) {
const keysToFetch = keys.filter((key) => !this.#cache.has(key));
if (keysToFetch.length > 0) {
const fetchedData = await this.#fetch(keysToFetch);
for (const [key, value] of Object.entries(fetchedData))
this.#cache.set(key, value);
}

return Object.fromEntries(keys.map((key) => ([key, this.#cache.get(key)])));
}
Comment on lines +96 to +113
Copy link
Member

Choose a reason for hiding this comment

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

On a second look, I will redesign these two function:

  1. get() will first try to get the value from #cache and if it does not exist, fetch it from the server.
  2. fetch() will just get the values from server and store it in cache. It will not return any value but only Promise. This function will become a "prefetch" action. The caller should only use get() for getting the values. The API usage would be simpler this way.

Copy link
Member Author

Choose a reason for hiding this comment

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

This suggestion is not consistent with usage. See createtorrent.html.

Copy link
Member

Choose a reason for hiding this comment

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

This suggestion is not consistent with usage. See createtorrent.html.

My suggestion will work in createtorrent.html and it will be clearer.

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer the current interface and think it makes sense.

Copy link
Member

@Chocobo1 Chocobo1 Sep 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I prefer the current interface and think it makes sense.

The interface is confusing. It will produce questions like these: get() isn't really getting the value. It is only getting from the cache. If the cache doesn't already hold the value, it fails. This contracts the assumption that get() is getting value from the server.
And fetch() is also returning the values. Which function is actually preferred to get values? Someone not familiar with the API internals will probably just abuse fetch() for everything. And get() will just be left out.
Also, in order to use get() a previous fetch() is required. Why this restriction? It leads to duplicate code.
And not every time the returned value from fetch() is put into use which leads to wasted computation.

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems to me you're imaging some convoluted, unlikely scenario because I made different design decisions than you would have. I can't imagine any developer actually facing the confusion you're describing. Good thing is, if people do somehow get confused by that, we can always change it later.

And not every time the returned value from fetch() is put into use which leads to wasted computation.

You're trying to over optimize JS code.

I would also like to note that get() was originally called getCached(), but was changed based on your recommendation. #23191 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

On a second thought, I don't insist on suggested version anymore. It will bring await/async to everywhere which isn't necessary.

However parts of my previous reply still applies:

And fetch() is also returning the values. Which function is actually preferred to get values? Someone not familiar with the API internals will probably just abuse fetch() for everything. And get() will just be left out.

And not every time the returned value from fetch() is put into use which leads to wasted computation.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can add some jsdoc if you really think people will get confused about which to use.


/**
* @param {Record<string, any>} data
*/
async set(data) {
await this.#set(data);

// update cache
for (const [key, value] of Object.entries(data))
this.#cache.set(key, value);
}
}

return exports();
})();
Object.freeze(window.qBittorrent.ClientData);
93 changes: 67 additions & 26 deletions src/webui/www/private/scripts/client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ window.qBittorrent.Client ??= (() => {
return {
setup: setup,
initializeCaches: initializeCaches,
initializeClientData: initializeClientData,
closeWindow: closeWindow,
closeFrameWindow: closeFrameWindow,
getSyncMainDataInterval: getSyncMainDataInterval,
Expand All @@ -56,15 +57,47 @@ window.qBittorrent.Client ??= (() => {
const tagMap = new Map();

let cacheAllSettled;
let clientDataPromise;
const setup = () => {
// fetch various data and store it in memory
clientDataPromise = window.qBittorrent.ClientData.fetch([
"show_search_engine",
"show_rss_reader",
"show_log_viewer",
"speed_in_browser_title_bar",
"show_top_toolbar",
"show_status_bar",
"show_filters_sidebar",
"hide_zero_status_filters",
"color_scheme",
"full_url_tracker_column",
"use_alt_row_colors",
"use_virtual_list",
"dblclick_complete",
"dblclick_download",
"dblclick_filter",
"search_in_filter",
"qbt_selected_log_levels",
"add_torrent_default_category",
]);

cacheAllSettled = Promise.allSettled([
window.qBittorrent.Cache.buildInfo.init(),
window.qBittorrent.Cache.preferences.init(),
window.qBittorrent.Cache.qbtVersion.init()
window.qBittorrent.Cache.qbtVersion.init(),
clientDataPromise,
]);
};

const initializeClientData = async () => {
try {
await clientDataPromise;
}
catch (error) {
console.error(`Failed to initialize client data. Reason: "${error}".`);
}
};

const initializeCaches = async () => {
const results = await cacheAllSettled;
for (const [idx, result] of results.entries()) {
Expand Down Expand Up @@ -223,8 +256,8 @@ let queueing_enabled = true;
let serverSyncMainDataInterval = 1500;
let customSyncMainDataInterval = null;
let useSubcategories = true;
const useAutoHideZeroStatusFilters = localPreferences.get("hide_zero_status_filters", "false") === "true";
const displayFullURLTrackerColumn = localPreferences.get("full_url_tracker_column", "false") === "true";
let useAutoHideZeroStatusFilters = false;
let displayFullURLTrackerColumn = false;

/* Categories filter */
const CATEGORIES_ALL = "b4af0e4c-e76d-4bac-a392-46cbc18d9655";
Expand All @@ -250,6 +283,8 @@ const TRACKERS_WARNING = "82a702c5-210c-412b-829f-97632d7557e9";
// Map<trackerHost: String, Map<trackerURL: String, torrents: Set>>
const trackerMap = new Map();

const clientData = window.qBittorrent.ClientData;

let selectedTracker = localPreferences.get("selected_tracker", TRACKERS_ALL);
let setTrackerFilter = () => {};

Expand All @@ -258,7 +293,13 @@ let selectedStatus = localPreferences.get("selected_filter", "all");
let setStatusFilter = () => {};
let toggleFilterDisplay = () => {};

window.addEventListener("DOMContentLoaded", (event) => {
window.addEventListener("DOMContentLoaded", async (event) => {
await window.qBittorrent.Client.initializeClientData();
window.qBittorrent.ColorScheme.update();

useAutoHideZeroStatusFilters = clientData.get("hide_zero_status_filters") === true;
displayFullURLTrackerColumn = clientData.get("full_url_tracker_column") === true;

window.qBittorrent.LocalPreferences.upgrade();

let isSearchPanelLoaded = false;
Expand Down Expand Up @@ -405,6 +446,13 @@ window.addEventListener("DOMContentLoaded", (event) => {
localPreferences.set(`filter_${filterListID.replace("FilterList", "")}_collapsed`, filterList.classList.toggle("invisible").toString());
};

const highlightSelectedStatus = () => {
const statusFilter = document.getElementById("statusFilterList");
const filterID = `${selectedStatus}_filter`;
for (const status of statusFilter.children)
status.classList.toggle("selectedFilter", (status.id === filterID));
};

new MochaUI.Panel({
id: "Filters",
title: "Panel",
Expand All @@ -426,35 +474,35 @@ window.addEventListener("DOMContentLoaded", (event) => {
initializeWindows();

// Show Top Toolbar is enabled by default
let showTopToolbar = localPreferences.get("show_top_toolbar", "true") === "true";
let showTopToolbar = clientData.get("show_top_toolbar") !== false;
Copy link
Member

@Chocobo1 Chocobo1 Sep 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Continuing #23191 (comment)

The code has become less straightforward.
Later someone will probably try to change it to (or write new code) clientData.get("show_top_toolbar") === true and only to find out it doesn't work reliably.
This kind of usage hides a lot of elusive details and it just make the code harder to comprehend, maintain and write.

Copy link
Member Author

Choose a reason for hiding this comment

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

This code is now more straightforward. Rather than some additional parameter that applies in unknown situations, the caller can use native JS to get their default value.

See #23191 (comment)

clientData.get("show_top_toolbar") === true and only to find out it doesn't work reliably.

In what way does this not work reliably?

Copy link
Member

Choose a reason for hiding this comment

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

Rather than some additional parameter that applies in unknown situations

It is always clear that the default value only applies when the actual value doesn't exist. This pattern is extensively used in our C++ code base and in Qt class methods. We never had a problem/confusion with it.

clientData.get("show_top_toolbar") === true and only to find out it doesn't work reliably.

In what way does this not work reliably?

You told me about it. === true comparison will fail on first use when the value doesn't exist. Which is the exact reason why you coded !== false instead.
This hidden pitfall is going to be a nightmare in terms of maintenance and readability. I rather let the default value be explicit and non optional than to allow such trap to exist.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is always clear that the default value only applies when the actual value doesn't exist. This pattern is extensively used in our C++ code base and in Qt class methods. We never had a problem/confusion with it.

Different languages have different paradigms. In JS, the nullish coalescing operator can be used to assign a default value when the value is nullish (either null or undefined). In our case, the value is only nullish when it does not exist.

This hidden pitfall is going to be a nightmare in terms of maintenance and readability. I rather let the default value be explicit and non optional than to allow such trap to exist.

Sorry, I really just don't see this as true. To me, my solution is modern idiomatic JS.

if (!showTopToolbar) {
document.getElementById("showTopToolbarLink").firstElementChild.style.opacity = "0";
document.getElementById("mochaToolbar").classList.add("invisible");
}

// Show Status Bar is enabled by default
let showStatusBar = localPreferences.get("show_status_bar", "true") === "true";
let showStatusBar = clientData.get("show_status_bar") !== false;
if (!showStatusBar) {
document.getElementById("showStatusBarLink").firstElementChild.style.opacity = "0";
document.getElementById("desktopFooterWrapper").classList.add("invisible");
}

// Show Filters Sidebar is enabled by default
let showFiltersSidebar = localPreferences.get("show_filters_sidebar", "true") === "true";
let showFiltersSidebar = clientData.get("show_filters_sidebar") !== false;
if (!showFiltersSidebar) {
document.getElementById("showFiltersSidebarLink").firstElementChild.style.opacity = "0";
document.getElementById("filtersColumn").classList.add("invisible");
document.getElementById("filtersColumn_handle").classList.add("invisible");
}

let speedInTitle = localPreferences.get("speed_in_browser_title_bar") === "true";
let speedInTitle = clientData.get("speed_in_browser_title_bar") === true;
if (!speedInTitle)
document.getElementById("speedInBrowserTitleBarLink").firstElementChild.style.opacity = "0";

// After showing/hiding the toolbar + status bar
window.qBittorrent.Client.showSearchEngine(localPreferences.get("show_search_engine") !== "false");
window.qBittorrent.Client.showRssReader(localPreferences.get("show_rss_reader") !== "false");
window.qBittorrent.Client.showLogViewer(localPreferences.get("show_log_viewer") === "true");
window.qBittorrent.Client.showSearchEngine(clientData.get("show_search_engine") !== false);
window.qBittorrent.Client.showRssReader(clientData.get("show_rss_reader") !== false);
window.qBittorrent.Client.showLogViewer(clientData.get("show_log_viewer") === true);

// After Show Top Toolbar
MochaUI.Desktop.setDesktopSize();
Expand Down Expand Up @@ -571,13 +619,6 @@ window.addEventListener("DOMContentLoaded", (event) => {
window.qBittorrent.Filters.clearStatusFilter();
};

const highlightSelectedStatus = () => {
const statusFilter = document.getElementById("statusFilterList");
const filterID = `${selectedStatus}_filter`;
for (const status of statusFilter.children)
status.classList.toggle("selectedFilter", (status.id === filterID));
};

const updateCategoryList = () => {
const categoryList = document.getElementById("categoryFilterList");
if (!categoryList)
Expand Down Expand Up @@ -1223,7 +1264,7 @@ window.addEventListener("DOMContentLoaded", (event) => {

document.getElementById("showTopToolbarLink").addEventListener("click", (e) => {
showTopToolbar = !showTopToolbar;
localPreferences.set("show_top_toolbar", showTopToolbar.toString());
clientData.set({ show_top_toolbar: showTopToolbar }).catch(console.error);
if (showTopToolbar) {
document.getElementById("showTopToolbarLink").firstElementChild.style.opacity = "1";
document.getElementById("mochaToolbar").classList.remove("invisible");
Expand All @@ -1237,7 +1278,7 @@ window.addEventListener("DOMContentLoaded", (event) => {

document.getElementById("showStatusBarLink").addEventListener("click", (e) => {
showStatusBar = !showStatusBar;
localPreferences.set("show_status_bar", showStatusBar.toString());
clientData.set({ show_status_bar: showStatusBar }).catch(console.error);
if (showStatusBar) {
document.getElementById("showStatusBarLink").firstElementChild.style.opacity = "1";
document.getElementById("desktopFooterWrapper").classList.remove("invisible");
Expand Down Expand Up @@ -1274,7 +1315,7 @@ window.addEventListener("DOMContentLoaded", (event) => {

document.getElementById("showFiltersSidebarLink").addEventListener("click", (e) => {
showFiltersSidebar = !showFiltersSidebar;
localPreferences.set("show_filters_sidebar", showFiltersSidebar.toString());
clientData.set({ show_filters_sidebar: showFiltersSidebar }).catch(console.error);
if (showFiltersSidebar) {
document.getElementById("showFiltersSidebarLink").firstElementChild.style.opacity = "1";
document.getElementById("filtersColumn").classList.remove("invisible");
Expand All @@ -1290,7 +1331,7 @@ window.addEventListener("DOMContentLoaded", (event) => {

document.getElementById("speedInBrowserTitleBarLink").addEventListener("click", (e) => {
speedInTitle = !speedInTitle;
localPreferences.set("speed_in_browser_title_bar", speedInTitle.toString());
clientData.set({ speed_in_browser_title_bar: speedInTitle }).catch(console.error);
if (speedInTitle)
document.getElementById("speedInBrowserTitleBarLink").firstElementChild.style.opacity = "1";
else
Expand All @@ -1300,19 +1341,19 @@ window.addEventListener("DOMContentLoaded", (event) => {

document.getElementById("showSearchEngineLink").addEventListener("click", (e) => {
window.qBittorrent.Client.showSearchEngine(!window.qBittorrent.Client.isShowSearchEngine());
localPreferences.set("show_search_engine", window.qBittorrent.Client.isShowSearchEngine().toString());
clientData.set({ show_search_engine: window.qBittorrent.Client.isShowSearchEngine() }).catch(console.error);
updateTabDisplay();
});

document.getElementById("showRssReaderLink").addEventListener("click", (e) => {
window.qBittorrent.Client.showRssReader(!window.qBittorrent.Client.isShowRssReader());
localPreferences.set("show_rss_reader", window.qBittorrent.Client.isShowRssReader().toString());
clientData.set({ show_rss_reader: window.qBittorrent.Client.isShowRssReader() }).catch(console.error);
updateTabDisplay();
});

document.getElementById("showLogViewerLink").addEventListener("click", (e) => {
window.qBittorrent.Client.showLogViewer(!window.qBittorrent.Client.isShowLogViewer());
localPreferences.set("show_log_viewer", window.qBittorrent.Client.isShowLogViewer().toString());
clientData.set({ show_log_viewer: window.qBittorrent.Client.isShowLogViewer() }).catch(console.error);
updateTabDisplay();
});

Expand Down Expand Up @@ -1369,7 +1410,7 @@ window.addEventListener("DOMContentLoaded", (event) => {
// main window tabs

const showTransfersTab = () => {
const showFiltersSidebar = localPreferences.get("show_filters_sidebar", "true") === "true";
const showFiltersSidebar = clientData.get("show_filters_sidebar") !== false;
if (showFiltersSidebar) {
document.getElementById("filtersColumn").classList.remove("invisible");
document.getElementById("filtersColumn_handle").classList.remove("invisible");
Expand Down
Loading
Loading