From c12940382874a03ea636329c22a2f83e906cdb2f Mon Sep 17 00:00:00 2001 From: snuyanzin Date: Tue, 15 Jan 2019 14:35:44 +0300 Subject: [PATCH 1/3] [SQLLINE-260] Validate given values for enum type properties --- src/main/java/sqlline/SqlLine.java | 7 --- src/main/java/sqlline/SqlLineOpts.java | 43 +++++++++++++++---- src/main/resources/sqlline/SqlLine.properties | 4 +- src/test/java/sqlline/SqlLineArgsTest.java | 30 +++++++++++-- 4 files changed, 62 insertions(+), 22 deletions(-) diff --git a/src/main/java/sqlline/SqlLine.java b/src/main/java/sqlline/SqlLine.java index a48dd731..d62bfe5f 100644 --- a/src/main/java/sqlline/SqlLine.java +++ b/src/main/java/sqlline/SqlLine.java @@ -1526,14 +1526,12 @@ String scanForDriver(String url) { return driver.getClass().getCanonicalName(); } - System.out.println("before"); scanDrivers(); if ((driver = findRegisteredDriver(url)) != null) { return driver.getClass().getCanonicalName(); } - System.out.println("return null"); return null; } catch (Exception e) { e.printStackTrace(); @@ -1603,11 +1601,6 @@ int print(ResultSet rs, DispatchCallback callback) throws SQLException { } } - if (f == null) { - error(loc("unknown-format", format, getOutputFormats().keySet())); - f = new TableOutputFormat(this); - } - Rows rows; if (getOpts().getIncremental()) { rows = new IncrementalRows(this, rs, callback); diff --git a/src/main/java/sqlline/SqlLineOpts.java b/src/main/java/sqlline/SqlLineOpts.java index 8edcaa2b..48b16ce3 100644 --- a/src/main/java/sqlline/SqlLineOpts.java +++ b/src/main/java/sqlline/SqlLineOpts.java @@ -103,6 +103,7 @@ public class SqlLineOpts implements Completer { put(MAX_HISTORY_ROWS, SqlLineOpts.this::setMaxHistoryRows); put(MODE, SqlLineOpts.this::setMode); put(NUMBER_FORMAT, SqlLineOpts.this::setNumberFormat); + put(OUTPUT_FORMAT, SqlLineOpts.this::setOutputFormat); put(TIME_FORMAT, SqlLineOpts.this::setTimeFormat); put(TIMESTAMP_FORMAT, SqlLineOpts.this::setTimestampFormat); } @@ -411,6 +412,13 @@ public void set(SqlLineProperty key, Object value) { ? (String) value : String.valueOf(value); valueToSet = DEFAULT.equalsIgnoreCase(strValue) ? key.defaultValue() : value; + if (!key.getAvailableValues().isEmpty() + && !key.getAvailableValues().contains(valueToSet.toString())) { + sqlLine.error( + sqlLine.loc("unknown-value", + key.propertyName(), value, key.getAvailableValues())); + return; + } break; case INTEGER: try { @@ -484,7 +492,7 @@ public void setNumberFormat(String numberFormat) { DecimalFormatSymbols.getInstance(Locale.ROOT)); nf.format(Integer.MAX_VALUE); } catch (Exception e) { - throw new IllegalArgumentException(e.getMessage()); + sqlLine.handleException(e); } propertiesMap.put(NUMBER_FORMAT, numberFormat); } @@ -572,9 +580,8 @@ public void setColorScheme(String colorScheme) { propertiesMap.put(COLOR_SCHEME, colorScheme); return; } - throw new IllegalArgumentException( - sqlLine.loc("unknown-colorscheme", colorScheme, - new TreeSet<>(BuiltInHighlightStyle.BY_NAME.keySet()))); + sqlLine.error(sqlLine.loc("unknown-value", COLOR_SCHEME.propertyName(), + colorScheme, COLOR_SCHEME.getAvailableValues())); } public String getColorScheme() { @@ -653,7 +660,7 @@ public void setCsvQuoteCharacter(String csvQuoteCharacter) { return; } } - throw new IllegalArgumentException("CsvQuoteCharacter is '" + sqlLine.error("CsvQuoteCharacter is '" + csvQuoteCharacter + "'; it must be a character of default"); } @@ -744,9 +751,29 @@ public void setMode(String mode) { keyMaps.put(LineReader.MAIN, keyMaps.get(LineReader.VIINS)); break; default: - throw new IllegalArgumentException( - sqlLine.loc( - "unknown-mode", mode, Arrays.asList(LineReader.EMACS, "vi"))); + sqlLine.error( + sqlLine.loc("unknown-value", MODE.propertyName(), + mode, Arrays.asList(LineReader.EMACS, "vi"))); + } + } + + public void setOutputFormat(String outputFormat) { + if (DEFAULT.equalsIgnoreCase(outputFormat)) { + set(OUTPUT_FORMAT, OUTPUT_FORMAT.defaultValue()); + return; + } + + Set availableFormats = + sqlLine.getOutputFormats().keySet().stream() + .map(t -> t.toUpperCase(Locale.ROOT)).collect(Collectors.toSet()); + if (availableFormats.contains(outputFormat.toUpperCase(Locale.ROOT))) { + set(OUTPUT_FORMAT, outputFormat); + } else { + sqlLine.error( + sqlLine.loc("unknown-value", + OUTPUT_FORMAT.propertyName(), + outputFormat, + sqlLine.getOutputFormats().keySet())); } } diff --git a/src/main/resources/sqlline/SqlLine.properties b/src/main/resources/sqlline/SqlLine.properties index 24b0305d..6d430a3f 100644 --- a/src/main/resources/sqlline/SqlLine.properties +++ b/src/main/resources/sqlline/SqlLine.properties @@ -197,9 +197,7 @@ driver: Driver: {0} (version {1}) autocommit-status: Autocommit status: {0} isolation-status: Transaction isolation: {0} isolation-level-not-supported: Transaction isolation level {0} is not supported. Default ({1}) will be used instead. -unknown-format: Unknown output format "{0}". Possible values: {1} -unknown-colorscheme: Unknown color scheme "{0}". Possible values: {1} -unknown-mode: Unknown mode "{0}". Possible values: {1} +unknown-value: Unknown {0} "{1}". Possible values: {2} closed: closed open: open diff --git a/src/test/java/sqlline/SqlLineArgsTest.java b/src/test/java/sqlline/SqlLineArgsTest.java index 947c4eef..82e5987f 100644 --- a/src/test/java/sqlline/SqlLineArgsTest.java +++ b/src/test/java/sqlline/SqlLineArgsTest.java @@ -18,6 +18,7 @@ import java.sql.SQLException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.TreeSet; @@ -1731,7 +1732,7 @@ public void testCustomOutputFormats() { + "!set outputformat json\n" + "values 1;"; checkScriptFile(script, true, equalTo(SqlLine.Status.OK), - containsString("Unknown output format \"json\"")); + containsString("Unknown outputFormat \"json\"")); } @Test @@ -1858,6 +1859,23 @@ public void testSetUsage() { containsString("Usage: set [all | []]")); } + @Test + public void testSetWrongValueToEnumTypeProperties() { + Collection nominalProperties = + Arrays.stream(BuiltInProperty.values()) + .filter(t -> !t.getAvailableValues().isEmpty()) + .collect(Collectors.toSet()); + final String wrongValue = "wrong_value"; + for (BuiltInProperty property : nominalProperties) { + final String script = + "!set " + property.propertyName() + " " + wrongValue + "\n"; + checkScriptFile(script, true, equalTo(SqlLine.Status.OK), + allOf(containsString("Unknown " + + property.propertyName() + " \"" + wrongValue + "\""), + not(containsString("Exception")))); + } + } + @Test public void testResetSuccess() { final String script = "!set timeout 200\n" @@ -2147,8 +2165,10 @@ public void testSetWrongColorScheme() { sqlLine.runCommands(dc, "!set colorscheme " + invalidColorScheme); assertThat(os.toString("UTF8"), containsString( - sqlLine.loc("unknown-colorscheme", invalidColorScheme, - new TreeSet<>(BuiltInHighlightStyle.BY_NAME.keySet())))); + sqlLine.loc("unknown-value", + BuiltInProperty.COLOR_SCHEME.propertyName(), + invalidColorScheme, + BuiltInProperty.COLOR_SCHEME.getAvailableValues()))); os.reset(); } catch (Exception e) { // fail @@ -2169,7 +2189,9 @@ public void testSetWrongEditingMode() { sqlLine.runCommands(dc, "!set mode " + invalidEditingMode); assertThat(os.toString("UTF8"), containsString( - sqlLine.loc("unknown-mode", invalidEditingMode, + sqlLine.loc("unknown-value", + BuiltInProperty.MODE.propertyName(), + invalidEditingMode, Arrays.asList(LineReader.EMACS, "vi")))); os.reset(); } catch (Exception e) { From 138b44bc9cffae39ea118fb45e16e464c180cd00 Mon Sep 17 00:00:00 2001 From: snuyanzin Date: Tue, 15 Jan 2019 15:17:04 +0300 Subject: [PATCH 2/3] [SQLLINE-260] Fix checkstyle issue --- src/main/java/sqlline/SqlLineOpts.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/sqlline/SqlLineOpts.java b/src/main/java/sqlline/SqlLineOpts.java index 48b16ce3..bda07d6e 100644 --- a/src/main/java/sqlline/SqlLineOpts.java +++ b/src/main/java/sqlline/SqlLineOpts.java @@ -762,7 +762,7 @@ public void setOutputFormat(String outputFormat) { set(OUTPUT_FORMAT, OUTPUT_FORMAT.defaultValue()); return; } - + Set availableFormats = sqlLine.getOutputFormats().keySet().stream() .map(t -> t.toUpperCase(Locale.ROOT)).collect(Collectors.toSet()); From 54bb58c2dfec7e240fc2a822c96710bfa6cc633a Mon Sep 17 00:00:00 2001 From: snuyanzin Date: Tue, 15 Jan 2019 16:56:03 +0300 Subject: [PATCH 3/3] [SQLLINE-260] Merge tests for properties with available values into one --- src/test/java/sqlline/SqlLineArgsTest.java | 53 +--------------------- 1 file changed, 2 insertions(+), 51 deletions(-) diff --git a/src/test/java/sqlline/SqlLineArgsTest.java b/src/test/java/sqlline/SqlLineArgsTest.java index 82e5987f..f4f39af2 100644 --- a/src/test/java/sqlline/SqlLineArgsTest.java +++ b/src/test/java/sqlline/SqlLineArgsTest.java @@ -33,7 +33,6 @@ import org.hsqldb.jdbc.JDBCResultSet; import org.hsqldb.jdbc.JDBCResultSetMetaData; import org.jline.builtins.Commands; -import org.jline.reader.LineReader; import org.jline.terminal.Terminal; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -1861,12 +1860,12 @@ public void testSetUsage() { @Test public void testSetWrongValueToEnumTypeProperties() { - Collection nominalProperties = + Collection propertiesWithAvailableValues = Arrays.stream(BuiltInProperty.values()) .filter(t -> !t.getAvailableValues().isEmpty()) .collect(Collectors.toSet()); final String wrongValue = "wrong_value"; - for (BuiltInProperty property : nominalProperties) { + for (BuiltInProperty property: propertiesWithAvailableValues) { final String script = "!set " + property.propertyName() + " " + wrongValue + "\n"; checkScriptFile(script, true, equalTo(SqlLine.Status.OK), @@ -2152,54 +2151,6 @@ public void testSave() { } } - @Test - public void testSetWrongColorScheme() { - final SqlLine sqlLine = new SqlLine(); - ByteArrayOutputStream os = new ByteArrayOutputStream(); - try { - SqlLine.Status status = - begin(sqlLine, os, false, "-e", "!set maxwidth 80"); - assertThat(status, equalTo(SqlLine.Status.OK)); - final DispatchCallback dc = new DispatchCallback(); - final String invalidColorScheme = "invalid"; - sqlLine.runCommands(dc, "!set colorscheme " + invalidColorScheme); - assertThat(os.toString("UTF8"), - containsString( - sqlLine.loc("unknown-value", - BuiltInProperty.COLOR_SCHEME.propertyName(), - invalidColorScheme, - BuiltInProperty.COLOR_SCHEME.getAvailableValues()))); - os.reset(); - } catch (Exception e) { - // fail - throw new RuntimeException(e); - } - } - - @Test - public void testSetWrongEditingMode() { - final SqlLine sqlLine = new SqlLine(); - ByteArrayOutputStream os = new ByteArrayOutputStream(); - try { - SqlLine.Status status = - begin(sqlLine, os, false, "-e", "!set maxwidth 80"); - assertThat(status, equalTo(SqlLine.Status.OK)); - DispatchCallback dc = new DispatchCallback(); - final String invalidEditingMode = "invalid"; - sqlLine.runCommands(dc, "!set mode " + invalidEditingMode); - assertThat(os.toString("UTF8"), - containsString( - sqlLine.loc("unknown-value", - BuiltInProperty.MODE.propertyName(), - invalidEditingMode, - Arrays.asList(LineReader.EMACS, "vi")))); - os.reset(); - } catch (Exception e) { - // fail - throw new RuntimeException(e); - } - } - @Test public void testScript() { final File file = createTempFile("sqlline", ".script");