Conversation
…nd add comprehensive type conversion tests
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 10 minutes and 49 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughFixes InherentTypeViolation and union resolution failures during Ballerina function execution from Micro Integrator by implementing strict type conversions before invocation, improving union member selection through enhanced type unwrapping, and correcting property naming in configuration and union parameter handling. Changes
Sequence DiagramsequenceDiagram
participant Synapse as Synapse/MI<br/>Properties
participant BalExec as BalExecutor
participant ParamHnd as ParamHandler
participant DataTrans as DataTransformer
participant Ballerina as Ballerina<br/>Runtime
Synapse->>BalExec: invoke callable with args
BalExec->>ParamHnd: setParameters(args, context, callable)
ParamHnd->>ParamHnd: loop: for each param
ParamHnd->>ParamHnd: check if BMap or BArray
alt Type Conversion Needed
ParamHnd->>DataTrans: getFunctionParameterType(callable, functionName, index)
DataTrans-->>ParamHnd: expectedType
ParamHnd->>DataTrans: convertValueToType(param, expectedType)
Note over DataTrans: unwrap INTERSECTION_TYPE<br/>unwrap TYPE_REFERENCED_TYPE<br/>resolve UNION_TAG with multi-pass
DataTrans-->>ParamHnd: converted value
ParamHnd->>ParamHnd: assign converted param to args[i]
else No Conversion
ParamHnd->>ParamHnd: assign param as-is
end
ParamHnd-->>BalExec: prepared args
BalExec->>Ballerina: invoke with prepared args
alt Success
Ballerina-->>BalExec: result
else BError
BalExec->>BalExec: log error message
BalExec-->>BalExec: throw BallerinaExecutionException
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
tests/src/test/java/io/ballerina/stdlib/mi/executor/ParamHandlerTest.java (1)
71-72: Please add coverage for the new callable-aware conversion path.This update only verifies the new signature with
null. It never exercises the BObject/Module lookup added inParamHandler.setParameters(...), so regressions in the new type-conversion branch will slip through.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/src/test/java/io/ballerina/stdlib/mi/executor/ParamHandlerTest.java` around lines 71 - 72, The test currently calls ParamHandler.setParameters(args, context, null) and thus never exercises the callable-aware conversion path; update the test to also invoke ParamHandler.setParameters with a non-null callable container (e.g., a BObject or Module instance) so the BObject/Module lookup in ParamHandler.setParameters is exercised, and add assertions verifying the converted parameter values/state after the call to catch regressions in the new type-conversion branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@native/src/main/java/io/ballerina/stdlib/mi/BalConnectorConfig.java`:
- Around line 153-166: The loop that builds connection args (using
paramHandler.getParameter and the local objectTypeName) never reshapes
BMap/BArray parameters into the expected inherent Ballerina types, so ensure
each param is converted before storing back into args: for each param returned
by paramHandler.getParameter in the loop, if param is a BMap or BArray, call the
existing conversion/transform helper used elsewhere (e.g., the
ParamHandler.convertValueToType or equivalent convertValueToType utility
referenced in the review) with context and the objectTypeName / the param's
expected type so it becomes the typed record/array; replace args[i] with the
converted value and handle exceptions (don’t swallow them silently) so
ValueCreator.createObjectValue(...) receives properly typed values similar to
the logic in io.ballerina.stdlib.mi.executor.ParamHandler.
In `@native/src/main/java/io/ballerina/stdlib/mi/executor/DataTransformer.java`:
- Around line 394-409: The catch block after convertValueToType currently logs
the exception and then falls back to returning the raw reconstructedBMap, which
can reintroduce InherentTypeViolation downstream (see ParamHandler usage);
change the behavior so that when convertValueToType(reconstructedBMap, recType)
throws, you throw a SynapseException (including the original exception message
or wrapping the exception) instead of returning the untyped value; apply the
same change to the analogous block that handles the other typed-record
conversion (the second convertValueToType/reconstructedBMap branch mentioned in
the comment).
- Around line 89-107: The conversion logic in DataTransformer that handles
TypeTags.INT_TAG and TypeTags.FLOAT_TAG currently uses ((Number)
sourceValue).longValue() which silently truncates fractional values (causing
lossy narrowing and wrong union-member selection); update the INT conversion to
detect fractional input (e.g., non-integer Number or BDecimal/BFloat with
fraction) and fail fast by throwing a type/coercion error instead of truncating,
and keep FLOAT conversion as doubleValue(); also add explicit handling for
TypeTags.DECIMAL_TAG in the union "Pass 2" coercion logic (the same pass that
currently handles numeric members) so decimal union members get a proper
numeric->decimal conversion (use BigDecimal construction from the numeric value)
rather than falling through to the string-only pass. Ensure you modify the
branches that reference TypeTags.INT_TAG, TypeTags.FLOAT_TAG,
TypeTags.DECIMAL_TAG and longValue()/doubleValue() in DataTransformer to
implement these checks and the new DECIMAL_TAG branch.
- Around line 198-233: createTypedRecordFromGeneric currently only copies
declared fields and drops undeclared entries; update it so when targetType is a
RecordType with a non-null/non-NEVER restFieldType (i.e., an open record) you
also iterate over genericMap keys that are not in targetType.getFields() and
insert them into typedRecord as rest fields: convert each undeclared value with
convertValueToType(genericValue, restFieldType) and put it under the same
BString key (ensuring you skip keys already handled by the declared-field loop);
keep the existing strict-mode closed-record validation unchanged.
In
`@tests/src/test/java/io/ballerina/stdlib/mi/executor/DataTransformerTest.java`:
- Around line 397-440: The test testCreateTypedRecordFromGeneric_ClosedRecord
doesn't exercise the closed-record rejection path because it mocks StructureType
and calls the non-strict overload; update the test to mock RecordType (use
RecordType instead of StructureType), ensure targetType instanceof RecordType by
returning that mock from targetType, and call the strict overload of
DataTransformer.createTypedRecordFromGeneric(...) with strict=true (or otherwise
invoke the API that enforces closed-record semantics) while providing a
sourceMap that contains an extra key ("unknown") so the method will hit the
closed-record branch and you can assert the expected rejection/behavior.
In `@tests/src/test/resources/expected/project7/functions/component.xml`:
- Around line 66-68: The <component name="postUsersMessages_importByUserId">
entry has a mismatched <file> value — update the <file> element currently set to
"postUsersMessages_importByUserId.xml" to exactly match the actual fixture
filename "postusersmessages_Importbyuserid.xml" (preserving case) so the file
reference in component.xml points to the real fixture and avoids case-sensitive
filesystem failures.
In
`@tool-migen-cli/src/main/java/io/ballerina/mi/generator/XmlPropertyWriter.java`:
- Around line 113-119: The code in XmlPropertyWriter unconditionally
dereferences parameter.typeInfo when writing record metadata (in the case
"record"), which crashes for params with no typeInfo and emits "null" strings;
update the "record" branch to check parameter.typeInfo != null before appending
the recordName/recordModule/recordOrg/recordVersion properties (and individually
guard each field if needed) so you only emit those <property .../> lines when
typeInfo and its non-null subfields exist, keeping the existing parameter.name
and parameter.typeName lines unchanged; reference the case "record" block, the
parameter variable, and the typeInfo property when making this change.
---
Nitpick comments:
In `@tests/src/test/java/io/ballerina/stdlib/mi/executor/ParamHandlerTest.java`:
- Around line 71-72: The test currently calls ParamHandler.setParameters(args,
context, null) and thus never exercises the callable-aware conversion path;
update the test to also invoke ParamHandler.setParameters with a non-null
callable container (e.g., a BObject or Module instance) so the BObject/Module
lookup in ParamHandler.setParameters is exercised, and add assertions verifying
the converted parameter values/state after the call to catch regressions in the
new type-conversion branch.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cbfc0c1f-762e-4486-8b70-ef0cf493f88c
📒 Files selected for processing (21)
CHANGELOG.mdnative/src/main/java/io/ballerina/stdlib/mi/BalConnectorConfig.javanative/src/main/java/io/ballerina/stdlib/mi/executor/BalExecutor.javanative/src/main/java/io/ballerina/stdlib/mi/executor/DataTransformer.javanative/src/main/java/io/ballerina/stdlib/mi/executor/ParamHandler.javatests/src/test/java/io/ballerina/stdlib/mi/executor/DataTransformerTest.javatests/src/test/java/io/ballerina/stdlib/mi/executor/ParamHandlerTest.javatests/src/test/resources/ballerina/project5/Dependencies.tomltests/src/test/resources/ballerina/project7/Dependencies.tomltests/src/test/resources/expected/project4/uischema/project4_Client.jsontests/src/test/resources/expected/project7/functions/component.xmltests/src/test/resources/expected/project7/functions/postusersmessages_Importbyuserid.xmltests/src/test/resources/expected/project7/uischema/postusersmessages_Importbyuserid.jsontests/src/test/resources/expected/typedescProject/functions/processTypedescAnydata.xmltests/src/test/resources/expected/typedescProject/functions/processTypedescRecord.xmltests/src/test/resources/expected/typedescProject/functions/processTypedescString.xmltests/src/test/resources/expected/typedescProject/functions/processTypedescUnion.xmltool-migen-cli/src/main/java/io/ballerina/mi/generator/JsonGenerator.javatool-migen-cli/src/main/java/io/ballerina/mi/generator/XmlPropertyWriter.javatool-migen-cli/src/main/java/io/ballerina/mi/util/Utils.javatool-migen-cli/src/main/resources/balConnector/functions/functions_template.xml
thuva9872
approved these changes
Apr 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This PR addresses critical runtime issues where Ballerina function invocations from WSO2 Micro Integrator (MI) would fail due to
InherentTypeViolation. This happens because generic data structures (maps/arrays) coming from the MI runtime were not being correctly cast into the specific inherent types required by the Ballerina runtime. Additionally, it improves the resolution of complex Union types and handling of Intersection types.Resolves wso2/product-integrator-mi#4828
Goals
Approach
DataTransformerto recursively unwrapIntersectionandTypeReferencetypes to reach the effective underlying type.createTypedRecordFromGeneric,createTypedArrayFromGeneric, andcreateTypedMapFromGenericto explicitly recreate values with the expected BallerinaTypedescriptors.Numbertypes into the exact Ballerina numeric types (int,float,decimal).XmlPropertyWriterto use a more consistentparamN_recordNamenaming convention and support recursive writing of nested record properties.User stories
As an integration developer using the Ballerina MI Gen tool, I want my generated connectors to handle complex data structures (like Unions of records or readonly configurations) seamlessly without encountering runtime type violations when data is passed from the MI sequence into Ballerina.
Release note
Fixed
InherentTypeViolationand Union resolution failures during Ballerina function execution from WSO2 Micro Integrator. This includes improved support for record inherent types, intersection types, and strict numeric type coercion.Documentation
N/A. This is a core runtime reliability fix and does not change the user-facing CLI commands or Ballerina syntax.
Training
N/A
Certification
N/A
Marketing
N/A
Automation tests
DataTransformerTest.java(refactored 270+ lines) to replace reference equality checks with value equality and added 30+ new test cases for complex type conversion scenarios.mi-testssuite, including new scenarios for Union types inTestUnionDataType.java.Security checks
Samples
Sample projects in
tests/src/test/resources/ballerina/such asunionProjectandrecordProjectdemonstrate the fixed behaviors.Related PRs
N/A
Migrations (if applicable)
N/A
Test environment
Learning
N/A