Merge pull request #1313 from daneshk/master

Allow nillable and optional fields to be read as record field
ballerina-platform · Dec 4, 2024 · cb542c5 · cb542c5
2 parents 87908f4 + 058cb5f
commit cb542c5
Show file tree

Hide file tree

Showing 6 changed files with 221 additions and 18 deletions.
diff --git a/ballerina/tests/csv_io.bal b/ballerina/tests/csv_io.bal
@@ -94,6 +94,22 @@ type Employee6 record {
     string age;
 };
 
+type Employee7 record {|
+    string name;
+    string designation;
+    string company;
+    string? age;
+    string? residence;
+|};
+
+type Employee8 record {|
+    string name;
+    string designation;
+    string company;
+    string age?;
+    string residence;
+|};
+
 type RefInt int;
 
 type RefStr string;
@@ -2000,3 +2016,167 @@ function testFileReadCsvRecordWithsingleHeaderLine() returns Error? {
     }
     test:assertEquals(i, 3);
 }
+
+@test:Config {}
+function testFileReadCsvRecordWithEmptyFieldValues() returns Error? {
+    string resourceFilePath1 = TEST_RESOURCE_PATH + "csvResourceFileWithEmptyValues.csv";
+    Employee7[] readContent = check fileReadCsv(resourceFilePath1);
+    Employee7[] content1 = [{
+            name: "Anne Hamiltom",
+            designation: "Software Engineer",
+            company: "Microsoft",
+            age: (),
+            residence: "New York"
+        },
+        {
+            name: "John Thomson",
+            designation: "Software Architect",
+            company: "WSO2",
+            age: "38 years",
+            residence: "Colombo"
+        },
+        {
+            name: "Mary Thompson",
+            designation: "Banker",
+            company: "Sampath Bank",
+            age: "30 years",
+            residence: ()
+        }
+    ];
+    test:assertEquals(readContent.length(), 3);
+    test:assertEquals(readContent, content1);
+}
+
+@test:Config {}
+function testFileReadCsvRecordStreamWithEmptyFieldValues() returns Error? {
+    string resourceFilePath1 = TEST_RESOURCE_PATH + "csvResourceFileWithEmptyValues.csv";
+    stream<Employee7, Error?> readContent = check fileReadCsvAsStream(resourceFilePath1);
+    Employee7[] content1 = [{
+            name: "Anne Hamiltom",
+            designation: "Software Engineer",
+            company: "Microsoft",
+            age: (),
+            residence: "New York"
+        },
+        {
+            name: "John Thomson",
+            designation: "Software Architect",
+            company: "WSO2",
+            age: "38 years",
+            residence: "Colombo"
+        },
+        {
+            name: "Mary Thompson",
+            designation: "Banker",
+            company: "Sampath Bank",
+            age: "30 years",
+            residence: ()
+        }
+    ];
+    int i = 0;
+    check readContent.forEach(function(Employee7 recordVal) {
+        test:assertEquals(recordVal, content1[i]);
+        i = i + 1;
+    });
+    test:assertEquals(i, 3);
+}
+
+@test:Config {}
+function testFileReadCsvRecordWithNillableField() returns Error? {
+    string resourceFilePath1 = TEST_RESOURCE_PATH + "csvResourceFile1.csv";
+    stream<Employee7, Error?> readContent = check fileReadCsvAsStream(resourceFilePath1);
+    Employee7[] content1 = [{
+            name: "Anne Hamiltom",
+            designation: "Software Engineer",
+            company: "Microsoft",
+            age: "26 years",
+            residence: "New York"
+        },
+        {
+            name: "John Thomson",
+            designation: "Software Architect",
+            company: "WSO2",
+            age: "38 years",
+            residence: "Colombo"
+        },
+        {
+            name: "Mary Thompson",
+            designation: "Banker",
+            company: "Sampath Bank",
+            age: "30 years",
+            residence: "Colombo"
+        }
+    ];
+    int i = 0;
+    check readContent.forEach(function(Employee7 recordVal) {
+        test:assertEquals(recordVal, content1[i]);
+        i = i + 1;
+    });
+    test:assertEquals(i, 3);
+}
+
+@test:Config {}
+function testFileReadCsvRecordWithOptionalFields() returns Error? {
+    string resourceFilePath1 = TEST_RESOURCE_PATH + "csvResourceFileWithMissingFields.csv";
+    stream<Employee8, Error?> readContent = check fileReadCsvAsStream(resourceFilePath1);
+    Employee8[] content1 = [{
+            name: "Anne Hamiltom",
+            designation: "Software Engineer",
+            company: "Microsoft",
+            residence: "New York"
+        },
+        {
+            name: "John Thomson",
+            designation: "Software Architect",
+            company: "WSO2",
+            residence: "Colombo"
+        },
+        {
+            name: "Mary Thompson",
+            designation: "Banker",
+            company: "Sampath Bank",
+            residence: "Colombo"
+        }
+    ];
+    int i = 0;
+    check readContent.forEach(function(Employee8 recordVal) {
+        test:assertEquals(recordVal, content1[i]);
+        i = i + 1;
+    });
+    test:assertEquals(i, 3);
+}
+
+@test:Config {}
+function testFileReadCsvRecordWithOptionalFieldsHasValues() returns Error? {
+    string resourceFilePath1 = TEST_RESOURCE_PATH + "csvResourceFile1.csv";
+    stream<Employee8, Error?> readContent = check fileReadCsvAsStream(resourceFilePath1);
+    Employee8[] content1 = [{
+            name: "Anne Hamiltom",
+            designation: "Software Engineer",
+            company: "Microsoft",
+            age: "26 years",
+            residence: "New York"
+        },
+        {
+            name: "John Thomson",
+            designation: "Software Architect",
+            company: "WSO2",
+            age: "38 years",
+            residence: "Colombo"
+        },
+        {
+            name: "Mary Thompson",
+            designation: "Banker",
+            company: "Sampath Bank",
+            age: "30 years",
+            residence: "Colombo"
+        }
+    ];
+    int i = 0;
+    check readContent.forEach(function(Employee8 recordVal) {
+        test:assertEquals(recordVal, content1[i]);
+        i = i + 1;
+    });
+    test:assertEquals(i, 3);
+}
+
diff --git a/ballerina/tests/negative.bal b/ballerina/tests/negative.bal
@@ -139,7 +139,7 @@ isolated function testFileCsvReadWithDefectiveRecords() returns Error? {
     string filePath = TEMP_DIR + "workers2.csv";
     Employee6[]|Error csvContent = fileReadCsv(filePath);
     test:assertTrue(csvContent is Error);
-    test:assertEquals((<Error>csvContent).message(), "The CSV file content header count(5) doesn't match with ballerina record field count(4). ");
+    test:assertEquals((<Error>csvContent).message(), "The csv file contains an additional column - residence.");
 }
 
 @test:Config {}
