pgyori commented on code in PR #6234:
URL: https://github.com/apache/nifi/pull/6234#discussion_r936709751
##########
nifi-nar-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/csv/CSVReader.java:
##########
@@ -79,6 +81,17 @@ public class CSVReader extends SchemaRegistryService
implements RecordReaderFact
.required(true)
.build();
+ public static final PropertyDescriptor TRIM_DOUBLE_QUOTE = new
PropertyDescriptor.Builder()
+ .name("Trim double quote")
+ .description("Whether or not to trim starting and ending double
quotes. For example: with trim string '\"test\"'"
Review Comment:
I think it would be useful to add to the description something like
"if set to 'false' it means full compliance with RFC-4180".
This is because a new user who hasn't used this before selects RFC-4180 as
the CSV format, they would probably only want to know what value of this
property guarantees that.
##########
nifi-nar-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/test/java/org/apache/nifi/csv/TestJacksonCSVRecordReader.java:
##########
@@ -57,9 +57,9 @@ private List<RecordField> getDefaultFields() {
return fields;
}
- private JacksonCSVRecordReader createReader(final InputStream in, final
RecordSchema schema, CSVFormat format) throws IOException {
+ private JacksonCSVRecordReader createReader(final InputStream in, final
RecordSchema schema, CSVFormat format, final boolean trimDoubleQuote) throws
IOException {
Review Comment:
Just like in the case of the constructor of CSVRecordReader, I recommend
overloading this method with a variant that does not contain the
`trimDoubleQuote` parameter, and it should call this method with
`trimDoubleQuote=true`.
##########
nifi-nar-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/csv/CSVRecordReader.java:
##########
@@ -44,11 +44,12 @@
public class CSVRecordReader extends AbstractCSVRecordReader {
private final CSVParser csvParser;
+ private final boolean trimDoubleQuote;
private List<RecordField> recordFields;
public CSVRecordReader(final InputStream in, final ComponentLog logger,
final RecordSchema schema, final CSVFormat csvFormat, final boolean hasHeader,
final boolean ignoreHeader,
Review Comment:
I recommend reintroducing the declaration of the original constructor, and
the implementation should just call this constructor with
`trimDoubleQuote=true`. This way everywhere where you called this constructor
with `trimDoubleQuote=true`, you could revert to calling the original
constructor.
##########
nifi-nar-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/csv/JacksonCSVRecordReader.java:
##########
@@ -52,11 +52,12 @@ public class JacksonCSVRecordReader extends
AbstractCSVRecordReader {
private final MappingIterator<String[]> recordStream;
private List<String> rawFieldNames = null;
private boolean allowDuplicateHeaderNames;
+ private final boolean trimDoubleQuote;
private volatile static CsvMapper mapper = new
CsvMapper().enable(CsvParser.Feature.WRAP_AS_ARRAY);
public JacksonCSVRecordReader(final InputStream in, final ComponentLog
logger, final RecordSchema schema, final CSVFormat csvFormat, final boolean
hasHeader, final boolean ignoreHeader,
Review Comment:
Same as in the case of CSVRecordReader: I recommend reintroducing the
declaration of the original constructor, and the implementation should just
call this constructor with trimDoubleQuote=true. This way everywhere where you
called this constructor with trimDoubleQuote=true, you could revert to calling
the original constructor.
##########
nifi-nar-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/csv/CSVReader.java:
##########
@@ -50,6 +50,8 @@
import java.util.List;
import java.util.Map;
+import static org.apache.nifi.csv.CSVUtils.CSV_FORMAT;
Review Comment:
I believe this is an accidental auto-import. Could you please remove it?
##########
nifi-nar-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/test/java/org/apache/nifi/csv/TestCSVRecordReader.java:
##########
@@ -63,9 +63,9 @@ private List<RecordField> getDefaultFields() {
return fields;
}
- private CSVRecordReader createReader(final InputStream in, final
RecordSchema schema, CSVFormat format) throws IOException {
+ private CSVRecordReader createReader(final InputStream in, final
RecordSchema schema, CSVFormat format, final boolean trimDoubleQuote) throws
IOException {
Review Comment:
Just like in the case of the constructor of CSVRecordReader, I recommend
overloading this method with a variant that does not contain the
`trimDoubleQuote` parameter, and it should call this method with
`trimDoubleQuote=true`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
