Ref T689 Settings validation can produce more descriptive error messages.

This commit is contained in:
Mat Sutcliffe
2019-06-23 17:08:45 +01:00
parent 6f9f6ccc9d
commit 668a77d083
16 changed files with 34 additions and 21 deletions

View File

@@ -33,7 +33,7 @@ namespace BlackCore
static const QString &humanReadable() { static const QString name("Hotkeys"); return name; }
//! \copydoc BlackMisc::TSettingTrait::isValid
static bool isValid(const BlackMisc::Input::CActionHotkeyList &value)
static bool isValid(const BlackMisc::Input::CActionHotkeyList &value, QString &)
{
for (const auto &actionHotkey : value)
{
@@ -63,7 +63,7 @@ namespace BlackCore
}
//! \copydoc BlackMisc::TSettingTrait::isValid
static bool isValid(const QStringList &pluginIdentifiers)
static bool isValid(const QStringList &pluginIdentifiers, QString &)
{
for (const QString &pluginIdentifier : pluginIdentifiers)
{

View File

@@ -202,7 +202,7 @@ namespace BlackCore
static const QString &humanReadable() { static const QString name("FSD message Logging"); return name; }
/* //! \copydoc BlackMisc::TSettingTrait::isValid
static bool isValid(const CRawFsdMessageSettings &setting)
static bool isValid(const CRawFsdMessageSettings &setting, QString &)
{
if (setting.areRawFsdMessagesEnabled()) { return !setting.getFileDir().isEmpty(); }
return true;

View File

@@ -92,7 +92,7 @@ namespace BlackGui
static const QString &humanReadable() { static const QString name("Background consolidation"); return name; }
//! \copydoc BlackMisc::TSettingTrait::isValid
static bool isValid(const int &valueInSeconds) { return valueInSeconds == -1 || (valueInSeconds >= minSecs() && valueInSeconds <= maxSecs()); }
static bool isValid(const int &valueInSeconds, QString &) { return valueInSeconds == -1 || (valueInSeconds >= minSecs() && valueInSeconds <= maxSecs()); }
//! \copydoc BlackMisc::TSettingTrait::defaultValue
static const int &defaultValue() { static const int i = 60; return i; }

View File

@@ -106,7 +106,7 @@ namespace BlackGui
static const QString &humanReadable() { static const QString name("View update"); return name; }
//! \copydoc BlackMisc::TSettingTrait::isValid
static bool isValid(const CViewUpdateSettings &settings) { return settings.isValid(); }
static bool isValid(const CViewUpdateSettings &settings, QString &) { return settings.isValid(); }
};
} // ns
} // ns

View File

@@ -109,7 +109,7 @@ namespace BlackMisc
static const QString &humanReadable() { static const QString name("Audio"); return name; }
//! \copydoc BlackMisc::TSettingTrait::isValid
static bool isValid(const BlackMisc::Audio::CSettings &value) { Q_UNUSED(value); return true; }
static bool isValid(const BlackMisc::Audio::CSettings &value, QString &) { Q_UNUSED(value); return true; }
};
} // namespace
} // namespace

View File

@@ -31,7 +31,7 @@ namespace BlackMisc
static const QString &humanReadable() { static const QString name("Voice setup"); return name; }
//! \copydoc BlackMisc::TSettingTrait::isValid
static bool isValid(const CVoiceSetup &setup) { return setup.validate().isSuccess(); }
static bool isValid(const CVoiceSetup &setup, QString &) { return setup.validate().isSuccess(); }
};
} // ns
} // ns

View File

@@ -451,7 +451,7 @@ namespace BlackMisc
//! Validator function. Return true if the argument is valid, false otherwise. Default
//! implementation just returns true. Reimplemented in derived class to support validation of the value.
static bool isValid(const T &) { return true; }
static bool isValid(const T &value, QString &reason) { Q_UNUSED(value); Q_UNUSED(reason); return true; }
//! Return the value to use in case the supplied value does not satisfy the validator.
//! Default implementation returns a default-constructed value.

View File

@@ -144,7 +144,7 @@ namespace BlackMisc
//! Validator function. Return true if the argument is valid, false otherwise. Default
//! implementation just returns true. Reimplemented in derived class to support validation of the value.
static bool isValid(const T &) { return true; }
static bool isValid(const T &value, QString &reason) { Q_UNUSED(value); Q_UNUSED(reason); return true; }
//! Return the value to use in case the supplied value does not satisfy the validator.
//! Default implementation returns a default-constructed value.

View File

@@ -52,7 +52,7 @@ namespace BlackMisc
static const char *key() { return "mapping/modelconverterxbin"; }
//! \copydoc BlackMisc::TSettingTrait::isValid
static bool isValid(const QString &value)
static bool isValid(const QString &value, QString &)
{
if (value.isEmpty()) { return true; }
const QFile f(value);

View File

@@ -36,8 +36,8 @@ namespace BlackMisc
//! \copydoc BlackMisc::TSettingTrait::defaultValue
static QString defaultValue() { return "tcp:host=127.0.0.1,port=45003"; }
//! \copydoc BlackMisc::TSettingTrait::defaultValue
static bool isValid(const QString &dBusAddress) { return BlackMisc::CDBusServer::isSessionOrSystemAddress(dBusAddress) || BlackMisc::CDBusServer::isQtDBusAddress(dBusAddress); }
//! \copydoc BlackMisc::TSettingTrait::isValid
static bool isValid(const QString &dBusAddress, QString &) { return BlackMisc::CDBusServer::isSessionOrSystemAddress(dBusAddress) || BlackMisc::CDBusServer::isQtDBusAddress(dBusAddress); }
};
} // ns
} // ns

View File

@@ -97,7 +97,7 @@ namespace BlackMisc
static const QString &humanReadable() { static const QString name("swift plugin"); return name; }
//! \copydoc BlackMisc::TSettingTrait::isValid
static bool isValid(const CSwiftPluginSettings &value) { return value.getEmulatedSimulator().isSingleSimulator(); }
static bool isValid(const CSwiftPluginSettings &value, QString &) { return value.getEmulatedSimulator().isSingleSimulator(); }
};
} // ns
} // ns

View File

@@ -35,8 +35,8 @@ namespace BlackMisc
//! \copydoc BlackMisc::TSettingTrait::defaultValue
static QString defaultValue() { return "tcp:host=127.0.0.1,port=45001"; }
//! \copydoc BlackMisc::TSettingTrait::defaultValue
static bool isValid(const QString &dBusAddress) { return BlackMisc::CDBusServer::isSessionOrSystemAddress(dBusAddress) || BlackMisc::CDBusServer::isQtDBusAddress(dBusAddress); }
//! \copydoc BlackMisc::TSettingTrait::isValid
static bool isValid(const QString &dBusAddress, QString &) { return BlackMisc::CDBusServer::isSessionOrSystemAddress(dBusAddress) || BlackMisc::CDBusServer::isQtDBusAddress(dBusAddress); }
};
} // ns
} // ns

View File

@@ -637,9 +637,11 @@ namespace BlackMisc
key.replace("%OwnerClass%", QString(parent()->metaObject()->className()).replace("::", "/"), Qt::CaseInsensitive);
key.replace("%OwnerName%", parent()->objectName(), Qt::CaseInsensitive);
QString unused;
Q_ASSERT_X(! m_elements.contains(key), "CValuePage", "Can't have two CCached in the same object referring to the same value");
Q_ASSERT_X(defaultValue.isValid() ? defaultValue.userType() == metaType : true, "CValuePage", "Metatype mismatch for default value");
Q_ASSERT_X(defaultValue.isValid() && validator ? validator(defaultValue) : true, "CValuePage", "Validator rejects default value");
Q_ASSERT_X(defaultValue.isValid() && validator ? validator(defaultValue, unused) : true, "CValuePage", "Validator rejects default value");
Q_UNUSED(unused);
auto &element = *(m_elements[key] = ElementPtr(new Element(key, name, metaType, validator, defaultValue)));
std::forward_as_tuple(element.m_value.uniqueWrite(), element.m_timestamp, element.m_saved) = m_cache->getValue(key);
@@ -839,6 +841,7 @@ namespace BlackMisc
CStatusMessage CValuePage::validate(const Element &element, const CVariant &value, CStatusMessage::StatusSeverity invalidSeverity) const
{
QString reason;
if (! value.isValid())
{
return CStatusMessage(this, invalidSeverity, u"Empty cache value %1", true) << element.m_nameWithKey;
@@ -847,9 +850,16 @@ namespace BlackMisc
{
return CStatusMessage(this).error(u"Expected %1 but got %2 for %3") << QMetaType::typeName(element.m_metaType) << value.typeName() << element.m_nameWithKey;
}
else if (element.m_validator && ! element.m_validator(value))
else if (element.m_validator && ! element.m_validator(value, reason))
{
return CStatusMessage(this).error(u"%1 is not valid for %2") << value.toQString() << element.m_nameWithKey;
if (reason.isEmpty())
{
return CStatusMessage(this).error(u"%1 is not valid for %2") << value.toQString() << element.m_nameWithKey;
}
else
{
return CStatusMessage(this).error(u"%1 (%2 for %3)") << reason << value.toQString() << element.m_nameWithKey;
}
}
else
{

View File

@@ -444,7 +444,10 @@ namespace BlackMisc
private:
template <typename F>
static Private::CValuePage::Validator wrap(F func) { return [func](const CVariant &value)->bool { return func(value.to<T>()); }; }
static Private::CValuePage::Validator wrap(F func)
{
return [func](const CVariant &value, QString &reason) -> bool { return func(value.to<T>(), reason); };
}
static Private::CValuePage::Validator wrap(std::nullptr_t) { return {}; }
template <typename F>

View File

@@ -72,7 +72,7 @@ namespace BlackMisc
struct Element;
//! Functor used for validating values.
using Validator = std::function<bool(const CVariant &)>;
using Validator = std::function<bool(const CVariant &, QString &)>;
//! Functor used to notify parent of changes.
using NotifySlot = std::pair<std::function<void(QObject *)>, void (QObject::*)()>;

View File

@@ -314,7 +314,7 @@ namespace BlackMiscTest
}
//! Is value between 0 - 100?
bool validator(int value)
bool validator(int value, QString &)
{
return value >= 0 && value <= 100;
}