Skip to content

Commit

Permalink
Merge pull request #16 from prakanth97/issue_6355
Browse files Browse the repository at this point in the history 
Refactor code to properly handle QulifiedName as key in map type and fix issue in converting to open record
  • Loading branch information
@hasithaa
hasithaa committed Jun 6, 2024
2 parents 0e66df4 + b9ba5e3 commit 810132d
Show file tree
Hide file tree
Showing 10 changed files with 397 additions and 115 deletions.
6 changes: 3 additions & 3 deletions ballerina/Ballerina.toml
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
org = "ballerina"
name = "data.xmldata"
version = "0.1.0"
version = "0.1.1"
authors = ["Ballerina"]
keywords = ["xml"]
repository = "https://github.com/ballerina-platform/module-ballerina-data-xmldata"
Expand All @@ -12,5 +12,5 @@ export = ["data.xmldata"]
[[platform.java17.dependency]]
groupId = "io.ballerina.lib"
artifactId = "data-native"
version = "0.1.0"
path = "../native/build/libs/data.xmldata-native-0.1.0.jar"
version = "0.1.1"
path = "../native/build/libs/data.xmldata-native-0.1.1-SNAPSHOT.jar"
2 changes: 1 addition & 1 deletion ballerina/CompilerPlugin.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ id = "constraint-compiler-plugin"
class = "io.ballerina.lib.data.xmldata.compiler.XmldataCompilerPlugin"

[[dependency]]
path = "../compiler-plugin/build/libs/data.xmldata-compiler-plugin-0.1.0.jar"
path = "../compiler-plugin/build/libs/data.xmldata-compiler-plugin-0.1.1-SNAPSHOT.jar"
2 changes: 1 addition & 1 deletion ballerina/Dependencies.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ distribution-version = "2201.9.0"
[[package]]
org = "ballerina"
name = "data.xmldata"
version = "0.1.0"
version = "0.1.1"
dependencies = [
{org = "ballerina", name = "io"},
{org = "ballerina", name = "jballerina.java"},
Expand Down
96 changes: 96 additions & 0 deletions ballerina/tests/fromXml_test.bal
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3144,3 +3144,99 @@ function testUnsupportedTypeNegative() {
|}|error err3 = parseAsType(xmlVal2);
test:assertEquals((<error>err3).message(), "unsupported input type");
}

@Namespace {
uri: "http://example.com/book"
}
type OpenBook1 record {
@Namespace {
uri: "http://example.com"
}
int id;
@Namespace {
uri: "http://example.com/book"
}
string title;
@Namespace {
uri: "http://example.com/book"
}
string author;
};

@test:Config
function testInvalidNamespaceInOpenRecordForParseString1() {
string xmldata = string `
<book xmlns="http://example.com/book">
<id>601970</id>
<title>string</title>
<author>string</author>
</book>`;
OpenBook1|Error err = parseString(xmldata);
test:assertTrue(err is error);
test:assertEquals((<error>err).message(), "undefined field 'id' in record 'data.xmldata:OpenBook1'");
}

@test:Config
function testInvalidNamespaceInOpenRecordForParseAsType1() {
xml xmldata = xml `
<book xmlns="http://example.com/book">
<id>601970</id>
<title>string</title>
<author>string</author>
</book>`;
OpenBook1|Error err = parseAsType(xmldata);
test:assertTrue(err is error);
test:assertEquals((<error>err).message(), "undefined field 'id' in record 'data.xmldata:OpenBook1'");
}

@Namespace {
uri: "http://example.com/book"
}
type OpenBook2 record {
AuthorOpen author;
@Namespace {
uri: "http://example.com/book"
}
string title;
};

type AuthorOpen record {
@Namespace {
uri: "http://example.com"
}
string name;
@Namespace {
uri: "http://example.com/book"
}
int age;
};

@test:Config
function testInvalidNamespaceInOpenRecordForParseString2() {
string xmldata = string `
<book xmlns="http://example.com/book">
<author>
<name>R.C Martin</name>
<age>60</age>
</author>
<title>Clean Code</title>
</book>`;
OpenBook2|Error err = parseString(xmldata);
test:assertTrue(err is error);
test:assertEquals((<error>err).message(), "undefined field 'name' in record 'data.xmldata:AuthorOpen'");
}

