Skip to content

feat: enhance ScribanJsonHelper to support root array input and outpu… - #180

Merged
AhmadRAbuhussein merged 2 commits into
releases/r8.0from
hamza/feature/json-mapper-root-array-support
Jun 9, 2026
Merged

feat: enhance ScribanJsonHelper to support root array input and outpu…#180
AhmadRAbuhussein merged 2 commits into
releases/r8.0from
hamza/feature/json-mapper-root-array-support

Conversation

@hamzahalq

@hamzahalq hamzahalq commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

…t with comprehensive unit tests

Summary by CodeRabbit

  • New Features

    • JSON template rendering now supports root arrays as inputs and outputs and enriches both objects and array elements with partner/global values.
    • Improved error messages on JSON parse failures, including the full rendered output.
  • Tests

    • Added comprehensive tests for array-based rendering and a helper to render/parse array results.

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Review Change Stack

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cf9a6aa0-a9ef-4f71-963c-de703bd5f3c7

📥 Commits

Reviewing files that changed from the base of the PR and between 5d8817b and 7e5bcbb.

📒 Files selected for processing (4)
  • SW.Bitween.Api/Resources/Mappers/Preview.cs
  • SW.Bitween.NativeAdapters/JsonMapper/ScribanJsonHelper.cs
  • SW.Bitween.UnitTests/ScribanJsonHelperRootArrayTests.cs
  • SW.Bitween.UnitTests/ScribanJsonTestHelper.cs

📝 Walkthrough

Walkthrough

ScribanJsonHelper.Render now accepts root JSON arrays or objects, exposes arrays as items (hoisting first-item fields), parses rendered output as a JToken (object or array), and applies recursive dotted-key expansion. Tests and a RenderArray helper validate array-input and array-output scenarios.

Changes

Root Array Support in Scriban JSON Rendering

Layer / File(s) Summary
Core Array Input/Output Rendering Logic
SW.Bitween.NativeAdapters/JsonMapper/ScribanJsonHelper.cs
Render accepts JArray or JObject roots; JArray is exposed as items with optional hoisting of first-element properties. Rendered output is parsed as any JToken, recursive ExpandDottedKeys runs across objects and arrays, and JSON parse errors include the rendered string.
Test helpers and root-array test coverage
SW.Bitween.UnitTests/ScribanJsonTestHelper.cs, SW.Bitween.UnitTests/ScribanJsonHelperRootArrayTests.cs
Added RenderArray helper and an 18-test MSTest suite covering array-input→object-output, object-input→array-output, and array-input→array-output cases (access, iteration, hoisting, injected values, filtering, primitives, nested arrays, empty arrays).
Preview enrichment for arrays and objects
SW.Bitween.Api/Resources/Mappers/Preview.cs
Refactored Preview.Handle to parse input once as JToken and apply __partner__/__globals__ enrichment to either a root JObject or to each JObject element within a root JArray. Removed the previous enriched flag logic.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • AhmadRAbuhussein
  • mmalkhatib

Poem

🐰 I nibble keys and hop through trees of JSON,
Arrays unfold like radishes for fun,
First items hoisted, dotted paths unwind,
Templates bloom, and tests keep bugs confined.
Hooray for hoists — a rabbit's render rhyme!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title is truncated but clearly refers to the main change: enhancing ScribanJsonHelper to support root array input/output. It accurately summarizes the primary feature added.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch hamza/feature/json-mapper-root-array-support

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
SW.Bitween.NativeAdapters/JsonMapper/ScribanJsonHelper.cs (1)

87-87: 💤 Low value

Consider security implications of including rendered output in exception messages.

The error message now includes the full rendered template output via \n\nRendered:\n{rendered}. While this significantly aids debugging, the rendered output might contain sensitive data (PII, secrets, or business-critical values) that could be exposed in logs or error tracking systems.

🔒 Optional mitigation strategies
  • Truncate the rendered output to a reasonable length (e.g., first 500 characters)
  • Add a configuration flag to control whether full output is included
  • Redact sensitive patterns before including in the exception
  • Document that exception handlers should treat this as sensitive data

Example truncation:

-throw new InvalidOperationException($"Template produced invalid JSON: {ex.Message}\n\nRendered:\n{rendered}");
+var preview = rendered.Length > 500 ? rendered.Substring(0, 500) + "..." : rendered;
+throw new InvalidOperationException($"Template produced invalid JSON: {ex.Message}\n\nRendered:\n{preview}");
🤖 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` at line 87, The
current throw in ScribanJsonHelper that appends the full rendered string (the
InvalidOperationException using rendered) can expose sensitive data; modify the
code that throws the exception so it does not include the entire rendered
output: truncate the rendered variable to a safe length (e.g., first 500
characters) and append an explicit marker like "...(truncated)" or gate
inclusion behind a configuration flag (e.g., a boolean in ScribanJsonHelper
settings) so full output is only included when explicitly enabled; locate the
throw site referencing rendered in ScribanJsonHelper.cs and replace the message
construction to use the truncated (or config-guarded) value.
🤖 Prompt for all review comments with 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.

Inline comments:
In `@SW.Bitween.UnitTests/ScribanJsonHelperRootArrayTests.cs`:
- Around line 90-114: The Preview integration only injects __partner__ /
__globals__ when inputJson parses to a JObject (the code path in Preview.Handle
checks if (JToken.Parse(inputJson) is JObject parsedObj)), so root-array
payloads never get enrichment and templates miss those fields; update
Preview.Handle to detect root JArray cases, wrap or transform the array into an
enriched object (e.g., add __partner__ / __globals__ to each element or create a
wrapper object that contains the array plus the hoisted fields) before passing
to the Scriban rendering pipeline, ensuring the same enrichment logic used for
JObject is applied to root arrays so templates can access __partner__ and
__globals__ end-to-end.

---

Nitpick comments:
In `@SW.Bitween.NativeAdapters/JsonMapper/ScribanJsonHelper.cs`:
- Line 87: The current throw in ScribanJsonHelper that appends the full rendered
string (the InvalidOperationException using rendered) can expose sensitive data;
modify the code that throws the exception so it does not include the entire
rendered output: truncate the rendered variable to a safe length (e.g., first
500 characters) and append an explicit marker like "...(truncated)" or gate
inclusion behind a configuration flag (e.g., a boolean in ScribanJsonHelper
settings) so full output is only included when explicitly enabled; locate the
throw site referencing rendered in ScribanJsonHelper.cs and replace the message
construction to use the truncated (or config-guarded) value.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 60fd5c07-b2cc-494a-8eef-f13180e691ea

📥 Commits

Reviewing files that changed from the base of the PR and between 5d8817b and 5b82b8a.

📒 Files selected for processing (3)
  • SW.Bitween.NativeAdapters/JsonMapper/ScribanJsonHelper.cs
  • SW.Bitween.UnitTests/ScribanJsonHelperRootArrayTests.cs
  • SW.Bitween.UnitTests/ScribanJsonTestHelper.cs

Comment thread SW.Bitween.UnitTests/ScribanJsonHelperRootArrayTests.cs
@AhmadRAbuhussein
AhmadRAbuhussein merged commit 584e218 into releases/r8.0 Jun 9, 2026
2 checks passed
@MusaMisto
MusaMisto deleted the hamza/feature/json-mapper-root-array-support branch July 2, 2026 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants