Skip to content

fix: refactor dotted key expansion to handle nested objects recursively #173

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mmalkhatib merged 1 commit into from
May 21, 2026
Merged

fix: refactor dotted key expansion to handle nested objects recursively #173

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 22 additions & 8 deletions SW.Bitween.NativeAdapters/JsonMapper/ScribanJsonHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,8 @@ public static string Render(string scribanTemplate, string inputJson)
throw new InvalidOperationException($"Template produced invalid JSON: {ex.Message}\n\nRendered:\n{rendered}");
}

// 8. Expand dotted keys into nested objects (e.g. "buyer.email" → {buyer:{email:...}})
var result = new JObject();
foreach (var prop in flat.Properties())
{
SetByPath(result, prop.Name, prop.Value);
}

return result.ToString(Formatting.Indented);
// 8. Expand dotted keys into nested objects recursively at all depths
return ExpandDottedKeys(flat).ToString(Formatting.Indented);
}

// ─── Helpers ──────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -140,6 +134,26 @@ public override bool TryGetValue(TemplateContext context, SourceSpan span, strin
}
}

/// <summary>Recursively expands dotted keys in all JObjects at every depth, including inside arrays.</summary>
private static JToken ExpandDottedKeys(JToken token)
{
if (token is JObject obj)
{
var result = new JObject();
foreach (var prop in obj.Properties())
SetByPath(result, prop.Name, ExpandDottedKeys(prop.Value));
return result;
Comment on lines +143 to +145

@coderabbitai coderabbitai Bot May 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Detect and block dotted-path collisions to prevent silent data loss.

Line 144 now applies SetByPath recursively at every depth. If an object has both a and a.b, one value is overwritten based on property order. Please fail fast on collision instead of replacing existing nodes.

💡 Proposed fix 
 private static void SetByPath(JObject root, string path, JToken value)
 {
     var parts = path.Split('.');
     JObject current = root;
     for (int i = 0; i < parts.Length - 1; i++)
     {
         var part = parts[i];
-        if (current[part] is not JObject child)
-        {
-            child = new JObject();
-            current[part] = child;
-        }
-        current = child;
+        var existing = current[part];
+        if (existing is JObject child)
+        {
+            current = child;
+            continue;
+        }
+
+        if (existing != null)
+            throw new InvalidOperationException(
+                $"Cannot expand dotted path '{path}': segment '{part}' is already a non-object value.");
+
+        var newChild = new JObject();
+        current[part] = newChild;
+        current = newChild;
     }
-    current[parts[^1]] = value;
+    var leaf = parts[^1];
+    if (current[leaf] is JObject && value is not JObject)
+        throw new InvalidOperationException(
+            $"Cannot set path '{path}': destination is already an object.");
+
+    current[leaf] = value;
 }
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@SW.Bitween.NativeAdapters/JsonMapper/ScribanJsonHelper.cs` around lines 143 -
145, The loop that calls SetByPath for each prop after ExpandDottedKeys can
silently overwrite when dotted paths collide (e.g., existing "a" vs incoming
"a.b"); modify the logic to detect such collisions and throw instead of
overwriting: either add a pre-check in the caller before SetByPath that inspects
the current result for existing nodes along the dotted path (using prop.Name and
the structure returned by ExpandDottedKeys) or harden SetByPath itself to
validate each path segment (in SetByPath, when traversing/creating nodes, throw
an exception if a scalar value exists where an object is required or an object
exists where a scalar is being set). Ensure the thrown exception includes the
conflicting path and the names of the existing and incoming node types so
callers can fail fast on collisions.

}
if (token is JArray arr)
{
var result = new JArray();
foreach (var item in arr)
result.Add(ExpandDottedKeys(item));
return result;
}
return token;
}

/// <summary>Sets a value at a dot-separated path inside a JObject, creating intermediate objects as needed.</summary>
private static void SetByPath(JObject root, string path, JToken value)
{
Expand Down