From 6ab70fcc832a81a90340bd93ad3f2a27d65e726c Mon Sep 17 00:00:00 2001 From: Christoph Kuhnke Date: Thu, 15 Sep 2022 09:13:15 +0200 Subject: [PATCH] Bug/258 fix data type properties #2 (#259) * #255: Improved documentation * #258 fixed * ported tests to run on windows, too --- doc/changes/changes_16.1.1.md | 10 ++++-- .../adapter/request/parser/RequestParser.java | 18 +++++----- .../request/parser/RequestParserTest.java | 35 +++++++++++-------- .../exasol/logging/CompactFormatterTest.java | 24 ++++++------- .../exasol/logging/RemoteLogManagerTest.java | 31 ++++++++++++---- 5 files changed, 72 insertions(+), 46 deletions(-) diff --git a/doc/changes/changes_16.1.1.md b/doc/changes/changes_16.1.1.md index 19c9e1a2..243aba86 100644 --- a/doc/changes/changes_16.1.1.md +++ b/doc/changes/changes_16.1.1.md @@ -1,11 +1,15 @@ -# Common module of Exasol Virtual Schemas Adapters 16.1.1, released 2022-09-13 +# Common module of Exasol Virtual Schemas Adapters 16.1.1, released 2022-09-15 -Code name: Documentation Update +Code name: Fixed data type properties and Documentation Update ## Summary -Updated documentation. +Fixed data type properties and updated documentation. ## Features * #255: Improved documentation + +## Bug Fixes + +* #258: Fixed data type properties diff --git a/src/main/java/com/exasol/adapter/request/parser/RequestParser.java b/src/main/java/com/exasol/adapter/request/parser/RequestParser.java index e068a93a..14c77eb4 100644 --- a/src/main/java/com/exasol/adapter/request/parser/RequestParser.java +++ b/src/main/java/com/exasol/adapter/request/parser/RequestParser.java @@ -71,12 +71,19 @@ private AbstractAdapterRequest parseRefreshRequest(final JsonObject root, final } private AbstractAdapterRequest parsePushdownRequest(final JsonObject root, final SchemaMetadataInfo metadataInfo) { - final SqlStatement statement = parsePushdownStatement(root); + final JsonObject pushdownJson = root.getJsonObject(PUSHDOW_REQUEST_KEY); final List involvedTables = parseInvolvedTables(root); - final List dataTypes = parseDataTypes(root); + final SqlStatement statement = parsePushdownStatement(pushdownJson, involvedTables); + final List dataTypes = parseDataTypes(pushdownJson); return new PushDownRequest(metadataInfo, statement, involvedTables, dataTypes); } + private SqlStatement parsePushdownStatement(final JsonObject pushdownJson, + final List involvedTables) { + final PushdownSqlParser pushdownSqlParser = PushdownSqlParser.createWithTablesMetadata(involvedTables); + return (SqlStatement) pushdownSqlParser.parseExpression(pushdownJson); + } + private List parseDataTypes(final JsonObject root) { if (!root.containsKey(SELECT_LIST_DATATYPES_KEY)) { return Collections.emptyList(); @@ -102,13 +109,6 @@ private SchemaMetadataInfo readSchemaMetadataInfo(final JsonObject root) { } } - private SqlStatement parsePushdownStatement(final JsonObject root) { - final List involvedTables = parseInvolvedTables(root); - final PushdownSqlParser pushdownSqlParser = PushdownSqlParser.createWithTablesMetadata(involvedTables); - final JsonObject jsonPushdownStatement = root.getJsonObject(PUSHDOW_REQUEST_KEY); - return (SqlStatement) pushdownSqlParser.parseExpression(jsonPushdownStatement); - } - /** * Create a {@link RequestParser} * diff --git a/src/test/java/com/exasol/adapter/request/parser/RequestParserTest.java b/src/test/java/com/exasol/adapter/request/parser/RequestParserTest.java index 9fb04a62..25d4abab 100644 --- a/src/test/java/com/exasol/adapter/request/parser/RequestParserTest.java +++ b/src/test/java/com/exasol/adapter/request/parser/RequestParserTest.java @@ -1,6 +1,7 @@ package com.exasol.adapter.request.parser; import static com.exasol.adapter.request.parser.json.builder.JsonEntry.*; +import static com.exasol.adapter.request.parser.json.builder.JsonEntry.array; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; import static org.hamcrest.collection.IsMapContaining.hasEntry; @@ -18,6 +19,7 @@ import com.exasol.adapter.metadata.TableMetadata; import com.exasol.adapter.request.*; import com.exasol.adapter.request.parser.json.builder.*; +import com.exasol.adapter.request.parser.json.builder.JsonParent.JsonObject; class RequestParserTest { private static final JsonKeyValue SCHEMA_METADATA_INFO = JsonEntry.entry("schemaMetadataInfo", @@ -76,7 +78,7 @@ void unsupportedPropertyType() { @Test void classicPushDownRequest() { - final AdapterRequest request = this.parser.parse(createPushDownRequest().render()); + final AdapterRequest request = this.parser.parse(createPushDownRequest(null).render()); assertThat("Request class", request, instanceOf(PushDownRequest.class)); final List involvedTables = ((PushDownRequest) request).getInvolvedTablesMetadata(); final List selectListDataTypes = ((PushDownRequest) request).getSelectListDataTypes(); @@ -88,13 +90,12 @@ void classicPushDownRequest() { @Test void pushDownRequestWithSelectListDataTypes() { - final String rawRequest = createPushDownRequest().addChild( // - entry("selectListDataTypes", array( // - object(entry("type", "DECIMAL"), // - entry("precision", 9), // - entry("scale", 10)), // - object(entry("type", "DOUBLE"))))) // - .render(); + final JsonKeyValue ltdt = entry("selectListDataTypes", array( // + object(entry("type", "DECIMAL"), // + entry("precision", 9), // + entry("scale", 10)), // + object(entry("type", "DOUBLE")))); // + final String rawRequest = createPushDownRequest(ltdt).render(); final AdapterRequest request = this.parser.parse(rawRequest); final List selectListDataTypes = ((PushDownRequest) request).getSelectListDataTypes(); assertAll(() -> assertThat(request.getType(), equalTo(AdapterRequestType.PUSHDOWN)), @@ -132,15 +133,19 @@ void requestWithoutSchemaMetadata() { assertThat(request.getVirtualSchemaName(), equalTo("UNKNOWN")); } - private JsonParent createPushDownRequest() { + private JsonParent createPushDownRequest(final JsonEntry selectListDataTypes) { + final JsonObject pdr = object( // + entry("type", "select"), // + entry("from", object( // + entry("name", "FOO"), // + entry("type", "table") // + ))); + if (selectListDataTypes != null) { + pdr.addChild(selectListDataTypes); + } return JsonEntry.object( // entry("type", "pushdown"), // - entry("pushdownRequest", object( // - entry("type", "select"), // - entry("from", object( // - entry("name", "FOO"), // - entry("type", "table") // - )))), // + entry("pushdownRequest", pdr), // entry("involvedTables", array(object( // entry("name", "FOO"), // entry("columns", array(object( // diff --git a/src/test/java/com/exasol/logging/CompactFormatterTest.java b/src/test/java/com/exasol/logging/CompactFormatterTest.java index 1af87461..80e4e8e4 100644 --- a/src/test/java/com/exasol/logging/CompactFormatterTest.java +++ b/src/test/java/com/exasol/logging/CompactFormatterTest.java @@ -1,8 +1,9 @@ package com.exasol.logging; +import static com.exasol.logging.RemoteLogManagerTest.*; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.text.MatchesPattern.matchesPattern; -import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertAll; import java.util.logging.Level; @@ -14,7 +15,6 @@ class CompactFormatterTest { private LogRecord record; private final CompactFormatter formatter = new CompactFormatter(); - private final String TIMESTAMP_PATTERN = "\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}.\\d{3}"; @BeforeEach void beforeEach() { @@ -24,51 +24,49 @@ void beforeEach() { @Test void testFormat() { final String formattedRecord = this.formatter.format(this.record); - assertThat(formattedRecord, matchesPattern(this.TIMESTAMP_PATTERN + " SEVERE +message\\n")); + assertThat(formattedRecord, matchesTimeStamp(" SEVERE +message")); } @Test void testFormatWithEmptyClass() { this.record.setSourceClassName(""); final String formattedRecord = this.formatter.format(this.record); - assertThat(formattedRecord, matchesPattern(this.TIMESTAMP_PATTERN + " SEVERE +message\\n")); + assertThat(formattedRecord, matchesTimeStamp(" SEVERE +message")); } @Test void testFormatWithSimpleClass() { this.record.setSourceClassName("example"); final String formattedRecord = this.formatter.format(this.record); - assertThat(formattedRecord, matchesPattern(this.TIMESTAMP_PATTERN + " SEVERE +\\[example\\] +message\\n")); + assertThat(formattedRecord, matchesTimeStamp(" SEVERE +\\[example\\] +message")); } @Test void testFormatWithFullClass() { this.record.setSourceClassName("com.exasol.example"); final String formattedRecord = this.formatter.format(this.record); - assertThat(formattedRecord, - matchesPattern(this.TIMESTAMP_PATTERN + " SEVERE +\\[c\\.e\\.example\\] +message\\n")); + assertThat(formattedRecord, matchesTimeStamp(" SEVERE +\\[c\\.e\\.example\\] +message")); } @Test void testFormatRobustAgainstDoubleDot() { this.record.setSourceClassName("com.exasol..example"); final String formattedRecord = this.formatter.format(this.record); - assertThat(formattedRecord, - matchesPattern(this.TIMESTAMP_PATTERN + " SEVERE +\\[c\\.e\\.\\.example\\] +message\\n")); + assertThat(formattedRecord, matchesTimeStamp(" SEVERE +\\[c\\.e\\.\\.example\\] +message")); } @Test void testFormatRobustAgainstEndDot() { this.record.setSourceClassName("com.exasol."); final String formattedRecord = this.formatter.format(this.record); - assertThat(formattedRecord, matchesPattern(this.TIMESTAMP_PATTERN + " SEVERE +\\[c\\.e\\.\\] +message\\n")); + assertThat(formattedRecord, matchesTimeStamp(" SEVERE +\\[c\\.e\\.\\] +message")); } @Test void testFormatRobustAgainstOnlyDot() { this.record.setSourceClassName("."); final String formattedRecord = this.formatter.format(this.record); - assertThat(formattedRecord, matchesPattern(this.TIMESTAMP_PATTERN + " SEVERE +\\[\\.\\] +message\\n")); + assertThat(formattedRecord, matchesTimeStamp(" SEVERE +\\[\\.\\] +message")); } @Test @@ -76,7 +74,7 @@ void testFormatWithPlaceholders() { final LogRecord recordWithPlaceholders = new LogRecord(Level.SEVERE, "message {0} : {1}"); recordWithPlaceholders.setParameters(new String[] { "foo", "bar" }); final String formattedRecord = this.formatter.format(recordWithPlaceholders); - assertThat(formattedRecord, matchesPattern(this.TIMESTAMP_PATTERN + " SEVERE +message foo : bar\\n")); + assertThat(formattedRecord, matchesTimeStamp(" SEVERE +message foo : bar")); } @Test @@ -88,7 +86,7 @@ void testFormatException() { final String formattedRecord = this.formatter.format(this.record); assertAll( () -> assertThat(formattedRecord, - matchesPattern(this.TIMESTAMP_PATTERN + " SEVERE the message(\\n.*)*")), + matchesPattern(TIMESTAMP_PATTERN + " SEVERE the message\\n(.*" + LINEFEED_PATTERN + ")*")), () -> assertThat(formattedRecord, containsString("the exception")), () -> assertThat(formattedRecord, containsString("the cause"))); } diff --git a/src/test/java/com/exasol/logging/RemoteLogManagerTest.java b/src/test/java/com/exasol/logging/RemoteLogManagerTest.java index 5f33ed9b..7e5899a8 100644 --- a/src/test/java/com/exasol/logging/RemoteLogManagerTest.java +++ b/src/test/java/com/exasol/logging/RemoteLogManagerTest.java @@ -1,13 +1,14 @@ package com.exasol.logging; -import static org.hamcrest.Matchers.matchesPattern; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.matchesPattern; import java.io.*; import java.net.*; import java.util.logging.Level; import java.util.logging.Logger; +import org.hamcrest.Matcher; import org.itsallcode.io.Capturable; import org.itsallcode.junit.sysextensions.SystemErrGuard; import org.junit.jupiter.api.*; @@ -15,11 +16,30 @@ @ExtendWith(SystemErrGuard.class) class RemoteLogManagerTest { + private static final String CLASS_TAG = "\\[c\\.e\\.l\\.RemoteLogManagerTest\\]"; - private static final String TIMESTAMP_PATTERN = "\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}.\\d{3}"; + static final String TIMESTAMP_PATTERN = "\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}.\\d{3}"; private static final Logger LOGGER = Logger.getLogger(RemoteLogManagerTest.class.getName()); + static final String LINEFEED_PATTERN = linefeedPattern(); + + static Matcher matchesTimeStamp(final String content) { + return matchesPattern(TIMESTAMP_PATTERN + content + LINEFEED_PATTERN); + } + private RemoteLogManager logManager; + static String linefeedPattern() { + switch (System.lineSeparator()) { + case "\r\n": + return "\\r\\n"; + case "\r": + return "\\r"; + case "\n": // falling through intentionally + default: + return "\\n"; + } + } + @BeforeEach void BeforeEach() { this.logManager = new RemoteLogManager(); @@ -35,7 +55,7 @@ void testSetupConsoleLogging(final Capturable stream) throws IOException { this.logManager.setupConsoleLogger(Level.INFO); stream.capture(); LOGGER.info("Hello."); - assertThat(stream.getCapturedData(), matchesPattern(TIMESTAMP_PATTERN + " INFO +" + CLASS_TAG + " Hello.\\n")); + assertThat(stream.getCapturedData(), matchesTimeStamp(" INFO +" + CLASS_TAG + " Hello.")); } @Test @@ -43,8 +63,7 @@ void testSetupConsoleLoggingWithMoreDetailedLogLevel(final Capturable stream) th this.logManager.setupConsoleLogger(Level.ALL); stream.capture(); LOGGER.finest(() -> "Hello."); - assertThat(stream.getCapturedData(), - matchesPattern(TIMESTAMP_PATTERN + " FINEST +" + CLASS_TAG + " Hello.\\n")); + assertThat(stream.getCapturedData(), matchesTimeStamp(" FINEST +" + CLASS_TAG + " Hello.")); } @Test @@ -103,6 +122,6 @@ private String readLogMessageFromSocket(final Socket socket) throws IOException void testFallBackFromRemoteLoggingToConsoleLogging(final Capturable stream) { stream.capture(); this.logManager.setupRemoteLogger("this.hostname.should.not.exist.exasol.com", 3000, Level.ALL); - assertThat(stream.getCapturedData(), matchesPattern(".*Falling back to console log.\n")); + assertThat(stream.getCapturedData(), matchesPattern(".*Falling back to console log." + System.lineSeparator())); } } \ No newline at end of file