sergey-chugunov-1985 commented on code in PR #12204: URL: https://github.com/apache/ignite/pull/12204#discussion_r2297873553
########## modules/core/src/test/resources/codegen/TestMapMessage.java: ########## @@ -0,0 +1,290 @@ +/* + * 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.util.BitSet; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.UUID; +import org.apache.ignite.internal.processors.affinity.AffinityTopologyVersion; +import org.apache.ignite.internal.processors.cache.version.GridCacheVersion; +import org.apache.ignite.lang.IgniteUuid; +import org.apache.ignite.plugin.extensions.communication.Message; + +public class TestMapMessage implements Message { Review Comment: What makes me uncomfortable about this test is that in all tested instances key type is always the same as value type. With this test we could mix up keyType parameter and valueType parameter in MessageSerializerGenerator and this test won't catch it. To fix it lets do the following: shift all value type parameters in all fields by one. So the first field will be `Map<boolean[], Long> boolArrayLongObjMap`, the second field will become `Map<byte[], boolean[]> byteArrayBoolArrayMap` and so on. ########## modules/core/src/main/java/org/apache/ignite/internal/GridDirectMap.java: ########## @@ -24,9 +24,11 @@ /** * Annotates map fields. + * Deprecated, see {@link Order} and {@link MessageProcessor} for details. Review Comment: Please rephrase this line in the following way: `Deprecated as not needed for new messages. See {@link Order} and {@link MessageProcessor} for details of how message serizalization code is generated`. ########## modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/message/CalciteMessageFactory.java: ########## @@ -28,8 +28,12 @@ public class CalciteMessageFactory implements MessageFactoryProvider { /** {@inheritDoc} */ @SuppressWarnings({"unchecked", "rawtypes"}) @Override public void registerAll(MessageFactory factory) { - for (MessageType type : MessageType.values()) - factory.register(type.directType(), (Supplier)type.factory()); + for (MessageType type : MessageType.values()) { + if (type.serializer() != null) Review Comment: Please add a comment that this check is temporary and will be removed when all messages are migrated to new serialization framework. ########## modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/metadata/FragmentDescription.java: ########## @@ -17,41 +17,37 @@ package org.apache.ignite.internal.processors.query.calcite.metadata; -import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.UUID; -import org.apache.ignite.internal.GridDirectMap; -import org.apache.ignite.internal.GridDirectTransient; +import org.apache.ignite.internal.Order; import org.apache.ignite.internal.processors.cache.GridCacheSharedContext; import org.apache.ignite.internal.processors.query.calcite.message.MarshalableMessage; import org.apache.ignite.internal.processors.query.calcite.message.MessageType; import org.apache.ignite.internal.util.UUIDCollectionMessage; import org.apache.ignite.internal.util.typedef.internal.U; -import org.apache.ignite.plugin.extensions.communication.Message; -import org.apache.ignite.plugin.extensions.communication.MessageCollectionItemType; -import org.apache.ignite.plugin.extensions.communication.MessageReader; -import org.apache.ignite.plugin.extensions.communication.MessageWriter; /** */ public class FragmentDescription implements MarshalableMessage { /** */ + @Order(0) private long fragmentId; /** */ + @Order(1) private FragmentMapping mapping; /** */ - private ColocationGroup target; + @Order(2) + private Map<Long, UUIDCollectionMessage> remoteSources0; Review Comment: Please add a TODO to remove this field (and MarshallableMessage interface I guess) when https://issues.apache.org/jira/browse/IGNITE-26133 is done. -- 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: notifications-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
