Type safety of `ConfigBuilder::set_default` in combination with `Config::try_deserialize`?

[stoically]

Currently, when setting defaults with ConfigBuilder::set_default there's no type safety when used in combination with Config::try_deserialize. Meaning, that if I change the type I'm serializing to, I might miss adjusting the set_default strings accordingly. Ideally those would be coupled to my type somehow.

Not sure if this even possible. I'd be interested in contributing if someone has an idea. Thanks for the config crate either way!

[matthiasbeyer]

I think with the current implementation this is actually not possible.

With the rewrite I am playing around with in my spare time (which is why it is only really slowly progressing) that should be possible though.

[stoically]

Is your rewrite public somewhere? I'd be interested to take a look!

Here's something I was testing, which is based on using `#[serde(default = "..")]`. Could certainly use some ergonomic improvements, but otherwise seems to work fine for my use case. What do you think, could this also be a possible way to approach the crate rewrite?

use std::{fs, ops::Deref};

use serde::de::DeserializeOwned;

use crate::{Error, Result};

/// Smart pointer to a given config type.
pub struct Config<T: DeserializeOwned> {
    inner: T,
}

impl<T: DeserializeOwned> Config<T> {

    /// Construct with the given JSON string.
    ///
    /// Tries to deserialize the JSON into the [`Config`]s generic inner type.
    pub fn new_from_str(json: &str) -> Result<Self> {
        let inner =
            serde_json::from_reader(json.as_bytes()).map_err(|source| Error::SerdeJson {
                msg: String::from("Deserializing config from reader failed"),
                source,
            })?;

        Ok(Self { inner })
    }
}

impl<T: DeserializeOwned> Deref for Config<T> {
    type Target = T;

    fn deref(&self) -> &Self::Target {
        &self.inner
    }
}

#[cfg(test)]
mod tests {
    use serde::{Deserialize, Serialize};
    use serde_json::json;

    use super::*;

    #[derive(Debug, Serialize, Deserialize)]
    pub struct TestConfig {
        #[serde(default = "test_default")]
        test: String,
    }

    fn test_default() -> String {
        String::from("value")
    }

    #[test]
    fn test() -> Result<()> {
        let config = Config::<TestConfig>::new_from_str("{}")?;

        assert_eq!(config.test, "value");

        Ok(())
    }
}

[matthiasbeyer]

Is your rewrite public somewhere? I'd be interested to take a look!

As said it is slowly progressing only, but yes: https://github.com/matthiasbeyer/config-rs/tree/rethink

Please note that I will rebase this branch as I see fit. Do not base anything serious on it!

[stoically]

Thanks for the pointer. It seems that the core approach is still to operate on string-based config keys, unless I'm missing something? Would you say that doing string-based operations is still a good way to approach a modern config crate? I feel like it's comparable to serde-jsons Value getters, which can be useful, but I feel like that in most cases it makes more sense to have concrete types already. So maybe the default assumption of the config crate could be "deserialize to concrete type by default, but if you need an escape hatch, here's a raw value you can call getters on"?

[matthiasbeyer]

It seems that the core approach is still to operate on string-based config keys

I'm not even sure anymore, have not touched this in quite some time.
I'm of course open to discussions how to improve this! You can, if you want, take my branch and propose patches for it!

Still, I'm wondering what a non-string-key-configfile would look like? None of the supported formats is able to have a non-string as a key, right? And what would that deserialize to when deserializing to a struct?
As said, though: I'm open to discussion and even patches for improving my rewrite! Make sure to have a look at the "re-think" labeled issues as well!
The more freedom for the user of the crate, the better - as long as it is reasonable and does not end in spaghetti-code 😉

[stoically]

"string-based config keys" wasn't well phrased. What I actually meant was "string-based getters". Which isn't entirely true anymore for the rethink rewrite, since it introduces the concept of accessors, so the means of accessing the value could be not string-based after all. Which sounds like an interesting approach of accessing the raw config when no type safety is needed, and definitely something that's important to have either way - and kinda comparable to serde_json::Value::get.

In my use case I want to operate on concrete types with type safe defaults instead of using getters. My current solution for that is laid out in #365 (comment). In my mind a great solution would be the ability to combine the two approaches, though, I'm not exactly sure how that could look like, especially in terms of API. But maybe for the concrete type scenario it actually is a good idea to just leverage serdes feature set - and only introduce something custom brew for the getters scenario?

[stoically]

Because, one big part of the config crate is the ability to parse from a multitude of sources in an easy way - combining that with serde for the concrete type scenario sounds great to me personally.

[matthiasbeyer]

Yes, default values are twofold:

1. you can have default configuration objects that you use as a baseline and then "layer" the user-provided configuration over that using this crates functionality (currently implemented or the one from the rewrite).
2. Using serde(default) attributes on your custom types.

Both are somewhat related, but with slightly different usecases! The first one is just "the default configuration for my app", the second one is more of a "assume a default for a specific option if the user does not provide a value".

The latter one can also be implemented using the first approach. The difference is that if you do not provide a default value layer and you want to deserialize into your own structure without having serde(default) annotations, that might fail if the user does not provide a value.

Just two techniques providing almost the same functionality, but with slightly different semantics 😉

Plus, as you said, if you do not deserialize into an own struct, you get default values via the layer if you want to.

[stoically]

Thanks for taking the time to write down your thoughts around this. Could you maybe expand on how 1. would look like for the "concrete type with type safe defaults"-scenario in terms of (pseudo-)code? I can't really imagine how that could work, unfortunately.

[matthiasbeyer]

I have something like this in mind:

Config::builder()
    .load_default(MyConfigDefault::default())
    .load(TomlLoader::from_path(my_config_file_path))
    .build()

Here, MyConfigDefault would be a custom struct that implements ConfigSource and provides default values.
The layering here makes it possible that if the user-provided toml file does not specify one key, we get an automatic fallback to the default value.

Does that answer your question?

---

That generated Config object can then be deserialized (if wanted) to a concrete MyConfig: serde::Deserialize, which of course then has concrete types as members.

[stoically]

I see, thanks for elaborating. But wouldn't that still mean that there's no type safety, given that I can change the fields of MyConfig without having to change MyConfigDefault? E.g. I refactor MyConfig::uwu to MyConfig::owo, but forget to do the same refactoring with MyConfigDefault.

[matthiasbeyer]

Hm, yes that's indeed an issue.