Address review suggestions

ballerina-platform · Jul 9, 2024 · efe4da6 · efe4da6
1 parent 6109556
commit efe4da6
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 28 deletions.
diff --git a/ballerina/tests/fromXml_test.bal b/ballerina/tests/fromXml_test.bal
@@ -2812,7 +2812,7 @@ function testProjectionWithXmlAttributeForParseAsType() returns error? {
     test:assertEquals(rec.A, "2");
 }
 
-type RecType record {
+type RecType1 record {
     string name;
     @Name {
         value: "name"
@@ -2821,25 +2821,44 @@ type RecType record {
     string duplicateName;
 };
 
+type RecType2 record {
+    record {|
+        string \#content;
+    |} name;
+    @Name {
+        value: "name"
+    }
+    @Attribute
+    string duplicateName;
+};
+
 @test:Config
 isolated function testElementAndAttributeInSameScopeHaveSameName() returns error? {
     string xmlStr = string `
     <Data name="Kevin">
         <name>Kanth</name>
     </Data>
     `; 
-    RecType rec = check parseString(xmlStr);
-    test:assertEquals(rec.name, "Kanth");
-    test:assertEquals(rec.duplicateName, "Kevin");
+    RecType1 rec11 = check parseString(xmlStr);
+    test:assertEquals(rec11.name, "Kanth");
+    test:assertEquals(rec11.duplicateName, "Kevin");
+
+    RecType2 rec12 = check parseString(xmlStr);
+    test:assertEquals(rec12.name.\#content, "Kanth");
+    test:assertEquals(rec12.duplicateName, "Kevin");  
 
     xml xmlVal = xml `
     <Data name="Kevin">
         <name>Kanth</name>
     </Data>
     `;
-    RecType rec2 = check parseAsType(xmlVal);
-    test:assertEquals(rec2.name, "Kanth");
-    test:assertEquals(rec2.duplicateName, "Kevin");
+    RecType1 rec21 = check parseAsType(xmlVal);
+    test:assertEquals(rec21.name, "Kanth");
+    test:assertEquals(rec21.duplicateName, "Kevin");
+
+    RecType2 rec22 = check parseAsType(xmlVal);
+    test:assertEquals(rec22.name.\#content, "Kanth");
+    test:assertEquals(rec22.duplicateName, "Kevin");  
 }
 
 type RecNs3 record {|

diff --git a/native/src/main/java/io/ballerina/lib/data/xmldata/xml/QualifiedNameMap.java b/native/src/main/java/io/ballerina/lib/data/xmldata/xml/QualifiedNameMap.java
@@ -1,6 +1,6 @@
 package io.ballerina.lib.data.xmldata.xml;
 
-import io.ballerina.lib.data.xmldata.utils.Constants;
+import io.ballerina.lib.data.xmldata.utils.DataUtils;
 
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -32,8 +32,9 @@ public V remove(QualifiedName qName) {
 
             List<QualifiedName> qNames = fields.get(localName);
             for (QualifiedName qualifiedName : fields.get(localName)) {
-                if (isSameNamespace(qualifiedName, qName)
-                        && isSameAttributeFlag(qualifiedName.getAttributeState(), qName.getAttributeState())) {
+                if (DataUtils.isSameNamespace(qualifiedName, qName)
+                        && DataUtils.isSameAttributeFlag(qualifiedName.getAttributeState(),
+                        qName.getAttributeState())) {
                     field = this.members.remove(qualifiedName);
                     qNames.remove(qualifiedName);
                     break;
@@ -57,26 +58,14 @@ public boolean contains(QualifiedName qName) {
             return false;
         }
         for (QualifiedName qualifiedName : stringToQNameMap.get(localName)) {
-            if (isSameNamespace(qualifiedName, qName)
-                    && isSameAttributeFlag(qualifiedName.getAttributeState(), qName.getAttributeState())) {
+            if (DataUtils.isSameNamespace(qualifiedName, qName)
+                    && DataUtils.isSameAttributeFlag(qualifiedName.getAttributeState(), qName.getAttributeState())) {
                 return true;
             }
         }
         return false;
     }
 
-    private boolean isSameNamespace(QualifiedName q1, QualifiedName q2) {
-        String ns1 = q1.getNamespaceURI();
-        String ns2 = q2.getNamespaceURI();
-        return  (ns1.equals(ns2) && q1.getPrefix().equals(q2.getPrefix()))
-                || ns1.equals(Constants.NS_ANNOT_NOT_DEFINED) || ns2.equals(Constants.NS_ANNOT_NOT_DEFINED);
-    }
-
-    private boolean isSameAttributeFlag(QualifiedName.AttributeState flag1, QualifiedName.AttributeState flag2) {
-        return (flag1 == QualifiedName.AttributeState.NOT_DEFINED
-                || flag2 == QualifiedName.AttributeState.NOT_DEFINED) || (flag1.equals(flag2));
-    }
-
     public boolean contains(String localName) {
         return stringToQNameMap.containsKey(localName);
     }
@@ -103,8 +92,8 @@ public V get(QualifiedName qName) {
             return null;
         }
         for (QualifiedName qualifiedName : stringToQNameMap.get(localName)) {
-            if (isSameNamespace(qualifiedName, qName)
-                    && isSameAttributeFlag(qualifiedName.getAttributeState(), qName.getAttributeState())) {
+            if (DataUtils.isSameNamespace(qualifiedName, qName)
+                    && DataUtils.isSameAttributeFlag(qualifiedName.getAttributeState(), qName.getAttributeState())) {
                 return members.get(qualifiedName);
             }
         }
@@ -130,8 +119,9 @@ public QualifiedName getMatchedQualifiedName(QualifiedName elementQName) {
             return null;
         }
         for (QualifiedName qualifiedName : stringToQNameMap.get(localName)) {
-            if (isSameNamespace(qualifiedName, elementQName)
-                    && isSameAttributeFlag(qualifiedName.getAttributeState(), elementQName.getAttributeState())) {
+            if (DataUtils.isSameNamespace(qualifiedName, elementQName)
+                    && DataUtils.isSameAttributeFlag(qualifiedName.getAttributeState(),
+                    elementQName.getAttributeState())) {
                 return qualifiedName;
             }
         }