Scripting: JSON handling fix #2954

[predic8]

Summary by CodeRabbit

• Bug Fixes

  • Improved JSON parameter binding behavior in scripting plugins.

• Logging & Observability

  • JSON parsing failure logs now provide cleaner, less verbose output.

• Tests

  • Added comprehensive test coverage for JSON parameter binding behavior.

[coderabbitai]

📝 Walkthrough

Walkthrough

ScriptingUtils updates JSON parameter binding to deserialize request bodies to Object.class instead of Map.class, enabling flexible type handling for JSON objects, arrays, strings, and numbers. Imports are reorganized from wildcards to explicit declarations, and logging behavior changes from warning-level with exception to info-level with message only. A new test suite validates the type conversions across JSON primitives and collections.

Changes

JSON Parameter Binding

Layer / File(s)	Summary
JSON deserialization logic and imports core/src/main/java/com/predic8/membrane/core/lang/ScriptingUtils.java	Import section replaces wildcard imports with explicit imports for Jackson, Membrane core classes, and utilities. JSON parsing now deserializes to Object.class for flexible type handling, and failure logging changes from warn-with-exception to info with message only.
JSON parameter binding validation tests core/src/test/java/com/predic8/membrane/core/lang/ScriptingUtilsTest.java	New @Nested test class json validates that createParameterBindings produces Map for objects, List for arrays, String for strings, and Integer for numbers, using a createBinding helper with mocked exchanges.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A hop through JSON's twisted vines,
Where Objects now replace old Lines,
The tests skip merrily to validate,
Each type transform—how elegant! ✨
Imports neat, the logic clean,
Best refactoring we've seen.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% 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.
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.
Title check	✅ Passed	The title accurately reflects the main change: fixing JSON handling in the scripting utilities, specifically for deserialization to Object.class instead of Map.class and adjusting error logging.

_{✏️ 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 fix-2954-json-array-payloads-in-scripts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

core/src/test/java/com/predic8/membrane/core/lang/ScriptingUtilsTest.java (1)

22-43: ⚡ Quick win

Add boolean and null JSON binding tests.

The suite currently skips two JSON primitive cases (true/false and null). Adding both keeps the new Object.class contract fully covered.

Proposed test additions

         `@Test`
         void number() throws URISyntaxException {
             assertInstanceOf(Integer.class, createBinding("7").get("json"));
         }

+        `@Test`
+        void bool() throws URISyntaxException {
+            assertInstanceOf(Boolean.class, createBinding("true").get("json"));
+        }
+
+        `@Test`
+        void nullValue() throws URISyntaxException {
+            var bindings = createBinding("null");
+            assertNotNull(bindings);
+            org.junit.jupiter.api.Assertions.assertNull(bindings.get("json"));
+        }
+
         private static `@NotNull` Map<String, Object> createBinding(String json) throws URISyntaxException {

🤖 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 `@core/src/test/java/com/predic8/membrane/core/lang/ScriptingUtilsTest.java`
around lines 22 - 43, Add two unit tests to cover the missing JSON primitives:
create a test method (e.g., booleanValue) that calls
createBinding("true").get("json") and assertInstanceOf(Boolean.class, ...) to
verify boolean binding, and a test method (e.g., nullValue) that calls
createBinding("null").get("json") and assertNull(...) to verify null binding;
reference createBinding(...) and the test class ScriptingUtilsTest to insert
these new `@Test` methods alongside the existing map/array/string/number tests.

🤖 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.

Nitpick comments:
In `@core/src/test/java/com/predic8/membrane/core/lang/ScriptingUtilsTest.java`:
- Around line 22-43: Add two unit tests to cover the missing JSON primitives:
create a test method (e.g., booleanValue) that calls
createBinding("true").get("json") and assertInstanceOf(Boolean.class, ...) to
verify boolean binding, and a test method (e.g., nullValue) that calls
createBinding("null").get("json") and assertNull(...) to verify null binding;
reference createBinding(...) and the test class ScriptingUtilsTest to insert
these new `@Test` methods alongside the existing map/array/string/number tests.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: dc1cc9af-c947-443e-9ae2-b39efb0f9e85

📥 Commits

Reviewing files that changed from the base of the PR and between b6622e0 and 5bfd871.

📒 Files selected for processing (2)

• core/src/main/java/com/predic8/membrane/core/lang/ScriptingUtils.java
• core/src/test/java/com/predic8/membrane/core/lang/ScriptingUtilsTest.java