Skip to content

Commit

Permalink
Merge pull request #613 from epage/lower
Browse files Browse the repository at this point in the history
Revert Fix uppercase lowercase isses (#354, #429)
  • Loading branch information
epage authored Dec 17, 2024
2 parents 943987c + 51eacda commit b03a0f5
Show file tree
Hide file tree
Showing 16 changed files with 185 additions and 127 deletions.
6 changes: 2 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@
[RON]: https://github.com/ron-rs/ron
[JSON5]: https://github.com/callum-oakley/json5-rs

Please note this library

- can not be used to write changed configuration values back to the configuration file(s)!
- Is case insensitive and all the keys are converted to lowercase internally
Please note that this library can not be used to write changed configuration
values back to the configuration file(s)!

## Usage

Expand Down
10 changes: 3 additions & 7 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,7 @@ impl Config {
where
T: Into<Value>,
{
self.defaults
.insert(key.to_lowercase().as_str().parse()?, value.into());
self.defaults.insert(key.parse()?, value.into());

#[allow(deprecated)]
self.refresh()
Expand All @@ -130,16 +129,15 @@ impl Config {
where
T: Into<Value>,
{
self.overrides
.insert(key.to_lowercase().as_str().parse()?, value.into());
self.overrides.insert(key.parse()?, value.into());

#[allow(deprecated)]
self.refresh()
}

#[deprecated(since = "0.12.0", note = "please use 'ConfigBuilder' instead")]
pub fn set_once(&mut self, key: &str, value: Value) -> Result<()> {
let expr: path::Expression = key.to_lowercase().as_str().parse()?;
let expr: path::Expression = key.parse()?;

// Traverse the cache using the path to (possibly) retrieve a value
if let Some(ref mut val) = expr.get_mut(&mut self.cache) {
Expand All @@ -151,8 +149,6 @@ impl Config {
}

fn get_value(&self, key: &str) -> Result<Value> {
let k = key.to_lowercase();
let key = k.as_str();
// Parse the key into a path expression
let expr: path::Expression = key.parse()?;

Expand Down
2 changes: 1 addition & 1 deletion src/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ impl EnumAccess {
fn variant_deserializer(&self, name: &str) -> Result<StrDeserializer<'_>> {
self.variants
.iter()
.find(|&&s| s.to_lowercase() == name.to_lowercase()) // changing to lowercase will enable deserialization of lowercase values to enums
.find(|&&s| s == name)
.map(|&s| StrDeserializer(s))
.ok_or_else(|| self.no_constructor_error(name))
}
Expand Down
7 changes: 2 additions & 5 deletions src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,10 @@ impl Environment {
/// To switch the default type back to type Strings you need to provide the keys which should be [`Vec<String>`] using this function.
pub fn with_list_parse_key(mut self, key: &str) -> Self {
if self.list_parse_keys.is_none() {
self.list_parse_keys = Some(vec![key.to_lowercase()]);
self.list_parse_keys = Some(vec![key.into()]);
} else {
self.list_parse_keys = self.list_parse_keys.map(|mut keys| {
keys.push(key.to_lowercase());
keys.push(key.into());
keys
});
}
Expand Down Expand Up @@ -289,9 +289,6 @@ impl Source for Environment {
ValueKind::Float(parsed)
} else if let Some(separator) = &self.list_separator {
if let Some(keys) = &self.list_parse_keys {
#[cfg(feature = "convert-case")]
let key = key.to_lowercase();

if keys.contains(&key) {
let v: Vec<Value> = value
.split(separator)
Expand Down
16 changes: 8 additions & 8 deletions src/path/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl Expression {
match *self {
Self::Identifier(ref id) => match root.kind {
ValueKind::Table(ref mut map) => Some(
map.entry(id.to_lowercase())
map.entry(id.clone())
.or_insert_with(|| Value::new(None, ValueKind::Nil)),
),

Expand All @@ -131,15 +131,15 @@ impl Expression {
Some(value) => {
if let ValueKind::Table(ref mut map) = value.kind {
Some(
map.entry(key.to_lowercase())
map.entry(key.clone())
.or_insert_with(|| Value::new(None, ValueKind::Nil)),
)
} else {
*value = Map::<String, Value>::new().into();

if let ValueKind::Table(ref mut map) = value.kind {
Some(
map.entry(key.to_lowercase())
map.entry(key.clone())
.or_insert_with(|| Value::new(None, ValueKind::Nil)),
)
} else {
Expand Down Expand Up @@ -193,25 +193,25 @@ impl Expression {
ValueKind::Table(ref incoming_map) => {
// Pull out another table
let target = if let ValueKind::Table(ref mut map) = root.kind {
map.entry(id.to_lowercase())
map.entry(id.clone())
.or_insert_with(|| Map::<String, Value>::new().into())
} else {
unreachable!();
};

// Continue the deep merge
for (key, val) in incoming_map {
Self::Identifier(key.to_lowercase()).set(target, val.clone());
Self::Identifier(key.clone()).set(target, val.clone());
}
}

_ => {
if let ValueKind::Table(ref mut map) = root.kind {
// Just do a simple set
if let Some(existing) = map.get_mut(&id.to_lowercase()) {
if let Some(existing) = map.get_mut(id) {
*existing = value;
} else {
map.insert(id.to_lowercase(), value);
map.insert(id.clone(), value);
}
}
}
Expand All @@ -224,7 +224,7 @@ impl Expression {
// Didn't find a table. Oh well. Make a table and do this anyway
*parent = Map::<String, Value>::new().into();
}
Self::Identifier(key.to_lowercase()).set(parent, value);
Self::Identifier(key.clone()).set(parent, value);
}
}

Expand Down
78 changes: 78 additions & 0 deletions tests/testsuite/case.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
use serde_derive::Deserialize;

use config::{Config, File, FileFormat};

#[test]
#[cfg(feature = "json")]
fn respect_field_case() {
#[derive(Deserialize, Debug)]
#[allow(non_snake_case)]
#[allow(dead_code)]
struct Kafka {
broker: String,
topic: String,
pollSleep: u64, //<---
}

let c = Config::builder()
.add_source(File::from_str(
r#"
{
"broker": "localhost:29092",
"topic": "rust",
"pollSleep": 1000
}
"#,
FileFormat::Json,
))
.build()
.unwrap();

c.try_deserialize::<Kafka>().unwrap();
}

#[test]
#[cfg(feature = "json")]
fn respect_renamed_field() {
#[derive(Deserialize, Debug)]
#[allow(dead_code)]
struct MyConfig {
#[serde(rename = "FooBar")]
foo_bar: String,
}

let c = Config::builder()
.add_source(File::from_str(
r#"
{
"FooBar": "Hello, world!"
}
"#,
FileFormat::Json,
))
.build()
.unwrap();

c.try_deserialize::<MyConfig>().unwrap();
}

#[test]
#[cfg(feature = "json")]
fn respect_path_case() {
let c = Config::builder()
.add_source(File::from_str(
r#"
{
"Student": [
{ "Name": "1" },
{ "Name": "2" }
]
}
"#,
FileFormat::Json,
))
.build()
.unwrap();

c.get_string("Student[0].Name").unwrap();
}
25 changes: 11 additions & 14 deletions tests/testsuite/env.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use config::{Config, Environment, Source};
use serde_derive::Deserialize;
use snapbox::{assert_data_eq, str};

use config::{Config, Environment, Source};

/// Reminder that tests using env variables need to use different env variable names, since
/// tests can be run in parallel
Expand Down Expand Up @@ -474,11 +476,13 @@ fn test_parse_string_and_list_ignore_list_parse_key_case() {
// using a struct in an enum here to make serde use `deserialize_any`
#[derive(Deserialize, Debug)]
#[serde(tag = "tag")]
#[allow(dead_code)]
enum TestStringEnum {
String(TestString),
}

#[derive(Deserialize, Debug)]
#[allow(dead_code)]
struct TestString {
string_val: String,
string_list: Vec<String>,
Expand All @@ -503,20 +507,13 @@ fn test_parse_string_and_list_ignore_list_parse_key_case() {
.build()
.unwrap();

let config: TestStringEnum = config.try_deserialize().unwrap();
let res = config.try_deserialize::<TestStringEnum>();

match config {
TestStringEnum::String(TestString {
string_val,
string_list,
}) => {
assert_eq!(String::from("test,string"), string_val);
assert_eq!(
vec![String::from("test"), String::from("string")],
string_list
);
}
}
assert!(res.is_err());
assert_data_eq!(
res.unwrap_err().to_string(),
str![[r#"invalid type: string "test,string", expected a sequence"#]]
);
},
);
}
Expand Down
21 changes: 11 additions & 10 deletions tests/testsuite/file_ini.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ rating = 4.5
match cap_settings {
Ok(v) => {
// this assertion will ensure that the map has only lowercase keys
assert_ne!(v.FOO, "FOO should be overridden");
assert_eq!(v.FOO, "FOO should be overridden");
assert_eq!(
lower_settings.foo,
"I HAVE BEEN OVERRIDDEN_WITH_UPPER_CASE".to_owned()
Expand Down Expand Up @@ -198,11 +198,12 @@ bar = "bar is a lowercase param"
.add_source(config::Environment::with_prefix("APPS").separator("_"))
.build()
.unwrap();
let val: EnumSettings = cfg.try_deserialize().unwrap();

assert_eq!(
val,
EnumSettings::Bar("I HAVE BEEN OVERRIDDEN_WITH_UPPER_CASE".to_owned())
let param = cfg.try_deserialize::<EnumSettings>();
assert!(param.is_err());
assert_data_eq!(
param.unwrap_err().to_string(),
str!["enum EnumSettings does not have variant constructor bar"]
);
}

Expand All @@ -226,11 +227,11 @@ bar = "bar is a lowercase param"
.build()
.unwrap();

let param: EnumSettings = cfg.try_deserialize().unwrap();

assert_eq!(
param,
EnumSettings::Bar("I have been overridden_with_lower_case".to_owned())
let param = cfg.try_deserialize::<EnumSettings>();
assert!(param.is_err());
assert_data_eq!(
param.unwrap_err().to_string(),
str!["enum EnumSettings does not have variant constructor bar"]
);
}

Expand Down
42 changes: 11 additions & 31 deletions tests/testsuite/file_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,27 +115,6 @@ fn test_error_parse() {
);
}

#[test]
fn test_json_vec() {
let c = Config::builder()
.add_source(File::from_str(
r#"
{
"WASTE": ["example_dir1", "example_dir2"]
}
"#,
FileFormat::Json,
))
.build()
.unwrap();

let v = c.get_array("WASTE").unwrap();
let mut vi = v.into_iter();
assert_eq!(vi.next().unwrap().into_string().unwrap(), "example_dir1");
assert_eq!(vi.next().unwrap().into_string().unwrap(), "example_dir2");
assert!(vi.next().is_none());
}

#[test]
fn test_override_uppercase_value_for_struct() {
#[derive(Debug, Deserialize, PartialEq)]
Expand Down Expand Up @@ -189,7 +168,7 @@ fn test_override_uppercase_value_for_struct() {
match cap_settings {
Ok(v) => {
// this assertion will ensure that the map has only lowercase keys
assert_ne!(v.FOO, "FOO should be overridden");
assert_eq!(v.FOO, "FOO should be overridden");
assert_eq!(
lower_settings.foo,
"I HAVE BEEN OVERRIDDEN_WITH_UPPER_CASE".to_owned()
Expand Down Expand Up @@ -279,11 +258,12 @@ fn test_override_uppercase_value_for_enums() {
.add_source(config::Environment::with_prefix("APPS").separator("_"))
.build()
.unwrap();
let val: EnumSettings = cfg.try_deserialize().unwrap();

assert_eq!(
val,
EnumSettings::Bar("I HAVE BEEN OVERRIDDEN_WITH_UPPER_CASE".to_owned())
let param = cfg.try_deserialize::<EnumSettings>();
assert!(param.is_err());
assert_data_eq!(
param.unwrap_err().to_string(),
str!["enum EnumSettings does not have variant constructor bar"]
);
}

Expand All @@ -309,11 +289,11 @@ fn test_override_lowercase_value_for_enums() {
.build()
.unwrap();

let param: EnumSettings = cfg.try_deserialize().unwrap();

assert_eq!(
param,
EnumSettings::Bar("I have been overridden_with_lower_case".to_owned())
let param = cfg.try_deserialize::<EnumSettings>();
assert!(param.is_err());
assert_data_eq!(
param.unwrap_err().to_string(),
str!["enum EnumSettings does not have variant constructor bar"]
);
}

Expand Down
Loading

0 comments on commit b03a0f5

Please sign in to comment.