Add backend changes to json/xml data conversion in data mapping#719
Merged
KavinduZoysa merged 4 commits intoballerina-platform:mainfrom Feb 19, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds converted-variable tracking and runtime type-conversion support to mapping generation, exposes new convertType LS endpoint and request record, adds convertExpression/convertType APIs, and includes tests and test resources for variable and function conversion scenarios. Changes
Sequence DiagramsequenceDiagram
participant Client
participant LSService as DataMapperService
participant Manager as DataMapManager
participant TypeEngine as TypeConversionEngine
Client->>LSService: convertType(request)
activate LSService
LSService->>LSService: load project & obtain SemanticModel
alt semantic model & file available
LSService->>Manager: convertType(filePath, semanticModel, codedata, typeName, variableName, isInput)
activate Manager
Manager->>Manager: resolve target, find ConvertedVariables
Manager->>TypeEngine: compute conversion expression & edits
activate TypeEngine
TypeEngine-->>Manager: conversion edits (JSON/TextEdits)
deactivate TypeEngine
Manager-->>LSService: DataMapperSourceResponse (edits)
deactivate Manager
else error
LSService->>LSService: catch Throwable and set error in response
end
LSService-->>Client: CompletableFuture<DataMapperSourceResponse>
deactivate LSService
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
flow-model-generator/modules/flow-model-generator-ls-extension/src/main/java/io/ballerina/flowmodelgenerator/extension/DataMapperService.java (1)
577-597: Consider adding Javadoc for consistency.The
convertTypemethod lacks Javadoc documentation, while similar methods likeconvertExpression(lines 549-555) andtransformationFunction(lines 495-502) have documentation. Adding Javadoc would maintain consistency and help API consumers understand the endpoint's purpose.📝 Suggested Javadoc
+ /** + * Converts a variable from one type to another for incompatible types. + * This API generates text edits to wrap or transform the variable with type conversion logic. + * + * `@param` request The request containing file path, code data, target type name, variable name, and direction + * `@return` Response containing the text edits needed to perform the type conversion + * `@since` 2.0.0 + */ `@JsonRequest` public CompletableFuture<DataMapperSourceResponse> convertType(DataMapperConvertTypeRequest request) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flow-model-generator/modules/flow-model-generator-ls-extension/src/main/java/io/ballerina/flowmodelgenerator/extension/DataMapperService.java` around lines 577 - 597, Add a Javadoc block for the DataMapperService.convertType method (signature: public CompletableFuture<DataMapperSourceResponse> convertType(DataMapperConvertTypeRequest request)) mirroring the style and content used for convertExpression and transformationFunction: briefly describe the endpoint's purpose (convert a type into mapping source), document the request parameter (DataMapperConvertTypeRequest) and the returned CompletableFuture<DataMapperSourceResponse), note that errors are captured in the response (response.setError), and include any relevant tags like `@param` and `@return`; place the Javadoc immediately above the convertType method.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java`:
- Around line 3846-3895: The getMatchingLetVar method currently throws
IllegalStateException when no matching let variable is found, causing LSP
failures; change getMatchingLetVar to return null instead of throwing (signature
stays private LetVariableDeclarationNode getMatchingLetVar(...)), and update all
callers (e.g., the call sites inside the big method that process letExpr and
letVarDeclarationNodes) to check for a null return and handle it by skipping
edits for that variable (i.e., do not add TextEdit or call addErrorReturn) or
return an empty edit list for the file; ensure you keep the existing null-safe
logic paths and only add null-checks around uses of the returned
LetVariableDeclarationNode to make the service resilient to stale/renamed
variable names.
- Around line 3897-3921: In addErrorReturn (DataMapManager.addErrorReturn) the
else-branch that handles functions with no existing return-type descriptor
inserts " error?" which is invalid; change the TextEdit payload for that branch
to include the "returns" keyword (i.e., " returns error?") so it matches the
other branches and produces a valid return-type-descriptor; update the TextEdit
created in that else path accordingly.
In
`@flow-model-generator/modules/flow-model-generator-ls-extension/src/test/java/io/ballerina/flowmodelgenerator/extension/DataMappingConvertTypeTest.java`:
- Around line 121-124: The clazz() override in DataMappingConvertTypeTest
wrongly returns DataMappingTypesTest.class; update the method so it returns
DataMappingConvertTypeTest.class (i.e., change the returned Class reference in
the clazz() method to DataMappingConvertTypeTest.class) to ensure the test
framework uses the correct test class.
- Around line 136-149: The Javadoc `@param` tags for the TestConfig record are out
of order compared to the record field declaration (record TestConfig(String
source, String description, JsonElement codedata, String typeName, String
variableName, boolean isInput, Map<String, List<TextEdit>> output)); update the
Javadoc `@param` list so the tag order matches the record field order: source,
description, codedata, typeName, variableName, isInput, output, keeping
descriptions the same.
In
`@flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/data_mapper_convert_type/source/variable1.bal`:
- Line 1: Remove the unused import statement "import ballerina/http;" from the
file (variable1.bal) since the http module is not referenced anywhere; locate
the import line and delete it so the file no longer contains the unused import.
In
`@flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/data_mapper_source/source/function_def3.bal`:
- Around line 11-13: The Student record literal in function transform3 is
missing the required password field and the expression incorrectly returns the
function name; update the Student construction (the variable currently named
transform3_var) to include the required password property (e.g., password:
<value> derived from input) and change the final expression so the function
returns that constructed variable (transform3_var) rather than the function name
transform3, preserving the declared returns json|error and reusing the existing
userConverted from check user.ensureType().
- Around line 15-17: The function transform4 contains invalid Ballerina syntax
and appears unused; either delete the entire function transform4 (including the
let binding using UserInfo and Student and the reference to transform3) or
replace it with syntactically valid code — e.g., a proper let/assignment and
valid expressions for userConverted and transform4_var — and if this is meant to
be a negative test, add a dedicated test case and a clear filename/label
indicating it is a negative test instead of leaving dead/invalid code in the
source.
---
Nitpick comments:
In
`@flow-model-generator/modules/flow-model-generator-ls-extension/src/main/java/io/ballerina/flowmodelgenerator/extension/DataMapperService.java`:
- Around line 577-597: Add a Javadoc block for the DataMapperService.convertType
method (signature: public CompletableFuture<DataMapperSourceResponse>
convertType(DataMapperConvertTypeRequest request)) mirroring the style and
content used for convertExpression and transformationFunction: briefly describe
the endpoint's purpose (convert a type into mapping source), document the
request parameter (DataMapperConvertTypeRequest) and the returned
CompletableFuture<DataMapperSourceResponse), note that errors are captured in
the response (response.setError), and include any relevant tags like `@param` and
`@return`; place the Javadoc immediately above the convertType method.
...-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java
Show resolved
Hide resolved
...-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java
Show resolved
Hide resolved
...sion/src/test/java/io/ballerina/flowmodelgenerator/extension/DataMappingConvertTypeTest.java
Show resolved
Hide resolved
...sion/src/test/java/io/ballerina/flowmodelgenerator/extension/DataMappingConvertTypeTest.java
Show resolved
Hide resolved
...odel-generator-ls-extension/src/test/resources/data_mapper_convert_type/source/variable1.bal
Outdated
Show resolved
Hide resolved
...-model-generator-ls-extension/src/test/resources/data_mapper_source/source/function_def3.bal
Outdated
Show resolved
Hide resolved
...-model-generator-ls-extension/src/test/resources/data_mapper_source/source/function_def3.bal
Outdated
Show resolved
Hide resolved
KavinduZoysa
force-pushed
the
fix-issue-2390
branch
from
ddef9e9 to
0cbf5a3
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java (1)
3771-3782:⚠️ Potential issue | 🟠 MajorString → boolean conversion should parse values, not just check empty string.
The current logic
((%s == "") ? false : true)incorrectly converts the string"false"totrue. Ballerina providesboolean:fromString()in the lang library, which correctly parses string values and handles errors.🔧 Correct conversion using Ballerina's standard function
- case BOOLEAN -> String.format - ("((%s == \"\") ? false : true)", expression); + case BOOLEAN -> String.format( + "(let boolean|error tmp = boolean:fromString(%s) in (tmp is error ? false : tmp))", + expression);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java` around lines 3771 - 3782, In DataMapManager where sourceKind == TypeDescKind.STRING you must replace the BOOLEAN branch that only checks empty string with logic that calls boolean:fromString and handles errors like the INT/FLOAT/DECIMAL cases; specifically update the switch's BOOLEAN case to create a temp let binding (e.g., let boolean|error tmp = boolean:fromString(expression) in (tmp is error ? false : tmp)) so string values like "false" are parsed correctly and errors default to false.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java`:
- Around line 378-392: The loop over convertedVariables.subMappings() casts
typedBindingPatternNode.bindingPattern() to CaptureBindingPatternNode without
checking its kind, which can throw for destructuring bindings; update the loop
in DataMapManager to first check that typedBindingPatternNode.bindingPattern()
is an instance (or has the correct kind) of CaptureBindingPatternNode before
casting and only then call variableName().text(); if it isn't a capture binding,
skip that ConvertedVariable (or handle appropriately) so getRefMappingPort(...)
is only invoked for true capture bindings.
In
`@flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/data_mapper_convert_type/source/variable1.bal`:
- Around line 19-25: Two duplicate function definitions for transform2 (both
signature "function transform2(json user, json student) returns json|error")
will not compile; keep only one implementation or rename one of them (e.g.,
transform2 and transform2Alternate). Locate the two transform2 functions in the
file and either remove the unwanted duplicate body or rename the second
occurrence and update any call sites; ensure the retained function correctly
uses the UserInfo userConverted = check user.ensureType() binding and returns a
valid json or error as declared.
- Around line 30-36: Both functions declare non-optional return types but have
empty bodies, causing compilation errors; update transform4 and transform5 to
end with explicit return statements that match their declared types—e.g., in
transform4 (returns json) add a return of a json value such as an empty object
(return {}); in transform5 (returns json|error) add an explicit return that
yields either a json (e.g., return {}) or an error as appropriate for the
function’s logic (ensure all control-flow paths return a value of type
json|error).
---
Outside diff comments:
In
`@flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java`:
- Around line 3771-3782: In DataMapManager where sourceKind ==
TypeDescKind.STRING you must replace the BOOLEAN branch that only checks empty
string with logic that calls boolean:fromString and handles errors like the
INT/FLOAT/DECIMAL cases; specifically update the switch's BOOLEAN case to create
a temp let binding (e.g., let boolean|error tmp = boolean:fromString(expression)
in (tmp is error ? false : tmp)) so string values like "false" are parsed
correctly and errors default to false.
---
Duplicate comments:
In
`@flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java`:
- Around line 3816-3895: getMatchingLetVar currently throws
IllegalStateException when no matching let variable is found, which can crash
convertType; change getMatchingLetVar to return null instead of throwing, and
update convertType to check the returned LetVariableDeclarationNode (the letVar
from calls inside both the !isInput and isInput branches) and if null perform a
graceful no-op by returning gson.toJsonTree(textEditsMap) (i.e., leave
textEditsMap empty) before attempting to use letVar; ensure both call sites that
assign LetVariableDeclarationNode letVar = getMatchingLetVar(...) handle the
null case.
...-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java
Show resolved
Hide resolved
...odel-generator-ls-extension/src/test/resources/data_mapper_convert_type/source/variable1.bal
Show resolved
Hide resolved
...odel-generator-ls-extension/src/test/resources/data_mapper_convert_type/source/variable1.bal
Show resolved
Hide resolved
KavinduZoysa
force-pushed
the
fix-issue-2390
branch
from
0cbf5a3 to
3bbf8e2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/data_mapper_convert_type/source/variable1.bal (1)
26-28: Variable shadows function name.The let binding
Student transform3 = {}creates a variable that shadows the enclosing function name. While syntactically valid, this could be confusing. If intentional for testing purposes, this is fine; otherwise, consider renaming the variable for clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/data_mapper_convert_type/source/variable1.bal` around lines 26 - 28, The let binding inside the function transform3 declares a variable named transform3 (Student transform3 = {}), which shadows the enclosing function name; rename that local variable to a non-conflicting identifier (for example studentVar, studentConverted, or studentObj) in the let binding within function transform3 so the variable no longer shares the function's name and avoids shadowing while preserving the same initialization semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java`:
- Around line 1299-1310: The category string in getRefOutputMappingPort
currently sets mappingPort.category = "converted_variable" which mismatches the
input-port naming; change it to "converted-variable" to match the input path
naming convention and ensure UI filtering/styling works; check getRefMappingPort
and any other places that set category for converted ports and make them use the
same "converted-variable" hyphenated value for consistency.
---
Duplicate comments:
In
`@flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java`:
- Around line 3883-3895: getMatchingLetVar currently throws
IllegalStateException when no matching LetVariableDeclarationNode is found;
change it to return null instead of throwing and update all call sites within
DataMapManager that invoke getMatchingLetVar to handle a null result (e.g., skip
processing, log a warning, or take the graceful fallback) so stale/renamed
variable names don't crash LSP requests. Keep the existing matching logic
(checking typedBindingPattern().bindingPattern() and comparing
((CaptureBindingPatternNode) ...).variableName().text()), just replace the throw
with return null and add null-checks around any usage of the returned
LetVariableDeclarationNode to handle the absent-case safely.
In
`@flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/data_mapper_convert_type/source/variable1.bal`:
- Around line 19-24: There are two conflicting function definitions named
transform2 (same parameter list) which causes a duplicate-definition compile
error; fix it by removing or renaming one of the transform2 functions (or
consolidating their logic into a single transform2) so only one function with
that name/signature remains, and ensure any references to UserInfo userConverted
and its initialization (the check user.ensureType() expression and the returned
json body) are preserved or merged appropriately.
- Line 1: Remove the unused import "import ballerina/http as _;" from the file;
locate the import statement (the line containing the aliased import of
ballerina/http) and delete it so the file no longer contains the unnecessary
underscore-aliased import.
---
Nitpick comments:
In
`@flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/data_mapper_convert_type/source/variable1.bal`:
- Around line 26-28: The let binding inside the function transform3 declares a
variable named transform3 (Student transform3 = {}), which shadows the enclosing
function name; rename that local variable to a non-conflicting identifier (for
example studentVar, studentConverted, or studentObj) in the let binding within
function transform3 so the variable no longer shares the function's name and
avoids shadowing while preserving the same initialization semantics.
...-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java
Outdated
Show resolved
Hide resolved
KavinduZoysa
force-pushed
the
fix-issue-2390
branch
from
3bbf8e2 to
f6a4c53
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java`:
- Around line 238-263: The setModuleInfo call is using targetNode.typeSymbol()
even when convertedVariables.output() exists, which causes module info to be
taken from the converted let-var type rather than the parent (the actual
refOutputPort type); change the logic so that if convertedVariables != null &&
convertedVariables.output() != null you pass
convertedVariables.output().parentType() (or its raw/appropriate TypeSymbol)
into setModuleInfo instead of targetNode.typeSymbol(), otherwise keep using
targetNode.typeSymbol(); update references around refOutputPort,
convertedVariables, and setModuleInfo accordingly and keep the existing
null/safety handling.
In
`@flow-model-generator/modules/flow-model-generator-ls-extension/src/test/java/io/ballerina/flowmodelgenerator/extension/DataMappingConvertTypeTest.java`:
- Around line 72-74: In DataMappingConvertTypeTest, the BufferedReader returned
by Files.newBufferedReader(configJsonPath) is never closed which can leak file
descriptors; wrap the reader in a try-with-resources (or otherwise ensure it is
closed) when calling gson.fromJson so that the BufferedReader created for
configJsonPath is closed after parsing TestConfig.
---
Duplicate comments:
In
`@flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java`:
- Around line 3883-3895: The method getMatchingLetVar currently throws
IllegalStateException when no matching LetVariableDeclarationNode is found;
change it to return null instead of throwing and update all callers (e.g.,
convertType where letVar is used) to null-check the result and skip applying
edits when letVar == null; specifically adjust getMatchingLetVar
signature/return behavior and modify call sites that assume a non-null
LetVariableDeclarationNode (references: getMatchingLetVar,
LetVariableDeclarationNode, BindingPatternNode, convertType) so stale/renamed
variable names no longer cause LSP failures.
In
`@flow-model-generator/modules/flow-model-generator-ls-extension/src/test/resources/data_mapper_source/source/function_def3.bal`:
- Around line 11-13: The Student record literal returned in function transform3
lacks the required password field; update the construction of the Student value
(the record assigned to variable transform3 in function transform3) to include a
valid password property (e.g., derive from userConverted or provide a safe
default or propagate an error) so the Student type requirement is satisfied;
ensure the record literal includes both username and password fields consistent
with the Student type used by transform3.
...-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java
Outdated
Show resolved
Hide resolved
...sion/src/test/java/io/ballerina/flowmodelgenerator/extension/DataMappingConvertTypeTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java (1)
3814-3815:⚠️ Potential issue | 🟡 MinorString-to-boolean conversion may produce unexpected results.
The current logic converts any non-empty string to
true, including strings like"false"or"0". This may not match user expectations where"false"should convert tofalse.Consider using a more semantic conversion:
🔧 Suggested alternative
- case BOOLEAN -> String.format - ("((%s == \"\") ? false : true)", expression); + case BOOLEAN -> String.format + ("(let string tmp = %s.toLowerAscii() in (tmp == \"true\" || tmp == \"1\"))", expression);Or document this behavior if it's intentional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java` around lines 3814 - 3815, The BOOLEAN branch in DataMapManager currently emits "((%s == \"\") ? false : true)" which treats any non-empty string (including "false") as true; change the generated expression to perform a semantic parse such as using Boolean.parseBoolean(%s) or an explicit equalsIgnoreCase check (e.g., "%s != null && %s.equalsIgnoreCase(\"true\")") so that strings like "false" map to false; update the BOOLEAN case in DataMapManager accordingly.
🧹 Nitpick comments (1)
flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java (1)
3425-3427: Remove the unusedParentPortrecord.The
ParentPortrecord is declared at line 3425 but is never instantiated or referenced anywhere in the codebase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java` around lines 3425 - 3427, Remove the unused record declaration ParentPort from DataMapManager: locate the private record ParentPort(String typeName, String variable) in the DataMapManager class and delete the entire record definition since it is never instantiated or referenced anywhere in the codebase; run a build or IDE symbol search afterward to confirm no usages remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java`:
- Around line 3814-3815: The BOOLEAN branch in DataMapManager currently emits
"((%s == \"\") ? false : true)" which treats any non-empty string (including
"false") as true; change the generated expression to perform a semantic parse
such as using Boolean.parseBoolean(%s) or an explicit equalsIgnoreCase check
(e.g., "%s != null && %s.equalsIgnoreCase(\"true\")") so that strings like
"false" map to false; update the BOOLEAN case in DataMapManager accordingly.
---
Duplicate comments:
In
`@flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java`:
- Around line 250-263: The refOutputPort is sometimes created for a parent when
convertedVariables.output() is present (via getRefOutputMappingPort) but
setModuleInfo is still using targetNode.typeSymbol() (which may have been
reassigned to the converted let-var type); update the logic before calling
setModuleInfo so that when convertedVariables.output() != null you pass the
parent reference type/symbol (the original parent type used to create
refOutputPort) instead of targetNode.typeSymbol(), i.e., detect the converted
output case and supply the parent type/symbol used for getRefOutputMappingPort
to setModuleInfo for refOutputPort.
- Around line 3916-3928: Change getMatchingLetVar so it returns null when no
matching LetVariableDeclarationNode is found instead of throwing
IllegalStateException (update the method signature/behavior around
getMatchingLetVar and the throw at the end). Then update its callers that expect
a LetVariableDeclarationNode (e.g., where letVar is obtained from
getMatchingLetVar when processing let expressions) to check for a null return
and, on null, bail out gracefully by returning gson.toJsonTree(textEditsMap)
(i.e., return an empty edits result) rather than letting an exception propagate.
---
Nitpick comments:
In
`@flow-model-generator/modules/flow-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java`:
- Around line 3425-3427: Remove the unused record declaration ParentPort from
DataMapManager: locate the private record ParentPort(String typeName, String
variable) in the DataMapManager class and delete the entire record definition
since it is never instantiated or referenced anywhere in the codebase; run a
build or IDE symbol search afterward to confirm no usages remain.
354c309 to
2787b73
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request adds backend support for automatic type conversion in the data mapping subsystem. The changes enable the system to detect, track, and automatically convert variables between incompatible types (e.g., json to specific record types, xml to records) by generating appropriate conversion expressions and propagating converted variable metadata throughout the mapping pipeline.
Changes:
- Introduces converted-variable tracking infrastructure with new
ConvertedVariableandConvertedVariablesrecord types that identify and manage type conversions in LET expressions - Extends
DataMapManagerwith conversion detection logic and addsconvertType()method to generate text edits for type conversions, including automatic error union handling - Exposes new
convertTypeJSON-RPC endpoint viaDataMapperServicewith correspondingDataMapperConvertTypeRequestmodel to surface conversion functionality to language-server clients
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| testng.xml | Adds DataMappingConvertTypeTest to the test suite configuration |
| function_def3.bal | New test source file with conversion scenarios using UserInfo and Student types |
| function_defn4.json | Test configuration for mapping with converted password field |
| function_def_transformed_type1.bal | Test source demonstrating multiple conversion scenarios with User, Foo, and SecondaryPhones types |
| function_def_transformed_type2.json | Model test configuration for converted xml variable scenario |
| function_def_transformed_type1.json | Model test configuration for multiple input/output conversions |
| variable1.bal | Primary test source file with 7 transform function variations for conversion testing |
| variable9.json through variable1.json | Twelve test configurations covering various conversion scenarios for inputs, outputs, and function body types |
| DataMappingSourceTest.java | Registers function_defn4.json test case |
| DataMappingModelTest.java | Registers two new model test cases for transformed types |
| DataMappingConvertTypeTest.java | New test class implementing comprehensive type conversion test harness with 12 test cases |
| DataMapperConvertTypeRequest.java | New request record defining the API contract for type conversion operations |
| DataMapperService.java | Adds convertType endpoint implementation that delegates to DataMapManager |
| DataMapManager.java | Core implementation: adds ConvertedVariable/ConvertedVariables records, implements conversion detection logic, adds convertType method, extends getMappings and getInputPorts to propagate converted variable metadata, implements helper methods for conversion tracking |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java
Outdated
Show resolved
Hide resolved
...-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java
Outdated
Show resolved
Hide resolved
...odel-generator-ls-extension/src/test/resources/data_mapper_convert_type/source/variable1.bal
Show resolved
Hide resolved
...-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java
Outdated
Show resolved
Hide resolved
...-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java
Outdated
Show resolved
Hide resolved
...-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java
Outdated
Show resolved
Hide resolved
...-model-generator-ls-extension/src/test/resources/data_mapper_source/source/function_def3.bal
Show resolved
Hide resolved
...-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java
Outdated
Show resolved
Hide resolved
...-model-generator-core/src/main/java/io/ballerina/flowmodelgenerator/core/DataMapManager.java
Outdated
Show resolved
Hide resolved
gimantha
approved these changes
Feb 19, 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
Addresses wso2/product-ballerina-integrator#2390
Overview
This pull request adds backend support for JSON/XML type conversion within the data mapping subsystem. It introduces converted-variable tracking and propagation across mapping generation, implements conversion helpers for common primitive types, and exposes conversion utilities via the language-server extension to automate edits needed when mapping between incompatible types.
Key changes
Core mapping enhancements
Conversion logic and helpers
Public API and service
Request model and tests
Impact
Enables automated type conversion support in data mappers, reducing manual edits when mapping between incompatible types and providing programmatic and language-server access to conversion edits. Review focus recommended for the newly exposed public API (convertExpression, convertType) and the duplicate service method entry.