From 8761bc53cab85caba9a4a4053cfd9e0970d46a1c Mon Sep 17 00:00:00 2001
From: Robert Varga <robert.varga@pantheon.tech>
Date: Fri, 24 Nov 2023 16:10:33 +0100
Subject: [PATCH] Eliminate SE_BAD_FIELD suppressions

We are implementing Serializable Proxy pattern, which should really be
enforced via throwing appropriate exceptions from
readObject()/writeObject() et al.

This retrofits the mininal set to keep SpotBugs happy, eliminating the
associated suppressions.

Change-Id: I8264114b1f0ed74123f0163d62d953dc45a8a9d1
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
---
 .../CommitLocalTransactionRequest.java        | 23 +++++++++++++++----
 .../access/commands/ConnectClientSuccess.java | 21 ++++++++++++++---
 .../commands/ModifyTransactionRequest.java    | 22 ++++++++++++++----
 .../commands/ReadTransactionSuccess.java      | 21 +++++++++++++++--
 .../cluster/access/concepts/Message.java      | 23 +++++++++++++++++++
 5 files changed, 97 insertions(+), 13 deletions(-)

diff --git a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/CommitLocalTransactionRequest.java b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/CommitLocalTransactionRequest.java
index c7502d51d4..7a3f771b47 100644
--- a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/CommitLocalTransactionRequest.java
+++ b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/CommitLocalTransactionRequest.java
@@ -11,7 +11,10 @@ import static java.util.Objects.requireNonNull;
 
 import akka.actor.ActorRef;
 import com.google.common.base.MoreObjects.ToStringHelper;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.io.ObjectStreamException;
 import java.util.Optional;
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
@@ -27,9 +30,6 @@ public final class CommitLocalTransactionRequest
     @java.io.Serial
     private static final long serialVersionUID = 1L;
 
-    @SuppressFBWarnings(value = "SE_BAD_FIELD", justification = "This field is not Serializable but this class "
-            + "implements writeReplace to delegate serialization to a Proxy class and thus instances of this class "
-            + "aren't serialized. FindBugs does not recognize this.")
     private final DataTreeModification mod;
     private final Exception delayedFailure;
     private final boolean coordinated;
@@ -74,4 +74,19 @@ public final class CommitLocalTransactionRequest
         return super.addToStringAttributes(toStringHelper).add("coordinated", coordinated)
                 .add("delayedError", delayedFailure);
     }
+
+    @java.io.Serial
+    private void readObject(final ObjectInputStream stream) throws IOException, ClassNotFoundException {
+        throwNSE();
+    }
+
+    @java.io.Serial
+    private void readObjectNoData() throws ObjectStreamException {
+        throwNSE();
+    }
+
+    @java.io.Serial
+    private void writeObject(final ObjectOutputStream stream) throws IOException {
+        throwNSE();
+    }
 }
diff --git a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ConnectClientSuccess.java b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ConnectClientSuccess.java
index a96404511e..ad0e3624e1 100644
--- a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ConnectClientSuccess.java
+++ b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ConnectClientSuccess.java
@@ -16,11 +16,13 @@ import akka.serialization.JavaSerializer;
 import akka.serialization.Serialization;
 import com.google.common.base.MoreObjects.ToStringHelper;
 import com.google.common.collect.ImmutableList;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.DataInput;
 import java.io.IOException;
 import java.io.ObjectInput;
+import java.io.ObjectInputStream;
 import java.io.ObjectOutput;
+import java.io.ObjectOutputStream;
+import java.io.ObjectStreamException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
@@ -76,8 +78,6 @@ public final class ConnectClientSuccess extends RequestSuccess<ClientIdentifier,
     private static final long serialVersionUID = 1L;
 
     private final @NonNull ImmutableList<ActorSelection> alternates;
-
-    @SuppressFBWarnings(value = "SE_BAD_FIELD", justification = "See justification above.")
     private final ReadOnlyDataTree dataTree;
     private final @NonNull ActorRef backend;
     private final int maxMessages;
@@ -142,4 +142,19 @@ public final class ConnectClientSuccess extends RequestSuccess<ClientIdentifier,
         return super.addToStringAttributes(toStringHelper).add("alternates", alternates)
                 .add("dataTree present", getDataTree().isPresent()).add("maxMessages", maxMessages);
     }
+
+    @java.io.Serial
+    private void readObject(final ObjectInputStream stream) throws IOException, ClassNotFoundException {
+        throwNSE();
+    }
+
+    @java.io.Serial
+    private void readObjectNoData() throws ObjectStreamException {
+        throwNSE();
+    }
+
+    @java.io.Serial
+    private void writeObject(final ObjectOutputStream stream) throws IOException {
+        throwNSE();
+    }
 }
