Copilot commented on code in PR #13758: URL: https://github.com/apache/ignite/pull/13758#discussion_r4410869087
########## modules/core/src/test/java/org/apache/ignite/internal/IgniteClientReconnectCustomBinaryContextTest.java: ########## @@ -0,0 +1,287 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal; + +import java.io.IOException; +import java.lang.reflect.Field; +import java.time.Duration; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.temporal.Temporal; +import java.util.Arrays; +import java.util.concurrent.atomic.AtomicBoolean; +import org.apache.ignite.Ignite; +import org.apache.ignite.IgniteCheckedException; +import org.apache.ignite.IgniteClientDisconnectedException; +import org.apache.ignite.binary.BinaryBasicIdMapper; +import org.apache.ignite.binary.BinaryBasicNameMapper; +import org.apache.ignite.binary.BinaryObjectException; +import org.apache.ignite.binary.BinaryReader; +import org.apache.ignite.binary.BinarySerializer; +import org.apache.ignite.binary.BinaryTypeConfiguration; +import org.apache.ignite.binary.BinaryWriter; +import org.apache.ignite.cache.CacheAtomicityMode; +import org.apache.ignite.cache.CacheMode; +import org.apache.ignite.cache.CacheWriteSynchronizationMode; +import org.apache.ignite.configuration.BinaryConfiguration; +import org.apache.ignite.configuration.CacheConfiguration; +import org.apache.ignite.configuration.IgniteConfiguration; +import org.apache.ignite.spi.discovery.tcp.TcpDiscoveryIoSession; +import org.apache.ignite.spi.discovery.tcp.TcpDiscoverySpi; +import org.apache.ignite.spi.discovery.tcp.ipfinder.TcpDiscoveryIpFinder; +import org.apache.ignite.spi.discovery.tcp.messages.TcpDiscoveryAbstractMessage; +import org.apache.ignite.testframework.GridTestUtils; +import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest; +import org.junit.Test; +import org.springframework.util.ReflectionUtils; + +import static java.time.temporal.ChronoField.DAY_OF_MONTH; +import static java.time.temporal.ChronoField.HOUR_OF_DAY; +import static java.time.temporal.ChronoField.MINUTE_OF_HOUR; +import static java.time.temporal.ChronoField.MONTH_OF_YEAR; +import static java.time.temporal.ChronoField.NANO_OF_SECOND; +import static java.time.temporal.ChronoField.SECOND_OF_MINUTE; +import static java.time.temporal.ChronoField.YEAR; + +/** + * Test binary configuration restored after client reconnect. + */ +public class IgniteClientReconnectCustomBinaryContextTest extends GridCommonAbstractTest { + /** */ + public static final String CACHE_NAME = "Test"; + + /** */ + private final AtomicBoolean block = new AtomicBoolean(false); + + /** */ + @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception { + IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName); + + cfg.setBinaryConfiguration( + new BinaryConfiguration() + .setTypeConfigurations( + Arrays.asList( + new BinaryTypeConfiguration(DateKey.class.getName()) + .setIdMapper(new BinaryBasicIdMapper(true)) + .setNameMapper(new BinaryBasicNameMapper(true)), + new BinaryTypeConfiguration("java.time.Ser") + .setSerializer(new LocalDateBinarySerializer()) + ) + ) + ); + + if (igniteInstanceName.equals("client")) { + TcpDiscoverySpi discoverySpi = (TcpDiscoverySpi)cfg.getDiscoverySpi(); + TcpDiscoveryIpFinder ipFinder = discoverySpi.getIpFinder(); + + BlockingDiscoverySpi clientDiscoverySpi = new BlockingDiscoverySpi(block); + clientDiscoverySpi.setIpFinder(ipFinder); + + cfg.setDiscoverySpi(clientDiscoverySpi); + + cfg.setFailureDetectionTimeout(30000); + cfg.setClientFailureDetectionTimeout(30000); + } + + return cfg; + } + + /** */ + @Test + public void testReconnect() throws Exception { + IgniteEx srv1 = startGrid(0); + IgniteEx client = startClientGrid("client"); + + srv1.getOrCreateCache(getCacheConfiguration()); + + DateKey storedKey = new DateKey(LocalDate.now()); + String val = "Hello"; + + // Put value to cache and ensure it is visible from client and server. + srv1.cache(CACHE_NAME).put(storedKey, val); + assertEquals(val, srv1.cache(CACHE_NAME).get(storedKey)); + assertEquals(val, client.cache(CACHE_NAME).get(storedKey)); + + // Wait for client disconnected. + block.set(true); + assertTrue(GridTestUtils.waitForCondition(() -> { + try { + client.cache(CACHE_NAME).get(storedKey); + } + catch (IgniteClientDisconnectedException ex) { + return true; + } + return false; + }, getTestTimeout())); + + assertTrue(client.context().clientDisconnected()); + block.set(false); + + // Wait for client to reconnect. + try { + client.cache(CACHE_NAME); + } + catch (IgniteClientDisconnectedException ex) { + ex.reconnectFuture().get(); + } + + // Ensure value is still there. + assertEquals(value, client.cache(CACHE_NAME).get(storedKey)); + + client.close(); + + Ignite client1 = startClientGrid("client"); + assertEquals(value, client1.cache(CACHE_NAME).get(storedKey)); Review Comment: The test defines `val` but later asserts against `value`, which is undefined and will fail compilation. Replace `value` with `val` in both assertions. ########## modules/core/src/test/java/org/apache/ignite/internal/IgniteClientReconnectCustomBinaryContextTest.java: ########## @@ -0,0 +1,287 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal; + +import java.io.IOException; +import java.lang.reflect.Field; +import java.time.Duration; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.temporal.Temporal; +import java.util.Arrays; +import java.util.concurrent.atomic.AtomicBoolean; +import org.apache.ignite.Ignite; +import org.apache.ignite.IgniteCheckedException; +import org.apache.ignite.IgniteClientDisconnectedException; +import org.apache.ignite.binary.BinaryBasicIdMapper; +import org.apache.ignite.binary.BinaryBasicNameMapper; +import org.apache.ignite.binary.BinaryObjectException; +import org.apache.ignite.binary.BinaryReader; +import org.apache.ignite.binary.BinarySerializer; +import org.apache.ignite.binary.BinaryTypeConfiguration; +import org.apache.ignite.binary.BinaryWriter; +import org.apache.ignite.cache.CacheAtomicityMode; +import org.apache.ignite.cache.CacheMode; +import org.apache.ignite.cache.CacheWriteSynchronizationMode; +import org.apache.ignite.configuration.BinaryConfiguration; +import org.apache.ignite.configuration.CacheConfiguration; +import org.apache.ignite.configuration.IgniteConfiguration; +import org.apache.ignite.spi.discovery.tcp.TcpDiscoveryIoSession; +import org.apache.ignite.spi.discovery.tcp.TcpDiscoverySpi; +import org.apache.ignite.spi.discovery.tcp.ipfinder.TcpDiscoveryIpFinder; +import org.apache.ignite.spi.discovery.tcp.messages.TcpDiscoveryAbstractMessage; +import org.apache.ignite.testframework.GridTestUtils; +import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest; +import org.junit.Test; +import org.springframework.util.ReflectionUtils; + +import static java.time.temporal.ChronoField.DAY_OF_MONTH; +import static java.time.temporal.ChronoField.HOUR_OF_DAY; +import static java.time.temporal.ChronoField.MINUTE_OF_HOUR; +import static java.time.temporal.ChronoField.MONTH_OF_YEAR; +import static java.time.temporal.ChronoField.NANO_OF_SECOND; +import static java.time.temporal.ChronoField.SECOND_OF_MINUTE; +import static java.time.temporal.ChronoField.YEAR; + +/** + * Test binary configuration restored after client reconnect. + */ +public class IgniteClientReconnectCustomBinaryContextTest extends GridCommonAbstractTest { + /** */ + public static final String CACHE_NAME = "Test"; + + /** */ + private final AtomicBoolean block = new AtomicBoolean(false); + + /** */ + @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception { + IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName); + + cfg.setBinaryConfiguration( + new BinaryConfiguration() + .setTypeConfigurations( + Arrays.asList( + new BinaryTypeConfiguration(DateKey.class.getName()) + .setIdMapper(new BinaryBasicIdMapper(true)) + .setNameMapper(new BinaryBasicNameMapper(true)), + new BinaryTypeConfiguration("java.time.Ser") + .setSerializer(new LocalDateBinarySerializer()) + ) + ) + ); + + if (igniteInstanceName.equals("client")) { + TcpDiscoverySpi discoverySpi = (TcpDiscoverySpi)cfg.getDiscoverySpi(); + TcpDiscoveryIpFinder ipFinder = discoverySpi.getIpFinder(); + + BlockingDiscoverySpi clientDiscoverySpi = new BlockingDiscoverySpi(block); + clientDiscoverySpi.setIpFinder(ipFinder); + + cfg.setDiscoverySpi(clientDiscoverySpi); + + cfg.setFailureDetectionTimeout(30000); + cfg.setClientFailureDetectionTimeout(30000); + } + + return cfg; + } + + /** */ + @Test + public void testReconnect() throws Exception { + IgniteEx srv1 = startGrid(0); + IgniteEx client = startClientGrid("client"); + + srv1.getOrCreateCache(getCacheConfiguration()); + + DateKey storedKey = new DateKey(LocalDate.now()); + String val = "Hello"; Review Comment: The test defines `val` but later asserts against `value`, which is undefined and will fail compilation. Replace `value` with `val` in both assertions. ########## modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryMetadata.java: ########## @@ -76,6 +76,10 @@ public class BinaryMetadata implements Externalizable { /** Enum ordinal to name mapping. */ private Map<Integer, String> ordinalToName; + /** Flag {@code true} if for optimization needs, already known object fields are not pushed into + * {@link #fields} structure. */ + private boolean partialFields; Review Comment: `BinaryMetadata` implements `Externalizable`, and this PR adds a new stateful field (`partialFields`). To avoid losing this state across serialization (e.g., inter-node communication, persistence, or any marshalling paths that rely on `Externalizable`), ensure `writeExternal`/`readExternal` are updated to include this field in a backward-compatible way (e.g., by appending and handling older streams safely). ########## modules/core/src/test/java/org/apache/ignite/internal/IgniteClientReconnectCustomBinaryContextTest.java: ########## @@ -0,0 +1,287 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal; + +import java.io.IOException; +import java.lang.reflect.Field; +import java.time.Duration; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.temporal.Temporal; +import java.util.Arrays; +import java.util.concurrent.atomic.AtomicBoolean; +import org.apache.ignite.Ignite; +import org.apache.ignite.IgniteCheckedException; +import org.apache.ignite.IgniteClientDisconnectedException; +import org.apache.ignite.binary.BinaryBasicIdMapper; +import org.apache.ignite.binary.BinaryBasicNameMapper; +import org.apache.ignite.binary.BinaryObjectException; +import org.apache.ignite.binary.BinaryReader; +import org.apache.ignite.binary.BinarySerializer; +import org.apache.ignite.binary.BinaryTypeConfiguration; +import org.apache.ignite.binary.BinaryWriter; +import org.apache.ignite.cache.CacheAtomicityMode; +import org.apache.ignite.cache.CacheMode; +import org.apache.ignite.cache.CacheWriteSynchronizationMode; +import org.apache.ignite.configuration.BinaryConfiguration; +import org.apache.ignite.configuration.CacheConfiguration; +import org.apache.ignite.configuration.IgniteConfiguration; +import org.apache.ignite.spi.discovery.tcp.TcpDiscoveryIoSession; +import org.apache.ignite.spi.discovery.tcp.TcpDiscoverySpi; +import org.apache.ignite.spi.discovery.tcp.ipfinder.TcpDiscoveryIpFinder; +import org.apache.ignite.spi.discovery.tcp.messages.TcpDiscoveryAbstractMessage; +import org.apache.ignite.testframework.GridTestUtils; +import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest; +import org.junit.Test; +import org.springframework.util.ReflectionUtils; + +import static java.time.temporal.ChronoField.DAY_OF_MONTH; +import static java.time.temporal.ChronoField.HOUR_OF_DAY; +import static java.time.temporal.ChronoField.MINUTE_OF_HOUR; +import static java.time.temporal.ChronoField.MONTH_OF_YEAR; +import static java.time.temporal.ChronoField.NANO_OF_SECOND; +import static java.time.temporal.ChronoField.SECOND_OF_MINUTE; +import static java.time.temporal.ChronoField.YEAR; + +/** + * Test binary configuration restored after client reconnect. + */ +public class IgniteClientReconnectCustomBinaryContextTest extends GridCommonAbstractTest { + /** */ + public static final String CACHE_NAME = "Test"; + + /** */ + private final AtomicBoolean block = new AtomicBoolean(false); + + /** */ + @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception { + IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName); + + cfg.setBinaryConfiguration( + new BinaryConfiguration() + .setTypeConfigurations( + Arrays.asList( + new BinaryTypeConfiguration(DateKey.class.getName()) + .setIdMapper(new BinaryBasicIdMapper(true)) + .setNameMapper(new BinaryBasicNameMapper(true)), + new BinaryTypeConfiguration("java.time.Ser") + .setSerializer(new LocalDateBinarySerializer()) + ) + ) + ); + + if (igniteInstanceName.equals("client")) { + TcpDiscoverySpi discoverySpi = (TcpDiscoverySpi)cfg.getDiscoverySpi(); + TcpDiscoveryIpFinder ipFinder = discoverySpi.getIpFinder(); + + BlockingDiscoverySpi clientDiscoverySpi = new BlockingDiscoverySpi(block); + clientDiscoverySpi.setIpFinder(ipFinder); + + cfg.setDiscoverySpi(clientDiscoverySpi); + + cfg.setFailureDetectionTimeout(30000); + cfg.setClientFailureDetectionTimeout(30000); + } + + return cfg; + } + + /** */ + @Test + public void testReconnect() throws Exception { + IgniteEx srv1 = startGrid(0); + IgniteEx client = startClientGrid("client"); + + srv1.getOrCreateCache(getCacheConfiguration()); + + DateKey storedKey = new DateKey(LocalDate.now()); + String val = "Hello"; + + // Put value to cache and ensure it is visible from client and server. + srv1.cache(CACHE_NAME).put(storedKey, val); + assertEquals(val, srv1.cache(CACHE_NAME).get(storedKey)); + assertEquals(val, client.cache(CACHE_NAME).get(storedKey)); + + // Wait for client disconnected. + block.set(true); + assertTrue(GridTestUtils.waitForCondition(() -> { + try { + client.cache(CACHE_NAME).get(storedKey); + } + catch (IgniteClientDisconnectedException ex) { + return true; + } + return false; + }, getTestTimeout())); + + assertTrue(client.context().clientDisconnected()); + block.set(false); + + // Wait for client to reconnect. + try { + client.cache(CACHE_NAME); + } + catch (IgniteClientDisconnectedException ex) { + ex.reconnectFuture().get(); + } + + // Ensure value is still there. + assertEquals(value, client.cache(CACHE_NAME).get(storedKey)); + + client.close(); + + Ignite client1 = startClientGrid("client"); + assertEquals(value, client1.cache(CACHE_NAME).get(storedKey)); Review Comment: The test defines `val` but later asserts against `value`, which is undefined and will fail compilation. Replace `value` with `val` in both assertions. ########## modules/core/src/test/java/org/apache/ignite/internal/IgniteClientReconnectCustomBinaryContextTest.java: ########## @@ -0,0 +1,287 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal; + +import java.io.IOException; +import java.lang.reflect.Field; +import java.time.Duration; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.temporal.Temporal; +import java.util.Arrays; +import java.util.concurrent.atomic.AtomicBoolean; +import org.apache.ignite.Ignite; +import org.apache.ignite.IgniteCheckedException; +import org.apache.ignite.IgniteClientDisconnectedException; +import org.apache.ignite.binary.BinaryBasicIdMapper; +import org.apache.ignite.binary.BinaryBasicNameMapper; +import org.apache.ignite.binary.BinaryObjectException; +import org.apache.ignite.binary.BinaryReader; +import org.apache.ignite.binary.BinarySerializer; +import org.apache.ignite.binary.BinaryTypeConfiguration; +import org.apache.ignite.binary.BinaryWriter; +import org.apache.ignite.cache.CacheAtomicityMode; +import org.apache.ignite.cache.CacheMode; +import org.apache.ignite.cache.CacheWriteSynchronizationMode; +import org.apache.ignite.configuration.BinaryConfiguration; +import org.apache.ignite.configuration.CacheConfiguration; +import org.apache.ignite.configuration.IgniteConfiguration; +import org.apache.ignite.spi.discovery.tcp.TcpDiscoveryIoSession; +import org.apache.ignite.spi.discovery.tcp.TcpDiscoverySpi; +import org.apache.ignite.spi.discovery.tcp.ipfinder.TcpDiscoveryIpFinder; +import org.apache.ignite.spi.discovery.tcp.messages.TcpDiscoveryAbstractMessage; +import org.apache.ignite.testframework.GridTestUtils; +import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest; +import org.junit.Test; +import org.springframework.util.ReflectionUtils; + +import static java.time.temporal.ChronoField.DAY_OF_MONTH; +import static java.time.temporal.ChronoField.HOUR_OF_DAY; +import static java.time.temporal.ChronoField.MINUTE_OF_HOUR; +import static java.time.temporal.ChronoField.MONTH_OF_YEAR; +import static java.time.temporal.ChronoField.NANO_OF_SECOND; +import static java.time.temporal.ChronoField.SECOND_OF_MINUTE; +import static java.time.temporal.ChronoField.YEAR; + +/** + * Test binary configuration restored after client reconnect. + */ +public class IgniteClientReconnectCustomBinaryContextTest extends GridCommonAbstractTest { + /** */ + public static final String CACHE_NAME = "Test"; + + /** */ + private final AtomicBoolean block = new AtomicBoolean(false); + + /** */ + @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception { + IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName); + + cfg.setBinaryConfiguration( + new BinaryConfiguration() + .setTypeConfigurations( + Arrays.asList( + new BinaryTypeConfiguration(DateKey.class.getName()) + .setIdMapper(new BinaryBasicIdMapper(true)) + .setNameMapper(new BinaryBasicNameMapper(true)), + new BinaryTypeConfiguration("java.time.Ser") + .setSerializer(new LocalDateBinarySerializer()) + ) + ) + ); + + if (igniteInstanceName.equals("client")) { + TcpDiscoverySpi discoverySpi = (TcpDiscoverySpi)cfg.getDiscoverySpi(); + TcpDiscoveryIpFinder ipFinder = discoverySpi.getIpFinder(); + + BlockingDiscoverySpi clientDiscoverySpi = new BlockingDiscoverySpi(block); + clientDiscoverySpi.setIpFinder(ipFinder); + + cfg.setDiscoverySpi(clientDiscoverySpi); + + cfg.setFailureDetectionTimeout(30000); + cfg.setClientFailureDetectionTimeout(30000); + } + + return cfg; + } + + /** */ + @Test + public void testReconnect() throws Exception { + IgniteEx srv1 = startGrid(0); + IgniteEx client = startClientGrid("client"); + + srv1.getOrCreateCache(getCacheConfiguration()); + + DateKey storedKey = new DateKey(LocalDate.now()); + String val = "Hello"; + + // Put value to cache and ensure it is visible from client and server. + srv1.cache(CACHE_NAME).put(storedKey, val); + assertEquals(val, srv1.cache(CACHE_NAME).get(storedKey)); + assertEquals(val, client.cache(CACHE_NAME).get(storedKey)); + + // Wait for client disconnected. + block.set(true); + assertTrue(GridTestUtils.waitForCondition(() -> { + try { + client.cache(CACHE_NAME).get(storedKey); + } + catch (IgniteClientDisconnectedException ex) { + return true; + } + return false; + }, getTestTimeout())); + + assertTrue(client.context().clientDisconnected()); + block.set(false); + + // Wait for client to reconnect. + try { + client.cache(CACHE_NAME); + } + catch (IgniteClientDisconnectedException ex) { + ex.reconnectFuture().get(); + } + + // Ensure value is still there. + assertEquals(value, client.cache(CACHE_NAME).get(storedKey)); + + client.close(); + + Ignite client1 = startClientGrid("client"); + assertEquals(value, client1.cache(CACHE_NAME).get(storedKey)); + } + + /** */ + private static CacheConfiguration<DateKey, String> getCacheConfiguration() { + CacheConfiguration<DateKey, String> ccfg = new CacheConfiguration<>(); + + ccfg.setName(CACHE_NAME); + ccfg.setWriteSynchronizationMode(CacheWriteSynchronizationMode.FULL_SYNC); + ccfg.setAtomicityMode(CacheAtomicityMode.ATOMIC); + ccfg.setTypes(DateKey.class, String.class); + ccfg.setIndexedTypes(DateKey.class, String.class); + ccfg.setCacheMode(CacheMode.PARTITIONED); + + return ccfg; + } + + /** */ + public static class DateKey { + /** */ + LocalDate date; + + /** */ + public DateKey(LocalDate date) { + this.date = date; + } + } + + /** */ + public static class LocalDateBinarySerializer implements BinarySerializer { + /** */ + private static final byte DURATION_TYPE = 1; + + /** */ + private static final byte LOCAL_DATE_TYPE = 3; + + /** */ + private static final byte LOCAL_DATE_TIME_TYPE = 5; + + /** */ + private final Field objectField; + + /** */ + private final Field typeField; + + /** */ + public LocalDateBinarySerializer() { + try { + Class<?> cls = Class.forName("java.time.Ser"); + objectField = ReflectionUtils.findField(cls, "object"); + objectField.setAccessible(true); + + typeField = ReflectionUtils.findField(cls, "type"); + typeField.setAccessible(true); + } + catch (ClassNotFoundException e) { + throw new RuntimeException(e); + } + } + + @Override public void writeBinary(Object obj, BinaryWriter writer) throws BinaryObjectException { + Byte type = (Byte)ReflectionUtils.getField(typeField, obj); + writer.writeByte("type", type); + if (type == DURATION_TYPE) { + Duration duration = (Duration)ReflectionUtils.getField(objectField, obj); + writer.writeString("duration", duration != null ? duration.toString() : null); + } + else { + Temporal date = (Temporal)ReflectionUtils.getField(objectField, obj); + writer.writeInt("year", date.get(YEAR)); + writer.writeInt("month", date.get(MONTH_OF_YEAR)); + writer.writeInt("day", date.get(DAY_OF_MONTH)); + if (type == LOCAL_DATE_TIME_TYPE) { + writer.writeInt("hour", date.get(HOUR_OF_DAY)); + writer.writeInt("minute", date.get(MINUTE_OF_HOUR)); + writer.writeInt("second", date.get(SECOND_OF_MINUTE)); + writer.writeInt("nano", date.get(NANO_OF_SECOND)); + } + } + } + + /** */ + @Override public void readBinary(Object obj, BinaryReader reader) throws BinaryObjectException { + byte type = reader.readByte("type"); + + if (type == DURATION_TYPE) { + String str = reader.readString("duration"); + Duration duration = str != null ? Duration.parse(str) : null; + ReflectionUtils.setField(objectField, obj, duration); + + } + else { + int year = reader.readInt("year"); + int month = reader.readInt("month"); + int day = reader.readInt("day"); + + LocalDate date = LocalDate.of(year, month, day); + if (type == LOCAL_DATE_TYPE) { + ReflectionUtils.setField(objectField, obj, date); + } + else { + LocalTime time = LocalTime.of( + reader.readInt("hour"), + reader.readInt("minute"), + reader.readInt("second"), + reader.readInt("nano") + ); + LocalDateTime dateTime = LocalDateTime.of(date, time); + ReflectionUtils.setField(objectField, obj, dateTime); + } + } + } + } + + /** + * Discovery SPI, that makes a node stop to processing a messages when {@code block} is set to {@code true}. Review Comment: The comment has grammatical issues and is a bit unclear. Consider rewording to something like: “Discovery SPI that stops processing messages when `block` is set to `true`.” ########## modules/core/src/test/java/org/apache/ignite/internal/client/thin/BinaryMetadataChannelFailureTest.java: ########## @@ -0,0 +1,178 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.internal.client.thin; + +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import org.apache.ignite.Ignition; +import org.apache.ignite.binary.BinaryBasicIdMapper; +import org.apache.ignite.binary.BinaryIdMapper; +import org.apache.ignite.binary.BinaryObject; +import org.apache.ignite.binary.BinaryObjectBuilder; +import org.apache.ignite.client.ClientCache; +import org.apache.ignite.client.IgniteClient; +import org.apache.ignite.configuration.BinaryConfiguration; +import org.apache.ignite.configuration.ClientConfiguration; +import org.apache.ignite.internal.IgniteEx; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +/** + * Checks the thin client binary metadata cache when a channel fails while a binary object is being built. + */ +@RunWith(Parameterized.class) +public class BinaryMetadataChannelFailureTest extends AbstractThinClientTest { + /** Type name. */ + private static final String TYPE_NAME = "TestType"; + + /** Field names. */ + private static final List<String> FIELDS = Arrays.asList("f0", "f1", "f2"); + + /** Field names. */ + private static final List<String> FIELDS_NEW = Arrays.asList("f0", "f1", "f2", "f3"); + + /** Field whose ID resolution triggers the channel failure. */ + private static final String HOOK_FIELD = "f1"; + + /** If {@code true} put a different value. */ + @Parameterized.Parameter(0) + public Boolean differentValue; + + /** Parameters. */ + @Parameterized.Parameters(name = "differentValue = {0}") + public static Collection<?> parameters() { + return List.of(true, false); + } + + /** {@inheritDoc} */ + @Override protected void afterTest() throws Exception { + super.afterTest(); + + stopAllGrids(); + } + + /** + * Channel failure happens after the builder has read the cached type metadata, but before the new schema is + * registered. The client must not end up with a cached type that lacks the fields of the registered schema. + * + * @throws Exception If failed. + */ + @Test + public void testChannelFailureDuringBuild() throws Exception { + IgniteEx srv = startGrid(0); + + HookIdMapper idMapper = new HookIdMapper(); + + ClientConfiguration cfg = getClientConfiguration(srv) + .setBinaryConfiguration(new BinaryConfiguration().setIdMapper(idMapper)); + + try (IgniteClient client = Ignition.startClient(cfg)) { + ClientCache<Integer, BinaryObject> cache = client.getOrCreateCache(DEFAULT_CACHE_NAME).withKeepBinary(); + + BinaryObjectBuilder builder = builder(client, FIELDS); + + // Type with all fields is registered and cached by the client. + cache.put(0, builder.build()); + + assertTrue(cache.get(0).type().fieldNames().containsAll(FIELDS)); + + idMapper.hook = () -> { + dropAllThinClientConnections(srv); + + // The next request detects the broken channel and runs the channel failure listeners. + client.cacheNames(); + }; + + if (differentValue) + builder = builder(client, FIELDS_NEW); + + cache.put(1, builder.build()); + + assertNull("Channel failure hook was not triggered", idMapper.hook); + + BinaryObject obj = cache.get(1); + + List<String> ethalon = differentValue ? FIELDS_NEW : FIELDS; + + for (String field : ethalon) + assertEquals(field, obj.<String>field(field)); + + assertTrue(srv.binary().type(TYPE_NAME).fieldNames().containsAll(ethalon)); + + Collection<String> clientFields = obj.type().fieldNames(); + + assertTrue("Client type metadata lacks fields of the object schema: " + clientFields, + clientFields.containsAll(ethalon)); + } + } + + /** + * @param client Client. + * @return Builder with all fields set. + */ + private static BinaryObjectBuilder builder(IgniteClient client, List<String> fields) { + BinaryObjectBuilder builder = client.binary().builder(TYPE_NAME); + + for (String field : fields) + builder.setField(field, field, String.class); + + return builder; + } + + /** Use it in tests. */ + private static BinaryObjectBuilder newBuilder1(IgniteClient client) { + BinaryObjectBuilder builder = client.binary().builder(TYPE_NAME); + + for (String field : FIELDS_NEW) + builder.setField(field, field, String.class); + + return builder; + } + Review Comment: This helper (`newBuilder1`) is not referenced anywhere in this new test file and duplicates the existing `builder(...)` helper. Removing it (or using it in the test) will reduce dead code and keep the test intent clearer. ########## modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryMetadata.java: ########## @@ -98,6 +102,27 @@ public BinaryMetadata(int typeId, String typeName, @Nullable Map<String, BinaryF this(typeId, typeName, fields, affKeyFieldName, null, isEnum, enumMap); } + /** + * Constructor. + * + * @param typeId Type ID. + * @param typeName Type name. + * @param fields Fields map. + * @param affKeyFieldName Affinity key field name. + * @param schemas Schemas. + * @param isEnum Enum flag. + * @param enumMap Enum name to ordinal mapping. + * @param partialFields Flag {@code true} if for optimization needs, already known object fields are not pushed into + * {@link #fields} structure. + */ + BinaryMetadata(int typeId, String typeName, @Nullable Map<String, BinaryFieldMetadata> fields, + @Nullable String affKeyFieldName, @Nullable Collection<BinarySchema> schemas, boolean isEnum, + @Nullable Map<String, Integer> enumMap, boolean partialFields) { + this(typeId, typeName, fields, affKeyFieldName, schemas, isEnum, enumMap); + + this.partialFields = partialFields; + } Review Comment: `BinaryMetadata` implements `Externalizable`, and this PR adds a new stateful field (`partialFields`). To avoid losing this state across serialization (e.g., inter-node communication, persistence, or any marshalling paths that rely on `Externalizable`), ensure `writeExternal`/`readExternal` are updated to include this field in a backward-compatible way (e.g., by appending and handling older streams safely). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
