Vladsz83 commented on code in PR #12708:
URL: https://github.com/apache/ignite/pull/12708#discussion_r2811306449
##########
modules/core/src/test/java/org/apache/ignite/testframework/GridTestUtils.java:
##########
@@ -221,16 +200,16 @@ public void afterDiscovery(DiscoveryCustomMessage
customMsg) {
public void ignite(IgniteEx ignite) {
// No-op.
}
+ }
- /**
- * Obtains {@link DiscoveryCustomMessage} from {@link
CustomMessageWrapper}.
- *
- * @param wrapper Wrapper of {@link DiscoveryCustomMessage}.
- * @return Unwrapped {@link DiscoveryCustomMessage}.
- */
- private DiscoveryCustomMessage unwrap(CustomMessageWrapper wrapper) {
- return U.field(wrapper, "delegate");
- }
+ /**
+ * Unwraps messsage if it is wrapped by {@link
SecurityAwareCustomMessageWrapper}.
+ *
+ * @param msg Message.
+ */
+ public static DiscoveryCustomMessage unwrap(DiscoveryCustomMessage msg) {
Review Comment:
Can we rename this method somehow, WDYT? Like `unwrapCustomMessage`? Yes,
here is a javadoc. But the method name is meaningless. We might move this
method into `GridDiscoveryManager` for example. There is usage of `instanceof
SecurityAwareCustomMessageWrapper`. Also in `ClientImpl`, `ServerImpl`.
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/DiscoverySpiCustomMessage.java:
##########
@@ -17,37 +17,28 @@
package org.apache.ignite.spi.discovery;
-import java.io.Serializable;
+import org.apache.ignite.internal.managers.discovery.DiscoCache;
import org.apache.ignite.internal.managers.discovery.DiscoveryCustomMessage;
import org.apache.ignite.internal.managers.discovery.GridDiscoveryManager;
-import org.jetbrains.annotations.Nullable;
+import org.apache.ignite.internal.processors.affinity.AffinityTopologyVersion;
+import org.apache.ignite.lang.IgniteUuid;
/**
* Message to send across ring.
*
* @see GridDiscoveryManager#sendCustomEvent(DiscoveryCustomMessage)
+ * TODO: Should be removed in
https://issues.apache.org/jira/browse/IGNITE-27778
*/
-public interface DiscoverySpiCustomMessage extends Serializable {
- /**
- * Called when custom message has been handled by all nodes.
- *
- * @return Ack message or {@code null} if ack is not required.
- */
- @Nullable public DiscoverySpiCustomMessage ackMessage();
+@Deprecated
+public abstract class DiscoverySpiCustomMessage implements
DiscoveryCustomMessage {
Review Comment:
Can we set the defaults for `isMutable()`, `ackMessage()`,
`createDiscoCache()` in `DiscoveryCustomMessage`? WDYT? There is a lot of
overrided `return null/false`. Other ticket?
##########
modules/core/src/main/java/org/apache/ignite/internal/managers/discovery/GridDiscoveryManager.java:
##########
@@ -592,8 +591,11 @@ private void onDiscovery0(DiscoveryNotification
notification) {
ClusterNode node = notification.getNode();
long topVer = notification.getTopVer();
- DiscoveryCustomMessage customMsg =
notification.getCustomMsgData() == null ? null
- :
((CustomMessageWrapper)notification.getCustomMsgData()).delegate();
+ DiscoveryCustomMessage customData =
notification.getCustomMsgData() == null ? null :
Review Comment:
Suggestion: `customData` is a bit confisung. Sounds like we do smth. like
`byte[] data = customMsg.getData()`. Let's use one variable named `customMsg`.
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/ClientImpl.java:
##########
@@ -504,7 +504,10 @@ else if (state == DISCONNECTED) {
try {
TcpDiscoveryCustomEventMessage msg;
- if (((CustomMessageWrapper)evt).delegate() instanceof
DiscoveryServerOnlyCustomMessage)
+ DiscoveryCustomMessage customMsg = evt instanceof
SecurityAwareCustomMessageWrapper ?
Review Comment:
Might be used the shared method `unwrap()` moved from `GridTestUtils`
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/DiscoveryNotification.java:
##########
@@ -42,7 +43,7 @@ public class DiscoveryNotification {
private @Nullable NavigableMap<Long, Collection<ClusterNode>> topHist;
/** Custom message data. */
- private @Nullable DiscoverySpiCustomMessage customMsgData;
+ private @Nullable DiscoveryCustomMessage customMsgData;
Review Comment:
I would rename `DiscoveryCustomMessage customMsgData` ->
`DiscoveryCustomMessage customMsg`. `Data` sounds like `byte[] data =
customMsg.getData()`. Also the code might be searched for
`DiscoveryCustomMessage data` -> `DiscoveryCustomMessage customMsg` and
`DiscoveryCustomMessage custom` -> `DiscoveryCustomMessage customMsg` .
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/ServerImpl.java:
##########
@@ -1017,11 +1017,14 @@ private void interruptPing(TcpDiscoveryNode node) {
}
/** {@inheritDoc} */
- @Override public void sendCustomEvent(DiscoverySpiCustomMessage evt) {
+ @Override public void sendCustomEvent(DiscoveryCustomMessage evt) {
try {
TcpDiscoveryCustomEventMessage msg;
- if (((CustomMessageWrapper)evt).delegate() instanceof
DiscoveryServerOnlyCustomMessage)
+ DiscoveryCustomMessage customMsg = evt instanceof
SecurityAwareCustomMessageWrapper ?
Review Comment:
Might be used the shared method `unwrap()` moved from `GridTestUtils`
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/DiscoverySpiCustomMessage.java:
##########
@@ -17,37 +17,28 @@
package org.apache.ignite.spi.discovery;
-import java.io.Serializable;
+import org.apache.ignite.internal.managers.discovery.DiscoCache;
import org.apache.ignite.internal.managers.discovery.DiscoveryCustomMessage;
import org.apache.ignite.internal.managers.discovery.GridDiscoveryManager;
-import org.jetbrains.annotations.Nullable;
+import org.apache.ignite.internal.processors.affinity.AffinityTopologyVersion;
+import org.apache.ignite.lang.IgniteUuid;
/**
* Message to send across ring.
*
* @see GridDiscoveryManager#sendCustomEvent(DiscoveryCustomMessage)
+ * TODO: Should be removed in
https://issues.apache.org/jira/browse/IGNITE-27778
*/
-public interface DiscoverySpiCustomMessage extends Serializable {
- /**
- * Called when custom message has been handled by all nodes.
- *
- * @return Ack message or {@code null} if ack is not required.
- */
- @Nullable public DiscoverySpiCustomMessage ackMessage();
+@Deprecated
Review Comment:
Suggestion: `@Deprecated(forRemoval = true)`
--
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]
