From ea40b6e3c935374056681cbc3093c482928d9258 Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Tue, 3 Dec 2024 15:10:11 +0530 Subject: [PATCH 1/5] Allow nil values to be read as records --- ballerina/tests/csv_io.bal | 72 +++++++++++++++++++ .../csvResourceFileWithEmptyValues.csv | 4 ++ .../io/nativeimpl/RecordChannelUtils.java | 7 -- 3 files changed, 76 insertions(+), 7 deletions(-) create mode 100644 ballerina/tests/resources/csvResourceFileWithEmptyValues.csv diff --git a/ballerina/tests/csv_io.bal b/ballerina/tests/csv_io.bal index fec49531..e256112a 100644 --- a/ballerina/tests/csv_io.bal +++ b/ballerina/tests/csv_io.bal @@ -94,6 +94,14 @@ type Employee6 record { string age; }; +type Employee7 record {| + string name; + string designation; + string company; + string? age; + string? residence; +|}; + type RefInt int; type RefStr string; @@ -2000,3 +2008,67 @@ 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 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); +} diff --git a/ballerina/tests/resources/csvResourceFileWithEmptyValues.csv b/ballerina/tests/resources/csvResourceFileWithEmptyValues.csv new file mode 100644 index 00000000..5f19cb0c --- /dev/null +++ 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/native/src/main/java/io/ballerina/stdlib/io/nativeimpl/RecordChannelUtils.java b/native/src/main/java/io/ballerina/stdlib/io/nativeimpl/RecordChannelUtils.java index e820664d..096eb6f2 100644 --- a/native/src/main/java/io/ballerina/stdlib/io/nativeimpl/RecordChannelUtils.java +++ b/native/src/main/java/io/ballerina/stdlib/io/nativeimpl/RecordChannelUtils.java @@ -159,9 +159,6 @@ record = textRecordChannel.read(); return returnStruct; } Map struct = (Map) 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)); @@ -225,10 +222,6 @@ record = textRecordChannel.getFields(line); return returnStruct; } final Map struct = (Map) 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); } From 39ef74783ee0e05189561c89918ee71500f596eb Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Tue, 3 Dec 2024 23:52:44 +0530 Subject: [PATCH 2/5] Update the changelog --- changelog.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/changelog.md b/changelog.md index 768e7570..b0bd2c5b 100644 --- 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 From 624e19d9b65108ab68da0e8a48d12aabb504cb9e Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Wed, 4 Dec 2024 13:59:29 +0530 Subject: [PATCH 3/5] support optional fields in record type --- ballerina/tests/csv_io.bal | 108 ++++++++++++++++++ ballerina/tests/negative.bal | 2 +- .../csvResourceFileWithMissingFields.csv | 4 + .../io/nativeimpl/RecordChannelUtils.java | 26 +++-- 4 files changed, 129 insertions(+), 11 deletions(-) create mode 100644 ballerina/tests/resources/csvResourceFileWithMissingFields.csv diff --git a/ballerina/tests/csv_io.bal b/ballerina/tests/csv_io.bal index e256112a..53f2572b 100644 --- a/ballerina/tests/csv_io.bal +++ b/ballerina/tests/csv_io.bal @@ -102,6 +102,14 @@ type Employee7 record {| string? residence; |}; +type Employee8 record {| + string name; + string designation; + string company; + string age?; + string residence; +|}; + type RefInt int; type RefStr string; @@ -2072,3 +2080,103 @@ function testFileReadCsvRecordStreamWithEmptyFieldValues() returns Error? { }); test:assertEquals(i, 3); } + +@test:Config {} +function testFileReadCsvRecordWithNillableField() returns Error? { + string resourceFilePath1 = TEST_RESOURCE_PATH + "csvResourceFile1.csv"; + stream 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 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 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 index 5a6b5503..348dac58 100644 --- 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((csvContent).message(), "The CSV file content header count(5) doesn't match with ballerina record field count(4). "); + test:assertEquals((csvContent).message(), "The csv file contains an additional column - residence."); } @test:Config {} diff --git a/ballerina/tests/resources/csvResourceFileWithMissingFields.csv b/ballerina/tests/resources/csvResourceFileWithMissingFields.csv new file mode 100644 index 00000000..23a02a1a --- /dev/null +++ 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/native/src/main/java/io/ballerina/stdlib/io/nativeimpl/RecordChannelUtils.java b/native/src/main/java/io/ballerina/stdlib/io/nativeimpl/RecordChannelUtils.java index 096eb6f2..98d9e8e4 100644 --- a/native/src/main/java/io/ballerina/stdlib/io/nativeimpl/RecordChannelUtils.java +++ b/native/src/main/java/io/ballerina/stdlib/io/nativeimpl/RecordChannelUtils.java @@ -21,6 +21,8 @@ import io.ballerina.runtime.api.TypeTags; 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.utils.StringUtils; @@ -315,16 +317,20 @@ public static Object closeStream(BObject iterator) { } private static void validateHeaders(ArrayList 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)); + } + }); } } From dc1ad87424c4c8bb00daaf5d3decb4a3f30101fa Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Wed, 4 Dec 2024 15:20:56 +0530 Subject: [PATCH 4/5] Add negative testcases --- ballerina/tests/negative.bal | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/ballerina/tests/negative.bal b/ballerina/tests/negative.bal index 348dac58..6e935449 100644 --- a/ballerina/tests/negative.bal +++ b/ballerina/tests/negative.bal @@ -239,3 +239,17 @@ function readCsvFileWithUnsupportedMappingType() { map[]|Error out = fileReadCsv(filePath); test:assertEquals((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((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((out).message(), "Field 'age' does not support nil value."); +} \ No newline at end of file From 058cb5f59fea56467bbc7f3dce3f75023caf6b3c Mon Sep 17 00:00:00 2001 From: Danesh Kuruppu Date: Wed, 4 Dec 2024 15:23:35 +0530 Subject: [PATCH 5/5] Update ballerina/tests/negative.bal --- ballerina/tests/negative.bal | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ballerina/tests/negative.bal b/ballerina/tests/negative.bal index 6e935449..8006a25a 100644 --- a/ballerina/tests/negative.bal +++ b/ballerina/tests/negative.bal @@ -252,4 +252,4 @@ function readCsvWithMissingValueAndOptionalField() { string filePath = TEST_RESOURCE_PATH + "csvResourceFileWithEmptyValues.csv"; Employee8[]|Error out = fileReadCsv(filePath); test:assertEquals((out).message(), "Field 'age' does not support nil value."); -} \ No newline at end of file +}