Skip to content

Commit 38026e8

Browse files
committed
fix(aliases): skip sub-resource arrays in scalar precompute and harden denorm Phase 2a
- In compute_aggregates_for_version, skip non-wildcard entries whose short name matches a detected sub-resource array. Without this filter, scalar alias normalization would overwrite the properly flattened sub-resource array with a non-flattened copy. - Populate alias_owned_normalized_roots for all non-wildcard scalar aliases (not just single-segment ones) so Phase 2a correctly skips nested alias roots like sku.name. - Change get_path_exact_or_ci to accept &[String] directly instead of &[&str], eliminating the per-op Vec<&str> allocation in the scalar denormalization loop.
1 parent 1675f96 commit 38026e8

8 files changed

Lines changed: 258 additions & 54 deletions

File tree

src/languages/azure_policy/aliases/denormalizer/mod.rs

Lines changed: 49 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,20 @@ pub(crate) mod sub_resource;
99
#[cfg(test)]
1010
mod tests;
1111

12+
use alloc::vec::Vec;
13+
1214
use crate::Value;
1315

1416
use crate::Rc;
1517

1618
use super::obj_map::{
1719
extract_type_field, find_key_ci, get_path_exact_or_ci, make_value, new_map,
18-
obj_get_exact_or_ci, obj_insert, remove_element_field, set_nested, val_str, ROOT_FIELDS,
20+
obj_get_exact_or_ci, obj_insert, remove_element_field_ci, set_nested, val_str, ROOT_FIELDS,
1921
};
2022
use super::types::ResolvedAliases;
2123
use super::AliasRegistry;
2224

23-
use super::normalizer::{apply_element_remap, ElementRemap};
25+
use super::normalizer::apply_element_remap_reverse;
2426

2527
use casing::{default_casing_map, denormalize_value, restore_casing};
2628

