Bug/258 fix data type properties #2

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

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<TableMetadata> involvedTables = parseInvolvedTables(root);
-        final List<DataType> dataTypes = parseDataTypes(root);
+        final SqlStatement statement = parsePushdownStatement(pushdownJson, involvedTables);
+        final List<DataType> dataTypes = parseDataTypes(pushdownJson);
         return new PushDownRequest(metadataInfo, statement, involvedTables, dataTypes);
     }
 
+    private SqlStatement parsePushdownStatement(final JsonObject pushdownJson,
+            final List<TableMetadata> involvedTables) {
+        final PushdownSqlParser pushdownSqlParser = PushdownSqlParser.createWithTablesMetadata(involvedTables);
+        return (SqlStatement) pushdownSqlParser.parseExpression(pushdownJson);
+    }
+
     private List<DataType> 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<TableMetadata> 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}
      *

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<TableMetadata> involvedTables = ((PushDownRequest) request).getInvolvedTablesMetadata();
         final List<DataType> 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<DataType> 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( //

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,59 +24,57 @@ 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
     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")));
     }

src/test/java/com/exasol/logging/RemoteLogManagerTest.java

@@ -1,25 +1,45 @@
 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.*;
 import org.junit.jupiter.api.extension.ExtendWith;
 
 @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<String> 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,16 +55,15 @@ 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
     void testSetupConsoleLoggingWithMoreDetailedLogLevel(final Capturable stream) throws IOException {
         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()));
     }
 }