@@ -239,3 +239,17 @@ function readCsvFileWithUnsupportedMappingType() {
     map<anydata>[]|Error out = fileReadCsv(filePath);
     test:assertEquals((<Error>out).message(), "Only 'string[]' and 'record{}' types are supported, but found 'map' ");
 }
+
+@test:Config {}
+function readCsvWithMissingColumnAndNillableField() {
+    string filePath = TEST_RESOURCE_PATH + "csvResourceFileWithMissingFields.csv";
+    Employee7[]|Error out = fileReadCsv(filePath);
+    test:assertEquals((<Error>out).message(), "The csv file does not contain the column - age.");
+}
+
+@test:Config {}
+function readCsvWithMissingValueAndOptionalField() {
+    string filePath = TEST_RESOURCE_PATH + "csvResourceFileWithEmptyValues.csv";
+    Employee8[]|Error out = fileReadCsv(filePath);
+    test:assertEquals((<Error>out).message(), "Field 'age' does not support nil value.");
+}
diff --git a/ballerina/tests/resources/csvResourceFileWithEmptyValues.csv b/ballerina/tests/resources/csvResourceFileWithEmptyValues.csv
@@ -0,0 +1,4 @@
+name, designation, company, age, residence
+Anne Hamiltom, Software Engineer, Microsoft,, New York
+John Thomson, Software Architect, WSO2, 38 years, Colombo
+Mary Thompson, Banker, Sampath Bank, 30 years,
diff --git a/ballerina/tests/resources/csvResourceFileWithMissingFields.csv b/ballerina/tests/resources/csvResourceFileWithMissingFields.csv
@@ -0,0 +1,4 @@
+name, designation, company, residence
+Anne Hamiltom, Software Engineer, Microsoft, New York
+John Thomson, Software Architect, WSO2, Colombo
+Mary Thompson, Banker, Sampath Bank, Colombo
diff --git a/changelog.md b/changelog.md
@@ -4,6 +4,8 @@ This file contains all the notable changes done to the Ballerina I/O package thr
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 ## [Unreleased]
+### Fixed
+- [The CSV file read as a record failed when a nillable field was empty](https://github.com/ballerina-platform/ballerina-library/issues/7433)
 
 ## [1.6.1] - 2024-08-06
 ### Changed

diff --git a/native/src/main/java/io/ballerina/stdlib/io/nativeimpl/RecordChannelUtils.java b/native/src/main/java/io/ballerina/stdlib/io/nativeimpl/RecordChannelUtils.java
@@ -20,6 +20,8 @@
 
 import io.ballerina.runtime.api.creators.TypeCreator;
 import io.ballerina.runtime.api.creators.ValueCreator;
+import io.ballerina.runtime.api.flags.SymbolFlags;
+import io.ballerina.runtime.api.types.Field;
 import io.ballerina.runtime.api.types.StructureType;
 import io.ballerina.runtime.api.types.Type;
 import io.ballerina.runtime.api.types.TypeTags;
@@ -158,9 +160,6 @@ record = textRecordChannel.read();
                         return returnStruct;
                     }
                     Map<String, Object> struct = (Map<String, Object>) returnStruct;
