From 2ba7258603166c2ae7110db0690298a849f50661 Mon Sep 17 00:00:00 2001 From: Igor Motov Date: Mon, 9 Apr 2018 17:01:24 -0400 Subject: [PATCH] SQL: Extract CSV spec parser into Utils class (elastic/x-pack-elasticsearch#4317) For Geo SQL we need to have a separate set of CSV tests. This commit extracts CSV spec parsing logic into a separate file for a more straight forward reuse. Relates elastic/x-pack-elasticsearch#4080 Original commit: elastic/x-pack-elasticsearch@29034ef051c55fad322bca31baca456527d39371 --- .../qa/sql/nosecurity/JdbcCsvSpecIT.java | 1 + .../xpack/qa/sql/security/JdbcCsvSpecIT.java | 1 + .../xpack/qa/sql/jdbc/CsvSpecTestCase.java | 155 +------------- .../xpack/qa/sql/jdbc/CsvTestUtils.java | 196 ++++++++++++++++++ .../xpack/qa/sql/jdbc/DebugCsvSpec.java | 34 ++- 5 files changed, 237 insertions(+), 150 deletions(-) create mode 100644 qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/CsvTestUtils.java diff --git a/qa/sql/no-security/src/test/java/org/elasticsearch/xpack/qa/sql/nosecurity/JdbcCsvSpecIT.java b/qa/sql/no-security/src/test/java/org/elasticsearch/xpack/qa/sql/nosecurity/JdbcCsvSpecIT.java index 09860d375b5..a245e6c85ef 100644 --- a/qa/sql/no-security/src/test/java/org/elasticsearch/xpack/qa/sql/nosecurity/JdbcCsvSpecIT.java +++ b/qa/sql/no-security/src/test/java/org/elasticsearch/xpack/qa/sql/nosecurity/JdbcCsvSpecIT.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.qa.sql.nosecurity; import org.elasticsearch.xpack.qa.sql.jdbc.CsvSpecTestCase; +import org.elasticsearch.xpack.qa.sql.jdbc.CsvTestUtils.CsvTestCase; public class JdbcCsvSpecIT extends CsvSpecTestCase { public JdbcCsvSpecIT(String fileName, String groupName, String testName, Integer lineNumber, CsvTestCase testCase) { diff --git a/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/JdbcCsvSpecIT.java b/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/JdbcCsvSpecIT.java index 3375b663404..e5fdf0baf45 100644 --- a/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/JdbcCsvSpecIT.java +++ b/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/JdbcCsvSpecIT.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.qa.sql.security; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.xpack.qa.sql.jdbc.CsvSpecTestCase; +import org.elasticsearch.xpack.qa.sql.jdbc.CsvTestUtils.CsvTestCase; import java.util.Properties; diff --git a/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/CsvSpecTestCase.java b/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/CsvSpecTestCase.java index 9e9fccd867a..e37688eb904 100644 --- a/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/CsvSpecTestCase.java +++ b/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/CsvSpecTestCase.java @@ -6,30 +6,18 @@ package org.elasticsearch.xpack.qa.sql.jdbc; import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; - -import org.elasticsearch.common.Strings; -import org.elasticsearch.common.collect.Tuple; -import org.elasticsearch.common.io.Streams; +import org.elasticsearch.xpack.qa.sql.jdbc.CsvTestUtils.CsvTestCase; import org.elasticsearch.xpack.sql.jdbc.jdbc.JdbcConfiguration; -import org.relique.io.TableReader; -import org.relique.jdbc.csv.CsvConnection; -import java.io.BufferedReader; -import java.io.BufferedWriter; -import java.io.IOException; -import java.io.Reader; -import java.io.StringReader; -import java.io.StringWriter; import java.sql.Connection; import java.sql.ResultSet; -import java.sql.SQLException; -import java.sql.Statement; import java.util.ArrayList; import java.util.List; -import java.util.Locale; import java.util.Properties; -import static org.hamcrest.Matchers.arrayWithSize; +import static org.elasticsearch.xpack.qa.sql.jdbc.CsvTestUtils.csvConnection; +import static org.elasticsearch.xpack.qa.sql.jdbc.CsvTestUtils.executeCsvQuery; +import static org.elasticsearch.xpack.qa.sql.jdbc.CsvTestUtils.specParser; /** * Tests comparing sql queries executed against our jdbc client @@ -60,37 +48,12 @@ public abstract class CsvSpecTestCase extends SpecBaseIntegrationTestCase { @Override protected final void doTest() throws Throwable { - assertMatchesCsv(testCase.query, testName, testCase.expectedResults); - } - - private void assertMatchesCsv(String query, String csvTableName, String expectedResults) throws SQLException, IOException { - Properties csvProperties = new Properties(); - csvProperties.setProperty("charset", "UTF-8"); - csvProperties.setProperty("separator", "|"); - csvProperties.setProperty("trimValues", "true"); - Tuple resultsAndTypes = extractColumnTypesAndStripCli(expectedResults); - csvProperties.setProperty("columnTypes", resultsAndTypes.v2()); - Reader reader = new StringReader(resultsAndTypes.v1()); - TableReader tableReader = new TableReader() { - @Override - public Reader getReader(Statement statement, String tableName) throws SQLException { - return reader; - } - - @Override - public List getTableNames(Connection connection) throws SQLException { - throw new UnsupportedOperationException(); - } - }; - try (Connection csv = new CsvConnection(tableReader, csvProperties, "") {}; + try (Connection csv = csvConnection(testCase.expectedResults); Connection es = esJdbc()) { // pass the testName as table for debugging purposes (in case the underlying reader is missing) - ResultSet expected = csv.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY) - .executeQuery("SELECT * FROM " + csvTableName); - // trigger data loading for type inference - expected.beforeFirst(); - ResultSet elasticResults = executeJdbcQuery(es, query); + ResultSet expected = executeCsvQuery(csv, testName); + ResultSet elasticResults = executeJdbcQuery(es, testCase.query); assertResults(expected, elasticResults); } } @@ -103,108 +66,4 @@ public abstract class CsvSpecTestCase extends SpecBaseIntegrationTestCase { return connectionProperties; } - private Tuple extractColumnTypesAndStripCli(String expectedResults) throws IOException { - try (StringReader reader = new StringReader(expectedResults); - BufferedReader bufferedReader = new BufferedReader(reader); - StringWriter writer = new StringWriter(); - BufferedWriter bufferedWriter = new BufferedWriter(writer)) { - - String header = bufferedReader.readLine(); - Tuple headerAndTypes; - - if (header.contains(":")) { - headerAndTypes = extractColumnTypesFromHeader(header); - } else { - // No type information in headers, no need to parse columns - trigger auto-detection - headerAndTypes = new Tuple<>(header, ""); - } - bufferedWriter.write(headerAndTypes.v1()); - bufferedWriter.newLine(); - - /* Read the next line. It might be a separator designed to look like the cli. - * If it is, then throw it out. If it isn't then keep it. - */ - String maybeSeparator = bufferedReader.readLine(); - if (maybeSeparator != null && false == maybeSeparator.startsWith("----")) { - bufferedWriter.write(maybeSeparator); - bufferedWriter.newLine(); - } - - bufferedWriter.flush(); - // Copy the rest of test - Streams.copy(bufferedReader, bufferedWriter); - return new Tuple<>(writer.toString(), headerAndTypes.v2()); - } - } - - private Tuple extractColumnTypesFromHeader(String header) { - String[] columnTypes = Strings.delimitedListToStringArray(header, "|", " \t"); - StringBuilder types = new StringBuilder(); - StringBuilder columns = new StringBuilder(); - for(String column : columnTypes) { - String[] nameType = Strings.delimitedListToStringArray(column, ":"); - assertThat("If at least one column has a type associated with it, all columns should have types", nameType, arrayWithSize(2)); - if(types.length() > 0) { - types.append(","); - columns.append("|"); - } - columns.append(nameType[0]); - types.append(resolveColumnType(nameType[1])); - } - return new Tuple<>(columns.toString(), types.toString()); - } - - private String resolveColumnType(String type) { - switch (type.toLowerCase(Locale.ROOT)) { - case "s": return "string"; - case "b": return "boolean"; - case "i": return "integer"; - case "l": return "long"; - case "f": return "float"; - case "d": return "double"; - case "ts": return "timestamp"; - default: return type; - } - } - - static CsvSpecParser specParser() { - return new CsvSpecParser(); - } - - private static class CsvSpecParser implements Parser { - private final StringBuilder data = new StringBuilder(); - private CsvTestCase testCase; - - @Override - public Object parse(String line) { - // beginning of the section - if (testCase == null) { - // pick up the query - testCase = new CsvTestCase(); - testCase.query = line.endsWith(";") ? line.substring(0, line.length() - 1) : line; - } - else { - // read data - if (line.startsWith(";")) { - testCase.expectedResults = data.toString(); - // clean-up and emit - CsvTestCase result = testCase; - testCase = null; - data.setLength(0); - return result; - } - else { - data.append(line); - data.append("\r\n"); - } - } - - return null; - } - } - - protected static class CsvTestCase { - String query; - String expectedResults; - } } diff --git a/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/CsvTestUtils.java b/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/CsvTestUtils.java new file mode 100644 index 00000000000..fbbc2285ed1 --- /dev/null +++ b/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/CsvTestUtils.java @@ -0,0 +1,196 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.qa.sql.jdbc; + +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.collect.Tuple; +import org.elasticsearch.common.io.Streams; +import org.relique.io.TableReader; +import org.relique.jdbc.csv.CsvConnection; + +import java.io.BufferedReader; +import java.io.BufferedWriter; +import java.io.IOException; +import java.io.Reader; +import java.io.StringReader; +import java.io.StringWriter; +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.List; +import java.util.Locale; +import java.util.Properties; + +import static org.hamcrest.Matchers.arrayWithSize; +import static org.junit.Assert.assertThat; + +/** + * Utility functions for CSV testing + */ +public final class CsvTestUtils { + + private CsvTestUtils() { + + } + + /** + * Executes a query on provided CSV connection. + *

+ * The supplied table name is only used for the test identification. + */ + public static ResultSet executeCsvQuery(Connection csv, String csvTableName) throws SQLException { + ResultSet expected = csv.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY) + .executeQuery("SELECT * FROM " + csvTableName); + // trigger data loading for type inference + expected.beforeFirst(); + return expected; + } + + /** + * Wraps CSV in the expectedResults into CSV Connection. + * + * Use {@link #executeCsvQuery} to obtain ResultSet from this connection + */ + public static Connection csvConnection(String expectedResults) throws IOException, SQLException { + Properties csvProperties = new Properties(); + csvProperties.setProperty("charset", "UTF-8"); + csvProperties.setProperty("separator", "|"); + csvProperties.setProperty("trimValues", "true"); + Tuple resultsAndTypes = extractColumnTypesAndStripCli(expectedResults); + csvProperties.setProperty("columnTypes", resultsAndTypes.v2()); + Reader reader = new StringReader(resultsAndTypes.v1()); + TableReader tableReader = new TableReader() { + @Override + public Reader getReader(Statement statement, String tableName) throws SQLException { + return reader; + } + + @Override + public List getTableNames(Connection connection) throws SQLException { + throw new UnsupportedOperationException(); + } + }; + return new CsvConnection(tableReader, csvProperties, "") { + }; + } + + private static Tuple extractColumnTypesAndStripCli(String expectedResults) throws IOException { + try (StringReader reader = new StringReader(expectedResults); + BufferedReader bufferedReader = new BufferedReader(reader); + StringWriter writer = new StringWriter(); + BufferedWriter bufferedWriter = new BufferedWriter(writer)) { + + String header = bufferedReader.readLine(); + Tuple headerAndTypes; + + if (header.contains(":")) { + headerAndTypes = extractColumnTypesFromHeader(header); + } else { + // No type information in headers, no need to parse columns - trigger auto-detection + headerAndTypes = new Tuple<>(header, ""); + } + bufferedWriter.write(headerAndTypes.v1()); + bufferedWriter.newLine(); + + /* Read the next line. It might be a separator designed to look like the cli. + * If it is, then throw it out. If it isn't then keep it. + */ + String maybeSeparator = bufferedReader.readLine(); + if (maybeSeparator != null && false == maybeSeparator.startsWith("----")) { + bufferedWriter.write(maybeSeparator); + bufferedWriter.newLine(); + } + + bufferedWriter.flush(); + // Copy the rest of test + Streams.copy(bufferedReader, bufferedWriter); + return new Tuple<>(writer.toString(), headerAndTypes.v2()); + } + } + + private static Tuple extractColumnTypesFromHeader(String header) { + String[] columnTypes = Strings.delimitedListToStringArray(header, "|", " \t"); + StringBuilder types = new StringBuilder(); + StringBuilder columns = new StringBuilder(); + for (String column : columnTypes) { + String[] nameType = Strings.delimitedListToStringArray(column, ":"); + assertThat("If at least one column has a type associated with it, all columns should have types", nameType, arrayWithSize(2)); + if (types.length() > 0) { + types.append(","); + columns.append("|"); + } + columns.append(nameType[0]); + types.append(resolveColumnType(nameType[1])); + } + return new Tuple<>(columns.toString(), types.toString()); + } + + private static String resolveColumnType(String type) { + switch (type.toLowerCase(Locale.ROOT)) { + case "s": + return "string"; + case "b": + return "boolean"; + case "i": + return "integer"; + case "l": + return "long"; + case "f": + return "float"; + case "d": + return "double"; + case "ts": + return "timestamp"; + default: + return type; + } + } + + /** + * Returns an instance of a parser for csv-spec tests. + */ + public static CsvSpecParser specParser() { + return new CsvSpecParser(); + } + + private static class CsvSpecParser implements SpecBaseIntegrationTestCase.Parser { + private final StringBuilder data = new StringBuilder(); + private CsvTestCase testCase; + + @Override + public Object parse(String line) { + // beginning of the section + if (testCase == null) { + // pick up the query + testCase = new CsvTestCase(); + testCase.query = line.endsWith(";") ? line.substring(0, line.length() - 1) : line; + } + else { + // read data + if (line.startsWith(";")) { + testCase.expectedResults = data.toString(); + // clean-up and emit + CsvTestCase result = testCase; + testCase = null; + data.setLength(0); + return result; + } + else { + data.append(line); + data.append("\r\n"); + } + } + + return null; + } + } + + public static class CsvTestCase { + String query; + String expectedResults; + } +} diff --git a/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/DebugCsvSpec.java b/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/DebugCsvSpec.java index 12e7b8b073c..46efeb0f55a 100644 --- a/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/DebugCsvSpec.java +++ b/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/DebugCsvSpec.java @@ -9,13 +9,22 @@ import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; import org.apache.logging.log4j.Logger; import org.elasticsearch.test.junit.annotations.TestLogging; +import org.elasticsearch.xpack.qa.sql.jdbc.CsvTestUtils.CsvTestCase; +import org.elasticsearch.xpack.sql.jdbc.jdbc.JdbcConfiguration; +import java.sql.Connection; import java.sql.ResultSet; import java.sql.SQLException; import java.util.List; +import java.util.Properties; + +import static org.elasticsearch.xpack.qa.sql.jdbc.CsvTestUtils.csvConnection; +import static org.elasticsearch.xpack.qa.sql.jdbc.CsvTestUtils.executeCsvQuery; +import static org.elasticsearch.xpack.qa.sql.jdbc.CsvTestUtils.specParser; @TestLogging(JdbcTestUtils.SQL_TRACE) -public abstract class DebugCsvSpec extends CsvSpecTestCase { +public abstract class DebugCsvSpec extends SpecBaseIntegrationTestCase { + private final CsvTestCase testCase; @ParametersFactory(shuffle = false, argumentFormatting = SqlSpecTestCase.PARAM_FORMATTING) public static List readScriptSpec() throws Exception { @@ -24,7 +33,8 @@ public abstract class DebugCsvSpec extends CsvSpecTestCase { } public DebugCsvSpec(String fileName, String groupName, String testName, Integer lineNumber, CsvTestCase testCase) { - super(fileName, groupName, testName, lineNumber, testCase); + super(fileName, groupName, testName, lineNumber); + this.testCase = testCase; } @Override @@ -43,4 +53,24 @@ public abstract class DebugCsvSpec extends CsvSpecTestCase { protected boolean logEsResultSet() { return true; } + + @Override + protected final void doTest() throws Throwable { + try (Connection csv = csvConnection(testCase.expectedResults); + Connection es = esJdbc()) { + + // pass the testName as table for debugging purposes (in case the underlying reader is missing) + ResultSet expected = executeCsvQuery(csv, testName); + ResultSet elasticResults = executeJdbcQuery(es, testCase.query); + assertResults(expected, elasticResults); + } + } + + // make sure ES uses UTC (otherwise JDBC driver picks up the JVM timezone per spec/convention) + @Override + protected Properties connectionProperties() { + Properties connectionProperties = new Properties(); + connectionProperties.setProperty(JdbcConfiguration.TIME_ZONE, "UTC"); + return connectionProperties; + } } \ No newline at end of file