Skip to content

Commit 61ff574

Browse files
GijsreynGijsreyn
committed
Fix comments
1 parent 779ff4b commit 61ff574

File tree

4 files changed

+100
-109
lines changed

4 files changed

+100
-109
lines changed

‎dsc/locales/en-us.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,3 +168,4 @@ failedToGetParentPath = "Error reading config path parent"
168168
dscConfigRootAlreadySet = "The current value of DSC_CONFIG_ROOT env var will be overridden"
169169
settingDscConfigRoot = "Setting DSC_CONFIG_ROOT env var as"
170170
removingUtf8Bom = "Removing UTF-8 BOM from input"
171+
parametersNotObject = "Parameters must be an object"

‎dsc/src/main.rs‎

Lines changed: 20 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -54,56 +54,8 @@ fn main() {
5454
generate(shell, &mut cmd, "dsc", &mut io::stdout());
5555
},
5656
SubCommand::Config { subcommand, parameters, parameters_file, system_root, as_group, as_assert, as_include } => {
57-
let merged_parameters = if parameters_file.is_some() && parameters.is_some() {
58-
// Both parameters and parameters_file provided - merge them with inline taking precedence
59-
let file_params = if let Some(file_name) = &parameters_file {
60-
if file_name == "-" {
61-
info!("{}", t!("main.readingParametersFromStdin"));
62-
let mut stdin = Vec::<u8>::new();
63-
match io::stdin().read_to_end(&mut stdin) {
64-
Ok(_) => {
65-
match String::from_utf8(stdin) {
66-
Ok(input) => Some(input),
67-
Err(err) => {
68-
error!("{}: {err}", t!("util.invalidUtf8"));
69-
exit(EXIT_INVALID_INPUT);
70-
}
71-
}
72-
},
73-
Err(err) => {
74-
error!("{}: {err}", t!("util.failedToReadStdin"));
75-
exit(EXIT_INVALID_INPUT);
76-
}
77-
}
78-
} else {
79-
info!("{}: {file_name}", t!("main.readingParametersFile"));
80-
match std::fs::read_to_string(file_name) {
81-
Ok(content) => Some(content),
82-
Err(err) => {
83-
error!("{} '{file_name}': {err}", t!("main.failedReadingParametersFile"));
84-
exit(util::EXIT_INVALID_INPUT);
85-
}
86-
}
87-
}
88-
} else {
89-
None
90-
};
91-
92-
// Parse both and merge
93-
if let (Some(file_content), Some(inline_content)) = (file_params, parameters.as_ref()) {
94-
info!("{}", t!("main.mergingParameters"));
95-
match util::merge_parameters(&file_content, inline_content) {
96-
Ok(merged) => Some(merged),
97-
Err(err) => {
98-
error!("{}: {err}", t!("main.failedMergingParameters"));
99-
exit(EXIT_INVALID_INPUT);
100-
}
101-
}
102-
} else {
103-
parameters.clone()
104-
}
105-
} else if let Some(file_name) = parameters_file {
106-
// Only parameters_file provided
57+
// Read parameters from file if provided
58+
let file_params = if let Some(file_name) = &parameters_file {
10759
if file_name == "-" {
10860
info!("{}", t!("main.readingParametersFromStdin"));
10961
let mut stdin = Vec::<u8>::new();
@@ -124,7 +76,7 @@ fn main() {
12476
}
12577
} else {
12678
info!("{}: {file_name}", t!("main.readingParametersFile"));
127-
match std::fs::read_to_string(&file_name) {
79+
match std::fs::read_to_string(file_name) {
12880
Ok(content) => Some(content),
12981
Err(err) => {
13082
error!("{} '{file_name}': {err}", t!("main.failedReadingParametersFile"));
@@ -133,7 +85,23 @@ fn main() {
13385
}
13486
}
13587
} else {
136-
parameters
88+
None
89+
};
90+
91+
let merged_parameters = match (file_params, parameters) {
92+
(Some(file_content), Some(inline_content)) => {
93+
info!("{}", t!("main.mergingParameters"));
94+
match util::merge_parameters(&file_content, &inline_content) {
95+
Ok(merged) => Some(merged),
96+
Err(err) => {
97+
error!("{}: {err}", t!("main.failedMergingParameters"));
98+
exit(EXIT_INVALID_INPUT);
99+
}
100+
}
101+
},
102+
(Some(file_content), None) => Some(file_content),
103+
(None, Some(inline_content)) => Some(inline_content),
104+
(None, None) => None,
137105
};
138106

139107
subcommand::config(&subcommand, &merged_parameters, system_root.as_ref(), &as_group, &as_assert, &as_include, progress_format);

‎dsc/src/util.rs‎

Lines changed: 78 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,61 @@ pub fn in_desired_state(test_result: &ResourceTestResult) -> bool {
579579
}
580580
}
581581

582+
/// Parse input string as JSON or YAML and return a serde_json::Value.
583+
///
584+
/// # Arguments
585+
///
586+
/// * `input` - The input string to parse (JSON or YAML format)
587+
/// * `context` - Context string for error messages (e.g., "file parameters", "inline parameters")
588+
///
589+
/// # Returns
590+
///
591+
/// * `Result<serde_json::Value, DscError>` - Parsed JSON value
592+
///
593+
/// # Errors
594+
///
595+
/// This function will return an error if the input cannot be parsed as valid JSON or YAML
596+
fn parse_input_to_json_value(input: &str, context: &str) -> Result<serde_json::Value, DscError> {
597+
match serde_json::from_str(input) {
598+
Ok(json) => Ok(json),
599+
Err(_) => {
600+
match serde_yaml::from_str::<serde_yaml::Value>(input) {
601+
Ok(yaml) => Ok(serde_json::to_value(yaml)?),
602+
Err(err) => {
603+
Err(DscError::Parser(t!(&format!("util.failedToParse{context}"), error = err.to_string()).to_string()))
604+
}
605+
}
606+
}
607+
}
608+
}
609+
610+
/// Convert parameter input to a map, handling different formats.
611+
///
612+
/// # Arguments
613+
///
614+
/// * `params` - Parameter string to convert (JSON or YAML format)
615+
/// * `context` - Context string for error messages
616+
///
617+
/// # Returns
618+
///
619+
/// * `Result<serde_json::Map<String, serde_json::Value>, DscError>` - Parameter map
620+
///
621+
/// # Errors
622+
///
623+
/// Returns an error if the input cannot be parsed or is not an object
624+
fn params_to_map(params: &str, context: &str) -> Result<serde_json::Map<String, serde_json::Value>, DscError> {
625+
let value = parse_input_to_json_value(params, context)?;
626+
627+
let Some(map) = value.as_object().cloned() else {
628+
return Err(DscError::Parser(t!("util.parametersNotObject").to_string()));
629+
};
630+
631+
Ok(map)
632+
}
633+
582634
/// Merge two parameter sets, with inline parameters taking precedence over file parameters.
635+
/// Top-level keys (like "parameters") are merged recursively, but parameter values themselves
636+
/// are replaced (not merged) when specified inline.
583637
///
584638
/// # Arguments
585639
///
@@ -598,67 +652,35 @@ pub fn in_desired_state(test_result: &ResourceTestResult) -> bool {
598652
pub fn merge_parameters(file_params: &str, inline_params: &str) -> Result<String, DscError> {
599653
use serde_json::Value;
600654

601-
// Parse file parameters
602-
let file_value: Value = match serde_json::from_str(file_params) {
603-
Ok(json) => json,
604-
Err(_) => {
605-
// YAML
606-
match serde_yaml::from_str::<serde_yaml::Value>(file_params) {
607-
Ok(yaml) => serde_json::to_value(yaml)?,
608-
Err(err) => {
609-
return Err(DscError::Parser(format!("Failed to parse file parameters: {err}")));
610-
}
611-
}
612-
}
613-
};
614-
615-
// Parse inline parameters
616-
let inline_value: Value = match serde_json::from_str(inline_params) {
617-
Ok(json) => json,
618-
Err(_) => {
619-
// YAML
620-
match serde_yaml::from_str::<serde_yaml::Value>(inline_params) {
621-
Ok(yaml) => serde_json::to_value(yaml)?,
622-
Err(err) => {
623-
return Err(DscError::Parser(format!("Failed to parse inline parameters: {err}")));
655+
// Convert both parameter inputs to maps
656+
let mut file_map = params_to_map(file_params, "FileParameters")?;
657+
let inline_map = params_to_map(inline_params, "InlineParameters")?;
658+
659+
// Merge top-level keys
660+
for (key, inline_value) in &inline_map {
661+
if key == "parameters" {
662+
// Special handling for "parameters" key - merge at parameter name level only
663+
// Within each parameter name, inline replaces (not merges)
664+
if let Some(file_params_value) = file_map.get_mut("parameters") {
665+
if let (Some(file_params_obj), Some(inline_params_obj)) = (file_params_value.as_object_mut(), inline_value.as_object()) {
666+
// For each parameter in inline, replace (not merge) in file
667+
for (param_name, param_value) in inline_params_obj {
668+
file_params_obj.insert(param_name.clone(), param_value.clone());
669+
}
670+
} else {
671+
// If one is not an object, inline replaces completely
672+
file_map.insert(key.clone(), inline_value.clone());
624673
}
674+
} else {
675+
// "parameters" key doesn't exist in file, add it
676+
file_map.insert(key.clone(), inline_value.clone());
625677
}
626-
}
627-
};
628-
629-
// Both must be objects to merge
630-
let Some(mut file_obj) = file_value.as_object().cloned() else {
631-
return Err(DscError::Parser("File parameters must be a JSON object".to_string()));
632-
};
633-
634-
let Some(inline_obj) = inline_value.as_object() else {
635-
return Err(DscError::Parser("Inline parameters must be a JSON object".to_string()));
636-
};
637-
638-
// Special handling for the "parameters" key - merge nested objects
639-
if let (Some(file_params_value), Some(inline_params_value)) = (file_obj.get("parameters"), inline_obj.get("parameters")) {
640-
if let (Some(mut file_params_obj), Some(inline_params_obj)) = (file_params_value.as_object().cloned(), inline_params_value.as_object()) {
641-
// Merge the nested parameters objects
642-
for (key, value) in inline_params_obj {
643-
file_params_obj.insert(key.clone(), value.clone());
644-
}
645-
file_obj.insert("parameters".to_string(), Value::Object(file_params_obj));
646678
} else {
647-
// If one is not an object, inline takes precedence
648-
file_obj.insert("parameters".to_string(), inline_params_value.clone());
649-
}
650-
} else if let Some(inline_params_value) = inline_obj.get("parameters") {
651-
// Only inline has parameters
652-
file_obj.insert("parameters".to_string(), inline_params_value.clone());
653-
}
654-
655-
// Merge other top-level keys: inline parameters override file parameters
656-
for (key, value) in inline_obj {
657-
if key != "parameters" {
658-
file_obj.insert(key.clone(), value.clone());
679+
// For other top-level keys, inline value replaces file value
680+
file_map.insert(key.clone(), inline_value.clone());
659681
}
660682
}
661683

662-
let merged = Value::Object(file_obj);
684+
let merged = Value::Object(file_map);
663685
Ok(serde_json::to_string(&merged)?)
664686
}

‎dsc/tests/dsc_parameters.tests.ps1‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ Describe 'Parameters tests' {
391391
$LASTEXITCODE | Should -Be 4
392392
$out | Should -BeNullOrEmpty
393393
$errorMessage = Get-Content -Path $TestDrive/error.log -Raw
394-
$errorMessage | Should -BeLike "*ERROR*Cannot read from STDIN for both parameters and input*"
394+
$errorMessage | Should -BeLike "*ERROR*Empty input provided*"
395395
}
396396

397397
It 'Invalid parameters read from STDIN result in error' {

0 commit comments

Comments
 (0)