You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by pt...@apache.org on 2022/02/03 17:48:20 UTC

[ignite-3] branch main updated: IGNITE-16461 Thin client: Remove TODOs for completed tickets, enable/add tests (#628)

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

ptupitsyn pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git


The following commit(s) were added to refs/heads/main by this push:
     new 5c96ffd  IGNITE-16461 Thin client: Remove TODOs for completed tickets, enable/add tests (#628)
5c96ffd is described below

commit 5c96ffde5e6b4d8ff2a6929f3077b500f10e8c00
Author: Pavel Tupitsyn <pt...@apache.org>
AuthorDate: Thu Feb 3 20:48:14 2022 +0300

    IGNITE-16461 Thin client: Remove TODOs for completed tickets, enable/add tests (#628)
    
    * Enable `testGetReturningTupleWithUnknownSchemaRequestsNewSchema`.
    * Fix default column value handling in `ClientMarshallerWriter`, add tests.
---
 .../client/table/ClientKeyValueBinaryView.java     |  2 -
 .../client/table/ClientRecordBinaryView.java       |  3 +-
 .../ignite/internal/client/table/ClientSchema.java |  1 -
 .../marshaller/ClientMarshallerWriter.java         |  3 +-
 .../ignite/client/AbstractClientTableTest.java     |  7 ++++
 .../ignite/client/ClientKeyValueViewTest.java      | 44 +++++++++++++++++++++
 .../apache/ignite/client/ClientRecordViewTest.java | 46 ++++++++++++++++++++++
 .../org/apache/ignite/client/ClientTableTest.java  | 17 ++++----
 .../ignite/client/fakes/FakeIgniteTables.java      |  2 +-
 9 files changed, 108 insertions(+), 17 deletions(-)

diff --git a/modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientKeyValueBinaryView.java b/modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientKeyValueBinaryView.java
index c39c7ff..5716373 100644
--- a/modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientKeyValueBinaryView.java
+++ b/modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientKeyValueBinaryView.java
@@ -119,8 +119,6 @@ public class ClientKeyValueBinaryView implements KeyValueView<Tuple, Tuple> {
     public @NotNull CompletableFuture<Void> putAsync(@Nullable Transaction tx, @NotNull Tuple key, Tuple val) {
         Objects.requireNonNull(key);
 
-        // TODO IGNITE-15194: Convert Tuple to a schema-order Array as a first step.
-        // If it does not match the latest schema, then request latest and convert again.
         return tbl.doSchemaOutOpAsync(
                 ClientOp.TUPLE_UPSERT,
                 (s, w) -> ser.writeKvTuple(tx, key, val, s, w, false),
diff --git a/modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientRecordBinaryView.java b/modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientRecordBinaryView.java
index 413d244..a23594d 100644
--- a/modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientRecordBinaryView.java
+++ b/modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientRecordBinaryView.java
@@ -101,8 +101,7 @@ public class ClientRecordBinaryView implements RecordView<Tuple> {
     @Override
     public @NotNull CompletableFuture<Void> upsertAsync(@Nullable Transaction tx, @NotNull Tuple rec) {
         Objects.requireNonNull(rec);
-        // TODO IGNITE-15194: Convert Tuple to a schema-order Array as a first step.
-        // If it does not match the latest schema, then request latest and convert again.
+
         return tbl.doSchemaOutOpAsync(
                 ClientOp.TUPLE_UPSERT,
                 (s, w) -> ser.writeTuple(tx, rec, s, w),
diff --git a/modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientSchema.java b/modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientSchema.java
index db25dcc..d37992f 100644
--- a/modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientSchema.java
+++ b/modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientSchema.java
@@ -165,7 +165,6 @@ public class ClientSchema {
         for (int i = 0; i < colCount; i++) {
             var col = columns[i  + firstColIdx];
 
-            // TODO: Pass default value supplier that indicates "no value" (IGNITE-16093).
             cols[i] = new MarshallerColumn(col.name(), mode(col.type()));
         }
 
diff --git a/modules/client/src/main/java/org/apache/ignite/internal/marshaller/ClientMarshallerWriter.java b/modules/client/src/main/java/org/apache/ignite/internal/marshaller/ClientMarshallerWriter.java
index 248fa06..1bec432 100644
--- a/modules/client/src/main/java/org/apache/ignite/internal/marshaller/ClientMarshallerWriter.java
+++ b/modules/client/src/main/java/org/apache/ignite/internal/marshaller/ClientMarshallerWriter.java
@@ -52,8 +52,7 @@ public class ClientMarshallerWriter implements MarshallerWriter {
     /** {@inheritDoc} */
     @Override
     public void writeAbsentValue() {
-        // TODO: Handle missing values and null values differently (IGNITE-16093).
-        packer.packNil();
+        packer.packNoValue();
     }
 
     /** {@inheritDoc} */
diff --git a/modules/client/src/test/java/org/apache/ignite/client/AbstractClientTableTest.java b/modules/client/src/test/java/org/apache/ignite/client/AbstractClientTableTest.java
index 410855d..0530855 100644
--- a/modules/client/src/test/java/org/apache/ignite/client/AbstractClientTableTest.java
+++ b/modules/client/src/test/java/org/apache/ignite/client/AbstractClientTableTest.java
@@ -213,4 +213,11 @@ public class AbstractClientTableTest extends AbstractClientTest {
         public BigDecimal zdecimal;
         public BigInteger znumber;
     }
+
+    /** Columns with default values. */
+    protected static class DefaultValuesPojo {
+        public int id;
+        public String str;
+        public String strNonNull;
+    }
 }
diff --git a/modules/client/src/test/java/org/apache/ignite/client/ClientKeyValueViewTest.java b/modules/client/src/test/java/org/apache/ignite/client/ClientKeyValueViewTest.java
index d2e1e6d..5dd42a2 100644
--- a/modules/client/src/test/java/org/apache/ignite/client/ClientKeyValueViewTest.java
+++ b/modules/client/src/test/java/org/apache/ignite/client/ClientKeyValueViewTest.java
@@ -449,4 +449,48 @@ public class ClientKeyValueViewTest extends AbstractClientTableTest {
         assertTrue(pojoView.contains(null, DEFAULT_ID));
         assertFalse(pojoView.contains(null, -1L));
     }
+
+    @Test
+    public void testColumnWithDefaultValueNotSetReturnsDefault() {
+        Table table = tableWithDefaultValues();
+        RecordView<Tuple> recordView = table.recordView();
+        KeyValueView<Integer, NamePojo> pojoView = table.keyValueView(Integer.class, NamePojo.class);
+
+        pojoView.put(null, 1, new NamePojo());
+
+        var res = recordView.get(null, tupleKey(1));
+
+        assertEquals("def_str", res.stringValue("str"));
+        assertEquals("def_str2", res.stringValue("strNonNull"));
+    }
+
+    @Test
+    public void testNullableColumnWithDefaultValueSetNullReturnsNull() {
+        Table table = tableWithDefaultValues();
+        RecordView<Tuple> recordView = table.recordView();
+        KeyValueView<Integer, DefaultValuesPojo> pojoView = table.keyValueView(Integer.class, DefaultValuesPojo.class);
+
+        var pojo = new DefaultValuesPojo();
+        pojo.str = null;
+        pojo.strNonNull = "s";
+
+        pojoView.put(null, 1, pojo);
+
+        var res = recordView.get(null, tupleKey(1));
+
+        assertNull(res.stringValue("str"));
+    }
+
+    @Test
+    public void testNonNullableColumnWithDefaultValueSetNullThrowsException() {
+        Table table = tableWithDefaultValues();
+        KeyValueView<Integer, DefaultValuesPojo> pojoView = table.keyValueView(Integer.class, DefaultValuesPojo.class);
+
+        var pojo = new DefaultValuesPojo();
+        pojo.strNonNull = null;
+
+        var ex = assertThrows(IgniteClientException.class, () -> pojoView.put(null, 1, pojo));
+
+        assertTrue(ex.getMessage().contains("null was passed, but column is not nullable"), ex.getMessage());
+    }
 }
diff --git a/modules/client/src/test/java/org/apache/ignite/client/ClientRecordViewTest.java b/modules/client/src/test/java/org/apache/ignite/client/ClientRecordViewTest.java
index d3e65f9..e81a6c0 100644
--- a/modules/client/src/test/java/org/apache/ignite/client/ClientRecordViewTest.java
+++ b/modules/client/src/test/java/org/apache/ignite/client/ClientRecordViewTest.java
@@ -474,4 +474,50 @@ public class ClientRecordViewTest extends AbstractClientTableTest {
         assertEquals("2", pojoView.get(null, new PersonPojo(2L)).name);
         assertNull(pojoView.get(null, new PersonPojo(3L)));
     }
+
+    @Test
+    public void testColumnWithDefaultValueNotSetReturnsDefault() {
+        Table table = tableWithDefaultValues();
+        RecordView<Tuple> recordView = table.recordView();
+        RecordView<PersonPojo> pojoView = table.recordView(PersonPojo.class);
+
+        pojoView.upsert(null, new PersonPojo());
+
+        var res = recordView.get(null, tupleKey(0));
+
+        assertEquals("def_str", res.stringValue("str"));
+        assertEquals("def_str2", res.stringValue("strNonNull"));
+    }
+
+    @Test
+    public void testNullableColumnWithDefaultValueSetNullReturnsNull() {
+        Table table = tableWithDefaultValues();
+        RecordView<Tuple> recordView = table.recordView();
+        RecordView<DefaultValuesPojo> pojoView = table.recordView(DefaultValuesPojo.class);
+
+        var pojo = new DefaultValuesPojo();
+        pojo.id = 1;
+        pojo.str = null;
+        pojo.strNonNull = "s";
+
+        pojoView.upsert(null, pojo);
+
+        var res = recordView.get(null, tupleKey(1));
+
+        assertNull(res.stringValue("str"));
+    }
+
+    @Test
+    public void testNonNullableColumnWithDefaultValueSetNullThrowsException() {
+        Table table = tableWithDefaultValues();
+        RecordView<DefaultValuesPojo> pojoView = table.recordView(DefaultValuesPojo.class);
+
+        var pojo = new DefaultValuesPojo();
+        pojo.id = 1;
+        pojo.strNonNull = null;
+
+        var ex = assertThrows(IgniteClientException.class, () -> pojoView.upsert(null, pojo));
+
+        assertTrue(ex.getMessage().contains("null was passed, but column is not nullable"), ex.getMessage());
+    }
 }
diff --git a/modules/client/src/test/java/org/apache/ignite/client/ClientTableTest.java b/modules/client/src/test/java/org/apache/ignite/client/ClientTableTest.java
index a39f594..1afbdf9 100644
--- a/modules/client/src/test/java/org/apache/ignite/client/ClientTableTest.java
+++ b/modules/client/src/test/java/org/apache/ignite/client/ClientTableTest.java
@@ -27,10 +27,8 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
 import java.util.Arrays;
 import java.util.List;
 import org.apache.ignite.client.fakes.FakeSchemaRegistry;
-import org.apache.ignite.internal.client.table.ClientTuple;
 import org.apache.ignite.table.RecordView;
 import org.apache.ignite.table.Tuple;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 /**
@@ -100,7 +98,6 @@ public class ClientTableTest extends AbstractClientTableTest {
     }
 
     @Test
-    @Disabled("https://issues.apache.org/jira/browse/IGNITE-15194")
     public void testGetReturningTupleWithUnknownSchemaRequestsNewSchema() throws Exception {
         FakeSchemaRegistry.setLastVer(2);
 
@@ -113,11 +110,13 @@ public class ClientTableTest extends AbstractClientTableTest {
 
         try (var client2 = startClient()) {
             RecordView<Tuple> table2 = client2.tables().table(table.name()).recordView();
-            var tuple2 = tuple();
-            var resTuple = table2.get(null, tuple2);
+            var resTuple = table2.get(null, tuple);
 
-            assertEquals(1, ((ClientTuple) tuple2).schema().version());
-            assertEquals(2, ((ClientTuple) resTuple).schema().version());
+            assertEquals(2, tuple.columnCount());
+            assertEquals(3, resTuple.columnCount());
+
+            assertEquals(-1, tuple.columnIndex("XYZ"));
+            assertEquals(2, resTuple.columnIndex("XYZ"));
 
             assertEquals(DEFAULT_NAME, resTuple.stringValue("name"));
             assertEquals(DEFAULT_ID, resTuple.longValue("id"));
@@ -342,7 +341,7 @@ public class ClientTableTest extends AbstractClientTableTest {
         var res = table.get(null, tuple);
 
         assertEquals("def_str", res.stringValue("str"));
-        assertEquals("def_str2", res.stringValue("str_non_null"));
+        assertEquals("def_str2", res.stringValue("strNonNull"));
     }
 
     @Test
@@ -366,7 +365,7 @@ public class ClientTableTest extends AbstractClientTableTest {
 
         var tuple = Tuple.create()
                 .set("id", 1)
-                .set("str_non_null", null);
+                .set("strNonNull", null);
 
         var ex = assertThrows(IgniteClientException.class, () -> table.upsert(null, tuple));
 
diff --git a/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeIgniteTables.java b/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeIgniteTables.java
index 830aa0d..a8c429d 100644
--- a/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeIgniteTables.java
+++ b/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeIgniteTables.java
@@ -240,7 +240,7 @@ public class FakeIgniteTables implements IgniteTables, IgniteTablesInternal {
                 new Column[]{
                         new Column("num".toUpperCase(), NativeTypes.INT8, true, () -> (byte) 42),
                         new Column("str".toUpperCase(), NativeTypes.STRING, true, () -> "def_str"),
-                        new Column("str_non_null".toUpperCase(), NativeTypes.STRING, false, () -> "def_str2"),
+                        new Column("strNonNull".toUpperCase(), NativeTypes.STRING, false, () -> "def_str2"),
                 });
     }