gui: Options — Capture tab, saveTo.allowedRoots, lock coverage to the schema
Options was missing 8 of the 43 real settings.* keys: capture.enabled, monitoredExtensions, monitoredMimeTypes, minSizeBytes, excludedHosts, bypassModifier, autoStartTypes, and saveTo.allowedRoots. The capture.* keys were dropped in an earlier pass on the mistaken read that the spec's "File Types" tab meant per-category extension lists (which do live on Category, not settings.*) — they're real settings.* keys for a real daemon feature (the extension's auto-capture policy), so the tab exists now, named "Capture" to match what it actually configures rather than the spec's label. New tst_optionsdialog case (allKeysMatchesTheSchemaExactly) loads Settings.schema.json itself at test time and diffs its property set against OptionsDialog::allKeys() — this drifted silently once already, so the regression is now a build-time gate an unused import or a future key addition would trip, not something that needs re-discovering by hand again. Verified against a real veloxd (not just mockd): settings.get across all 43 keys, a settings.set/get round trip on a scalar (connection.timeoutSec) and on array-valued keys in the shapes OptionsDialog::currentValues() actually produces (capture.monitoredExtensions, proxy.bypassHosts, saveTo.allowedRoots), and event.settings.changed fanning out to a second subscribed client — all round-tripped and restored to their original values afterward. The real OptionsDialog widget also loads and renders correctly against that same daemon's live defaults with no crash under ASan+UBSan. Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01NSCdCWFXBTSBBK3MzWtJiC
This commit is contained in:
@@ -9,6 +9,7 @@
|
||||
#include <QJsonArray>
|
||||
#include <QLabel>
|
||||
#include <QLineEdit>
|
||||
#include <QPlainTextEdit>
|
||||
#include <QPointer>
|
||||
#include <QPushButton>
|
||||
#include <QSpinBox>
|
||||
@@ -18,6 +19,7 @@
|
||||
|
||||
#include "rpc/Protocol.hpp"
|
||||
#include "rpc/RpcClient.hpp"
|
||||
#include "util/Theme.hpp"
|
||||
|
||||
namespace velox::gui {
|
||||
namespace {
|
||||
@@ -43,7 +45,7 @@ void setBufferCombo(QComboBox *combo, qint64 bytes) {
|
||||
combo->setCurrentIndex(combo->count() / 2); // an unrecognized value: land near the middle
|
||||
}
|
||||
|
||||
QStringList splitHosts(const QString &text) {
|
||||
QStringList splitCsv(const QString &text) {
|
||||
QStringList out;
|
||||
for (const QString &h : text.split(QLatin1Char(','), Qt::SkipEmptyParts)) {
|
||||
out << h.trimmed();
|
||||
@@ -51,6 +53,14 @@ QStringList splitHosts(const QString &text) {
|
||||
return out;
|
||||
}
|
||||
|
||||
QString joinArray(const QJsonArray &a) {
|
||||
QStringList items;
|
||||
for (const QJsonValue &v : a) {
|
||||
items << v.toString();
|
||||
}
|
||||
return items.join(QStringLiteral(", "));
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
QStringList OptionsDialog::allKeys() {
|
||||
@@ -61,10 +71,18 @@ QStringList OptionsDialog::allKeys() {
|
||||
"general.confirmOnExit",
|
||||
"general.language",
|
||||
"general.checkForUpdates",
|
||||
"capture.enabled",
|
||||
"capture.monitoredExtensions",
|
||||
"capture.monitoredMimeTypes",
|
||||
"capture.minSizeBytes",
|
||||
"capture.excludedHosts",
|
||||
"capture.bypassModifier",
|
||||
"capture.autoStartTypes",
|
||||
"saveTo.defaultDir",
|
||||
"saveTo.tempDir",
|
||||
"saveTo.fileExistsPolicy",
|
||||
"saveTo.createSubfolderPerSite",
|
||||
"saveTo.allowedRoots",
|
||||
"connection.preset",
|
||||
"connection.maxSegmentsPerDownload",
|
||||
"connection.bufferBytes",
|
||||
@@ -112,13 +130,14 @@ OptionsDialog::OptionsDialog(rpc::RpcClient *client, QWidget *parent)
|
||||
resize(560, 480);
|
||||
|
||||
buildGeneralTab();
|
||||
buildCaptureTab();
|
||||
buildSaveToTab();
|
||||
buildConnectionTab();
|
||||
buildDownloadsTab();
|
||||
buildProxyTab();
|
||||
buildSoundsTab();
|
||||
|
||||
statusLabel_->setStyleSheet(QStringLiteral("color: #c0392b;"));
|
||||
statusLabel_->setStyleSheet(theme::errorLabelStyle());
|
||||
statusLabel_->hide();
|
||||
|
||||
auto *buttons = new QDialogButtonBox(
|
||||
@@ -183,6 +202,37 @@ void OptionsDialog::buildGeneralTab() {
|
||||
tabs_->addTab(page, tr("General"));
|
||||
}
|
||||
|
||||
void OptionsDialog::buildCaptureTab() {
|
||||
auto *page = new QWidget(this);
|
||||
captureEnabled_ = new QCheckBox(tr("Capture downloads from the browser extension"), page);
|
||||
monitoredExtensions_ = new QLineEdit(page);
|
||||
monitoredExtensions_->setPlaceholderText(tr("comma-separated, e.g. zip, iso, mp4"));
|
||||
monitoredMimeTypes_ = new QLineEdit(page);
|
||||
monitoredMimeTypes_->setPlaceholderText(tr("comma-separated, e.g. application/zip"));
|
||||
minSizeKiB_ = new QSpinBox(page);
|
||||
minSizeKiB_->setRange(0, 2000000);
|
||||
minSizeKiB_->setSuffix(tr(" KiB"));
|
||||
excludedHosts_ = new QLineEdit(page);
|
||||
excludedHosts_->setPlaceholderText(tr("comma-separated, e.g. *.google.com"));
|
||||
bypassModifier_ = new QComboBox(page);
|
||||
bypassModifier_->addItem(tr("Alt"), QStringLiteral("alt"));
|
||||
bypassModifier_->addItem(tr("Ctrl"), QStringLiteral("ctrl"));
|
||||
bypassModifier_->addItem(tr("Shift"), QStringLiteral("shift"));
|
||||
bypassModifier_->addItem(tr("None"), QStringLiteral("none"));
|
||||
autoStartTypes_ = new QLineEdit(page);
|
||||
autoStartTypes_->setPlaceholderText(tr("extensions that skip the File Info dialog"));
|
||||
|
||||
auto *form = new QFormLayout(page);
|
||||
form->addRow(captureEnabled_);
|
||||
form->addRow(tr("Monitored extensions:"), monitoredExtensions_);
|
||||
form->addRow(tr("Monitored MIME types:"), monitoredMimeTypes_);
|
||||
form->addRow(tr("Minimum size:"), minSizeKiB_);
|
||||
form->addRow(tr("Never capture from:"), excludedHosts_);
|
||||
form->addRow(tr("Bypass-capture modifier key:"), bypassModifier_);
|
||||
form->addRow(tr("Auto-start these types:"), autoStartTypes_);
|
||||
tabs_->addTab(page, tr("Capture"));
|
||||
}
|
||||
|
||||
void OptionsDialog::buildSaveToTab() {
|
||||
auto *page = new QWidget(this);
|
||||
defaultDir_ = new QLineEdit(page);
|
||||
@@ -193,6 +243,11 @@ void OptionsDialog::buildSaveToTab() {
|
||||
fileExistsPolicy_->addItem(tr("Overwrite"), QStringLiteral("overwrite"));
|
||||
fileExistsPolicy_->addItem(tr("Resume"), QStringLiteral("resume"));
|
||||
createSubfolderPerSite_ = new QCheckBox(tr("Create a subfolder per site"), page);
|
||||
allowedRoots_ = new QPlainTextEdit(page);
|
||||
allowedRoots_->setPlaceholderText(
|
||||
tr("One directory per line — every save path must "
|
||||
"canonicalize inside one of these"));
|
||||
allowedRoots_->setMaximumHeight(80);
|
||||
|
||||
auto *defaultDirRow = new QWidget(page);
|
||||
auto *defaultDirLayout = new QHBoxLayout(defaultDirRow);
|
||||
@@ -215,6 +270,7 @@ void OptionsDialog::buildSaveToTab() {
|
||||
form->addRow(tr("Temp folder:"), tempDirRow);
|
||||
form->addRow(tr("If a file already exists:"), fileExistsPolicy_);
|
||||
form->addRow(createSubfolderPerSite_);
|
||||
form->addRow(tr("Allowed save roots:"), allowedRoots_);
|
||||
tabs_->addTab(page, tr("Save To"));
|
||||
}
|
||||
|
||||
@@ -390,12 +446,27 @@ void OptionsDialog::populateFrom(const QJsonObject &v) {
|
||||
language_->setCurrentIndex(langIdx >= 0 ? langIdx : 0);
|
||||
checkForUpdates_->setChecked(v.value("general.checkForUpdates").toBool(true));
|
||||
|
||||
captureEnabled_->setChecked(v.value("capture.enabled").toBool(true));
|
||||
monitoredExtensions_->setText(joinArray(v.value("capture.monitoredExtensions").toArray()));
|
||||
monitoredMimeTypes_->setText(joinArray(v.value("capture.monitoredMimeTypes").toArray()));
|
||||
minSizeKiB_->setValue(static_cast<int>(v.value("capture.minSizeBytes").toDouble() / 1024));
|
||||
excludedHosts_->setText(joinArray(v.value("capture.excludedHosts").toArray()));
|
||||
const int bypassIdx =
|
||||
bypassModifier_->findData(v.value("capture.bypassModifier").toString("alt"));
|
||||
bypassModifier_->setCurrentIndex(bypassIdx >= 0 ? bypassIdx : 0);
|
||||
autoStartTypes_->setText(joinArray(v.value("capture.autoStartTypes").toArray()));
|
||||
|
||||
defaultDir_->setText(v.value("saveTo.defaultDir").toString());
|
||||
tempDir_->setText(v.value("saveTo.tempDir").toString());
|
||||
const int policyIdx =
|
||||
fileExistsPolicy_->findData(v.value("saveTo.fileExistsPolicy").toString("ask"));
|
||||
fileExistsPolicy_->setCurrentIndex(policyIdx >= 0 ? policyIdx : 0);
|
||||
createSubfolderPerSite_->setChecked(v.value("saveTo.createSubfolderPerSite").toBool());
|
||||
QStringList roots;
|
||||
for (const QJsonValue &r : v.value("saveTo.allowedRoots").toArray()) {
|
||||
roots << r.toString();
|
||||
}
|
||||
allowedRoots_->setPlainText(roots.join(QLatin1Char('\n')));
|
||||
|
||||
const int presetIdx =
|
||||
connectionPreset_->findData(v.value("connection.preset").toString("auto"));
|
||||
@@ -452,10 +523,28 @@ QJsonObject OptionsDialog::currentValues() const {
|
||||
v["general.language"] = language_->currentData().toString();
|
||||
v["general.checkForUpdates"] = checkForUpdates_->isChecked();
|
||||
|
||||
v["capture.enabled"] = captureEnabled_->isChecked();
|
||||
v["capture.monitoredExtensions"] =
|
||||
QJsonArray::fromStringList(splitCsv(monitoredExtensions_->text()));
|
||||
v["capture.monitoredMimeTypes"] =
|
||||
QJsonArray::fromStringList(splitCsv(monitoredMimeTypes_->text()));
|
||||
v["capture.minSizeBytes"] = static_cast<qint64>(minSizeKiB_->value()) * 1024;
|
||||
v["capture.excludedHosts"] = QJsonArray::fromStringList(splitCsv(excludedHosts_->text()));
|
||||
v["capture.bypassModifier"] = bypassModifier_->currentData().toString();
|
||||
v["capture.autoStartTypes"] = QJsonArray::fromStringList(splitCsv(autoStartTypes_->text()));
|
||||
|
||||
v["saveTo.defaultDir"] = defaultDir_->text();
|
||||
v["saveTo.tempDir"] = tempDir_->text();
|
||||
v["saveTo.fileExistsPolicy"] = fileExistsPolicy_->currentData().toString();
|
||||
v["saveTo.createSubfolderPerSite"] = createSubfolderPerSite_->isChecked();
|
||||
QStringList roots;
|
||||
for (const QString &line : allowedRoots_->toPlainText().split(QLatin1Char('\n'))) {
|
||||
const QString trimmed = line.trimmed();
|
||||
if (!trimmed.isEmpty()) {
|
||||
roots << trimmed;
|
||||
}
|
||||
}
|
||||
v["saveTo.allowedRoots"] = QJsonArray::fromStringList(roots);
|
||||
|
||||
v["connection.preset"] = connectionPreset_->currentData().toString();
|
||||
v["connection.maxSegmentsPerDownload"] = maxSegmentsPerDownload_->value();
|
||||
@@ -479,7 +568,7 @@ QJsonObject OptionsDialog::currentValues() const {
|
||||
v["proxy.host"] = proxyHost_->text();
|
||||
v["proxy.port"] = proxyPort_->value();
|
||||
v["proxy.username"] = proxyUsername_->text();
|
||||
v["proxy.bypassHosts"] = QJsonArray::fromStringList(splitHosts(proxyBypassHosts_->text()));
|
||||
v["proxy.bypassHosts"] = QJsonArray::fromStringList(splitCsv(proxyBypassHosts_->text()));
|
||||
v["proxy.pacUrl"] = proxyPacUrl_->text();
|
||||
|
||||
v["sounds.enabled"] = soundsEnabled_->isChecked();
|
||||
|
||||
@@ -1,11 +1,12 @@
|
||||
// The Options dialog. Lane GUI.
|
||||
//
|
||||
// docs/03-gui-spec.md §4: every control here maps 1:1 onto a settings.* key from
|
||||
// contracts/schema/types/Settings.schema.json. The spec's "File Types" and "Site Logins"
|
||||
// tabs have no backing key (per-category extension lists live on Category via
|
||||
// category.upsert, not settings.*; login credentials go to the Secret Service) — a real
|
||||
// tab either binds to a real key or does not exist here, so those two are left out rather
|
||||
// than shipped as fake affordances.
|
||||
// contracts/schema/types/Settings.schema.json. The spec's "File Types" tab turned out to
|
||||
// have real backing after all (capture.monitoredExtensions/monitoredMimeTypes/
|
||||
// autoStartTypes are settings.* keys, not Category — a mistake in an earlier pass here,
|
||||
// caught while checking this dialog covers all 43 keys against the now-live real veloxd);
|
||||
// it is named "Capture" below to match what it actually configures. "Site Logins" still
|
||||
// has no settings.* key (credentials go to the Secret Service) and stays out.
|
||||
|
||||
#pragma once
|
||||
|
||||
@@ -16,6 +17,7 @@ class QCheckBox;
|
||||
class QComboBox;
|
||||
class QLabel;
|
||||
class QLineEdit;
|
||||
class QPlainTextEdit;
|
||||
class QSpinBox;
|
||||
class QTabWidget;
|
||||
|
||||
@@ -45,6 +47,7 @@ class OptionsDialog : public QDialog {
|
||||
|
||||
private:
|
||||
void buildGeneralTab();
|
||||
void buildCaptureTab();
|
||||
void buildSaveToTab();
|
||||
void buildConnectionTab();
|
||||
void buildDownloadsTab();
|
||||
@@ -67,11 +70,21 @@ class OptionsDialog : public QDialog {
|
||||
QComboBox *language_;
|
||||
QCheckBox *checkForUpdates_;
|
||||
|
||||
// Capture
|
||||
QCheckBox *captureEnabled_;
|
||||
QLineEdit *monitoredExtensions_;
|
||||
QLineEdit *monitoredMimeTypes_;
|
||||
QSpinBox *minSizeKiB_;
|
||||
QLineEdit *excludedHosts_;
|
||||
QComboBox *bypassModifier_;
|
||||
QLineEdit *autoStartTypes_;
|
||||
|
||||
// Save To
|
||||
QLineEdit *defaultDir_;
|
||||
QLineEdit *tempDir_;
|
||||
QComboBox *fileExistsPolicy_;
|
||||
QCheckBox *createSubfolderPerSite_;
|
||||
QPlainTextEdit *allowedRoots_;
|
||||
|
||||
// Connection
|
||||
QComboBox *connectionPreset_;
|
||||
|
||||
@@ -82,12 +82,14 @@ set_tests_properties(gui_fileinfodialog PROPERTIES
|
||||
LABELS "gui"
|
||||
ENVIRONMENT "QT_QPA_PLATFORM=offscreen")
|
||||
|
||||
# tst_optionsdialog — OptionsDialog::diffChanged, the "only send what changed" logic.
|
||||
# Red when an unchanged key gets resent, or a key missing from `original` stops
|
||||
# counting as changed.
|
||||
# tst_optionsdialog — OptionsDialog::diffChanged, the "only send what changed" logic, and
|
||||
# allKeys() against the real schema.
|
||||
# Red when an unchanged key gets resent, a key missing from `original` stops counting
|
||||
# as changed, or allKeys() drifts from Settings.schema.json in either direction.
|
||||
add_executable(tst_optionsdialog tst_optionsdialog.cpp)
|
||||
target_compile_features(tst_optionsdialog PRIVATE cxx_std_23)
|
||||
target_compile_options(tst_optionsdialog PRIVATE -Wall -Wextra -Wpedantic -Werror)
|
||||
target_compile_definitions(tst_optionsdialog PRIVATE VELOX_REPO_ROOT="${CMAKE_SOURCE_DIR}")
|
||||
target_link_libraries(tst_optionsdialog PRIVATE velox-gui-lib Qt6::Widgets Qt6::Test)
|
||||
add_test(NAME gui_optionsdialog COMMAND tst_optionsdialog)
|
||||
set_tests_properties(gui_optionsdialog PROPERTIES
|
||||
|
||||
@@ -5,6 +5,8 @@
|
||||
// effect" contract and spams event.settings.changed with noise), or a key present in
|
||||
// `current` but absent from `original` stops being treated as changed.
|
||||
|
||||
#include <QFile>
|
||||
#include <QJsonDocument>
|
||||
#include <QJsonObject>
|
||||
#include <QtTest>
|
||||
|
||||
@@ -20,6 +22,7 @@ class TstOptionsDialog : public QObject {
|
||||
void onlyChangedKeysAreReturned();
|
||||
void keyAbsentFromOriginalCountsAsChanged();
|
||||
void allKeysAreNonEmptyAndUnique();
|
||||
void allKeysMatchesTheSchemaExactly();
|
||||
};
|
||||
|
||||
void TstOptionsDialog::identicalValuesProduceNoDiff() {
|
||||
@@ -50,5 +53,24 @@ void TstOptionsDialog::allKeysAreNonEmptyAndUnique() {
|
||||
QCOMPARE(QSet<QString>(keys.begin(), keys.end()).size(), keys.size());
|
||||
}
|
||||
|
||||
// Red when a key is added to (or removed from) Settings.schema.json without the same
|
||||
// change landing here — either direction is a real bug: an invented key settings.set
|
||||
// would reject with -32602, or a real key the dialog silently never shows.
|
||||
void TstOptionsDialog::allKeysMatchesTheSchemaExactly() {
|
||||
QFile f(QStringLiteral(VELOX_REPO_ROOT "/contracts/schema/types/Settings.schema.json"));
|
||||
QVERIFY2(f.open(QIODevice::ReadOnly), qUtf8Printable(f.errorString()));
|
||||
const QJsonObject schema = QJsonDocument::fromJson(f.readAll()).object();
|
||||
const QJsonObject properties = schema.value("properties").toObject();
|
||||
QVERIFY(!properties.isEmpty());
|
||||
|
||||
QSet<QString> schemaKeys;
|
||||
for (auto it = properties.constBegin(); it != properties.constEnd(); ++it) {
|
||||
schemaKeys.insert(it.key());
|
||||
}
|
||||
const QStringList dialogKeysList = OptionsDialog::allKeys();
|
||||
const QSet<QString> dialogKeys(dialogKeysList.begin(), dialogKeysList.end());
|
||||
QCOMPARE(dialogKeys, schemaKeys);
|
||||
}
|
||||
|
||||
QTEST_MAIN(TstOptionsDialog)
|
||||
#include "tst_optionsdialog.moc"
|
||||
|
||||
Reference in New Issue
Block a user