You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@metamodel.apache.org by ka...@apache.org on 2019/06/20 10:01:28 UTC

[metamodel] 03/07: Did some more structural changes, so we don't need to synchronize access to the CSVParser.

This is an automated email from the ASF dual-hosted git repository.

kaspersor pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/metamodel.git

commit 89d01981394b9ca187a09f105339ff4cd9abf3b3
Author: Arjan Seijkens <a....@quadient.com>
AuthorDate: Fri Jun 14 11:26:04 2019 +0200

    Did some more structural changes, so we don't need to synchronize access to the CSVParser.
---
 .../org/apache/metamodel/csv/CsvConfiguration.java | 17 +++++++++
 .../org/apache/metamodel/csv/CsvDataContext.java   | 16 +-------
 .../apache/metamodel/csv/SingleLineCsvDataSet.java | 43 +++++++++++++++++++++-
 .../org/apache/metamodel/csv/SingleLineCsvRow.java |  7 +---
 .../apache/metamodel/csv/SingleLineCsvRowTest.java |  4 +-
 5 files changed, 63 insertions(+), 24 deletions(-)

diff --git a/csv/src/main/java/org/apache/metamodel/csv/CsvConfiguration.java b/csv/src/main/java/org/apache/metamodel/csv/CsvConfiguration.java
index 332ef4b..d9f0f72 100644
--- a/csv/src/main/java/org/apache/metamodel/csv/CsvConfiguration.java
+++ b/csv/src/main/java/org/apache/metamodel/csv/CsvConfiguration.java
@@ -26,6 +26,10 @@ import org.apache.metamodel.schema.naming.ColumnNamingStrategy;
 import org.apache.metamodel.util.BaseObject;
 import org.apache.metamodel.util.FileHelper;
 
+import com.opencsv.CSVParserBuilder;
+import com.opencsv.ICSVParser;
+import com.opencsv.RFC4180ParserBuilder;
+
 /**
  * Represents the configuration for reading/parsing CSV files.
  */
@@ -195,4 +199,17 @@ public final class CsvConfiguration extends BaseObject implements Serializable {
                 + ", separatorChar=" + separatorChar + ", quoteChar=" + quoteChar + ", escapeChar=" + escapeChar
                 + ", failOnInconsistentRowLength=" + failOnInconsistentRowLength + "]";
     }
+    
+    public ICSVParser createParser() {
+        if (getEscapeChar() == getQuoteChar()) {
+            return new RFC4180ParserBuilder().withSeparator(getSeparatorChar()).withQuoteChar(getQuoteChar()).build();
+        } else {
+            return new CSVParserBuilder()
+                    .withSeparator(getSeparatorChar())
+                    .withQuoteChar(getQuoteChar())
+                    .withEscapeChar(getEscapeChar())
+                    .build();
+        }
+    }
+
 }
diff --git a/csv/src/main/java/org/apache/metamodel/csv/CsvDataContext.java b/csv/src/main/java/org/apache/metamodel/csv/CsvDataContext.java
index 34b25fa..515ebdd 100644
--- a/csv/src/main/java/org/apache/metamodel/csv/CsvDataContext.java
+++ b/csv/src/main/java/org/apache/metamodel/csv/CsvDataContext.java
@@ -48,10 +48,8 @@ import org.apache.metamodel.util.UrlResource;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.opencsv.CSVParserBuilder;
 import com.opencsv.CSVReader;
 import com.opencsv.ICSVParser;
