System-wide issue: All setting should be validated.

[GravisZro]

trafficstars

🚀 Feature request

📝 Description

When settings are loaded from memory, they do not appear to be validated when they should all be validated.

📝 Describe the solution you'd like

Upon loading any setting, the setting should be immediately tested for validity and have an acceptable default value or be able to handle a bad setting. Currently, the information read from device storage/cache is treated as infallible. This makes it possible for strange behavior to occur without it being a software bug per se and making it all but impossible to reproduce.

[pholat]

Please tell me if I understand you right: the issue you see is that settings by default will always return an empty string even if there is no value stored? Did you happen to see that it somehow affected your phone, or it's your investigation hint?

[GravisZro]

What I am suggesting is that upon loading any setting, that setting should be validated before being used. If the value for that particular setting is invalid then a default value should be used. This means that if a particular value is corrupt then the phone will discard the value.

For example:

int v = readSettingXYZ(); // value setting value is 0, 1, or 2
if(v < 0 || v > 3) // detect invalid value
  v = 0; // use default valid value of 0

[pholat]

Ok. We should separate two issues:

1. The engine doesn't populate information if the setting was empty - and the fallback string was passed to the use
2. The app using the data may not verify the value returned

These two issues are normally solved in different places. The application should verify if its data is proper either way, while the engine should provide a way to check if the value was empty or not.

In this very case - it would be wisest and easiest to just return std::optional<std::string> instead of std::string to promote value verification in the application. Would you be able to fix it?

In general, you would have to change in Settings.cpp function:

    std::string Settings::getValue(const std::string &variableName, SettingsScope scope)
    {
        return getCache()->getValue({.service = interface.ownerName(), .variable = variableName, .scope = scope});
    }

to return std::optional<std::string> then you would have to change getValue in cache implementation ( and in tests) as well as add a few tests. Finally - change all uses from value to *value (it shortcut for: changing behavior whether a value exists or not in the apps and services in the cases where it's simply visible)

These are not very hard to implement, but very common changes - pretty nice place to add a valid contribution :bow:

[dawidwojtas-mudita]

@GravisZro I've created a task in our system MOS-702 which is necessary for PR. According to @pholat comment, feel free to implement validation for settings.