Make Payload Serializable 10/101210/3
authorRobert Varga <robert.varga@pantheon.tech>
Fri, 13 May 2022 12:52:16 +0000 (14:52 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Wed, 18 May 2022 09:41:40 +0000 (11:41 +0200)
All payloads are serialized via Java serialization, which is not great,
but needs to be expressed in the API contract. Also codify that each
payload needs to have a serializable proxy.

JIRA: CONTROLLER-2037
Change-Id: I99f146413e42af68bb33b942c2222c091a87cf3f
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/messages/KVv1.java [new file with mode: 0644]
opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/messages/KeyValue.java
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/IdentifiablePayload.java
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/Payload.java
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/persisted/NoopPayload.java
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/persisted/ServerConfigurationPayload.java
opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/MockRaftActorContext.java
opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorDelegatingPersistentDataProviderTest.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/persisted/CommitTransactionPayload.java

diff --git a/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/messages/KVv1.java b/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/messages/KVv1.java
new file mode 100644 (file)
index 0000000..7721a8b
--- /dev/null
@@ -0,0 +1,26 @@
+/*
+ * Copyright (c) 2022 PANTHEON.tech, s.r.o. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.controller.cluster.example.messages;
+
+import java.io.Serializable;
+
+final class KVv1 implements Serializable {
+    private static final long serialVersionUID = 1L;
+
+    private final String key;
+    private final String value;
+
+    KVv1(String key, String value) {
+        this.key = key;
+        this.value = value;
+    }
+
+    Object readResolve() {
+        return new KeyValue(key, value);
+    }
+}
index f76dc28..9312183 100644 (file)
@@ -7,11 +7,11 @@
  */
 package org.opendaylight.controller.cluster.example.messages;
 
-import java.io.Serializable;
 import org.opendaylight.controller.cluster.raft.messages.Payload;
 
-public class KeyValue extends Payload implements Serializable {
+public final class KeyValue extends Payload {
     private static final long serialVersionUID = 1L;
+
     private String key;
     private String value;
 
@@ -31,14 +31,6 @@ public class KeyValue extends Payload implements Serializable {
         return value;
     }
 
-    public void setKey(String key) {
-        this.key = key;
-    }
-
-    public void setValue(String value) {
-        this.value = value;
-    }
-
     @Override
     public String toString() {
         return "KeyValue{" + "key='" + key + '\'' + ", value='" + value + '\'' + '}';
@@ -48,4 +40,9 @@ public class KeyValue extends Payload implements Serializable {
     public int size() {
         return value.length() + key.length();
     }
+
+    @Override
+    protected Object writeReplace() {
+        return new KVv1(value, key);
+    }
 }
index f57690b..dad696d 100644 (file)
@@ -11,5 +11,6 @@ import org.opendaylight.yangtools.concepts.Identifiable;
 import org.opendaylight.yangtools.concepts.Identifier;
 
 public abstract class IdentifiablePayload<T extends Identifier> extends Payload implements Identifiable<T> {
+    private static final long serialVersionUID = 1L;
 
 }
index b70eace..78a2497 100644 (file)
@@ -7,6 +7,8 @@
  */
 package org.opendaylight.controller.cluster.raft.messages;
 
+import java.io.Serializable;
+
 /**
  * An instance of a {@link Payload} class is meant to be used as the Payload for {@link AppendEntries}.
  *
@@ -14,11 +16,20 @@ package org.opendaylight.controller.cluster.raft.messages;
  * When an actor which is derived from RaftActor attempts to persistData it must pass an instance of the Payload class.
  * Similarly when state needs to be applied to the derived RaftActor it will be passed an instance of the Payload class.
  */
-public abstract class Payload {
+public abstract class Payload implements Serializable {
+    private static final long serialVersionUID = 1L;
+
     /**
      * Return the estimate of in-memory size of this payload.
      *
      * @return An estimate of the in-memory size of this payload.
      */
     public abstract int size();
+
+    /**
+     * Return the serialization proxy for this object.
+     *
+     * @return Serialization proxy
+     */
+    protected abstract Object writeReplace();
 }
index 2acd03a..800ea45 100644 (file)
@@ -17,7 +17,7 @@ import org.opendaylight.controller.cluster.raft.messages.Payload;
  *
  * @author Thomas Pantelis
  */
-public final class NoopPayload extends Payload implements Serializable, ControlMessage {
+public final class NoopPayload extends Payload implements ControlMessage {
     public static final NoopPayload INSTANCE = new NoopPayload();
 
     // There is no need for Externalizable
@@ -40,7 +40,8 @@ public final class NoopPayload extends Payload implements Serializable, ControlM
         return 0;
     }
 
-    private Object writeReplace() {
+    @Override
+    protected Object writeReplace() {
         return PROXY;
     }
 }
index 9486ba6..eb66817 100644 (file)
@@ -15,7 +15,6 @@ import java.io.IOException;
 import java.io.ObjectInput;
 import java.io.ObjectOutput;
 import java.io.ObjectOutputStream;
-import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.List;
 import org.eclipse.jdt.annotation.NonNull;
@@ -29,7 +28,7 @@ import org.slf4j.LoggerFactory;
  *
  * @author Thomas Pantelis
  */
-public final class ServerConfigurationPayload extends Payload implements PersistentPayload, Serializable {
+public final class ServerConfigurationPayload extends Payload implements PersistentPayload {
     private static final class Proxy implements Externalizable {
         private static final long serialVersionUID = 1L;
 
@@ -43,7 +42,7 @@ public final class ServerConfigurationPayload extends Payload implements Persist
         }
 
         Proxy(final ServerConfigurationPayload payload) {
-            this.serverConfig = payload.getServerConfig();
+            serverConfig = payload.getServerConfig();
         }
 
         @Override
@@ -134,7 +133,8 @@ public final class ServerConfigurationPayload extends Payload implements Persist
         return "ServerConfigurationPayload [serverConfig=" + serverConfig + "]";
     }
 
-    private Object writeReplace() {
+    @Override
+    protected Object writeReplace() {
         return new Proxy(this);
     }
 }
index ea4766f..0d40079 100644 (file)
@@ -8,6 +8,8 @@
 
 package org.opendaylight.controller.cluster.raft;
 
+import static java.util.Objects.requireNonNull;
+
 import akka.actor.ActorRef;
 import akka.actor.ActorSelection;
 import akka.actor.ActorSystem;
@@ -19,6 +21,7 @@ import java.io.OutputStream;
 import java.io.Serializable;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.function.Consumer;
 import org.opendaylight.controller.cluster.DataPersistenceProvider;
@@ -56,8 +59,8 @@ public class MockRaftActorContext extends RaftActorContextImpl {
 
             @Override
             public void update(final long newTerm, final String newVotedFor) {
-                this.currentTerm = newTerm;
-                this.votedFor = newVotedFor;
+                currentTerm = newTerm;
+                votedFor = newVotedFor;
 
                 // TODO : Write to some persistent state
             }
@@ -109,7 +112,7 @@ public class MockRaftActorContext extends RaftActorContextImpl {
     }
 
     @Override public ActorSystem getActorSystem() {
-        return this.system;
+        return system;
     }
 
     @Override public ActorSelection getPeerActorSelection(final String peerId) {
@@ -200,21 +203,22 @@ public class MockRaftActorContext extends RaftActorContextImpl {
         }
     }
 
-    public static class MockPayload extends Payload implements Serializable {
+    public static final class MockPayload extends Payload {
         private static final long serialVersionUID = 3121380393130864247L;
-        private String value = "";
-        private int size;
+
+        private final String data;
+        private final int size;
 
         public MockPayload() {
+            this("");
         }
 
         public MockPayload(final String data) {
-            this.value = data;
-            size = value.length();
+            this(data, data.length());
         }
 
         public MockPayload(final String data, final int size) {
-            this(data);
+            this.data = requireNonNull(data);
             this.size = size;
         }
 
@@ -225,37 +229,39 @@ public class MockRaftActorContext extends RaftActorContextImpl {
 
         @Override
         public String toString() {
-            return value;
+            return data;
         }
 
         @Override
         public int hashCode() {
-            final int prime = 31;
-            int result = 1;
-            result = prime * result + (value == null ? 0 : value.hashCode());
-            return result;
+            return data.hashCode();
         }
 
         @Override
         public boolean equals(final Object obj) {
-            if (this == obj) {
-                return true;
-            }
-            if (obj == null) {
-                return false;
-            }
-            if (getClass() != obj.getClass()) {
-                return false;
-            }
-            MockPayload other = (MockPayload) obj;
-            if (value == null) {
-                if (other.value != null) {
-                    return false;
-                }
-            } else if (!value.equals(other.value)) {
-                return false;
-            }
-            return true;
+            return this == obj || obj instanceof MockPayload other && Objects.equals(data, other.data)
+                && size == other.size;
+        }
+
+        @Override
+        protected Object writeReplace() {
+            return new MockPayloadProxy(data, size);
+        }
+    }
+
+    private static final class MockPayloadProxy implements Serializable {
+        private static final long serialVersionUID = 1L;
+
+        private final String value;
+        private final int size;
+
+        MockPayloadProxy(String value, int size) {
+            this.value = value;
+            this.size = size;
+        }
+
+        Object readResolve() {
+            return new MockPayload(value, size);
         }
     }
 
@@ -264,19 +270,19 @@ public class MockRaftActorContext extends RaftActorContextImpl {
 
         public  MockReplicatedLogBuilder createEntries(final int start, final int end, final int term) {
             for (int i = start; i < end; i++) {
-                this.mockLog.append(new SimpleReplicatedLogEntry(i, term,
+                mockLog.append(new SimpleReplicatedLogEntry(i, term,
                         new MockRaftActorContext.MockPayload(Integer.toString(i))));
             }
             return this;
         }
 
         public  MockReplicatedLogBuilder addEntry(final int index, final int term, final MockPayload payload) {
-            this.mockLog.append(new SimpleReplicatedLogEntry(index, term, payload));
+            mockLog.append(new SimpleReplicatedLogEntry(index, term, payload));
             return this;
         }
 
         public ReplicatedLog build() {
-            return this.mockLog;
+            return mockLog;
         }
     }
 
index 333e01f..33247d3 100644 (file)
@@ -98,12 +98,22 @@ public class RaftActorDelegatingPersistentDataProviderTest {
     }
 
     static class TestNonPersistentPayload extends Payload {
+        private static final long serialVersionUID = 1L;
+
         @Override
         public int size() {
             return 0;
         }
+
+        @Override
+        protected Object writeReplace() {
+            // Not needed
+            throw new UnsupportedOperationException();
+        }
     }
 
     static class TestPersistentPayload extends TestNonPersistentPayload implements PersistentPayload {
+        private static final long serialVersionUID = 1L;
+
     }
 }