Skip to content
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

Jump to bottom

Improve JSON to Record Conversion #43482

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Improve JSON to Record Conversion #43482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
599b739
Remove java tuples dependency
Shadow-Devil Oct 10, 2024
2c7aa57
Convert nulls to json
Shadow-Devil Oct 10, 2024
e2ca2c1
Implement closed record with json rest fields for json-mapper
Shadow-Devil Oct 10, 2024
2356c62
Refactor JsonToRecordMapperTests
Shadow-Devil Oct 10, 2024
100e4cb
Implement closed record with json rest fields for json-to-record-conv…
Shadow-Devil Oct 10, 2024
f0da83e
Refactor JsonToRecordConverter
Shadow-Devil Oct 10, 2024
ed399d0
Refactor JsonToRecordMapperTests
Shadow-Devil Oct 11, 2024
fcf7a11
Fix long line
Shadow-Devil Oct 13, 2024
5bbeff1
Fix checkstyle issue in JsonToRecordMapperTests.java
gimantha Oct 14, 2024
f745593
Fix spotbugs
Shadow-Devil Oct 14, 2024
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
2 changes: 0 additions & 2 deletions gradle/libs.versions.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ jaegerThriftVersion="0.31.0"
jakartaActivationVersion="1.2.2"
javaDiffUtilsVersion="4.5"
javassistVersion="3.24.1-GA"
javaTuplesVersion="1.2"
javaxMailVersion="1.6.2"
javaxTransactionApiVersion="1.3"
javaxWsRsApiVersion="2.1.1"
Expand Down Expand Up @@ -197,7 +196,6 @@ jaeger-core = { module = "io.jaegertracing:jaeger-core", version.ref = "jaegerCo
jaeger-thrift = { module = "io.jaegertracing:jaeger-thrift", version.ref = "jaegerThriftVersion"}
jakarta-activation = { module = "jakarta.activation:jakarta.activation-api", version.ref = "jakartaActivationVersion"}
java-diff-utils = { module = "io.github.java-diff-utils:java-diff-utils", version.ref = "javaDiffUtilsVersion"}
java-tuples = { module = "org.javatuples:javatuples", version.ref = "javaTuplesVersion"}
javassist = { module = "org.javassist:javassist", version.ref = "javassistVersion"}
javax-mail = { module = "com.sun.mail:javax.mail", version.ref = "javaxMailVersion"}
javax-transaction-api = { module = "javax.transaction:javax.transaction-api", version.ref = "javaxTransactionApiVersion"}
Expand Down
2 changes: 0 additions & 2 deletions misc/json-to-record-converter/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,11 @@ dependencies {
implementation project(':identifier-util')
implementation libs.gson
implementation libs.apache.commons.lang3
implementation libs.java.tuples

testImplementation libs.testng
testImplementation project(':ballerina-lang')
testImplementation project(':formatter:formatter-core')
testImplementation project(':language-server:language-server-commons')
testImplementation libs.java.tuples
}

test {
Expand Down
57 changes: 30 additions & 27 deletions misc/json-to-record-converter/src/main/java/io/ballerina/jsonmapper/JsonToRecordMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import io.ballerina.compiler.syntax.tree.OptionalTypeDescriptorNode;
import io.ballerina.compiler.syntax.tree.ParenthesisedTypeDescriptorNode;
import io.ballerina.compiler.syntax.tree.RecordFieldNode;
import io.ballerina.compiler.syntax.tree.RecordRestDescriptorNode;
import io.ballerina.compiler.syntax.tree.RecordTypeDescriptorNode;
import io.ballerina.compiler.syntax.tree.SimpleNameReferenceNode;
import io.ballerina.compiler.syntax.tree.SyntaxKind;
Expand All @@ -52,7 +53,6 @@
import org.ballerinalang.formatter.core.options.ForceFormattingOptions;
import org.ballerinalang.formatter.core.options.FormattingOptions;
import org.ballerinalang.langserver.commons.workspace.WorkspaceManager;
import org.javatuples.Pair;

import java.util.AbstractMap;
import java.util.ArrayList;
Expand Down Expand Up @@ -237,8 +237,7 @@ private static void generateRecords(JsonObject jsonObject, String recordName, bo
List<DiagnosticMessage> diagnosticMessages,
boolean isNullAsOptional) {
Token recordKeyWord = AbstractNodeFactory.createToken(SyntaxKind.RECORD_KEYWORD);
Token bodyStartDelimiter = AbstractNodeFactory.createToken(isClosed ? SyntaxKind.OPEN_BRACE_PIPE_TOKEN :
SyntaxKind.OPEN_BRACE_TOKEN);
Token bodyStartDelimiter = AbstractNodeFactory.createToken(SyntaxKind.OPEN_BRACE_PIPE_TOKEN);

List<Node> recordFields = new ArrayList<>();
if (recordToTypeDescNodes.containsKey(recordName)) {
Expand Down Expand Up @@ -270,21 +269,25 @@ private static void generateRecords(JsonObject jsonObject, String recordName, bo
isNullAsOptional);
}
}

NodeList<Node> fieldNodes = AbstractNodeFactory.createNodeList(recordFields);
Token bodyEndDelimiter = AbstractNodeFactory.createToken(isClosed ? SyntaxKind.CLOSE_BRACE_PIPE_TOKEN :
SyntaxKind.CLOSE_BRACE_TOKEN);
Token bodyEndDelimiter = AbstractNodeFactory.createToken(SyntaxKind.CLOSE_BRACE_PIPE_TOKEN);
RecordRestDescriptorNode restDescriptorNode = isClosed ? null :
NodeFactory.createRecordRestDescriptorNode(
NodeFactory.createBuiltinSimpleNameReferenceNode(SyntaxKind.JSON_KEYWORD,
AbstractNodeFactory.createToken(SyntaxKind.JSON_KEYWORD)),
AbstractNodeFactory.createToken(SyntaxKind.ELLIPSIS_TOKEN),
AbstractNodeFactory.createToken(SyntaxKind.SEMICOLON_TOKEN));
RecordTypeDescriptorNode recordTypeDescriptorNode =
NodeFactory.createRecordTypeDescriptorNode(recordKeyWord, bodyStartDelimiter,
fieldNodes, null, bodyEndDelimiter);
fieldNodes, restDescriptorNode, bodyEndDelimiter);

if (moveBefore == null || moveBefore.equals(recordName)) {
recordToTypeDescNodes.put(recordName, recordTypeDescriptorNode);
} else {
List<Map.Entry<String, NonTerminalNode>> typeDescNodes = new ArrayList<>(recordToTypeDescNodes.entrySet());
List<String> recordNames = typeDescNodes.stream().map(Map.Entry::getKey).toList();
Map.Entry<String, NonTerminalNode> mapEntry =
new AbstractMap.SimpleEntry<>(recordName, recordTypeDescriptorNode);
typeDescNodes.add(recordNames.indexOf(moveBefore), mapEntry);
typeDescNodes.add(recordNames.indexOf(moveBefore), Map.entry(recordName, recordTypeDescriptorNode));
recordToTypeDescNodes.clear();
typeDescNodes.forEach(node -> recordToTypeDescNodes.put(node.getKey(), node.getValue()));
}
Expand Down Expand Up @@ -376,23 +379,24 @@ private static void updateRecordFields(JsonObject jsonObject, Map<String, JsonEl
Map<String, RecordFieldNode> previousRecordFieldToNodes,
Map<String, RecordFieldNode> newRecordFieldToNodes,
boolean isNullAsOptional) {
Map<String, Pair<RecordFieldNode, RecordFieldNode>> intersectingRecordFields =
Map<String, Map.Entry<RecordFieldNode, RecordFieldNode>> intersectingRecordFields =
intersection(previousRecordFieldToNodes, newRecordFieldToNodes);
Map<String, RecordFieldNode> differencingRecordFields =
difference(previousRecordFieldToNodes, newRecordFieldToNodes);

for (Map.Entry<String, Pair<RecordFieldNode, RecordFieldNode>> entry : intersectingRecordFields.entrySet()) {
boolean isOptional = entry.getValue().getValue0().questionMarkToken().isPresent();
for (Map.Entry<String, Map.Entry<RecordFieldNode, RecordFieldNode>> entry :
intersectingRecordFields.entrySet()) {
boolean isOptional = entry.getValue().getKey().questionMarkToken().isPresent();
Map<String, String> jsonEscapedFieldToFields = jsonNodes.entrySet().stream()
.collect(Collectors.toMap(jsonEntry -> escapeIdentifier(jsonEntry.getKey()), Map.Entry::getKey));
Map.Entry<String, JsonElement> jsonEntry = new AbstractMap.SimpleEntry<>(jsonEscapedFieldToFields
.get(entry.getKey()), jsonNodes.get(jsonEscapedFieldToFields.get(entry.getKey())));
if (!entry.getValue().getValue0().typeName().toSourceCode()
.equals(entry.getValue().getValue1().typeName().toSourceCode())) {
TypeDescriptorNode node1 = (TypeDescriptorNode) entry.getValue().getValue0().typeName();
TypeDescriptorNode node2 = (TypeDescriptorNode) entry.getValue().getValue1().typeName();
if (!entry.getValue().getKey().typeName().toSourceCode()
.equals(entry.getValue().getValue().typeName().toSourceCode())) {
TypeDescriptorNode node1 = (TypeDescriptorNode) entry.getValue().getKey().typeName();
TypeDescriptorNode node2 = (TypeDescriptorNode) entry.getValue().getValue().typeName();

TypeDescriptorNode nonAnyDataNode = null;
TypeDescriptorNode nonJSONDataNode = null;
Copy link
Member

Choose a reason for hiding this comment

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

Shall we follow the standard camelCase?

Suggested change
TypeDescriptorNode nonJSONDataNode = null;
TypeDescriptorNode nonJsonDataNode = null;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I will fix that soon

IdentifierToken optionalFieldName = null;
boolean alreadyOptionalTypeDesc = false;

Expand All @@ -404,15 +408,15 @@ private static void updateRecordFields(JsonObject jsonObject, Map<String, JsonEl
OptionalTypeDescriptorNode optionalTypeDescNode = (OptionalTypeDescriptorNode) node2;
node2 = (TypeDescriptorNode) optionalTypeDescNode.typeDescriptor();
alreadyOptionalTypeDesc = true;
} else if ((node1.kind().equals(SyntaxKind.ANYDATA_KEYWORD) ||
node2.kind().equals(SyntaxKind.ANYDATA_KEYWORD))) {
} else if ((node1.kind().equals(SyntaxKind.JSON_KEYWORD) ||
node2.kind().equals(SyntaxKind.JSON_KEYWORD))) {
if (isNullAsOptional) {
nonAnyDataNode = NodeParser.parseTypeDescriptor(node1.kind().equals(SyntaxKind.ANYDATA_KEYWORD)
nonJSONDataNode = NodeParser.parseTypeDescriptor(node1.kind().equals(SyntaxKind.JSON_KEYWORD)
? node2.toSourceCode() : node1.toSourceCode());
optionalFieldName = AbstractNodeFactory.createIdentifierToken(entry.getKey() +
SyntaxKind.QUESTION_MARK_TOKEN.stringValue());
} else {
nonAnyDataNode = NodeParser.parseTypeDescriptor(node1.kind().equals(SyntaxKind.ANYDATA_KEYWORD)
nonJSONDataNode = NodeParser.parseTypeDescriptor(node1.kind().equals(SyntaxKind.JSON_KEYWORD)
? node2.toSourceCode() + SyntaxKind.QUESTION_MARK_TOKEN.stringValue() :
node1.toSourceCode() + SyntaxKind.QUESTION_MARK_TOKEN.stringValue());
}
Expand All @@ -426,7 +430,7 @@ private static void updateRecordFields(JsonObject jsonObject, Map<String, JsonEl
RecordFieldNode recordField =
(RecordFieldNode) getRecordField(jsonEntry, existingFieldNames, updatedFieldNames, isOptional);
recordField = recordField.modify()
.withTypeName(nonAnyDataNode == null ? unionTypeDescNode : nonAnyDataNode)
.withTypeName(nonJSONDataNode == null ? unionTypeDescNode : nonJSONDataNode)
.withFieldName(optionalFieldName == null ? recordField.fieldName() : optionalFieldName)
.apply();
recordFields.add(recordField);
Expand Down Expand Up @@ -471,7 +475,7 @@ private static void updateRecordFields(JsonObject jsonObject, Map<String, JsonEl
private static Node getRecordField(Map.Entry<String, JsonElement> entry, List<String> existingFieldNames,
Map<String, String> updatedFieldNames,
boolean isOptionalField) {
Token typeName = AbstractNodeFactory.createToken(SyntaxKind.ANYDATA_KEYWORD);
Token typeName = AbstractNodeFactory.createToken(SyntaxKind.JSON_KEYWORD);
Token questionMarkToken = AbstractNodeFactory.createToken(SyntaxKind.QUESTION_MARK_TOKEN);
TypeDescriptorNode fieldTypeName = NodeFactory.createBuiltinSimpleNameReferenceNode(typeName.kind(), typeName);
IdentifierToken fieldName = AbstractNodeFactory.createIdentifierToken(escapeIdentifier(entry.getKey().trim()));
Expand All @@ -497,8 +501,7 @@ private static Node getRecordField(Map.Entry<String, JsonElement> entry, List<St
fieldTypeName, fieldName,
optionalFieldToken, semicolonToken);
} else if (entry.getValue().isJsonArray()) {
Map.Entry<String, JsonArray> jsonArrayEntry =
new AbstractMap.SimpleEntry<>(entry.getKey(), entry.getValue().getAsJsonArray());
Map.Entry<String, JsonArray> jsonArrayEntry = Map.entry(entry.getKey(), entry.getValue().getAsJsonArray());
ArrayTypeDescriptorNode arrayTypeName =
getArrayTypeDescriptorNode(jsonArrayEntry, existingFieldNames, updatedFieldNames);
recordFieldNode = NodeFactory.createRecordFieldNode(null, null,
Expand Down Expand Up @@ -584,7 +587,7 @@ private static ArrayTypeDescriptorNode getArrayTypeDescriptorNode(Map.Entry<Stri
typeDescriptorNodes.add(tempTypeNode);
}
} else if (element.isJsonNull()) {
Token tempTypeName = AbstractNodeFactory.createToken(SyntaxKind.ANYDATA_KEYWORD);
Token tempTypeName = AbstractNodeFactory.createToken(SyntaxKind.JSON_KEYWORD);
TypeDescriptorNode tempTypeNode =
NodeFactory.createBuiltinSimpleNameReferenceNode(tempTypeName.kind(), tempTypeName);
if (!typeDescriptorNodes.stream().map(Node::toSourceCode)
Expand Down Expand Up @@ -634,7 +637,7 @@ private static ArrayTypeDescriptorNode getArrayTypeDescriptorNode(Map.Entry<Stri
private static TypeDescriptorNode createUnionTypeDescriptorNode(List<TypeDescriptorNode> typeNames,
boolean isOptional) {
if (typeNames.isEmpty()) {
Token typeName = AbstractNodeFactory.createToken(SyntaxKind.ANYDATA_KEYWORD);
Token typeName = AbstractNodeFactory.createToken(SyntaxKind.JSON_KEYWORD);
return NodeFactory.createBuiltinSimpleNameReferenceNode(typeName.kind(), typeName);
} else if (typeNames.size() == 1) {
return typeNames.get(0);
Expand Down
25 changes: 11 additions & 14 deletions ...on-to-record-converter/src/main/java/io/ballerina/jsonmapper/util/ListOperationUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@

package io.ballerina.jsonmapper.util;

import org.javatuples.Pair;

import java.util.LinkedHashMap;
import java.util.Map;

Expand All @@ -39,12 +37,11 @@ private ListOperationUtils() {}
* @param mapTwo Second set of Key, Value pairs to be compared with other
* @return {@link Map} Intersection of first and second set of Key Value pairs
*/
public static <K, V> Map<K, Pair<V, V>> intersection(Map<K, V> mapOne, Map<K, V> mapTwo) {
Map<K, Pair<V, V>> intersection = new LinkedHashMap<>();
for (Map.Entry<K, V> key: mapOne.entrySet()) {
if (mapTwo.containsKey(key.getKey())) {
Pair<V, V> valuePair = new Pair<>(mapOne.get(key.getKey()), mapTwo.get(key.getKey()));
intersection.put(key.getKey(), valuePair);
public static <K, V> Map<K, Map.Entry<V, V>> intersection(Map<K, V> mapOne, Map<K, V> mapTwo) {
Map<K, Map.Entry<V, V>> intersection = new LinkedHashMap<>();
for (Map.Entry<K, V> entry: mapOne.entrySet()) {
if (mapTwo.containsKey(entry.getKey())) {
intersection.put(entry.getKey(), Map.entry(entry.getValue(), mapTwo.get(entry.getKey())));
}
}
return intersection;
Expand All @@ -59,9 +56,9 @@ public static <K, V> Map<K, Pair<V, V>> intersection(Map<K, V> mapOne, Map<K, V>
*/
public static <K, V> Map<K, V> union(Map<K, V> mapOne, Map<K, V> mapTwo) {
Map<K, V> union = new LinkedHashMap<>(mapOne);
for (Map.Entry<K, V> key: mapTwo.entrySet()) {
if (!mapOne.containsKey(key.getKey())) {
union.put(key.getKey(), mapTwo.get(key.getKey()));
for (Map.Entry<K, V> entry: mapTwo.entrySet()) {
if (!mapOne.containsKey(entry.getKey())) {
union.put(entry.getKey(), entry.getValue());
}
}
return union;
Expand All @@ -76,9 +73,9 @@ public static <K, V> Map<K, V> union(Map<K, V> mapOne, Map<K, V> mapTwo) {
*/
public static <K, V> Map<K, V> difference(Map<K, V> mapOne, Map<K, V> mapTwo) {
Map<K, V> unionMap = union(mapOne, mapTwo);
Map<K, Pair<V, V>> intersectionMap = intersection(mapOne, mapTwo);
for (Map.Entry<K, Pair<V, V>> key: intersectionMap.entrySet()) {
unionMap.remove(key.getKey());
Map<K, Map.Entry<V, V>> intersectionMap = intersection(mapOne, mapTwo);
for (K key: intersectionMap.keySet()) {
unionMap.remove(key);
}
return unionMap;
}
Expand Down
1 change: 0 additions & 1 deletion misc/json-to-record-converter/src/main/java/module-info.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
requires io.ballerina.language.server.commons;
requires io.ballerina.parser;
requires io.ballerina.tools.api;
requires javatuples;
requires org.apache.commons.lang3;

exports io.ballerina.jsonmapper;
Expand Down
Loading
Loading