-                    if (record.length != structType.getFields().size()) {
-                        return IOUtils.createError("Record type and CSV file does not match.");
-                    }
                     outList.add(ValueCreator.createRecordValue(describingType.getPackage(),
                             describingType.getName(), struct));
 
@@ -224,10 +223,6 @@ record = textRecordChannel.getFields(line);
                     return returnStruct;
                 }
                 final Map<String, Object> struct = (Map<String, Object>) returnStruct;
-                if (record.length != structType.getFields().size()) {
-                    bufferedReader.close();
-                    return IOUtils.createError("Record type and CSV file does not match.");
-                }
                 return ValueCreator.createRecordValue(describingType.getPackage(), describingType.getName(),
                         struct);
             }
@@ -321,16 +316,20 @@ public static Object closeStream(BObject iterator) {
     }
 
     private static void validateHeaders(ArrayList<String> headers, StructureType structType) {
-        if (headers.size() != structType.getFields().size()) {
-            throw IOUtils.createError(String.format("The CSV file content header count" +
-                            "(%s) doesn't match with ballerina record field count(%s). ",
-                    headers.size(), structType.getFields().size()));
-        }
-        for (String key : structType.getFields().keySet()) {
-            if (!headers.contains(key.trim())) {
-                throw IOUtils.createError(String.format("The Record does not contain the " +
-                        "field - %s. ", key.trim()));
+
+        structType.getFields().forEach((key, value) -> {
+            Field field = (Field) value;
+            if (!headers.contains(field.getFieldName().trim()) &&
+                    !SymbolFlags.isFlagOn(field.getFlags(), SymbolFlags.OPTIONAL)) {
+                throw IOUtils.createError(String.format("The csv file does not contain the " +
+                        "column - %s.", field.getFieldName().trim()));
             }
-        }
+        });
+
+        headers.forEach(header -> {
+            if (structType.getFields().get(header) == null) {
+                throw IOUtils.createError(String.format("The csv file contains an additional column - %s.", header));
+            }
+        });
     }
 }