anton-vinogradov commented on code in PR #13095:
URL: https://github.com/apache/ignite/pull/13095#discussion_r3564819814
##########
modules/core/src/main/java/org/apache/ignite/internal/managers/communication/IgniteMessageFactoryImpl.java:
##########
@@ -69,15 +79,38 @@ public IgniteMessageFactoryImpl(MessageFactoryProvider[]
factories) {
initialized = true;
}
- /** {@inheritDoc} */
- @Override public void register(short directType, Supplier<Message>
supplier, MessageSerializer serializer) throws IgniteException {
+ /**
+ * Registers a message type with a serializer, an optional marshaller, and
an optional deployer.
+ *
+ * @param directType Direct type.
+ * @param supplier Message factory.
+ * @param serializer Message serializer.
+ * @param marshaller Message marshaller, or {@code null} for
NonMarshallableMessage types.
Review Comment:
Done.
##########
modules/core/src/main/java/org/apache/ignite/internal/managers/communication/IgniteMessageFactoryImpl.java:
##########
@@ -69,15 +79,38 @@ public IgniteMessageFactoryImpl(MessageFactoryProvider[]
factories) {
initialized = true;
}
- /** {@inheritDoc} */
- @Override public void register(short directType, Supplier<Message>
supplier, MessageSerializer serializer) throws IgniteException {
+ /**
+ * Registers a message type with a serializer, an optional marshaller, and
an optional deployer.
+ *
+ * @param directType Direct type.
+ * @param supplier Message factory.
+ * @param serializer Message serializer.
+ * @param marshaller Message marshaller, or {@code null} for
NonMarshallableMessage types.
+ * @param deployer Message deployer, or {@code null} for messages
without deployable fields.
+ */
+ @Override public void register(short directType, Supplier<Message>
supplier, MessageSerializer serializer,
+ @Nullable MessageMarshaller marshaller, @Nullable
GridCacheMessageDeployer deployer) throws IgniteException {
if (initialized) {
throw new IllegalStateException("Message factory is already
initialized. " +
"Registration of new message types is forbidden.");
}
try {
- supplier.get().registerAsDirectType(directType);
+ Message msg = supplier.get();
+
+ if (marshaller == null && msg instanceof MarshallableMessage) {
+ throw new IgniteException("Message implements
MarshallableMessage but is registered without" +
+ " a marshaller, so it would be sent unmarshalled
[directType=" + directType +
Review Comment:
Reworded both: "Failed to register a message: it implements
MarshallableMessage/DeployableMessage but no marshaller/deployer is provided
[directType=…, cls=…]".
##########
modules/core/src/main/java/org/apache/ignite/internal/managers/communication/IgniteMessageFactoryImpl.java:
##########
Review Comment:
It does throw — `UnknownMessageException extends IgniteException` when
nothing is registered under the direct type. Made the javadoc precise (`@throws
UnknownMessageException`) and dropped the misleading `@Nullable` from `create`
— it never returns null.
##########
modules/core/src/main/java/org/apache/ignite/internal/managers/communication/IgniteMessageFactory.java:
##########
@@ -0,0 +1,91 @@
+/*
+ * 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.managers.communication;
+
+import java.util.function.Supplier;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.processors.cache.GridCacheMessageDeployer;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.apache.ignite.plugin.extensions.communication.MessageFactory;
+import org.apache.ignite.plugin.extensions.communication.MessageMarshaller;
+import org.apache.ignite.plugin.extensions.communication.MessageSerializer;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Internal extension of {@link MessageFactory} that also registers and
resolves the marshal and deployment
+ * companions of a message. Kept out of the public factory interface:
marshallers and deployers are bound to
+ * kernal and cache internals ({@code GridKernalContext}, {@code
GridCacheSharedContext}) the public SPI module
+ * must not depend on.
+ *
+ * @see MessageMarshaller
+ * @see GridCacheMessageDeployer
+ */
+public interface IgniteMessageFactory extends MessageFactory {
+ /** {@inheritDoc} */
+ @Override default void register(short directType, Supplier<Message>
supplier, MessageSerializer serializer)
+ throws IgniteException {
+ register(directType, supplier, serializer, null, null);
+ }
+
+ /**
+ * Register message factory with given direct type, serializer, and
marshaller. All messages must be registered
+ * during construction of class which implements this interface.
+ *
+ * @param directType Direct type.
+ * @param supplier Message factory.
+ * @param serializer Message serializer.
+ * @param marshaller Message marshaller, or {@code null} for
non-marshallable messages.
+ * @throws IgniteException In case of attempt to register message with
direct type which is already registered.
+ */
+ default void register(short directType, Supplier<Message> supplier,
MessageSerializer serializer,
+ @Nullable MessageMarshaller marshaller) throws IgniteException {
+ register(directType, supplier, serializer, marshaller, null);
+ }
+
+ /**
+ * Register message factory with given direct type, serializer,
marshaller, and deployer. All messages must be
+ * registered during construction of class which implements this interface.
+ *
+ * @param directType Direct type.
+ * @param supplier Message factory.
+ * @param serializer Message serializer.
+ * @param marshaller Message marshaller, or {@code null} for
non-marshallable messages.
+ * @param deployer Message deployer, or {@code null} for messages without
deployable fields.
+ * @throws IgniteException In case of attempt to register message with
direct type which is already registered.
+ */
+ public void register(short directType, Supplier<Message> supplier,
MessageSerializer serializer,
Review Comment:
Declined — the registry is runtime-keyed (by `directType`), so generics
can't prove the serializer↔message correspondence: the unchecked casts at the
`resolve()` points stay either way, and a wildcard would only relocate the
`@SuppressWarnings`. Also the public `MessageFactory` (nio module) is master's
interface with raw types — diverging there buys permanent merge friction for no
type-safety gain.
##########
modules/core/src/main/java/org/apache/ignite/internal/managers/communication/IgniteMessageFactoryImpl.java:
##########
@@ -42,6 +45,13 @@ public class IgniteMessageFactoryImpl implements
MessageFactory {
/** Message serializers. */
private final MessageSerializer[] msgSerializers =
(MessageSerializer[])Array.newInstance(MessageSerializer.class, ARR_SIZE);
+ /** Message marshallers (null entry = no marshaller registered for that
type). */
+ private final MessageMarshaller[] msgMarshallers =
(MessageMarshaller[])Array.newInstance(MessageMarshaller.class, ARR_SIZE);
Review Comment:
Declined — the registry is runtime-keyed (by `directType`), so generics
can't prove the serializer↔message correspondence: the unchecked casts at the
`resolve()` points stay either way, and a wildcard would only relocate the
`@SuppressWarnings`. Also the public `MessageFactory` (nio module) is master's
interface with raw types — diverging there buys permanent merge friction for no
type-safety gain.
##########
modules/core/src/main/java/org/apache/ignite/internal/managers/communication/IgniteMessageFactoryImpl.java:
##########
@@ -132,6 +163,16 @@ public IgniteMessageFactoryImpl(MessageFactoryProvider[]
factories) {
return serializer;
}
+ /** {@inheritDoc} */
+ @Override public @Nullable MessageMarshaller marshaller(short directType) {
Review Comment:
Declined — the registry is runtime-keyed (by `directType`), so generics
can't prove the serializer↔message correspondence: the unchecked casts at the
`resolve()` points stay either way, and a wildcard would only relocate the
`@SuppressWarnings`. Also the public `MessageFactory` (nio module) is master's
interface with raw types — diverging there buys permanent merge friction for no
type-safety gain.
##########
modules/core/src/main/java/org/apache/ignite/internal/managers/communication/IgniteMessageFactoryImpl.java:
##########
@@ -132,6 +163,16 @@ public IgniteMessageFactoryImpl(MessageFactoryProvider[]
factories) {
return serializer;
}
+ /** {@inheritDoc} */
+ @Override public @Nullable MessageMarshaller marshaller(short directType) {
+ return msgMarshallers[directTypeToIndex(directType)];
+ }
+
+ /** {@inheritDoc} */
+ @Override public @Nullable GridCacheMessageDeployer deployer(short
directType) {
Review Comment:
Kept — `directType` is the codebase-wide canonical term, tied to the public
`Message#directType()` accessor (master uses it throughout as well); renaming
it only around the factory would detach the parameter from the accessor it
corresponds to.
--
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]