-import com.opencsv.RFC4180ParserBuilder;
 
 /**
  * DataContext implementation for reading CSV files.
@@ -299,21 +297,11 @@ public final class CsvDataContext extends QueryPostprocessDataContext implements
             return new CsvDataSet(csvReader, columns, maxRowsOrNull, columnCount, failOnInconsistentRowLength);
         }
 
-        return new SingleLineCsvDataSet(reader, createParser(), columns, maxRowsOrNull, columnCount,
-                failOnInconsistentRowLength);
+        return new SingleLineCsvDataSet(reader, columns, maxRowsOrNull, columnCount, _configuration);
     }
 
     private ICSVParser createParser() {
-        final ICSVParser parser;
-        if (_configuration.getEscapeChar() == _configuration.getQuoteChar()) {
-            parser = new RFC4180ParserBuilder().withSeparator(_configuration.getSeparatorChar())
-                    .withQuoteChar(_configuration.getQuoteChar()).build();
-        } else {
-            parser = new CSVParserBuilder().withSeparator(_configuration.getSeparatorChar())
-                    .withQuoteChar(_configuration.getQuoteChar()).withEscapeChar(_configuration.getEscapeChar())
-                    .build();
-        }
-        return parser;
+        return _configuration.createParser();
     }
 
     protected CSVReader createCsvReader(int skipLines) {
diff --git a/csv/src/main/java/org/apache/metamodel/csv/SingleLineCsvDataSet.java b/csv/src/main/java/org/apache/metamodel/csv/SingleLineCsvDataSet.java
index 4b24892..c734060 100644
--- a/csv/src/main/java/org/apache/metamodel/csv/SingleLineCsvDataSet.java
+++ b/csv/src/main/java/org/apache/metamodel/csv/SingleLineCsvDataSet.java
@@ -44,13 +44,48 @@ final class SingleLineCsvDataSet extends AbstractDataSet {
     private final ICSVParser _csvParser;
     private final int _columnsInTable;
     private final boolean _failOnInconsistentRowLength;
-
+    private final CsvConfiguration _csvConfiguration;
+    
     private volatile int _rowNumber;
     private volatile Integer _rowsRemaining;
     private volatile Row _row;
 
+    /**
+     * @param reader
+     * @param csvParser
+     * @param columns
+     * @param maxRows
+     * @param columnsInTable
+     * @param failOnInconsistentRowLength
+     * 
+     * @deprecated When instantiating a {@link SingleLineCsvDataSet} using this constructor the {@link ICSVParser} can
+     *             be reused in different threads in a parallel manner which is not promised to work by the
+     *             {@link ICSVParser}. Use
+     *             {@link #SingleLineCsvDataSet(BufferedReader, CsvConfiguration, List, Integer, int, boolean)} instead.
+     */
+    @Deprecated
     public SingleLineCsvDataSet(BufferedReader reader, ICSVParser csvParser, List<Column> columns, Integer maxRows,
-                                int columnsInTable, boolean failOnInconsistentRowLength) {
+            int columnsInTable, boolean failOnInconsistentRowLength) {
+        this(reader, csvParser, columns, maxRows, columnsInTable, failOnInconsistentRowLength, null);
+    }
+
+    /**
+     * @param reader
+     * @param columns
+     * @param maxRows
+     * @param columnsInTable
+     * @param csvParser
+     * @param failOnInconsistentRowLength
+     */
+    public SingleLineCsvDataSet(final BufferedReader reader, final List<Column> columns, final Integer maxRows,
+            final int columnsInTable, final CsvConfiguration csvConfiguration) {
+        this(reader, null, columns, maxRows, columnsInTable, csvConfiguration.isFailOnInconsistentRowLength(),
+                csvConfiguration);
+    }
+
+    private SingleLineCsvDataSet(final BufferedReader reader, final ICSVParser csvParser, final List<Column> columns,
+            final Integer maxRows, final int columnsInTable, final boolean failOnInconsistentRowLength,
+            final CsvConfiguration csvConfiguration) {
         super(columns.stream().map(SelectItem::new).collect(Collectors.toList()));
         _reader = reader;
         _csvParser = csvParser;
@@ -58,6 +93,7 @@ final class SingleLineCsvDataSet extends AbstractDataSet {
         _failOnInconsistentRowLength = failOnInconsistentRowLength;
         _rowNumber = 0;
         _rowsRemaining = maxRows;
+        _csvConfiguration = csvConfiguration;
     }
 
     @Override
@@ -95,6 +131,9 @@ final class SingleLineCsvDataSet extends AbstractDataSet {
     }
 
     protected ICSVParser getCsvParser() {
+        if (_csvConfiguration != null) {
+            return _csvConfiguration.createParser();
+        }
         return _csvParser;
     }
 
diff --git a/csv/src/main/java/org/apache/metamodel/csv/SingleLineCsvRow.java b/csv/src/main/java/org/apache/metamodel/csv/SingleLineCsvRow.java
index 64ff013..fe9fda5 100644
--- a/csv/src/main/java/org/apache/metamodel/csv/SingleLineCsvRow.java
+++ b/csv/src/main/java/org/apache/metamodel/csv/SingleLineCsvRow.java
@@ -29,8 +29,6 @@ import org.apache.metamodel.schema.Column;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.opencsv.ICSVParser;
-
 /**
  * Specialized row implementation for single-line CSV values
  */
@@ -101,10 +99,7 @@ final class SingleLineCsvRow extends AbstractRow {
 
     private String[] parseLine() {
         try {
-            final ICSVParser csvParser = _dataSet.getCsvParser();
-            synchronized (csvParser) {
-                return csvParser.parseLine(_line);
-            }
+            return _dataSet.getCsvParser().parseLine(_line);
         } catch (IOException e) {
             if (_failOnInconsistentRowLength) {
                 throw new MetaModelException("Failed to parse CSV line no. " + _rowNumber + ": " + _line, e);
diff --git a/csv/src/test/java/org/apache/metamodel/csv/SingleLineCsvRowTest.java b/csv/src/test/java/org/apache/metamodel/csv/SingleLineCsvRowTest.java
index cc02ea4..a2f0eea 100644
--- a/csv/src/test/java/org/apache/metamodel/csv/SingleLineCsvRowTest.java
+++ b/csv/src/test/java/org/apache/metamodel/csv/SingleLineCsvRowTest.java
@@ -46,6 +46,7 @@ public class SingleLineCsvRowTest {
         columns.add(new MutableColumn("1"));
         columns.add(new MutableColumn("2"));
         CSVParser csvParser = new CSVParser();
+        @SuppressWarnings("deprecation")
         final SingleLineCsvDataSet dataSet = new SingleLineCsvDataSet(null, csvParser, columns, null, 2, false);
         final SingleLineCsvRow originalRow = new SingleLineCsvRow(dataSet, "foo,bar", 2, false, 1);
 
@@ -75,10 +76,9 @@ public class SingleLineCsvRowTest {
         columns.add(new MutableColumn("h").setColumnNumber(8));
         columns.add(new MutableColumn("J").setColumnNumber(10));
 
-        final CSVParser csvParser = new CSVParser();
         try (final DataSet dataSet = new SingleLineCsvDataSet(FileHelper
                 .getBufferedReader(new FileResource("src/test/resources/empty_fields.csv").read(),
-                        FileHelper.UTF_8_CHARSET), csvParser, columns, null, 11, false)) {
+                        FileHelper.UTF_8_CHARSET), columns, null, 11, new CsvConfiguration())) {
             dataSet.toRows().parallelStream().forEach(row -> {
                 for (int i = 0; i < 5; i++) {
                     assertEquals("", row.getValue(i));