@test:Config
function testInvalidNamespaceInOpenRecordForParseAsType2() {
xml xmldata = xml `
<book xmlns="http://example.com/book">
<author>
<name>R.C Martin</name>
<age>60</age>
</author>
<title>Clean Code</title>
</book>`;
OpenBook2|Error err = parseAsType(xmldata);
test:assertTrue(err is error);
test:assertEquals((<error>err).message(), "undefined field 'name' in record 'data.xmldata:AuthorOpen'");
}
1 change: 1 addition & 0 deletions native/src/main/java/io/ballerina/lib/data/xmldata/utils/Constants.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,5 @@ private Constants() {}
public static final BString TEXT_FIELD_NAME = StringUtils.fromString("textFieldName");
public static final BString ALLOW_DATA_PROJECTION = StringUtils.fromString("allowDataProjection");
public static final String NON_NUMERIC_STRING_REGEX = "[^a-zA-Z\\d\s]";
public static final String NS_ANNOT_NOT_DEFINED = "$$ns_annot_not_defined$$";
}
49 changes: 32 additions & 17 deletions native/src/main/java/io/ballerina/lib/data/xmldata/utils/DataUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import io.ballerina.lib.data.xmldata.FromString;
import io.ballerina.lib.data.xmldata.xml.QualifiedName;
import io.ballerina.lib.data.xmldata.xml.QualifiedNameMap;
import io.ballerina.runtime.api.PredefinedTypes;
import io.ballerina.runtime.api.TypeTags;
import io.ballerina.runtime.api.creators.TypeCreator;
Expand Down Expand Up @@ -89,7 +90,7 @@ public static QualifiedName validateAndGetXmlNameFromRecordAnnotation(RecordType
prefix == null ? "" : prefix.getValue());
}
}
return new QualifiedName(QualifiedName.NS_ANNOT_NOT_DEFINED, localName, "");
return new QualifiedName(Constants.NS_ANNOT_NOT_DEFINED, localName, "");
}

public static void validateTypeNamespace(String prefix, String uri, RecordType recordType) {
Expand Down Expand Up @@ -134,19 +135,33 @@ public static Map<QualifiedName, Field> getAllFieldsInRecordType(RecordType reco
}
}

Map<QualifiedName, Field> fields = new HashMap<>();
Map<QualifiedName, Field> fieldMap = new HashMap<>();
Map<String, List<QualifiedName>> fieldNames = new HashMap<>();
Map<String, Field> recordFields = recordType.getFields();
for (String key : recordFields.keySet()) {
QualifiedName modifiedQName = modifiedNames.getOrDefault(key,
new QualifiedName(QualifiedName.NS_ANNOT_NOT_DEFINED, key, ""));
if (fields.containsKey(modifiedQName)) {
throw DiagnosticLog.error(DiagnosticErrorCode.DUPLICATE_FIELD, modifiedQName.getLocalPart());
} else if (analyzerData.attributeHierarchy.peek().containsKey(modifiedQName)) {
continue;
QualifiedName modifiedQName =
modifiedNames.getOrDefault(key, new QualifiedName(Constants.NS_ANNOT_NOT_DEFINED, key, ""));
String localName = modifiedQName.getLocalPart();
if (fieldMap.containsKey(modifiedQName)) {
throw DiagnosticLog.error(DiagnosticErrorCode.DUPLICATE_FIELD, localName);
} else if (fieldNames.containsKey(localName)) {
if (modifiedQName.getNamespaceURI().equals(Constants.NS_ANNOT_NOT_DEFINED)) {
throw DiagnosticLog.error(DiagnosticErrorCode.DUPLICATE_FIELD, localName);
}
List<QualifiedName> qNames = fieldNames.get(localName);
qNames.forEach(qName -> {
if (qName.getNamespaceURI().equals(Constants.NS_ANNOT_NOT_DEFINED)) {
throw DiagnosticLog.error(DiagnosticErrorCode.DUPLICATE_FIELD, localName);
}
});
fieldMap.put(modifiedQName, recordFields.get(key));
fieldNames.get(localName).add(modifiedQName);
} else if (!analyzerData.attributeHierarchy.peek().contains(modifiedQName)) {
fieldMap.put(modifiedQName, recordFields.get(key));
fieldNames.put(localName, new ArrayList<>(List.of(modifiedQName)));
}
fields.put(modifiedQName, recordFields.get(key));
}
return fields;
return fieldMap;
}