@@ -101,12 +103,7 @@ pub fn denormalize_with_aliases(
101103
// Phase 2b: Aliased scalar fields → versioned ARM paths.
102104
if let Some(agg) = selected_agg {
103105
for op in &agg.scalar_aliases_denormalize {
104-
let segments: alloc::vec::Vec<&str> = op
105-
.normalized_path_segments
106-
.iter()
107-
.map(|segment| segment.as_str())
108-
.collect();
109-
let val = get_path_exact_or_ci(obj, &segments);
106+
let val = get_path_exact_or_ci(obj, &op.normalized_path_segments);
110107
let val = match val {
111108
Some(v) => v,
112109
None => continue,
@@ -126,6 +123,28 @@ pub fn denormalize_with_aliases(
126123
}
127124

128125
// Phase 2c + 2d: Use precomputed renames/remaps.
126+
//
127+
// For data-plane resources, non-root fields live in `result` (no
128+
// "properties" wrapper) but Phases 2c/2d/3 all operate on
129+
// `properties`. Stage data-plane fields into `properties` so the
130+
// subsequent phases can process them uniformly; Phase 4 merges
131+
// them back into `result` at the top level.
132+
if is_data_plane {
133+
let keys_to_move: Vec<Value> = result
134+
.keys()
135+
.filter(|k| {
136+
val_str(k)
137+
.is_some_and(|s| !ROOT_FIELDS.iter().any(|f| f.eq_ignore_ascii_case(s)))
138+
})
139+
.cloned()
140+
.collect();
141+
for k in keys_to_move {
142+
if let Some(v) = result.remove(&k) {
143+
properties.entry(k).or_insert(v);
144+
}
145+
}
146+
}
147+
129148
// Phase 2c: Precomputed array base renames.
130149
for (alias_base_lc, arm_base) in &agg.array_renames_denormalize {
131150
if let Some(key) = find_key_ci(&properties, alias_base_lc) {
@@ -137,14 +156,16 @@ pub fn denormalize_with_aliases(
137156

138157
// Phase 2d: Precomputed reverse element-level field remaps.
139158
// target_field is already stored with original ARM casing.
159+
// Uses case-insensitive lookups because Phase 2a restored key casing
160+
// but array_chain/source_field are lowercased.
140161
for rev in &agg.reverse_element_remaps {
141-
let remap = ElementRemap {
142-
array_chain: rev.array_chain.clone(),
143-
source_field: rev.source_field.clone(),
144-
target_field: rev.target_field.clone(),
145-
};
146-
apply_element_remap(&mut properties, &remap, false);
147-
remove_element_field(&mut properties, &rev.array_chain, &rev.cleanup_field);
162+
apply_element_remap_reverse(
163+
&mut properties,
164+
&rev.array_chain,
165+
&rev.source_field_segments,
166+
&rev.target_field,
167+
);
168+
remove_element_field_ci(&mut properties, &rev.array_chain, &rev.cleanup_field);
148169
}
149170
}
150171

@@ -160,14 +181,22 @@ pub fn denormalize_with_aliases(
160181

161182
// Phase 4: Attach properties to result.
162183
if !properties.is_empty() {
163-
let props_key = Value::from("properties");
164-
if let Some(Value::Object(existing_rc)) = result.get_mut(&props_key) {
165-
let existing = Rc::make_mut(existing_rc);
184+
if is_data_plane {
185+
// Data-plane resources have no "properties" wrapper; merge
186+
// directly into the top-level result.
166187
for (k, v) in properties {
167-
existing.entry(k).or_insert(v);
188+
result.entry(k).or_insert(v);
168189
}
169190
} else {
170-
obj_insert(&mut result, "properties", make_value(properties));
191+
let props_key = Value::from("properties");
192+
if let Some(Value::Object(existing_rc)) = result.get_mut(&props_key) {
193+
let existing = Rc::make_mut(existing_rc);
194+
for (k, v) in properties {
195+
existing.entry(k).or_insert(v);
196+
}
197+
} else {
198+
obj_insert(&mut result, "properties", make_value(properties));
199+
}
171200
}
172201
}
173202

src/languages/azure_policy/aliases/denormalizer/sub_resource.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,7 @@ pub fn rewrap_precomputed_sub_resource_arrays(
5656
ops: &[PrecomputedSubResourceRewrap],
5757
) {
5858
for op in ops {
59-
let parent_parts: Vec<&str> = op.parent_path.iter().map(|part| part.as_str()).collect();
60-
61-
for_each_array_object_in_path_ci(properties, &parent_parts, &mut |parent| {
59+
for_each_array_object_in_path_ci(properties, &op.parent_path, &mut |parent| {
6260
if let Some(Value::Array(arr)) = obj_get_mut_ci(parent, &op.array_name) {
6361
let inner = crate::Rc::make_mut(arr);
6462
for elem in inner.iter_mut() {

src/languages/azure_policy/aliases/mod.rs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -225,11 +225,8 @@ impl AliasRegistry {
225225
// so that overwritten aliases that no longer indicate sub-resource
226226
// wrapping cause stale classifications to be removed.
227227
existing.sub_resource_arrays = redetect_sub_resource_arrays(&existing.entries);
228-
// Recompute aggregates after merge.
229-
let (default_agg, versioned_agg) =
230-
precompute_aggregates(&existing.entries, &existing.sub_resource_arrays);
231-
existing.default_aggregates = default_agg;
232-
existing.versioned_aggregates = versioned_agg;
228+
// Recompute casing map and aggregates after merge.
229+
existing.rebuild_aggregates();
233230
} else {
234231
self.types.insert(lc_type, resolved);
235232
}
@@ -480,6 +477,17 @@ fn compute_aggregates_for_version(
480477
let selected_path = entry.select_path(api_version);
481478

482479
if !entry.is_wildcard {
480+
// Skip non-wildcard entries whose short name matches a detected
481+
// sub-resource array. These are handled by the sub-resource
482+
// rewrap path; including them here would overwrite the properly
483+
// flattened array with a non-flattened copy.
484+
if sub_resource_arrays.contains(&entry.short_name.to_ascii_lowercase()) {
485+
continue;
486+
}
487+
488+
// Only mark root as alias-owned when the alias IS the root
489+
// (single-segment path). Multi-segment aliases share a root key
490+
// that may contain non-alias siblings which Phase 2a must preserve.
483491
if entry.normalized_output_segments().len() == 1 {
484492
let root = entry.normalized_root_key();
485493
let stripped = root.strip_prefix("_p_").unwrap_or(root);
@@ -536,9 +544,12 @@ fn compute_aggregates_for_version(
536544
target_field: target_lc.clone(),
537545
});
538546

547+
let source_field_segments: Vec<String> =
548+
target_lc.split('.').map(String::from).collect();
539549
reverse_element_remaps.push(PrecomputedReverseRemap {
540550
array_chain,
541551
source_field: target_lc.clone(),
552+
source_field_segments,
542553
target_field: arm_leaf.to_string(),
543554
cleanup_field: target_lc,
544555
});

src/languages/azure_policy/aliases/normalizer/element_remap.rs

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,31 +8,12 @@ use alloc::vec::Vec;
88

99
use crate::Value;
1010

11-
use super::super::obj_map::{for_each_array_object_in_chain, get_path, set_nested, ObjMap};
11+
use super::super::obj_map::{
12+
for_each_array_object_in_chain, for_each_array_object_in_chain_ci, get_path,
13+
get_path_exact_or_ci, set_nested, ObjMap,
14+
};
1215
use super::super::types::PrecomputedRemap;
1316

14-
/// Describes a field remapping inside each element of a (possibly nested) array.
15-
pub struct ElementRemap {
16-
/// Chain of array navigations for nested `[*]` levels.
17-
pub(crate) array_chain: Vec<Vec<String>>,
18-
/// Dot-separated path to read within the innermost array element.
19-
pub(crate) source_field: String,
20-
/// Dot-separated path to write within the innermost array element.
21-
pub(crate) target_field: String,
22-
}
23-
24-
/// Apply an element-level field remap to each element of an array (or nested
25-
/// array chain).
26-
///
27-
/// When `lowercase` is `true` (normalizer), target path segments are
28-
/// lowercased. When `false` (denormalizer), they are written verbatim
29-
/// so that restored casing is preserved.
30-
pub fn apply_element_remap(result: &mut ObjMap, remap: &ElementRemap, lowercase: bool) {
31-
for_each_array_object_in_chain(result, &remap.array_chain, &mut |element| {
32-
remap_deep_field(element, &remap.source_field, &remap.target_field, lowercase);
33-
});
34-
}
35-
3617
/// Apply a precomputed element remap (from [`PrecomputedRemap`]) without
3718
/// any per-call string splitting or allocation.
3819
pub fn apply_element_remap_precomputed(result: &mut ObjMap, remap: &PrecomputedRemap) {
@@ -51,6 +32,26 @@ fn remap_deep_field(obj: &mut ObjMap, source: &str, target: &str, lowercase: boo
5132
set_nested(obj, target, val, lowercase);
5233
}
5334

35+
/// Apply a reverse (denormalization) element-level field remap using
36+
/// case-insensitive lookups for both array chain traversal and source field
37+
/// reads. This is necessary because Phase 2a restores key casing before
38+
/// Phase 2d runs, so the lowercased chain/source segments would miss with
39+
/// exact-case lookups.
40+
pub fn apply_element_remap_reverse(
41+
result: &mut ObjMap,
42+
array_chain: &[Vec<String>],
43+
source_segments: &[String],
44+
target: &str,
45+
) {
46+
for_each_array_object_in_chain_ci(result, array_chain, &mut |element| {
47+
let val = match get_path_exact_or_ci(element, source_segments) {
48+
Some(v) => v.clone(),
49+
None => return,
50+
};
51+
set_nested(element, target, val, false);
52+
});
53+
}
54+
5455
/// Read a value at a dot-separated path from an ObjMap.
5556
fn read_dotted_path(obj: &ObjMap, path: &str) -> Option<Value> {
5657
let segments: Vec<&str> = path.split('.').collect();

src/languages/azure_policy/aliases/normalizer/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ mod element_remap;
1111
mod flatten;
1212

1313
// Re-export items used by the denormalizer.
14-
pub(crate) use element_remap::{apply_element_remap, ElementRemap};
14+
pub(crate) use element_remap::apply_element_remap_reverse;
1515

1616
use crate::Value;
1717

0 commit comments

Comments
 (0)