diff --git a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ModifyTransactionRequest.java b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ModifyTransactionRequest.java
index c83289570d..b1ddd38930 100644
--- a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ModifyTransactionRequest.java
+++ b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ModifyTransactionRequest.java
@@ -10,10 +10,12 @@ package org.opendaylight.controller.cluster.access.commands;
 import akka.actor.ActorRef;
 import com.google.common.base.MoreObjects.ToStringHelper;
 import com.google.common.collect.ImmutableList;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.IOException;
 import java.io.ObjectInput;
+import java.io.ObjectInputStream;
 import java.io.ObjectOutput;
+import java.io.ObjectOutputStream;
+import java.io.ObjectStreamException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
@@ -74,9 +76,6 @@ public final class ModifyTransactionRequest extends TransactionRequest<ModifyTra
     @java.io.Serial
     private static final long serialVersionUID = 1L;
 
-    @SuppressFBWarnings(value = "SE_BAD_FIELD", justification = "This field is not Serializable but this class "
-            + "implements writeReplace to delegate serialization to a Proxy class and thus instances of this class "
-            + "aren't serialized. FindBugs does not recognize this.")
     private final List<TransactionModification> modifications;
     private final PersistenceProtocol protocol;
 
@@ -116,4 +115,19 @@ public final class ModifyTransactionRequest extends TransactionRequest<ModifyTra
     protected ModifyTransactionRequest cloneAsVersion(final ABIVersion version) {
         return new ModifyTransactionRequest(this, version);
     }
+
+    @java.io.Serial
+    private void readObject(final ObjectInputStream stream) throws IOException, ClassNotFoundException {
+        throwNSE();
+    }
+
+    @java.io.Serial
+    private void readObjectNoData() throws ObjectStreamException {
+        throwNSE();
+    }
+
+    @java.io.Serial
+    private void writeObject(final ObjectOutputStream stream) throws IOException {
+        throwNSE();
+    }
 }
diff --git a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ReadTransactionSuccess.java b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ReadTransactionSuccess.java
index 4f9734f060..a03766e9da 100644
--- a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ReadTransactionSuccess.java
+++ b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/commands/ReadTransactionSuccess.java
@@ -9,7 +9,10 @@ package org.opendaylight.controller.cluster.access.commands;
 
 import static java.util.Objects.requireNonNull;
 
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.io.ObjectStreamException;
 import java.util.Optional;
 import org.opendaylight.controller.cluster.access.ABIVersion;
 import org.opendaylight.controller.cluster.access.concepts.SliceableMessage;
@@ -25,7 +28,6 @@ public final class ReadTransactionSuccess extends TransactionSuccess<ReadTransac
     @java.io.Serial
     private static final long serialVersionUID = 1L;
 
-    @SuppressFBWarnings(value = "SE_BAD_FIELD", justification = "interface-based best effort")
     private final Optional<NormalizedNode> data;
 
     private ReadTransactionSuccess(final ReadTransactionSuccess request, final ABIVersion version) {
@@ -52,4 +54,19 @@ public final class ReadTransactionSuccess extends TransactionSuccess<ReadTransac
     protected ReadTransactionSuccess cloneAsVersion(final ABIVersion version) {
         return new ReadTransactionSuccess(this, version);
     }
+
+    @java.io.Serial
+    private void readObject(final ObjectInputStream stream) throws IOException, ClassNotFoundException {
+        throwNSE();
+    }
+
+    @java.io.Serial
+    private void readObjectNoData() throws ObjectStreamException {
+        throwNSE();
+    }
+
+    @java.io.Serial
+    private void writeObject(final ObjectOutputStream stream) throws IOException {
+        throwNSE();
+    }
 }
diff --git a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/concepts/Message.java b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/concepts/Message.java
index 3c8ce59b89..9748264e7f 100644
--- a/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/concepts/Message.java
+++ b/opendaylight/md-sal/cds-access-api/src/main/java/org/opendaylight/controller/cluster/access/concepts/Message.java
@@ -16,8 +16,12 @@ import com.google.common.base.MoreObjects.ToStringHelper;
 import java.io.DataInput;
 import java.io.Externalizable;
 import java.io.IOException;
+import java.io.NotSerializableException;
 import java.io.ObjectInput;
+import java.io.ObjectInputStream;
 import java.io.ObjectOutput;
+import java.io.ObjectOutputStream;
+import java.io.ObjectStreamException;
 import java.io.Serializable;
 import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.controller.cluster.access.ABIVersion;
@@ -198,4 +202,23 @@ public abstract class Message<T extends WritableIdentifier, C extends Message<T,
     protected final Object writeReplace() {
         return externalizableProxy(version);
     }
+
+    protected final void throwNSE() throws NotSerializableException {
+        throw new NotSerializableException(getClass().getName());
+    }
+
+    @java.io.Serial
+    private void readObject(final ObjectInputStream stream) throws IOException, ClassNotFoundException {
+        throwNSE();
+    }
+
+    @java.io.Serial
+    private void readObjectNoData() throws ObjectStreamException {
+        throwNSE();
+    }
+
+    @java.io.Serial
+    private void writeObject(final ObjectOutputStream stream) throws IOException {
+        throwNSE();
+    }
 }
-- 
2.36.6