@SuppressWarnings("unchecked")
Expand Down Expand Up @@ -177,7 +192,7 @@ public static QualifiedName getFieldNameFromRecord(Map<BString, Object> fieldAnn
prefix == null ? "" : prefix.getValue());
}
}
return new QualifiedName(QualifiedName.NS_ANNOT_NOT_DEFINED, fieldName, "");
return new QualifiedName(Constants.NS_ANNOT_NOT_DEFINED, fieldName, "");
}

@SuppressWarnings("unchecked")
Expand Down Expand Up @@ -216,7 +231,7 @@ public static Object convertStringToExpType(BString value, Type expType) {
}

public static void validateRequiredFields(BMap<BString, Object> currentMapValue, XmlAnalyzerData analyzerData) {
Map<QualifiedName, Field> fields = analyzerData.fieldHierarchy.peek();
Map<QualifiedName, Field> fields = analyzerData.fieldHierarchy.peek().getMembers();
for (QualifiedName key : fields.keySet()) {
// Validate required array size
Field field = fields.get(key);
Expand All @@ -236,7 +251,7 @@ public static void validateRequiredFields(BMap<BString, Object> currentMapValue,
}
}

Map<QualifiedName, Field> attributes = analyzerData.attributeHierarchy.peek();
Map<QualifiedName, Field> attributes = analyzerData.attributeHierarchy.peek().getMembers();
for (QualifiedName key : attributes.keySet()) {
Field field = attributes.get(key);
String fieldName = field.getFieldName();
Expand Down Expand Up @@ -289,8 +304,8 @@ public static MapType getMapTypeFromConstraintType(Type constraintType) {
}

public static void updateExpectedTypeStacks(RecordType recordType, XmlAnalyzerData analyzerData) {
analyzerData.attributeHierarchy.push(new HashMap<>(getAllAttributesInRecordType(recordType)));
analyzerData.fieldHierarchy.push(new HashMap<>(getAllFieldsInRecordType(recordType, analyzerData)));
analyzerData.attributeHierarchy.push(new QualifiedNameMap(getAllAttributesInRecordType(recordType)));
analyzerData.fieldHierarchy.push(new QualifiedNameMap(getAllFieldsInRecordType(recordType, analyzerData)));
analyzerData.restTypes.push(recordType.getRestFieldType());
}

Expand Down Expand Up @@ -774,8 +789,8 @@ private static String addAttributeToRecord(BString prefix, BString uri, String k
*/
public static class XmlAnalyzerData {
public final Stack<Object> nodesStack = new Stack<>();
public final Stack<Map<QualifiedName, Field>> fieldHierarchy = new Stack<>();
public final Stack<Map<QualifiedName, Field>> attributeHierarchy = new Stack<>();
public final Stack<QualifiedNameMap<Field>> fieldHierarchy = new Stack<>();
public final Stack<QualifiedNameMap<Field>> attributeHierarchy = new Stack<>();
public final Stack<Type> restTypes = new Stack<>();
public RecordType rootRecord;
public Field currentField;
Expand Down
6 changes: 1 addition & 5 deletions native/src/main/java/io/ballerina/lib/data/xmldata/xml/QualifiedName.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
* @since 0.1.0
*/
public class QualifiedName {
public static final String NS_ANNOT_NOT_DEFINED = "$$ns_annot_not_defined$$";
private String localPart;
private String namespaceURI;
private String prefix;
Expand Down Expand Up @@ -59,7 +58,7 @@ public String getPrefix() {

@Override
public int hashCode() {
return localPart.hashCode();
return prefix.hashCode() ^ namespaceURI.hashCode() ^ localPart.hashCode();
}

@Override
Expand All @@ -72,9 +71,6 @@ public boolean equals(Object objectToTest) {
return false;
}

if (qName.namespaceURI.equals(NS_ANNOT_NOT_DEFINED) || namespaceURI.equals(NS_ANNOT_NOT_DEFINED)) {
return localPart.equals(qName.localPart);
}
return localPart.equals(qName.localPart) && namespaceURI.equals(qName.namespaceURI) &&
prefix.equals(qName.prefix);
}
Expand Down
Loading

0 comments on commit 810132d

Please sign in to comment.